diff --git a/src/acp-agent.ts b/src/acp-agent.ts index 21f111c2..ca6f3920 100644 --- a/src/acp-agent.ts +++ b/src/acp-agent.ts @@ -240,6 +240,7 @@ import { type FileChangeReportTurnState, type FileChangeReportUnavailableReason, type NativeFileChangeReporter, + recordSubagentFileEdits, supportsAgentFileChangeReport, } from "./file-change-audit.js"; import { @@ -733,6 +734,9 @@ type Turn = { * turn. The state is turn-owned so a late control response can never be * rebound to a newer prompt. */ fileChangeReport?: FileChangeReportTurnState; + /** Files that subagents of this turn edited with a file tool, collected only + * when a report was requested (see {@link recordSubagentFileEdits}). */ + editedPaths?: Set; /** Set once the deferred has been resolved/rejected, so the consumer never * settles a turn twice (idle + handoff + stream-end can all race). */ settled: boolean; @@ -7770,6 +7774,12 @@ export class ClaudeAcpAgent { let runningTerminal: string | undefined; if (message.type === "user") { rememberResolvedToolNames(session, content); + recordSubagentFileEdits( + session.activeTurn, + message.parent_tool_use_id, + content, + session.toolUseCache, + ); const backgroundBashTask = backgroundBashTaskFromToolResult( content, message.tool_use_result, diff --git a/src/file-change-audit.ts b/src/file-change-audit.ts index 9a654746..94adf3ea 100644 --- a/src/file-change-audit.ts +++ b/src/file-change-audit.ts @@ -19,6 +19,10 @@ export type FileChangeReportTurnState = { export type NativeFileChangeReportTurn = { promptUuid: string; fileChangeReport?: FileChangeReportTurnState; + /** Raw `file_path`/`notebook_path` values of the file-tool edits that + * subagents of this turn completed. Checkpoints do not track files that + * only a subagent edited. */ + editedPaths?: Set; }; export type FileChangeReportWorkspace = { @@ -94,6 +98,41 @@ export function agentFileChangeReportMeta( return withAirMeta(undefined, AGENT_FILE_CHANGE_REPORT_CAPABILITY, result); } +const FILE_EDIT_TOOLS = new Set(["Edit", "Write", "MultiEdit", "NotebookEdit"]); + +/** + * Adds to `turn.editedPaths` the `file_path` (`notebook_path` for NotebookEdit) + * of each successful file-tool edit among the tool results of a subagent + * message. Root-agent messages (`parentToolUseId === null`) are skipped: the + * checkpoint already tracks their files. Must run before `toAcpNotifications` + * drops the tool use cache entries. + */ +export function recordSubagentFileEdits( + turn: (NativeFileChangeReportTurn & { settled: boolean }) | null | undefined, + parentToolUseId: string | null, + content: unknown, + toolUses: Record, +): void { + if (parentToolUseId === null || !turn || turn.settled || !turn.fileChangeReport) return; + if (!Array.isArray(content)) return; + for (const block of content) { + if ( + block?.type !== "tool_result" || + block.is_error === true || + typeof block.tool_use_id !== "string" + ) { + continue; + } + const toolUse = toolUses[block.tool_use_id]; + if (!toolUse || !FILE_EDIT_TOOLS.has(toolUse.name)) continue; + const input = toolUse.input as Record | undefined; + const editedPath = toolUse.name === "NotebookEdit" ? input?.notebook_path : input?.file_path; + if (typeof editedPath === "string" && editedPath.length > 0) { + (turn.editedPaths ??= new Set()).add(editedPath); + } + } +} + export function createNativeFileChangeReporter( options: NativeFileChangeReporterOptions, ): NativeFileChangeReporter { @@ -101,7 +140,7 @@ export function createNativeFileChangeReporter( const timeoutMs = options.timeoutMs ?? AGENT_FILE_CHANGE_REPORT_TIMEOUT_MS; const requestIds = new Set(); - const publish = async (state: FileChangeReportTurnState, result: AgentFileChangeReportResult) => { + async function publish(state: FileChangeReportTurnState, result: AgentFileChangeReportResult) { if (state.phase === "finished") return; state.phase = "finished"; try { @@ -109,24 +148,55 @@ export function createNativeFileChangeReporter( } catch (error) { options.logError(`Failed to publish file-change report ${state.requestId}: ${error}`); } - }; + } - const finishUnavailable = async ( + async function finishUnavailable( state: FileChangeReportTurnState, reason: FileChangeReportUnavailableReason, - ) => { + ) { await publish(state, { version: 1, requestId: state.requestId, status: "unavailable", reason, }); - }; + } - const report = async ( + async function publishReported(state: FileChangeReportTurnState, reportedPaths: string[]) { + const normalized = normalizeReportedPaths(reportedPaths, workspace); + await publish( + state, + fitReportedAgentFileChangeReport({ + version: 1, + requestId: state.requestId, + status: "reported", + paths: normalized.paths, + // Checkpoints miss Bash mutations. Subagent file-tool edits are added + // from the message stream, but the list is still not exhaustive. + declaredComplete: false, + truncated: normalized.truncated, + }), + ); + } + + // Used when the checkpoint preview fails: the subagent edits seen in the + // stream are still worth reporting. + async function finishWithoutPreview( + state: FileChangeReportTurnState, + editedPaths: Set | undefined, + reason: FileChangeReportUnavailableReason, + ) { + if (editedPaths && editedPaths.size > 0) { + await publishReported(state, [...editedPaths]); + } else { + await finishUnavailable(state, reason); + } + } + + async function report( turn: NativeFileChangeReportTurn | null | undefined, query: Pick, - ) => { + ) { const state = turn?.fileChangeReport; if (!state || state.phase !== "requested") return; state.phase = "collecting"; @@ -145,35 +215,22 @@ export function createNativeFileChangeReporter( }), ]); if (preview === null) { - await finishUnavailable(state, "timeout"); + await finishWithoutPreview(state, turn.editedPaths, "timeout"); return; } if (!preview.canRewind || !Array.isArray(preview.filesChanged)) { - await finishUnavailable(state, "invalidOutput"); + await finishWithoutPreview(state, turn.editedPaths, "invalidOutput"); return; } - const normalized = normalizeReportedPaths(preview.filesChanged, workspace); - await publish( - state, - fitReportedAgentFileChangeReport({ - version: 1, - requestId: state.requestId, - status: "reported", - paths: normalized.paths, - // Checkpoints cover Claude file tools, but not every mutation source - // (notably Bash and most subagents), so this list is never exhaustive. - declaredComplete: false, - truncated: normalized.truncated, - }), - ); + await publishReported(state, [...preview.filesChanged, ...(turn.editedPaths ?? [])]); } catch (error) { options.logError(`Failed to inspect Claude file checkpoint ${turn.promptUuid}: ${error}`); - await finishUnavailable(state, "providerError"); + await finishWithoutPreview(state, turn.editedPaths, "providerError"); } finally { if (timer) clearTimeout(timer); } - }; + } return { request(meta) { diff --git a/src/tests/file-change-audit-integration.test.ts b/src/tests/file-change-audit-integration.test.ts index 8537d4a7..af7fe690 100644 --- a/src/tests/file-change-audit-integration.test.ts +++ b/src/tests/file-change-audit-integration.test.ts @@ -11,7 +11,13 @@ import { import type { Pushable } from "../utils.js"; type Scenario = - "reported" | "providerError" | "localOnly" | "waitForCancel" | "waitForCheckpointCancel"; + | "reported" + | "providerError" + | "localOnly" + | "waitForCancel" + | "waitForCheckpointCancel" + | "subagentEdit" + | "subagentEditError"; let scenario: Scenario = "reported"; let observedOptions: Options | undefined; let replayMessages: unknown[] = []; @@ -47,7 +53,10 @@ vi.mock("@anthropic-ai/claude-agent-sdk", async () => { } return Promise.resolve({ canRewind: true, - filesChanged: ["src/changed.ts"], + filesChanged: + scenario === "subagentEdit" || scenario === "subagentEditError" + ? ["README.md"] + : ["src/changed.ts"], insertions: 4, deletions: 1, }); @@ -172,6 +181,48 @@ async function* runTurn(input: Pushable, interrupted: Promise { resolveCheckpointPreview(); await agent.dispose(); }); + + /** A subagent's successful file-tool edit is added to the checkpoint paths. */ + it("reports files that only a subagent edited alongside the checkpoint paths", async () => { + scenario = "subagentEdit"; + const updates: SessionNotification[] = []; + const { agent, sessionId } = await createAgent(updates); + + await agent.prompt(prompt(sessionId, "request-subagent", "Delegate an edit")); + + expect(reports(updates)).toEqual([ + { + version: 1, + requestId: "request-subagent", + status: "reported", + paths: [path.join(process.cwd(), "README.md"), path.join(process.cwd(), "GEMINI.md")], + declaredComplete: false, + truncated: false, + }, + ]); + await agent.dispose(); + }); + + /** A subagent edit whose tool result is an error did not change the file. */ + it("does not report a subagent edit that failed", async () => { + scenario = "subagentEditError"; + const updates: SessionNotification[] = []; + const { agent, sessionId } = await createAgent(updates); + + await agent.prompt(prompt(sessionId, "request-subagent-error", "Delegate an edit")); + + expect(reports(updates)).toEqual([ + { + version: 1, + requestId: "request-subagent-error", + status: "reported", + paths: [path.join(process.cwd(), "README.md")], + declaredComplete: false, + truncated: false, + }, + ]); + await agent.dispose(); + }); }); diff --git a/src/tests/file-change-audit.test.ts b/src/tests/file-change-audit.test.ts index af3fe155..76cf25ac 100644 --- a/src/tests/file-change-audit.test.ts +++ b/src/tests/file-change-audit.test.ts @@ -8,6 +8,7 @@ import { createNativeFileChangeReporter, type AgentFileChangeReportResult, type NativeFileChangeReporter, + recordSubagentFileEdits, } from "../file-change-audit.js"; function createSupport(options: { @@ -26,7 +27,11 @@ function createSupport(options: { }); } -function requestedTurn(reporter: NativeFileChangeReporter, requestId: string) { +function requestedTurn( + reporter: NativeFileChangeReporter, + requestId: string, + editedPaths?: string[], +) { const fileChangeReport = reporter.request({ jetbrains: { air: { @@ -35,7 +40,11 @@ function requestedTurn(reporter: NativeFileChangeReporter, requestId: string) { }, }); expect(fileChangeReport).toBeDefined(); - return { promptUuid: "prompt-uuid", fileChangeReport }; + return { + promptUuid: "prompt-uuid", + fileChangeReport, + ...(editedPaths ? { editedPaths: new Set(editedPaths) } : {}), + }; } describe("native agent file-change report", () => { @@ -235,4 +244,163 @@ describe("native agent file-change report", () => { expect(turn.fileChangeReport?.phase).toBe("finished"); expect(logError).toHaveBeenCalledWith(expect.stringContaining("transport closed")); }); + + /** Subagent-edited paths are unioned with the checkpoint paths and deduped. */ + it("adds subagent edited paths to the checkpoint paths", async () => { + const published: AgentFileChangeReportResult[] = []; + const cwd = process.cwd(); + const support = createSupport({ cwd, publish: async (result) => void published.push(result) }); + const turn = requestedTurn(support, "request-union", [path.join(cwd, "GEMINI.md"), "src/a.ts"]); + + await support.report(turn, { + rewindFiles: vi.fn(async () => ({ + canRewind: true, + filesChanged: ["src/a.ts", "README.md"], + })), + }); + + expect(published).toMatchObject([ + { + status: "reported", + paths: [ + path.join(cwd, "src/a.ts"), + path.join(cwd, "README.md"), + path.join(cwd, "GEMINI.md"), + ], + declaredComplete: false, + }, + ]); + }); + + /** When the checkpoint cannot be previewed, the subagent edits are still reported. */ + it("reports subagent edited paths when the checkpoint cannot be previewed", async () => { + const published: AgentFileChangeReportResult[] = []; + const cwd = process.cwd(); + const support = createSupport({ cwd, publish: async (result) => void published.push(result) }); + const turn = requestedTurn(support, "request-fallback", ["GEMINI.md"]); + + await support.report(turn, { + rewindFiles: vi.fn(async () => ({ canRewind: false, error: "checkpoint missing" })), + }); + + expect(published).toEqual([ + { + version: 1, + requestId: "request-fallback", + status: "reported", + paths: [path.join(cwd, "GEMINI.md")], + declaredComplete: false, + truncated: false, + }, + ]); + }); + + /** A checkpoint preview timeout falls back to the subagent edited paths. */ + it("reports subagent edited paths when the checkpoint preview times out", async () => { + const published: AgentFileChangeReportResult[] = []; + const cwd = process.cwd(); + const support = createSupport({ + cwd, + publish: async (result) => void published.push(result), + timeoutMs: 1, + }); + const turn = requestedTurn(support, "request-timeout-fallback", ["GEMINI.md"]); + const never = new Promise(() => {}); + + await support.report(turn, { rewindFiles: vi.fn(() => never) }); + + expect(published).toMatchObject([ + { status: "reported", paths: [path.join(cwd, "GEMINI.md")], declaredComplete: false }, + ]); + }); + + /** A thrown checkpoint error falls back to the subagent edited paths. */ + it("reports subagent edited paths when the checkpoint preview throws", async () => { + const published: AgentFileChangeReportResult[] = []; + const cwd = process.cwd(); + const support = createSupport({ cwd, publish: async (result) => void published.push(result) }); + const turn = requestedTurn(support, "request-throw-fallback", ["GEMINI.md"]); + + await support.report(turn, { + rewindFiles: vi.fn(async () => { + throw new Error("boom"); + }), + }); + + expect(published).toMatchObject([{ status: "reported", paths: [path.join(cwd, "GEMINI.md")] }]); + }); + + /** Without subagent edits an unusable checkpoint is still unavailable. */ + it("stays unavailable when there are no subagent edited paths", async () => { + const published: AgentFileChangeReportResult[] = []; + const support = createSupport({ publish: async (result) => void published.push(result) }); + const turn = requestedTurn(support, "request-empty", []); + + await support.report(turn, { + rewindFiles: vi.fn(async () => ({ canRewind: false, error: "checkpoint missing" })), + }); + + expect(published).toEqual([ + { version: 1, requestId: "request-empty", status: "unavailable", reason: "invalidOutput" }, + ]); + }); +}); + +describe("recordSubagentFileEdits", () => { + const toolUses = { + edit: { name: "Edit", input: { file_path: "/work/a.ts" } }, + write: { name: "Write", input: { file_path: "/work/b.ts" } }, + notebook: { name: "NotebookEdit", input: { notebook_path: "/work/n.ipynb" } }, + read: { name: "Read", input: { file_path: "/work/c.ts" } }, + }; + const results = (...ids: string[]) => + ids.map((id) => ({ type: "tool_result", tool_use_id: id, content: "ok" })); + const reportTurn = () => ({ + promptUuid: "prompt-uuid", + fileChangeReport: { requestId: "request", phase: "requested" as const }, + settled: false, + editedPaths: undefined as Set | undefined, + }); + + /** Edit, Write and NotebookEdit results of a subagent add their paths; Read does not. */ + it("collects the paths of subagent file-tool edits", () => { + const turn = reportTurn(); + recordSubagentFileEdits( + turn, + "toolu_agent", + results("edit", "write", "notebook", "read"), + toolUses, + ); + expect([...(turn.editedPaths ?? [])]).toEqual(["/work/a.ts", "/work/b.ts", "/work/n.ipynb"]); + }); + + /** A failed subagent Edit changed no file, so its path is skipped. */ + it("skips error results", () => { + const turn = reportTurn(); + recordSubagentFileEdits( + turn, + "toolu_agent", + [{ type: "tool_result", tool_use_id: "edit", content: "failed", is_error: true }], + toolUses, + ); + expect(turn.editedPaths).toBeUndefined(); + }); + + /** Root-agent edits are left to the checkpoint, so a root message adds nothing. */ + it("skips root-agent messages", () => { + const turn = reportTurn(); + recordSubagentFileEdits(turn, null, results("edit"), toolUses); + expect(turn.editedPaths).toBeUndefined(); + }); + + /** A settled turn, or one without a requested report, collects nothing. */ + it("skips turns that are settled or have no report", () => { + const settled = { ...reportTurn(), settled: true }; + recordSubagentFileEdits(settled, "toolu_agent", results("edit"), toolUses); + expect(settled.editedPaths).toBeUndefined(); + + const unrequested = { ...reportTurn(), fileChangeReport: undefined }; + recordSubagentFileEdits(unrequested, "toolu_agent", results("edit"), toolUses); + expect(unrequested.editedPaths).toBeUndefined(); + }); });