fix(acp): show shell command in ACP tool calls (#32304)

Co-authored-by: Mert Can Demir <validatedev@gmail.com>
This commit is contained in:
Shoubhit Dash 2026-06-14 14:46:27 +02:00 committed by GitHub
commit 51461429f4
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
4 changed files with 93 additions and 29 deletions

View file

@ -80,10 +80,11 @@ export class Subscription {
async replayMessage(message: SessionMessageResponse) { async replayMessage(message: SessionMessageResponse) {
if (message.info.role !== "assistant" && message.info.role !== "user") return if (message.info.role !== "assistant" && message.info.role !== "user") return
const cwd = message.info.role === "assistant" ? message.info.path?.cwd : undefined
for (const part of message.parts) { for (const part of message.parts) {
await this.recordFetchedPart(message.info.sessionID, message, part) await this.recordFetchedPart(message.info.sessionID, message, part)
if (part.type === "tool") { if (part.type === "tool") {
await this.handleToolPart(message.info.sessionID, part) await this.handleToolPart(message.info.sessionID, part, cwd ?? process.cwd())
continue continue
} }
await this.replayContentPart(message, part) await this.replayContentPart(message, part)
@ -146,7 +147,7 @@ export class Subscription {
}), }),
) )
if (part.type === "tool") { if (part.type === "tool") {
await this.handleToolPart(session.id, part) await this.handleToolPart(session.id, part, session.cwd)
} }
} }
@ -231,8 +232,8 @@ export class Subscription {
) )
} }
private async handleToolPart(sessionId: string, part: ToolPart) { private async handleToolPart(sessionId: string, part: ToolPart, cwd: string) {
await this.toolStart(sessionId, part) await this.toolStart(sessionId, part, cwd)
switch (part.state.status) { switch (part.state.status) {
case "pending": case "pending":
@ -240,7 +241,7 @@ export class Subscription {
return return
case "running": case "running":
await this.runningTool(sessionId, part) await this.runningTool(sessionId, part, cwd)
return return
case "completed": case "completed":
@ -253,6 +254,7 @@ export class Subscription {
toolCallId: part.callID, toolCallId: part.callID,
toolName: part.tool, toolName: part.tool,
state: part.state, state: part.state,
cwd,
}), }),
}, },
}) })
@ -268,6 +270,7 @@ export class Subscription {
toolCallId: part.callID, toolCallId: part.callID,
toolName: part.tool, toolName: part.tool,
state: part.state, state: part.state,
cwd,
}), }),
}, },
}) })
@ -275,7 +278,7 @@ export class Subscription {
} }
} }
private async runningTool(sessionId: string, part: ToolPart) { private async runningTool(sessionId: string, part: ToolPart, cwd: string) {
if (part.state.status !== "running") return if (part.state.status !== "running") return
const output = part.tool === "bash" ? shellOutputSnapshot(part.state) : undefined const output = part.tool === "bash" ? shellOutputSnapshot(part.state) : undefined
@ -289,6 +292,7 @@ export class Subscription {
toolCallId: part.callID, toolCallId: part.callID,
toolName: part.tool, toolName: part.tool,
state: part.state, state: part.state,
cwd,
}), }),
}, },
}) })
@ -306,12 +310,13 @@ export class Subscription {
toolName: part.tool, toolName: part.tool,
state: part.state, state: part.state,
output, output,
cwd,
}), }),
}, },
}) })
} }
private async toolStart(sessionId: string, part: ToolPart) { private async toolStart(sessionId: string, part: ToolPart, cwd: string) {
if (this.toolStarts.has(part.callID)) return if (this.toolStarts.has(part.callID)) return
this.toolStarts.add(part.callID) this.toolStarts.add(part.callID)
await this.input.connection.sessionUpdate({ await this.input.connection.sessionUpdate({
@ -322,6 +327,7 @@ export class Subscription {
toolCallId: part.callID, toolCallId: part.callID,
toolName: part.tool, toolName: part.tool,
state: part.state, state: part.state,
cwd,
}), }),
}, },
}) })

View file

