fix(web): list the file change a failed edit or write already applied

apps/web/AGENTS.md

@@ -330,7 +330,8 @@ When to add or change one:
   protection, or bring back a Host-owned browser.
 - File delivery recognition is a frontend projection, not a persisted or
   filesystem-verified registry. Settled turns collect local Markdown links
-  and supported structured successful write/edit outputs; inline basenames
+  and supported structured write/edit outputs that recorded a change (a call
+  whose own error came after the write still counts); inline basenames
   require an unambiguous observed output. References use the existing
   workspace file API and containment checks, never browser `file:` navigation.
   Delivery cards (under the closing reply of a turn) toggle only their own
@@ -344,6 +345,11 @@ When to add or change one:
   profile Web sessions use (`edit_file` and `write_file` record the same
   patches as `edit` / `write`; a `write_file` from older sessions has none and
   stays untracked; `changed: false` is no change) as well as `edit` / `write`.
+  Whether a call changed a file is read from its result details (`patch`, or
+  the reason none was recorded), never from `isError`: a call can fail after
+  its write landed (a cancelled `then_run`, a result-preparation failure) and
+  still carry the change, which must stay listed, undoable and replayable,
+  while a call that failed before writing records neither and is no change.
   Browser-renderable pages open in the
   embedded browser on click, with a system-browser action; both
   browsers load them from `src/site-preview.ts`, a separate token-scoped

apps/web/client/src/state/session.ts

