diff --git a/apps/cli/test/edit-tool-no-full-redraw.test.ts b/apps/cli/test/edit-tool-no-full-redraw.test.ts index fc84da19..24292913 100644 --- a/apps/cli/test/edit-tool-no-full-redraw.test.ts +++ b/apps/cli/test/edit-tool-no-full-redraw.test.ts @@ -198,6 +198,59 @@ describe("edit tool TUI rendering", () => { expect(rendered).toContain("line 150 changed"); }); + it("keeps the applied change on a Step card whose call failed after writing", async () => { + const dir = await mkdtemp(join(tmpdir(), "pi-edit-failed-")); + tempDirs.push(dir); + const filePath = join(dir, "cancelled-check.txt"); + await writeFile(filePath, "before\n", "utf8"); + + const terminal = new FakeTerminal(); + const tui: TUI = new TuiMainScreen(terminal); + const component = new ToolExecutionComponent( + "edit", + "tool-call-failed", + { path: filePath, edits: [{ oldText: "before", newText: "after" }] }, + { presentation: "step" }, + createEditToolDefinition(process.cwd()), + tui, + process.cwd(), + ); + tui.addChild(component); + tui.start(); + await waitForRender(); + + // The edit landed, then the check the call ran alongside was cancelled: the + // recorded change is real, the call is not. Recomputed from the args against + // the file it already changed, the preview finds no match and reports an error + // of its own, which is not what the call did. + await writeFile(filePath, "after\n", "utf8"); + component.setArgsComplete(); + tui.requestRender(); + await waitForRender(); + await waitForRender(); + // The args-based preview, computed against the file the call already changed. + expect(component.render(200).join("\n")).toContain("Could not find"); + component.updateResult( + { + content: [{ type: "text", text: "Successfully replaced 1 occurrence(s) in cancelled-check.txt.\n\nthen_run: $ check\nCommand aborted" }], + details: { diff: "-1 before\n+1 after\n", patch: "--- cancelled-check.txt\n+++ cancelled-check.txt\n@@ -1,1 +1,1 @@\n-before\n+after\n", firstChangedLine: 1 }, + isError: true, + }, + false, + ); + tui.requestRender(); + await waitForRender(); + await waitForRender(); + + const rendered = component.render(200).join("\n"); + // The diff comes from the result, so it replaces the stale preview error. + expect(rendered).toContain("-1 before"); + expect(rendered).toContain("+1 after"); + expect(rendered).not.toContain("Could not find"); + // The call still reads as failed. + expect(rendered).toContain("Command aborted"); + }); + it("shows a preflight error without rendering a diff when the edits do not apply", async () => { const dir = await mkdtemp(join(tmpdir(), "pi-edit-preflight-")); tempDirs.push(dir); @@ -232,4 +285,44 @@ describe("edit tool TUI rendering", () => { expect(rendered).not.toContain("+1 "); expect(rendered).not.toContain("-1 "); }); + + it("shows no diff for a failed call that recorded nothing", async () => { + const dir = await mkdtemp(join(tmpdir(), "pi-edit-nodetail-")); + tempDirs.push(dir); + const filePath = join(dir, "unchanged.txt"); + await writeFile(filePath, "line 0\n", "utf8"); + + const terminal = new FakeTerminal(); + const tui: TUI = new TuiMainScreen(terminal); + const component = new ToolExecutionComponent( + "edit", + "tool-call-nodetail", + { path: filePath, edits: [{ oldText: "does not exist", newText: "replacement" }] }, + { presentation: "step" }, + createEditToolDefinition(process.cwd()), + tui, + process.cwd(), + ); + tui.addChild(component); + tui.start(); + await waitForRender(); + component.setArgsComplete(); + tui.requestRender(); + await waitForRender(); + + // The renderer reads a result's diff whatever the call's own status is, so a + // failure that carries no change record must not invent one. + component.updateResult( + { content: [{ type: "text", text: "No matches for search string in unchanged.txt" }], details: {}, isError: true }, + false, + ); + tui.requestRender(); + await waitForRender(); + await waitForRender(); + + const rendered = component.render(200).join("\n"); + expect(rendered).toContain("No matches"); + expect(rendered).not.toContain("-1 line 0"); + expect(rendered).not.toContain("+1 "); + }); }); diff --git a/packages/coding-agent/src/core/tools/edit.ts b/packages/coding-agent/src/core/tools/edit.ts index 47c6a02f..b95c031a 100644 --- a/packages/coding-agent/src/core/tools/edit.ts +++ b/packages/coding-agent/src/core/tools/edit.ts @@ -416,7 +416,10 @@ export function createEditToolDefinition( ? JSON.stringify({ path: previewInput.path, edits: previewInput.edits }) : undefined; const typedResult = result as EditToolResultLike; - const resultDiff = !context.isError ? typedResult.details?.diff : undefined; + // A call can fail after its edit already landed (a cancelled `then_run`, a + // result-preparation failure). Its details still describe the change, so show + // them like any applied edit; a failure before the write carries no diff. + const resultDiff = typedResult.details?.diff; let changed = false; if (callComponent) { if (typeof resultDiff === "string") { diff --git a/packages/coding-agent/test/step-then-run-command-parity.test.ts b/packages/coding-agent/test/step-then-run-command-parity.test.ts index 964b4193..806d4ff3 100644 --- a/packages/coding-agent/test/step-then-run-command-parity.test.ts +++ b/packages/coding-agent/test/step-then-run-command-parity.test.ts @@ -258,6 +258,57 @@ describe("then_run uses run_command execution", () => { expect(await readFile(join(directory, "sample.txt"), "utf8")).toBe("applied\n"); }); + it("still shows the edit diff after the caller cancels the check", async () => { + initTheme("dark", false); + const controller = new AbortController(); + const exec = vi.fn(async () => { + controller.abort(); + throw new Error("aborted"); + }); + const { edit } = profile({ exec }); + await writeFile(join(directory, "sample.txt"), "before\n"); + const args = { path: "sample.txt", search: "before", replace: "after", then_run: "check" }; + const failure = await edit.execute("edit-cancelled", args, controller.signal, undefined, undefined as never).then( + () => undefined, + (error: { result?: { details?: { diff?: unknown; patch?: unknown }; isError?: boolean }; name?: string }) => + error, + ); + // The edit landed before the check was cancelled, so the failed call still + // carries the receipt its renderer reads. + expect(await readFile(join(directory, "sample.txt"), "utf8")).toBe("after\n"); + expect(failure?.name).toBe("ToolExecutionError"); + expect(failure?.result?.isError).toBe(true); + expect(failure?.result?.details?.diff).toBe("-1 before\n+1 after"); + expect(failure?.result?.details?.patch).toEqual(expect.stringContaining("\n-before\n+after")); + + // Rendering the failed call must not hide that change: the args-based preview + // cannot apply after the write, so only the recorded diff can show it. + const state: { callComponent?: { render(width: number): string[] } } = {}; + const context = { + args, + toolCallId: "edit-cancelled", + state, + cwd: directory, + executionStarted: true, + argsComplete: true, + isPartial: false, + expanded: true, + showImages: false, + invalidate() {}, + }; + edit.renderCall!(args, theme, { ...context, lastComponent: undefined, isError: false } as never); + edit.renderResult!(failure!.result as never, { expanded: true, isPartial: false }, theme, { + ...context, + lastComponent: undefined, + isError: true, + } as never); + + const rendered = state.callComponent?.render(200).join("\n") ?? ""; + // Both sides of the recorded diff, which no other part of the card produces. + expect(rendered).toContain("-1 before"); + expect(rendered).toContain("+1 after"); + }); + it("retains an applied mutation while reporting a real check executor fault", async () => { const exec = vi.fn(async () => { throw new Error("executor unavailable");