diff --git a/ui/src/components/task-chat/tool-taxonomy.test.ts b/ui/src/components/task-chat/tool-taxonomy.test.ts index 9f12f6bd26f..52f9fb09c0b 100644 --- a/ui/src/components/task-chat/tool-taxonomy.test.ts +++ b/ui/src/components/task-chat/tool-taxonomy.test.ts @@ -9,7 +9,7 @@ import { SquareTerminal, Wrench, } from "lucide-react"; -import { mcpToolSegment, toolTaxonomy } from "./tool-taxonomy"; +import { isGenericToolName, mcpToolSegment, toolTaxonomy } from "./tool-taxonomy"; describe("toolTaxonomy", () => { it("maps each family to its icon and verb", () => { @@ -77,6 +77,33 @@ describe("toolTaxonomy", () => { }); }); +describe("toolTaxonomy multi-word ACP titles", () => { + it("classifies by the first word", () => { + expect(toolTaxonomy("Read File").icon).toBe(BookOpen); + expect(toolTaxonomy("Edit File").icon).toBe(Pencil); + expect(toolTaxonomy("Write File").icon).toBe(Pencil); + expect(toolTaxonomy("Terminal").icon).toBe(SquareTerminal); + }); +}); + +describe("isGenericToolName", () => { + it("flags acpx placeholder names, including status-suffixed variants", () => { + expect(isGenericToolName("tool call")).toBe(true); + expect(isGenericToolName("tool call (completed)")).toBe(true); + expect(isGenericToolName("Tool Call (failed)")).toBe(true); + expect(isGenericToolName("acp_tool")).toBe(true); + expect(isGenericToolName("tool")).toBe(true); + expect(isGenericToolName("")).toBe(true); + expect(isGenericToolName(undefined)).toBe(true); + }); + + it("keeps real names", () => { + expect(isGenericToolName("Terminal")).toBe(false); + expect(isGenericToolName("Read")).toBe(false); + expect(isGenericToolName("mcp__linear__search_issues")).toBe(false); + }); +}); + describe("mcpToolSegment", () => { it("extracts and capitalizes the tool segment", () => { expect(mcpToolSegment("mcp__linear-server__search_issues")).toBe("Search_issues"); diff --git a/ui/src/components/task-chat/tool-taxonomy.ts b/ui/src/components/task-chat/tool-taxonomy.ts index f3746a0231d..6ea54e8173e 100644 --- a/ui/src/components/task-chat/tool-taxonomy.ts +++ b/ui/src/components/task-chat/tool-taxonomy.ts @@ -34,6 +34,22 @@ export interface ToolTaxonomyEntry { verbLabel: string; } +/** + * Placeholder names that carry no tool identity. acpx fills a tool_call_update + * whose ACP payload omits `title` with the literal "tool call", and older + * stored logs carry "tool call (completed)" / "acp_tool" variants — none of + * these should ever displace a real name like "Terminal" or "Read". + */ +const GENERIC_TOOL_NAMES = new Set(["tool", "tool call", "tool_call", "acp_tool"]); + +export function isGenericToolName(name: string | undefined | null): boolean { + const raw = (name ?? "") + .trim() + .toLowerCase() + .replace(/\s*\((?:pending|in_progress|completed|failed|cancelled)\)$/, ""); + return !raw || GENERIC_TOOL_NAMES.has(raw); +} + /** * Collapse an MCP tool name ("mcp__server__tool") to its tool segment, * capitalized — the same rule toolDisplayName() applies. Returns null for @@ -57,12 +73,19 @@ const FAMILY_META: Record, Omit { - if (n === "read" || n === "notebookread") return "read"; - if (n === "edit" || n === "write" || n === "multiedit" || n === "notebookedit") return "edit"; - if (n === "bash" || n === "shell" || n === "run" || n.includes("terminal")) return "terminal"; - if (n === "grep" || n === "glob" || n.includes("search")) return "search"; + // ACP titles are often multi-word ("Read File", "Edit File") — the first + // word carries the family. + const first = n.split(/[\s_:-]+/, 1)[0] ?? ""; + if (first === "read" || n === "notebookread") return "read"; + if (first === "edit" || first === "write" || n === "multiedit" || n === "notebookedit") { + return "edit"; + } + if (first === "bash" || first === "shell" || first === "run" || n.includes("terminal")) { + return "terminal"; + } + if (first === "grep" || first === "glob" || n.includes("search")) return "search"; if (n.includes("fetch") || n.includes("web") || n.includes("http")) return "web"; - if (n === "task" || n === "agent") return "agent"; + if (first === "task" || first === "agent") return "agent"; return "other"; } diff --git a/ui/src/components/task-chat/transcript-adapter.test.ts b/ui/src/components/task-chat/transcript-adapter.test.ts index f26dbe28474..ff7ae53bd72 100644 --- a/ui/src/components/task-chat/transcript-adapter.test.ts +++ b/ui/src/components/task-chat/transcript-adapter.test.ts @@ -1,6 +1,10 @@ import { describe, expect, it } from "vitest"; import type { TranscriptEntry } from "@/adapters"; -import { deriveRunStatusLabel, toolDisplayName } from "./transcript-adapter"; +import { + deriveRunStatusLabel, + toolDisplayName, + transcriptToTaskChatItems, +} from "./transcript-adapter"; const TS = "2026-07-31T12:00:00.000Z"; @@ -58,4 +62,115 @@ describe("toolDisplayName", () => { expect(toolDisplayName("")).toBe("Tool"); expect(toolDisplayName("tool")).toBe("Tool"); }); + + it("maps acpx placeholder names to the generic Tool label", () => { + expect(toolDisplayName("tool call")).toBe("Tool"); + expect(toolDisplayName("tool call (completed)")).toBe("Tool"); + expect(toolDisplayName("acp_tool")).toBe("Tool"); + }); +}); + +describe("transcriptToTaskChatItems tool_call updates", () => { + const opts = { runId: "run-1", running: false }; + + function update(toolUseId: string, status: string): TranscriptEntry { + // Mirrors what a persisted acpx tool_call_update line parses to: the + // literal placeholder name plus a synthesized { text, status } input. + return { + kind: "tool_call", + ts: TS, + name: "tool call", + toolUseId, + input: { text: `tool call (${status})`, status }, + } as TranscriptEntry; + } + + it("keeps the initial real name and target when a generic update arrives", () => { + const items = transcriptToTaskChatItems( + [ + toolCall("Terminal", { command: "pnpm test" }), + update("tool-Terminal", "completed"), + { + kind: "tool_result", + ts: TS, + toolUseId: "tool-Terminal", + toolName: "tool call", + content: "ok", + } as TranscriptEntry, + ], + opts, + ); + expect(items).toHaveLength(1); + const tool = items[0]; + expect(tool.kind).toBe("tool"); + if (tool.kind !== "tool") return; + expect(tool.name).toBe("Terminal"); + expect(tool.rawName).toBe("Terminal"); + expect(tool.target).toBe("pnpm test"); + expect(tool.status).toBe("completed"); + }); + + it("keeps identity on retitle and uses the invocation as the target", () => { + // Real stored sequence: "Terminal" (pending) → retitle to the command → + // generic "tool call" completion updates. + const items = transcriptToTaskChatItems( + [ + { kind: "tool_call", ts: TS, name: "Terminal", toolUseId: "tc-2" } as TranscriptEntry, + { kind: "tool_call", ts: TS, name: "ls -la", toolUseId: "tc-2" } as TranscriptEntry, + update("tc-2", "completed"), + ], + opts, + ); + expect(items).toHaveLength(1); + if (items[0].kind !== "tool") return; + expect(items[0].name).toBe("Terminal"); + expect(items[0].rawName).toBe("Terminal"); + expect(items[0].target).toBe("ls -la"); + }); + + it("never renders the synthesized { text, status } input as a target", () => { + const items = transcriptToTaskChatItems([update("tc-orphan", "pending")], opts); + expect(items).toHaveLength(1); + if (items[0].kind !== "tool") return; + expect(items[0].target).toBeUndefined(); + }); + + it("upgrades a generic-named call when a later entry carries the real name", () => { + const items = transcriptToTaskChatItems( + [ + { kind: "tool_call", ts: TS, name: "tool call", toolUseId: "tc-1" } as TranscriptEntry, + { kind: "tool_call", ts: TS, name: "Read", toolUseId: "tc-1" } as TranscriptEntry, + ], + opts, + ); + expect(items).toHaveLength(1); + if (items[0].kind !== "tool") return; + expect(items[0].name).toBe("Read"); + }); +}); + +describe("deriveRunStatusLabel with generic tail updates", () => { + it("recovers the real tool name from the call's initial entry", () => { + const status = deriveRunStatusLabel([ + toolCall("Terminal", { command: "ls -la" }), + { + kind: "tool_call", + ts: TS, + name: "tool call", + toolUseId: "tool-Terminal", + } as TranscriptEntry, + ]); + expect(status.label).toBe("Running a command"); + expect(status.toolName).toBe("Terminal"); + }); + + it("treats a tail retitle as the invocation, not the identity", () => { + const status = deriveRunStatusLabel([ + { kind: "tool_call", ts: TS, name: "Terminal", toolUseId: "tc-3" } as TranscriptEntry, + { kind: "tool_call", ts: TS, name: "pnpm test", toolUseId: "tc-3" } as TranscriptEntry, + ]); + expect(status.label).toBe("Running a command"); + expect(status.toolName).toBe("Terminal"); + expect(status.detail).toBe("Terminal · pnpm test"); + }); }); diff --git a/ui/src/components/task-chat/transcript-adapter.ts b/ui/src/components/task-chat/transcript-adapter.ts index 1480e15ff1b..eef8f0d3741 100644 --- a/ui/src/components/task-chat/transcript-adapter.ts +++ b/ui/src/components/task-chat/transcript-adapter.ts @@ -12,7 +12,7 @@ import type { TaskChatToolItem, TaskChatTurnItem, } from "./task-chat-model"; -import { mcpToolSegment, toolTaxonomy } from "./tool-taxonomy"; +import { isGenericToolName, mcpToolSegment, toolTaxonomy } from "./tool-taxonomy"; const TERMINAL_STATUSES = new Set([ "failed", @@ -68,8 +68,13 @@ export function summarizeToolInput(input: unknown): string | undefined { const value = record[key]; if (typeof value === "string" && value.trim()) return clip(value, TARGET_MAX); } + // The acpx log parser synthesizes { text, status } onto tool inputs from the + // event's summary line — presentation noise, not call parameters. const entries = Object.entries(record).filter( - ([, v]) => typeof v === "string" || typeof v === "number" || typeof v === "boolean", + ([k, v]) => + (typeof v === "string" || typeof v === "number" || typeof v === "boolean") && + k !== "text" && + k !== "status", ); if (entries.length === 0) return undefined; return clip(entries.map(([k, v]) => `${k}: ${String(v)}`).join(", "), TARGET_MAX); @@ -82,7 +87,7 @@ export function summarizeToolInput(input: unknown): string | undefined { */ export function toolDisplayName(name: string | undefined | null): string { const raw = (name ?? "").trim(); - if (!raw || raw === "tool") return "Tool"; + if (isGenericToolName(raw)) return "Tool"; const mcp = mcpToolSegment(raw); if (mcp) return mcp; return raw.charAt(0).toUpperCase() + raw.slice(1); @@ -187,19 +192,32 @@ export function transcriptToTaskChatItems( } case "tool_call": { const toolCallId = entry.toolUseId || `tool-${i}`; - const item: TaskChatToolItem = { - id: `${runId}:tool:${toolCallId}`, - kind: "tool", - name: toolDisplayName(entry.name), - rawName: entry.name ?? undefined, - target: summarizeToolInput(entry.input), - status: "in_progress", - }; - if (toolIndexById.has(toolCallId)) { - items[toolIndexById.get(toolCallId)!] = item; - lastToolIndex = toolIndexById.get(toolCallId)!; + const existingIndex = toolIndexById.get(toolCallId); + const existing = existingIndex != null ? items[existingIndex] : undefined; + if (existing?.kind === "tool") { + // tool_call_update for a call already in the list: merge. Updates + // often omit the title (acpx fills in a literal "tool call"), so a + // generic name must never displace the initial call's real one. ACP + // also retitles a call to its invocation once known ("Terminal" → + // "ls -la"); that is detail for the target slot, not a new identity. + if (!isGenericToolName(entry.name)) { + if (isGenericToolName(existing.rawName)) { + existing.name = toolDisplayName(entry.name); + existing.rawName = entry.name ?? undefined; + } else if (!existing.target && entry.name !== existing.rawName) { + existing.target = clip(entry.name!, TARGET_MAX); + } + } + lastToolIndex = existingIndex!; } else { - items.push(item); + items.push({ + id: `${runId}:tool:${toolCallId}`, + kind: "tool", + name: toolDisplayName(entry.name), + rawName: entry.name ?? undefined, + target: summarizeToolInput(entry.input), + status: "in_progress", + }); toolIndexById.set(toolCallId, items.length - 1); lastToolIndex = items.length - 1; } @@ -213,7 +231,7 @@ export function transcriptToTaskChatItems( const existing = items[idx]; if (existing.kind === "tool") { existing.status = entry.isError ? "failed" : "completed"; - if (existing.name === "Tool" && entry.toolName) { + if (isGenericToolName(existing.rawName) && !isGenericToolName(entry.toolName)) { existing.name = toolDisplayName(entry.toolName); existing.rawName = entry.toolName; } @@ -330,12 +348,34 @@ export function deriveRunStatusLabel(entries: readonly TranscriptEntry[]): { for (let i = entries.length - 1; i >= 0; i--) { const entry = entries[i]; if (entry.kind === "tool_call") { - const target = summarizeToolInput(entry.input); - const display = toolDisplayName(entry.name); + // A tail update may carry the generic placeholder name or a retitle to + // the invocation ("Terminal" → "ls -la"): the call's FIRST real name is + // its identity, and a differing later title is the invocation detail. + let name = entry.name; + let invocation: string | undefined; + if (entry.toolUseId) { + for (const prev of entries) { + if (prev === entry) break; + if ( + prev.kind === "tool_call" && + prev.toolUseId === entry.toolUseId && + !isGenericToolName(prev.name) + ) { + if (!isGenericToolName(entry.name) && entry.name !== prev.name) { + invocation = entry.name; + } + name = prev.name; + break; + } + } + } + const target = + summarizeToolInput(entry.input) ?? (invocation ? clip(invocation, TARGET_MAX) : undefined); + const display = toolDisplayName(name); return { - label: toolTaxonomy(entry.name).verbLabel, + label: toolTaxonomy(name).verbLabel, detail: target ? `${display} · ${target}` : display, - toolName: entry.name ?? undefined, + toolName: name ?? undefined, }; } if (entry.kind === "tool_result") break;