@@ -368,24 +368,32 @@ const FILE_CHANGE_TOOLS: ReadonlySet<string> = new Set(["edit", "write", "edit_f
 
 /**
  * The file change of a finished edit/write call. `edit`, `write` and Step's
- * `edit_file` record a unified patch in their result details; `write` says why
- * when it recorded none. Step's `write_file` records no patch (an untracked
- * change), and `changed: false` when the file already held the content.
+ * `edit_file` record a unified patch in their result details, and say why when
+ * they recorded none. Step's `write_file` records the same patch as `write`; a
+ * `write_file` from older sessions has neither a patch nor a reason, and both
+ * report `changed: false` when the file already held the content.
  * `result` is the recorded tool result, or a live `tool_execution_end`'s
  * result with the event's own `isError`.
+ *
+ * Whether the file was written is answered by the change record, not by
+ * `isError`: a call can fail after its write landed (a cancelled `then_run`, a
+ * failure preparing the result) and still record the patch. A call that failed
+ * before writing records neither a patch nor a reason for its absence.
  */
 function fileChange(
 	name: string,
 	args: unknown,
 	result: { readonly details?: unknown; readonly isError?: boolean } | undefined,
 ): FileChange | undefined {
-	if (!FILE_CHANGE_TOOLS.has(name) || !result || result.isError) return undefined;
+	if (!FILE_CHANGE_TOOLS.has(name) || !result) return undefined;
 	const details = (result.details ?? {}) as {
 		patch?: unknown;
 		untracked?: unknown;
 		changed?: unknown;
 	};
 	if (details.changed === false) return undefined;
+	if (result.isError === true && typeof details.patch !== "string" && details.untracked === undefined)
+		return undefined;
 	const input = (args ?? {}) as { path?: unknown; file_path?: unknown };
 	const path = typeof input.path === "string" ? input.path : input.file_path;
 	if (typeof path !== "string" || !path.trim()) return undefined;

apps/web/src/turn-changes.ts

@@ -27,8 +27,10 @@ import {
 
 /**
  * File-changing tools: the built-in names and the Step tool profile's, which
- * Web sessions use. Step's `edit_file` records the same patch as `edit`; its
- * `write_file` records none (skipped) and `changed: false` for a no-op.
+ * Web sessions use. Step's `edit_file` records the same patch as `edit`, and
+ * its `write_file` records the same patch (or why it recorded none) as
+ * `write`; a `write_file` from older sessions has neither. Both report
+ * `changed: false` for a file that already held the content.
  */
 const FILE_CHANGE_TOOLS: ReadonlySet<string> = new Set(["edit", "write", "edit_file", "write_file"]);
 
@@ -37,6 +39,17 @@ function toolPath(input: Readonly<Record<string, unknown>>): string | undefined
 	return typeof value === "string" && value.trim() ? value : undefined;
 }
 
+/**
+ * Whether a finished edit/write result records what the call left in the file:
+ * a patch, or why it recorded none. A call can fail after its write landed (a
+ * cancelled `then_run`, a failure preparing the result) and still carry that
+ * record, so `isError` does not answer this; a call that failed before writing
+ * records neither, and must not be counted as a change.
+ */
+function recordsAppliedChange(details: WriteToolDetails & { changed?: unknown }): boolean {
+	return typeof details.patch === "string" || details.untracked !== undefined;
+}
+
 /**
  * Why a revert was refused; `path` is the file as the tool call named it.
  * `modified` means the file no longer matches the turn's patches and nothing
@@ -252,7 +265,7 @@ export function turnRevertNotice(
 	].join("\n");
 }
 
-/** Successful edit/write calls of the turn that starts at the user entry `turnId`, in order. */
+/** Edit/write calls of the turn that starts at the user entry `turnId`, in order, that recorded a change. */
 export function turnChanges(entries: readonly SessionEntry[], turnId: string): TurnChangeSet {
 	const start = entries.findIndex((entry) => entry.id === turnId && isUserMessage(entry));
 	const changes: TurnChange[] = [];
@@ -276,10 +289,13 @@ export function turnChanges(entries: readonly SessionEntry[], turnId: string): T
 				calls.set(String(part.id), { name: String(part.name ?? ""), path: toolPath(args) });
 			}
 		}
-		if (message.role !== "toolResult" || message.isError || !message.toolCallId) continue;
+		if (message.role !== "toolResult" || !message.toolCallId) continue;
 		const call = calls.get(message.toolCallId);
 		if (!call?.path || !FILE_CHANGE_TOOLS.has(call.name)) continue;
 		const details = (message.details ?? {}) as WriteToolDetails & { changed?: unknown };
+		// A call that failed before writing recorded nothing; one that failed after
+		// writing still carries the change it applied.
+		if (message.isError === true && !recordsAppliedChange(details)) continue;
 		// Step's write_file left a file that already held the content untouched.
 		if (details.changed === false) continue;
 		if (typeof details.patch !== "string") {
@@ -553,10 +569,13 @@ async function replayFile(
 				calls.set(String(part.id), { name: String(part.name ?? ""), path: toolPath(args), content: args.content });
 			}
 		}
-		if (message.role !== "toolResult" || message.isError || !message.toolCallId) continue;
+		if (message.role !== "toolResult" || !message.toolCallId) continue;
 		const call = calls.get(message.toolCallId);
 		if (!call?.path || !FILE_CHANGE_TOOLS.has(call.name) || !(await sameFile(call.path))) continue;
 		const details = (message.details ?? {}) as WriteToolDetails & { changed?: unknown };
+		// A call that failed before writing recorded nothing to replay; one that
+		// failed after writing left exactly what its record describes.
+		if (message.isError === true && !recordsAppliedChange(details)) continue;
 		const write = call.name === "write" || call.name === "write_file";
 		// A write names the whole content, even when the file already held it (`changed: false`).
 		if (write && typeof call.content === "string") content = recordedText(call.content);

apps/web/test/client-live-turn-changes.test.ts

@@ -90,12 +90,30 @@ test("a finished edit or write shows in the turn's changes before any snapshot",
 	]);
 });
 
