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 } }, + ) })