jerry/claude-agent-acp
1
0
Fork 0
mirror of https://github.com/agentclientprotocol/claude-agent-acp.git synced 2026-10-27 04:30:59 +00:00
Code Issues Packages Projects Releases Wiki Activity

fix: Only resolve a denied tool call the client was told about (#923)

Browse Source 
`permission_denied` forwarded an SDK frame as a `tool_call_update`
without checking that the referenced `tool_call` was ever announced,
leaving it the last emitter without that check after #916.

The id is the executing call's own, and the frame is enqueued from
inside canUseTool, so it lands between the call's `tool_use` and the
`tool_result` carrying the rejection — verified against the bundled CLI,
where the deny short-circuit only emits for pre-ask decisions (rules,
mode, classifier, asyncAgent) and fires exactly once per denial. In that
flow the call is announced and in flight, so the guard changes nothing.

It changes the denials that arrive outside that window. The
cancelled-turn guard drops the assistant message carrying the tool_use,
so no `tool_call` is emitted, and a denial for it can still land
afterwards — the same premise the `tool_result` fallback in
`toAcpNotifications` gates on `wasEmitted` for. Ids leave
`emittedToolCalls` at `tool_result`, so the check also stops a denial
trailing its own result from flipping a call the client has seen finish
back to failed.

While here: a denial raised inside a subagent identifies it by
`agent_id`, never by the Agent/Task call that spawned it, so the update
went out unattributed while the `tool_call` it resolves sits in the
subagent's transcript. Resolve the parent through `liveBackgroundTasks`,
as the canUseTool path already does, and leave the update unattributed
rather than dropped when `task_started` was never seen for that agent
id.

`permission_denied` had no test coverage. Add cases for the in-flight
resolve, the reason fallback, the never-announced and already-resolved
drops, and subagent attribution with and without a recorded parent.

Fixes #918

Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
Ben Brandt
2026-07-27 12:49:30 +02:00
committed by GitHub
parent 5559ba890c
commit 8f67b6a92b
2 changed files with 257 additions and 0 deletions
Download Patch File Download Diff File Expand all files Collapse all files

23
src/acp-agent.ts
View File

@@ -2768,6 +2768,28 @@ export class ClaudeAcpAgent {
// already emitted as a `tool_call`, so mark it failed with the
// rejection reason — otherwise the client shows a tool call
// that silently never resolves.
//
// The id is the executing call's own, and the frame lands
// between its `tool_use` and its `tool_result` (the SDK enqueues
// it from inside canUseTool), so the call is normally in flight
// here. Not always: the assistant message carrying the tool_use
// is dropped by the cancelled-turn guard below, and a denial for
// it can still arrive afterwards — the case the `tool_result`
// fallback in `toAcpNotifications` gates on `wasEmitted` for.
// Drop the update rather than reference a tool call the client
// was never given (see `ensureToolCallEmitted`, issue #851).
if (!session.emittedToolCalls.has(message.tool_use_id)) {
break;
}
// A denial inside a subagent identifies the subagent by
// `agent_id` (as canUseTool does with `agentID`), never by the
// Agent/Task call that spawned it. Resolve it the same way so
// the update lands in the subagent's transcript alongside the
// `tool_call` it resolves, which carries the parent stamped from
// `parent_tool_use_id` (see `liveBackgroundTasks`).
const parentToolUseId = message.agent_id
? session.liveBackgroundTasks.get(message.agent_id)?.parentToolUseId
: undefined;
const reason = message.decision_reason ?? message.message;
await sendUpdate({
sessionId: message.session_id,
@@ -2784,6 +2806,7 @@ export class ClaudeAcpAgent {
_meta: {
claudeCode: {
toolName: message.tool_name,
...(parentToolUseId ? { parentToolUseId } : {}),
toolResponse: {
decisionReasonType: message.decision_reason_type,
decisionReason: message.decision_reason,