-test("a failed, unchanged or unfinished call adds no change", () => {
+test("a failed call that changed the file still shows its change", () => {
 	open();
-	runTool("c1", "write_file", { path: "a.json" }, { details: { patch: patch("a.json") } }, true);
-	runTool("c2", "write_file", { path: "b.json" }, { details: { changed: false } });
-	agent({ type: "tool_execution_start", toolCallId: "c3", toolName: "edit_file", args: { path: "c.json" } });
-	agent({ type: "tool_execution_end", toolCallId: "c4", toolName: "run_command", result: { details: {} } });
+	// A cancelled then_run: the edit landed, the call reports an error.
+	runTool("c1", "edit_file", { path: "a.ts" }, { details: { patch: patch("a.ts") } }, true);
+	runTool("c2", "write_file", { path: "b.json" }, { details: { changed: true, patch: patch("b.json") } }, true);
+	assert.deepEqual(shownFiles(), [
+		["a.ts", 1],
+		["b.json", 1],
+	]);
+	// The change shows beside a row that still reads as failed; the two facts are separate.
+	const states = getState()
+		.timelineItems.filter((item) => item.kind === "tool")
+		.map((item) => item.state);
+	assert.deepEqual(states, ["failed", "failed"]);
+});
+
+test("a failed call that never wrote adds no change", () => {
+	open();
+	// The search string did not match, so the call recorded no change.
+	runTool("c1", "edit_file", { path: "a.ts" }, { details: { changed: false } }, true);
+	runTool("c2", "write_file", { path: "b.json" }, {}, true);
+	runTool("c3", "write_file", { path: "d.json" }, { details: { changed: false } });
+	agent({ type: "tool_execution_start", toolCallId: "c4", toolName: "edit_file", args: { path: "c.json" } });
+	agent({ type: "tool_execution_end", toolCallId: "c5", toolName: "run_command", result: { details: {} } });
 	assert.deepEqual(shownFiles(), []);
 });
 

apps/web/test/turn-changes.test.ts

@@ -111,7 +111,7 @@ test("revertTurnChanges restores the turn's files and deletes created ones", asy
 			}),
 			entry("r1", { role: "toolResult", toolCallId: "c1", details: { patch: PATCHES.two } }),
 			entry("r2", { role: "toolResult", toolCallId: "c2", details: { patch: PATCHES.fresh, created: true } }),
-			entry("r3", { role: "toolResult", toolCallId: "c3", isError: true, details: { patch: "x" } }),
+			entry("r3", { role: "toolResult", toolCallId: "c3", isError: true }),
 			entry("u3", { role: "user", content: "third" }),
 		];
 		assert.deepEqual(turnChanges(entries, "u1"), { changes: [], skipped: [] });
@@ -162,6 +162,69 @@ test("turnChanges lists writes without a patch as skipped", async () => {
 	});
 });
 