@ -1,3 +1,4 @@
import { isAbsolute, resolve } from "path"
import type { ToolCall, ToolCallContent, ToolCallLocation, ToolCallUpdate, ToolKind } from "@agentclientprotocol/sdk" import type { ToolCall, ToolCallContent, ToolCallLocation, ToolCallUpdate, ToolKind } from "@agentclientprotocol/sdk"
export type ToolInput = Record<string, unknown> export type ToolInput = Record<string, unknown>
@ -69,10 +70,16 @@ export function toToolKind(toolName: string): ToolKind {
} }
} }
export function toLocations(toolName: string, input: ToolInput): ToolCallLocation[] { export function toLocations(toolName: string, input: ToolInput, cwd?: string): ToolCallLocation[] {
const tool = toolName.toLocaleLowerCase() const tool = toolName.toLocaleLowerCase()
switch (tool) { switch (tool) {
case "bash":
case "shell": {
const workdir = shellWorkdir(input, cwd)
return workdir ? [{ path: workdir }] : []
}
case "read": case "read":
case "edit": case "edit":
case "write": case "write":
@ -88,10 +95,6 @@ export function toLocations(toolName: string, input: ToolInput): ToolCallLocatio
case "context7_get_library_docs": case "context7_get_library_docs":
return locationFrom(input.path) return locationFrom(input.path)
case "bash":
case "shell":
return []
default: default:
return [] return []
} }
@ -122,14 +125,15 @@ export function pendingToolCall(input: {
readonly toolCallId: string readonly toolCallId: string
readonly toolName: string readonly toolName: string
readonly state: { readonly input: ToolInput; readonly title?: string } readonly state: { readonly input: ToolInput; readonly title?: string }
readonly cwd?: string
}): ToolCall { }): ToolCall {
return { return {
toolCallId: input.toolCallId, toolCallId: input.toolCallId,
title: input.state.title || input.toolName, title: toolTitle(input.toolName, input.state.input, input.state.title),
kind: toToolKind(input.toolName), kind: toToolKind(input.toolName),
status: "pending", status: "pending",
locations: toLocations(input.toolName, input.state.input), locations: toLocations(input.toolName, input.state.input, input.cwd),
rawInput: input.state.input, rawInput: rawInput(input.toolName, input.state.input, input.cwd),
} }
} }
@ -138,6 +142,7 @@ export function runningToolUpdate(input: {
readonly toolName: string readonly toolName: string
readonly state: RunningToolState readonly state: RunningToolState
readonly output?: string readonly output?: string
readonly cwd?: string
}): ToolCallUpdate { }): ToolCallUpdate {
const content = input.output const content = input.output
? [ ? [
@ -155,9 +160,9 @@ export function runningToolUpdate(input: {
toolCallId: input.toolCallId, toolCallId: input.toolCallId,
status: "in_progress", status: "in_progress",
kind: toToolKind(input.toolName), kind: toToolKind(input.toolName),
title: input.state.title ?? input.toolName, title: toolTitle(input.toolName, input.state.input, input.state.title),
locations: toLocations(input.toolName, input.state.input), locations: toLocations(input.toolName, input.state.input, input.cwd),
rawInput: input.state.input, rawInput: rawInput(input.toolName, input.state.input, input.cwd),
...(content ? { content } : {}), ...(content ? { content } : {}),
} }
} }
@ -166,29 +171,32 @@ export function duplicateRunningToolUpdate(input: {
readonly toolCallId: string readonly toolCallId: string
readonly toolName: string readonly toolName: string
readonly state: RunningToolState readonly state: RunningToolState
readonly cwd?: string
}): ToolCallUpdate { }): ToolCallUpdate {
return { return {
toolCallId: input.toolCallId, toolCallId: input.toolCallId,
status: "in_progress", status: "in_progress",
kind: toToolKind(input.toolName), kind: toToolKind(input.toolName),
title: input.state.title ?? input.toolName, title: toolTitle(input.toolName, input.state.input, input.state.title),
locations: toLocations(input.toolName, input.state.input), locations: toLocations(input.toolName, input.state.input, input.cwd),
rawInput: input.state.input, rawInput: rawInput(input.toolName, input.state.input, input.cwd),
} }
} }
export function completedToolUpdate(input: { export function completedToolUpdate(input: {
readonly toolCallId: string readonly toolCallId: string
readonly toolName: string readonly toolName: string
readonly state: CompletedToolState & { readonly title: string } readonly state: CompletedToolState & { readonly title?: string }
readonly cwd?: string
}): ToolCallUpdate { }): ToolCallUpdate {
return { return {
toolCallId: input.toolCallId, toolCallId: input.toolCallId,
status: "completed", status: "completed",
kind: toToolKind(input.toolName), kind: toToolKind(input.toolName),
title: input.state.title, title: toolTitle(input.toolName, input.state.input, input.state.title),
locations: toLocations(input.toolName, input.state.input, input.cwd),
content: completedToolContent(input.toolName, input.state), content: completedToolContent(input.toolName, input.state),
rawInput: input.state.input, rawInput: rawInput(input.toolName, input.state.input, input.cwd),
rawOutput: completedToolRawOutput(input.state), rawOutput: completedToolRawOutput(input.state),
} }
} }
@ -197,13 +205,15 @@ export function errorToolUpdate(input: {
readonly toolCallId: string readonly toolCallId: string
readonly toolName: string readonly toolName: string
readonly state: ErrorToolState readonly state: ErrorToolState
readonly cwd?: string
}): ToolCallUpdate { }): ToolCallUpdate {
return { return {
toolCallId: input.toolCallId, toolCallId: input.toolCallId,
status: "failed", status: "failed",
kind: toToolKind(input.toolName), kind: toToolKind(input.toolName),
title: input.toolName, title: toolTitle(input.toolName, input.state.input, undefined),
rawInput: input.state.input, locations: toLocations(input.toolName, input.state.input, input.cwd),
rawInput: rawInput(input.toolName, input.state.input, input.cwd),
content: [ content: [
{ {
type: "content", type: "content",
@ -253,6 +263,42 @@ export function shellOutputSnapshot(state: { readonly metadata?: unknown }) {
return stringValue((state.metadata as Record<string, unknown>).output) return stringValue((state.metadata as Record<string, unknown>).output)
} }
// For shell tools, surface the actual command as the title so it stays visible
// before output lands; non-shell tools keep their model-provided title.
function toolTitle(toolName: string, input: ToolInput, fallback: string | undefined) {
if (isShell(toolName)) return shellCommand(input) ?? stringValue(input.description) ?? fallback ?? toolName
return fallback || toolName
}
// Enrich shell rawInput with the resolved working directory so clients can show
// where the command runs, unless the model already specified one.
function rawInput(toolName: string, input: ToolInput, cwd?: string): ToolInput {
if (!isShell(toolName)) return input
if (input.cwd || input.workdir) return input
const workdir = shellWorkdir(input, cwd)
return workdir ? { ...input, cwd: workdir } : input
}
function shellWorkdir(input: ToolInput, cwd?: string) {
const explicit = stringValue(input.workdir) ?? stringValue(input.cwd)
return resolvePath(explicit, cwd) ?? cwd
}
function resolvePath(value: string | undefined, cwd?: string) {
if (!value) return undefined
if (isAbsolute(value)) return value
return resolve(cwd ?? process.cwd(), value)
}
function shellCommand(input: ToolInput) {
return stringValue(input.command) ?? stringValue(input.cmd)
}
function isShell(toolName: string) {
const tool = toolName.toLocaleLowerCase()
return tool === "bash" || tool === "shell"
}
export const mapToolKind = toToolKind export const mapToolKind = toToolKind
export const extractLocations = toLocations export const extractLocations = toLocations
export const buildCompletedToolContent = completedToolContent export const buildCompletedToolContent = completedToolContent

View file

@ -517,7 +517,7 @@ describe("acp event routing", () => {
expect(harness.updates).toHaveLength(0) expect(harness.updates).toHaveLength(0)
}) })
it("emits synthetic pending before the first running tool update", async () => { it("exposes the shell command on the synthetic pending tool call", async () => {
const harness = createHarness() const harness = createHarness()
await Effect.runPromise(harness.session.create({ id: "ses_tool", cwd: "/workspace" })) await Effect.runPromise(harness.session.create({ id: "ses_tool", cwd: "/workspace" }))
@ -527,7 +527,14 @@ describe("acp event routing", () => {
"tool_call", "tool_call",
"tool_call_update", "tool_call_update",
]) ])
expect(harness.updates[0]?.update).toMatchObject({ status: "pending", toolCallId: "call_1" }) expect(harness.updates[0]?.update).toMatchObject({
status: "pending",
toolCallId: "call_1",
title: "printf hello",
kind: "execute",
locations: [{ path: "/workspace" }],
rawInput: { cmd: "printf hello", cwd: "/workspace" },
})
expect(harness.updates[1]?.update).toMatchObject({ status: "in_progress", toolCallId: "call_1" }) expect(harness.updates[1]?.update).toMatchObject({ status: "in_progress", toolCallId: "call_1" })
}) })

View file

@ -37,7 +37,12 @@ describe("acp tool conversion", () => {
expect(toLocations("external_directory", { directories: ["/tmp/outside"], patterns: ["/tmp/outside/*"] })).toEqual([ expect(toLocations("external_directory", { directories: ["/tmp/outside"], patterns: ["/tmp/outside/*"] })).toEqual([
{ path: "/tmp/outside" }, { path: "/tmp/outside" },
]) ])
expect(toLocations("bash", { filePath: "/tmp/nope.ts", path: "/tmp" })).toEqual([]) expect(toLocations("bash", { cmd: "pwd" }, "/workspace")).toEqual([{ path: "/workspace" }])
expect(toLocations("bash", { command: "pwd", workdir: "subdir" }, "/workspace")).toEqual([
{ path: "/workspace/subdir" },
])
expect(toLocations("bash", { command: "pwd", workdir: "/abs/dir" }, "/workspace")).toEqual([{ path: "/abs/dir" }])
expect(toLocations("bash", { command: "printf hello" })).toEqual([])
expect(toLocations("read", { path: "/tmp/missing-file-path.ts" })).toEqual([]) expect(toLocations("read", { path: "/tmp/missing-file-path.ts" })).toEqual([])
}) })