From 0bc684ca0bb13d3aa89345fb60828c108088516f Mon Sep 17 00:00:00 2001 From: JJ Lee Date: Thu, 3 Sep 2026 08:18:05 -0400 Subject: [PATCH] fix(session): derive agent-file filter from tool filediff only (#742) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Remove patch-part file lists from the agentFilesAbsolute set in session.diff(). Patch parts captured all worktree changes between step-start and step-finish snapshots (including modifications by other sessions or external processes), causing the Files Changed tab to show files the current session never edited. The filter is now derived exclusively from completed tool parts with filediff metadata — the same metadata that edit/write/patch/apply_patch tools already emit. Patch parts retain their snapshot hashes (used for the from/to diff range) but their files arrays no longer feed the filter. Also fix: when the primary diff path (snapshot-based) runs and finds zero net changes, it now returns [] directly instead of falling through to the filediff- accumulation fallback, which would surface stale metadata for reverted files. Closes harmoniqs/amicode#742 --- packages/opencode/src/session/session.ts | 16 +- .../test/server/session-diff-scoped.test.ts | 210 +++++++++++++++++- 2 files changed, 216 insertions(+), 10 deletions(-) diff --git a/packages/opencode/src/session/session.ts b/packages/opencode/src/session/session.ts index 050eb53ee..8945693d5 100644 --- a/packages/opencode/src/session/session.ts +++ b/packages/opencode/src/session/session.ts @@ -857,10 +857,13 @@ const layer: Layer.Layer< if (part.type === "step-finish" && part.snapshot) { lastStepFinish = part.snapshot } - if (part.type === "patch" && part.files) { - for (const file of part.files) agentFilesAbsolute.add(file) - } - // In-flight file tracking: also collect files from completed tool parts with filediff metadata + // NOTE: patch-part file lists are intentionally excluded from the + // agent filter (#742). They capture all worktree changes between + // step-start and step-finish snapshots (including external edits), + // which caused cross-session contamination. The filter is now + // derived exclusively from tool filediff metadata below. + + // Collect files from completed tool parts with filediff metadata if (part.type === "tool") { const toolPart = part as { tool?: string; state?: { status?: string; metadata?: Record } } if (toolPart.state?.status === "completed") { @@ -936,7 +939,10 @@ const layer: Layer.Layer< results = [...results, ...extDiffs] } - if (results.length > 0) return results + // Primary path ran (snapshots + agent files exist): trust its result, + // even if empty (all diffs were zero). The fallbacks below are for + // legacy sessions without snapshot infrastructure, not supplements. + return results } } diff --git a/packages/opencode/test/server/session-diff-scoped.test.ts b/packages/opencode/test/server/session-diff-scoped.test.ts index 5d4620943..56f5b5a4f 100644 --- a/packages/opencode/test/server/session-diff-scoped.test.ts +++ b/packages/opencode/test/server/session-diff-scoped.test.ts @@ -1,9 +1,12 @@ /** - * Integration test for session-scoped diffs (#174). + * Integration test for session-scoped diffs (#174, #742). * * Verifies that GET /session/:id/diff returns the net diff (session-start - * snapshot vs current state) filtered to only files the agent touched (tracked - * via PatchParts). This replaces the old per-turn/per-message diff model. + * snapshot vs current state) filtered to only files the agent touched. + * + * After #742 the agent-touched file filter is derived exclusively from tool + * filediff metadata — patch-part file lists no longer feed the filter (they + * were the cross-session contamination vector). */ import { afterEach, describe, expect } from "bun:test" import { LayerNode } from "@opencode-ai/core/effect/layer-node" @@ -136,7 +139,7 @@ describe("Session.diff — session-scoped agent diffs (#174)", () => { // Also write a file that the agent did NOT touch (external change) yield* fs.writeWithDirs(path.join(test.directory, "external.txt"), "external change") - // Record a patch part with agent-touched files + // Record a patch part (provides snapshot hash range, but files no longer feed the filter) const agentFiles = [ path.join(test.directory, "new-file.ts"), path.join(test.directory, "existing.txt"), @@ -150,6 +153,41 @@ describe("Session.diff — session-scoped agent diffs (#174)", () => { files: agentFiles, }) + // Record tool parts with filediff metadata — this is the agent file filter source (#742) + const now = Date.now() + yield* Session.use.updatePart({ + id: PartID.ascending(), + sessionID: session.id, + messageID: userMsgID, + type: "tool", + callID: "call_write_1", + tool: "write", + state: { + status: "completed", + input: {}, + output: "", + title: "new-file.ts", + metadata: { filediff: { file: path.join(test.directory, "new-file.ts") } }, + time: { start: now, end: now }, + }, + } as any) + yield* Session.use.updatePart({ + id: PartID.ascending(), + sessionID: session.id, + messageID: userMsgID, + type: "tool", + callID: "call_edit_1", + tool: "edit", + state: { + status: "completed", + input: {}, + output: "", + title: "existing.txt", + metadata: { filediff: { file: path.join(test.directory, "existing.txt") } }, + time: { start: now, end: now }, + }, + } as any) + // Query the session diff via the HTTP endpoint const response = yield* requestInDirectory( pathFor(SessionPaths.diff, { sessionID: session.id }), @@ -206,7 +244,7 @@ describe("Session.diff — session-scoped agent diffs (#174)", () => { snapshot: startHash!, }) - // Record the file as agent-touched + // Record the file as agent-touched via patch part (snapshot hash) and filediff yield* Session.use.updatePart({ id: PartID.ascending(), sessionID: session.id, @@ -215,6 +253,23 @@ describe("Session.diff — session-scoped agent diffs (#174)", () => { hash: startHash!, files: [path.join(test.directory, "reverted.txt")], }) + const now = Date.now() + yield* Session.use.updatePart({ + id: PartID.ascending(), + sessionID: session.id, + messageID: userMsgID, + type: "tool", + callID: "call_edit_reverted", + tool: "edit", + state: { + status: "completed", + input: {}, + output: "", + title: "reverted.txt", + metadata: { filediff: { file: path.join(test.directory, "reverted.txt") } }, + time: { start: now, end: now }, + }, + } as any) // File is back to its original content → net diff is zero // (we didn't actually change it from the snapshot state) @@ -247,4 +302,149 @@ describe("Session.diff — session-scoped agent diffs (#174)", () => { }), { git: true, config: { formatter: false, lsp: false } }, ) + + it.instance( + "patch-part files alone do not feed the agent filter (#742 contamination fix)", + () => + Effect.gen(function* () { + const test = yield* TestInstance + const session = yield* withSession({ title: "contamination" }) + const snapshot = yield* Snapshot.Service + const fs = yield* FSUtil.Service + + // Write files and take a session-start snapshot + yield* fs.writeWithDirs(path.join(test.directory, "agent-file.ts"), "original") + const startHash = yield* snapshot.track() + expect(startHash).toBeTruthy() + + const userMsgID = MessageID.ascending() + yield* Session.use.updateMessage({ + id: userMsgID, + sessionID: session.id, + role: "user", + time: { created: Date.now() }, + agent: "build", + model: { providerID: ProviderV2.ID.make("test"), modelID: ModelV2.ID.make("model") }, + } satisfies SessionV1.User) + + yield* Session.use.updatePart({ + id: PartID.ascending(), + sessionID: session.id, + messageID: userMsgID, + type: "step-start", + snapshot: startHash!, + }) + + // Simulate an external change picked up by snapshot.patch() + yield* fs.writeWithDirs(path.join(test.directory, "agent-file.ts"), "modified by someone else") + + // Record a patch part listing the file — but NO tool part with filediff + yield* Session.use.updatePart({ + id: PartID.ascending(), + sessionID: session.id, + messageID: userMsgID, + type: "patch", + hash: startHash!, + files: [path.join(test.directory, "agent-file.ts")], + }) + + // The diff should be empty: patch-part files should NOT feed the filter + const response = yield* requestInDirectory( + pathFor(SessionPaths.diff, { sessionID: session.id }), + test.directory, + ) + expect(response.status).toBe(200) + expect(yield* response.json).toEqual([]) + }), + { git: true, config: { formatter: false, lsp: false } }, + ) + + it.instance( + "only filediff-tracked files appear even when patch parts list extra files (#742)", + () => + Effect.gen(function* () { + const test = yield* TestInstance + const session = yield* withSession({ title: "partial-overlap" }) + const snapshot = yield* Snapshot.Service + const fs = yield* FSUtil.Service + + // Write three files and take the session-start snapshot + yield* fs.writeWithDirs(path.join(test.directory, "a.ts"), "original a") + yield* fs.writeWithDirs(path.join(test.directory, "b.ts"), "original b") + yield* fs.writeWithDirs(path.join(test.directory, "c.ts"), "original c") + const startHash = yield* snapshot.track() + expect(startHash).toBeTruthy() + + const userMsgID = MessageID.ascending() + yield* Session.use.updateMessage({ + id: userMsgID, + sessionID: session.id, + role: "user", + time: { created: Date.now() }, + agent: "build", + model: { providerID: ProviderV2.ID.make("test"), modelID: ModelV2.ID.make("model") }, + } satisfies SessionV1.User) + + yield* Session.use.updatePart({ + id: PartID.ascending(), + sessionID: session.id, + messageID: userMsgID, + type: "step-start", + snapshot: startHash!, + }) + + // Modify all three files on disk + yield* fs.writeWithDirs(path.join(test.directory, "a.ts"), "modified a") + yield* fs.writeWithDirs(path.join(test.directory, "b.ts"), "modified b") + yield* fs.writeWithDirs(path.join(test.directory, "c.ts"), "modified c") + + // Patch part claims all three files (the contamination vector) + yield* Session.use.updatePart({ + id: PartID.ascending(), + sessionID: session.id, + messageID: userMsgID, + type: "patch", + hash: startHash!, + files: [ + path.join(test.directory, "a.ts"), + path.join(test.directory, "b.ts"), + path.join(test.directory, "c.ts"), + ], + }) + + // But the agent only edited a.ts — only it has filediff metadata + const now = Date.now() + yield* Session.use.updatePart({ + id: PartID.ascending(), + sessionID: session.id, + messageID: userMsgID, + type: "tool", + callID: "call_edit_a", + tool: "edit", + state: { + status: "completed", + input: {}, + output: "", + title: "a.ts", + metadata: { filediff: { file: path.join(test.directory, "a.ts") } }, + time: { start: now, end: now }, + }, + } as any) + + const response = yield* requestInDirectory( + pathFor(SessionPaths.diff, { sessionID: session.id }), + test.directory, + ) + expect(response.status).toBe(200) + const diffs = (yield* response.json) as Array<{ file: string }> + const files = diffs.map((d) => d.file) + + // Only a.ts should appear — b.ts and c.ts were NOT agent-edited + expect(files).toContain("a.ts") + expect(files).not.toContain("b.ts") + expect(files).not.toContain("c.ts") + expect(diffs.length).toBe(1) + }), + { git: true, config: { formatter: false, lsp: false } }, + ) })