+test("a change the call applied stays listed when the call failed afterwards", async () => {
+	await withRoot(async (root) => {
+		// The session's own edit tool records the patch; only the check it ran
+		// alongside was cancelled, so the call reports an error.
+		const s = session(root);
+		await writeFile(join(root, "a.txt"), "one\n");
+		ask(s, "u1");
+		await edit(s, "c1", "a.txt", "one", "two");
+		(s.entries.find((candidate) => candidate.id === "r-c1")!.message as { isError?: boolean }).isError = true;
+		ask(s, "u2");
+
+		// The change is listed, it is no skip, and the undo the user asked for works
+		// instead of reporting a turn without changes.
+		const { changes, skipped } = turnChanges(s.entries, "u1");
+		assert.deepEqual(
+			changes.map((change) => change.path),
+			["a.txt"],
+		);
+		assert.deepEqual(skipped, []);
+		assert.equal(await undo(s, "u1"), "ok");
+		assert.equal(await content(join(root, "a.txt")), "one\n");
+		assert.deepEqual(s.recorded, ["u1"]);
+	});
+});
+
+test("a failed later turn that did apply its change is suggested before this one", async () => {
+	await withRoot(async (root) => {
+		const s = session(root);
+		await writeFile(join(root, "a.txt"), "zero\n");
+		ask(s, "u1");
+		await write(s, "w1", "a.txt", "one\n");
+		ask(s, "u2");
+		await edit(s, "e2", "a.txt", "one", "two");
+		// The second turn's call failed afterwards, but its edit is what the file holds.
+		(s.entries.find((candidate) => candidate.id === "r-e2")!.message as { isError?: boolean }).isError = true;
+		// Undoing u1 needs u2 undone first, exactly as for a successful turn.
+		assert.equal(await undo(s, "u1"), "conflict");
+		assert.equal(await content(join(root, "a.txt")), "two\n");
+		assert.equal(await undo(s, "u2"), "ok");
+		assert.equal(await undo(s, "u1"), "ok");
+		assert.equal(await content(join(root, "a.txt")), "zero\n");
+	});
+});
+
+test("a failed call that recorded nothing is neither a change nor a skip", async () => {
+	await withRoot(async (root) => {
+		const entries = [
+			entry("u1", { role: "user", content: "go" }),
+			entry("a1", {
+				role: "assistant",
+				content: [
+					{ type: "toolCall", id: "c1", name: "edit", arguments: { path: "a.txt" } },
+					{ type: "toolCall", id: "c2", name: "write", arguments: { path: "b.txt" } },
+				],
+			}),
+			// The search string did not match, and the write never ran.
+			entry("r1", { role: "toolResult", toolCallId: "c1", isError: true, details: { changed: false } }),
+			entry("r2", { role: "toolResult", toolCallId: "c2", isError: true }),
+		];
+		assert.deepEqual(turnChanges(entries, "u1"), { changes: [], skipped: [] });
+	});
+});
+
 test("reverting a write restores the overwritten file's line endings and BOM", async () => {
 	await withRoot(async (root) => {
 		await writeFile(join(root, "lf.txt"), "a\nb\n");
@@ -370,7 +433,8 @@ test("later turns that recorded no change to the file are not suggested", async
 		ask(s, "u3");
 		await write(s, "w3", "b.txt", "another file\n");
 		ask(s, "u4");
-		// A failed write whose patch would explain the file, and a write without a patch, both to a.txt.
+		// Two failed writes to a.txt that never reached the file: one that stopped
+		// before recording anything, and one whose result carries no change record.
 		s.entries.push(
 			entry("a-w4", {
 				role: "assistant",
@@ -379,13 +443,8 @@ test("later turns that recorded no change to the file are not suggested", async
 					{ type: "toolCall", id: "w5", name: "write", arguments: { path: "a.txt" } },
 				],
 			}),
-			entry("r-w4", {
-				role: "toolResult",
-				toolCallId: "w4",
-				isError: true,
-				details: { patch: "--- a.txt\n+++ a.txt\n@@ -1,1 +1,1 @@\n-one\n+two\n" },
-			}),
-			entry("r-w5", { role: "toolResult", toolCallId: "w5" }),
+			entry("r-w4", { role: "toolResult", toolCallId: "w4", isError: true }),
+			entry("r-w5", { role: "toolResult", toolCallId: "w5", isError: true, details: {} }),
 		);
 		assert.equal(await undo(s, "u1"), "modified");
 		assert.equal(await content(join(root, "a.txt")), "two\n");
@@ -562,6 +621,26 @@ test("each turn's delivery is rebuilt as that turn left the file", async () => {
 	});
 });
 
+test("a turn's version stays exact when its call failed after writing", async () => {
+	await withRoot(async (root) => {
+		const s = session(root);
+		ask(s, "u1");
+		await write(s, "w1", "index.html", "<p>one</p>\n");
+		ask(s, "u2");
+		await edit(s, "e2", "index.html", "one", "two");
+		// The check the edit ran alongside was cancelled: the file changed, the call is an error.
+		const result = s.entries.find((candidate) => candidate.id === "r-e2") as SessionEntry;
+		const message = result.message as { details?: { patch?: string } };
+		(result.message as { isError?: boolean }).isError = true;
+		assert.equal(typeof message.details?.patch, "string");
+		// The recorded patch is still the truth about that turn, so a later turn's
+		// delivery rebuilds it instead of falling back to the file on disk.
+		ask(s, "u3");
+		await edit(s, "e3", "index.html", "two", "three");
+		assert.deepEqual(await fileAtTurnEnd(s.entries, "u2", "index.html", root), { text: "<p>two</p>\n", exact: true });
+	});
+});
+
 test("a turn's version is inexact when neither replay nor reversal reaches it", async () => {
 	await withRoot(async (root) => {
 		const s = session(root);
@@ -580,6 +659,7 @@ test("a turn's version is inexact when neither replay nor reversal reaches it",
 });
 
 /** A Step-profile call as Web sessions record it: `write_file` keeps no patch, `edit_file` the edit's. */
+/** A `write_file` result as version 0.84.4 and older recorded it: no patch, no reason. */
 async function stepWrite(s: Session, id: string, path: string, content: string, changed = true): Promise<void> {
 	const input = { path, content };
 	s.entries.push(entry(`a-${id}`, { role: "assistant", content: [{ type: "toolCall", id, name: "write_file", arguments: input }] }));
@@ -601,7 +681,7 @@ async function stepEdit(s: Session, id: string, path: string, search: string, re
 	s.entries.push(entry(`r-${id}`, { role: "toolResult", toolCallId: id, details: result.details }));
 }
 
-test("Step's edit_file patches are turn changes; a write_file is untracked, or nothing when unchanged", async () => {
+test("Step's edit_file patches are turn changes; an older write_file without one is untracked", async () => {
 	await withRoot(async (root) => {
 		const s = session(root);
 		ask(s, "u1");