diff --git a/docs/adr/0001-recover-full-bash-command-from-session.md b/docs/adr/0001-recover-full-bash-command-from-session.md index d68f09b..ac9b942 100644 --- a/docs/adr/0001-recover-full-bash-command-from-session.md +++ b/docs/adr/0001-recover-full-bash-command-from-session.md @@ -8,12 +8,12 @@ status: accepted ## Decision -For direct calls to Pi's native `bash` tool, capture the session manager at `session_start`. During authorization, use `details.toolCallId` to find exactly one matching assistant `toolCall` block in the current session and read its structured `arguments.command` value. +For direct calls to Pi's native `bash` tool, capture the session manager at `session_start`. During authorization, walk the session in reverse to the latest assistant message containing a `toolCall` block whose `id` equals `details.toolCallId`, require exactly one such block *within that message*, and read its structured `arguments.command` value. Proceed only when all evidence agrees: - the request and recovered tool call both identify the native `bash` tool; -- the tool-call ID has exactly one match; +- the tool-call ID has exactly one match within the latest assistant message that contains it (a cross-message reuse resolves to the latest, which is the call being authorized); - `arguments.command` is a string; - the complete input matches the strict wrapper grammar; - the inner command is not another recognized wrapper. @@ -35,7 +35,8 @@ The implementation must retain regression tests for: - inner `allow`, `ask`, and `deny` mapping; - compound input such as `timeout 60s pnpm test && git push`; - unsupported and nested wrapper syntax; -- missing, duplicate, malformed, and forwarded session evidence; +- missing, within-message duplicate, malformed, and forwarded session evidence; +- a tool-call id reused across messages (resolved to the latest); - Authorizer registration and disposal. The end-to-end experiment confirmed that permission-system checks every Bash command unit but invokes the Authorizer once for the aggregated `ask`; an Authorizer `allow` then releases the complete tool call. See [context ownership](../research/ai-bash-context-ownership.md) and [minimal Bash judgment evidence](../research/ai-bash-judge-input-minimality.md) for the underlying permission and evidence boundaries. diff --git a/packages/pi-permission-inner-cmd/src/recovery.ts b/packages/pi-permission-inner-cmd/src/recovery.ts index 34759b7..13d26a0 100644 --- a/packages/pi-permission-inner-cmd/src/recovery.ts +++ b/packages/pi-permission-inner-cmd/src/recovery.ts @@ -33,17 +33,21 @@ function extractBashCommand(block: ToolCallBlock): string | undefined { /** * Recover the complete native Bash command for one tool call. * - * Walks session entries, finds the assistant `toolCall` block whose `id` equals - * `toolCallId`, and reads its structured `arguments.command`. Proceeds only - * when, per ADR 0001: + * The tool call being authorized is always the most recent one, so entries are + * walked in reverse and the search stops at the first (latest) assistant + * message that contains a `toolCall` block whose `id` equals `toolCallId`. * - * - exactly one block matches the id (no duplicate), - * - that block names the native Bash tool, - * - `arguments.command` is a string. + * Per ADR 0001, the id must match exactly one block *within that single + * message*. An earlier message reusing the same id is a stale, already-resolved + * call and is irrelevant to the current authorization; but two matching blocks + * inside one message cannot be disambiguated (we cannot tell which one + * `details.toolCallId` refers to), so that case stays fail-closed. The matched + * block must then name the native Bash tool and carry a string + * `arguments.command`. * - * Any other outcome — no match, duplicate id, a non-Bash tool call, a - * non-string command, or malformed entries — returns `undefined` so the caller - * defers fail-closed. + * Any other outcome — no match, a within-message duplicate id, a non-Bash tool + * call, a non-string command, or malformed entries — returns `undefined` so the + * caller defers fail-closed. * * @returns the complete Bash command, or `undefined`. */ @@ -51,33 +55,34 @@ export function recoverNativeBashCommand( entries: ReadonlyArray, toolCallId: string, ): string | undefined { - let matches = 0; - let command: string | undefined; - - for (const entry of entries) { + for (let i = entries.length - 1; i >= 0; i--) { + const entry = entries[i]; if (entry.type !== "message") { continue; } - const message = entry.message as { role?: unknown; content?: unknown }; + const message = entry.message; if (message.role !== "assistant") { continue; } - const content = message.content; - if (!Array.isArray(content)) { + + const matches: ToolCallBlock[] = []; + for (const block of message.content) { + if (isToolCallBlock(block) && block.id === toolCallId) { + matches.push(block); + } + } + + if (matches.length === 0) { continue; } - for (const block of content) { - if (!isToolCallBlock(block) || block.id !== toolCallId) { - continue; - } - matches += 1; - // Keep walking the whole session so a duplicate id (two matching - // blocks) is detected even when the first match was unusable. - if (matches === 1) { - command = extractBashCommand(block); - } - } + + // Latest message containing the id. Uniqueness only has to hold within + // this one message (see above); a cross-message reuse resolves to the + // latest, which is the call currently being authorized. + return matches.length === 1 + ? extractBashCommand(matches[0]) + : undefined; } - return matches === 1 ? command : undefined; + return undefined; } diff --git a/packages/pi-permission-inner-cmd/test/recovery.test.ts b/packages/pi-permission-inner-cmd/test/recovery.test.ts index 67d22ba..7f35d9e 100644 --- a/packages/pi-permission-inner-cmd/test/recovery.test.ts +++ b/packages/pi-permission-inner-cmd/test/recovery.test.ts @@ -3,10 +3,10 @@ import type { SessionEntry } from "@earendil-works/pi-coding-agent"; import { recoverNativeBashCommand } from "../src/recovery"; /** Build a minimal assistant message entry carrying the given content blocks. */ -function assistantEntry(content: unknown[]): SessionEntry { +function assistantEntry(content: unknown[], id = "entry-1"): SessionEntry { return { type: "message", - id: "entry-1", + id, parentId: null, timestamp: "2026-08-08T00:00:00.000Z", message: { @@ -156,6 +156,25 @@ describe("recoverNativeBashCommand", () => { expect(recoverNativeBashCommand(entries, "call_1")).toBeUndefined(); }); + it("returns the latest command when the id recurs across messages", () => { + // A cross-message id reuse resolves to the latest block, which is the + // call currently being authorized; the earlier block is already- + // resolved history and must not fail-closed the recovery. + const entries = [ + assistantEntry( + [bashToolCall("call_1", "timeout 30s rm -rf /")], + "entry-a", + ), + assistantEntry( + [bashToolCall("call_1", "timeout 30s pnpm test")], + "entry-b", + ), + ]; + expect(recoverNativeBashCommand(entries, "call_1")).toBe( + "timeout 30s pnpm test", + ); + }); + it("tolerates a malformed content block that is not a tool call", () => { const entries = [ assistantEntry([