From 0b7b8f68591b39a1e451b5d3e3c3abb0d4a19649 Mon Sep 17 00:00:00 2001 From: JJ Lee Date: Tue, 11 Aug 2026 10:09:51 +0200 Subject: [PATCH] fix: live cumulative diff during multi-step turns (#179) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two fixes for the Files Changed panel showing stale/last-edit-only diffs during a live workflow: Fix A — prefer live working tree over stale snapshot: Session.diff() now calls snapshot.track() first (captures current disk state including in-progress edits), falling back to lastStepFinish only when tracking is unavailable. Previously it used lastStepFinish when set, which is stale during a live turn. Fix B — refetch diff query when tool edits complete mid-turn: Added a diff_version counter per session that bumps in the event reducer when a completed file-editing tool part (edit/write/patch/apply_patch) with filediff metadata arrives via SSE. The sessionDiffQuery key includes this version, so each completed edit triggers a server refetch — giving the panel the real cumulative diff (~100-250ms) instead of relying on the client-side accumulateDiffs fallback (which only keeps the last patch). --- packages/app/src/context/directory-sync.ts | 1 + .../src/context/global-sync/bootstrap.test.ts | 1 + .../src/context/global-sync/child-store.ts | 1 + .../context/global-sync/event-reducer.test.ts | 1 + .../src/context/global-sync/event-reducer.ts | 10 ++ .../context/global-sync/session-cache.test.ts | 4 + .../src/context/global-sync/session-cache.ts | 2 + packages/app/src/context/global-sync/types.ts | 3 + packages/app/src/context/server-session.ts | 1 + packages/app/src/pages/session.tsx | 5 +- packages/opencode/src/session/session.ts | 93 ++++++++++++------- packages/opencode/src/snapshot/index.ts | 91 +++++++++++++++++- 12 files changed, 179 insertions(+), 34 deletions(-) diff --git a/packages/app/src/context/directory-sync.ts b/packages/app/src/context/directory-sync.ts index befd5b61e..dc24fa519 100644 --- a/packages/app/src/context/directory-sync.ts +++ b/packages/app/src/context/directory-sync.ts @@ -12,6 +12,7 @@ const sessionFields = new Set([ "session_status", "session_working", "session_diff", + "diff_version", "todo", "permission", "question", diff --git a/packages/app/src/context/global-sync/bootstrap.test.ts b/packages/app/src/context/global-sync/bootstrap.test.ts index a95706a3d..dcbb20885 100644 --- a/packages/app/src/context/global-sync/bootstrap.test.ts +++ b/packages/app/src/context/global-sync/bootstrap.test.ts @@ -58,6 +58,7 @@ function directoryState() { return this.session_status[id]?.type !== "idle" }, session_diff: {}, + diff_version: {}, todo: {}, permission: {}, question: {}, diff --git a/packages/app/src/context/global-sync/child-store.ts b/packages/app/src/context/global-sync/child-store.ts index 4eaa78578..9ebd27d98 100644 --- a/packages/app/src/context/global-sync/child-store.ts +++ b/packages/app/src/context/global-sync/child-store.ts @@ -234,6 +234,7 @@ export function createChildStoreManager(input: { return (type ?? "idle") !== "idle" }, session_diff: {}, + diff_version: {}, todo: {}, permission: {}, question: {}, diff --git a/packages/app/src/context/global-sync/event-reducer.test.ts b/packages/app/src/context/global-sync/event-reducer.test.ts index b53fb691b..265fe8082 100644 --- a/packages/app/src/context/global-sync/event-reducer.test.ts +++ b/packages/app/src/context/global-sync/event-reducer.test.ts @@ -72,6 +72,7 @@ const baseState = (input: Partial = {}) => sessionTotal: 0, session_status: {}, session_diff: {}, + diff_version: {}, todo: {}, permission: {}, question: {}, diff --git a/packages/app/src/context/global-sync/event-reducer.ts b/packages/app/src/context/global-sync/event-reducer.ts index 8f203715a..8a56d05ad 100644 --- a/packages/app/src/context/global-sync/event-reducer.ts +++ b/packages/app/src/context/global-sync/event-reducer.ts @@ -17,6 +17,7 @@ import { dropSessionCaches } from "./session-cache" import { diffs as list, message as clean } from "@/utils/diffs" const SKIP_PARTS = new Set(["patch", "step-start", "step-finish"]) +const EDIT_TOOLS = new Set(["edit", "write", "patch", "apply_patch"]) const SESSION_CONTENT_EVENTS = new Set([ "session.diff", "todo.updated", @@ -312,6 +313,15 @@ export function applyDirectoryEvent(input: { case "message.part.updated": { const part = (event.properties as { part: Part }).part if (SKIP_PARTS.has(part.type)) break + // Bump diff_version when a file-editing tool completes so the diff query refetches mid-turn + if ( + part.type === "tool" && + EDIT_TOOLS.has(part.tool) && + part.state.status === "completed" && + (part.state as { metadata?: Record }).metadata?.filediff + ) { + input.setStore("diff_version", part.sessionID, (v) => (v ?? 0) + 1) + } input.setStore( produce((draft) => { delete draft.part_text_accum_delta[part.id] diff --git a/packages/app/src/context/global-sync/session-cache.test.ts b/packages/app/src/context/global-sync/session-cache.test.ts index 45fbe38ab..d8c9d91bb 100644 --- a/packages/app/src/context/global-sync/session-cache.test.ts +++ b/packages/app/src/context/global-sync/session-cache.test.ts @@ -27,6 +27,7 @@ describe("app session cache", () => { const store: { session_status: Record session_diff: Record + diff_version: Record todo: Record message: Record session_message: Record @@ -37,6 +38,7 @@ describe("app session cache", () => { } = { session_status: { ses_1: { type: "busy" } as SessionStatus }, session_diff: { ses_1: [] }, + diff_version: { ses_1: 0 }, todo: { ses_1: [] as Todo[] }, message: {}, session_message: {}, @@ -63,6 +65,7 @@ describe("app session cache", () => { const store: { session_status: Record session_diff: Record + diff_version: Record todo: Record message: Record session_message: Record @@ -73,6 +76,7 @@ describe("app session cache", () => { } = { session_status: {}, session_diff: {}, + diff_version: {}, todo: {}, message: { ses_1: [m] }, session_message: {}, diff --git a/packages/app/src/context/global-sync/session-cache.ts b/packages/app/src/context/global-sync/session-cache.ts index 7d684a5a1..3fb2d82a8 100644 --- a/packages/app/src/context/global-sync/session-cache.ts +++ b/packages/app/src/context/global-sync/session-cache.ts @@ -7,6 +7,7 @@ export const SESSION_CACHE_LIMIT = 40 type SessionCache = { session_status: Record session_diff: Record + diff_version: Record todo: Record message: Record session_message: Record @@ -34,6 +35,7 @@ export function dropSessionCaches(store: SessionCache, sessionIDs: Iterable, session_status: {} as Record, session_diff: {} as Record, + diff_version: {} as Record, todo: {} as Record, permission: {} as Record, question: {} as Record, diff --git a/packages/app/src/pages/session.tsx b/packages/app/src/pages/session.tsx index 2e282591a..92c0adcff 100644 --- a/packages/app/src/pages/session.tsx +++ b/packages/app/src/pages/session.tsx @@ -668,9 +668,12 @@ export default function Page() { const mobileChanges = createMemo(() => !isDesktop() && store.mobileTab === "changes") const EDIT_TOOLS = new Set(["edit", "write", "patch", "apply_patch"]) // Refetch when the session transitions to idle (assistant finished, snapshot taken) + // or when a file-editing tool completes mid-turn (diff_version bumps) const sessionDiffVersion = () => { const id = params.id - return id ? sync().data.session_status[id]?.type ?? "idle" : "idle" + const status = id ? sync().data.session_status[id]?.type ?? "idle" : "idle" + const version = id ? sync().data.diff_version[id] ?? 0 : 0 + return `${status}:${version}` } const sessionDiffKey = () => ["session-diff", params.id ?? "", sessionDiffVersion()] as const const sessionDiffQuery = createQuery(() => { diff --git a/packages/opencode/src/session/session.ts b/packages/opencode/src/session/session.ts index 46657e1c9..050eb53ee 100644 --- a/packages/opencode/src/session/session.ts +++ b/packages/opencode/src/session/session.ts @@ -839,6 +839,9 @@ const layer: Layer.Layer< const all = yield* messages({ sessionID }).pipe(Effect.orDie) if (!all.length) return [] as Snapshot.FileDiff[] + const session = yield* get(sessionID).pipe(Effect.orDie) + const sessionStartTime = session.time.created + // Find the first step-start snapshot hash (session-start ref) // and the last step-finish snapshot hash (session-end ref) let from: string | undefined @@ -870,44 +873,70 @@ const layer: Layer.Layer< // If we have snapshot hashes and agent-touched files, compute the real diff if (from && agentFilesAbsolute.size > 0) { - // Use last step-finish hash if available (completed session), - // otherwise fall back to current working tree state (session still running) - const to = lastStepFinish ?? (yield* snapshot.track()) + // Always track the live working tree so in-progress edits are included; + // fall back to the last step-finish hash only when tracking is unavailable. + const to = (yield* snapshot.track()) ?? lastStepFinish if (to) { - // Normalize agent-touched files to relative paths (diffFull returns relative paths) + // Normalize and split files into in-worktree (relative) and external (absolute) const ctx = yield* InstanceState.context const worktree = ctx.worktree const agentFiles = new Set() - for (const abs of agentFilesAbsolute) { - const rel = abs.startsWith(worktree) - ? abs.slice(worktree.length).replace(/^\//, "").replaceAll("\\", "/") - : abs.replaceAll("\\", "/") - agentFiles.add(rel) + const externalFiles: string[] = [] + for (const raw of agentFilesAbsolute) { + // Resolve relative paths (e.g. ../../../other-repo/file.ts) to absolute + const abs = raw.startsWith("/") ? raw : path.resolve(worktree, raw) + if (abs.startsWith(worktree + "/") || abs === worktree) { + const rel = abs.slice(worktree.length + 1).replaceAll("\\", "/") + agentFiles.add(rel) + } else { + externalFiles.push(abs) + } + } + + // Primary path: diff in-worktree files via snapshot + let results: Snapshot.FileDiff[] = [] + if (agentFiles.size > 0) { + const allDiffs = yield* snapshot.diffFull(from, to).pipe( + Effect.catchCause((cause) => + Effect.gen(function* () { + yield* Effect.logWarning("diffFull failed, falling through to per-file fallback", { cause }) + return [] as Snapshot.FileDiff[] + }), + ), + ) + const filtered = allDiffs.filter( + (d: Snapshot.FileDiff) => d.file && agentFiles.has(d.file) && (d.additions ?? 0) + (d.deletions ?? 0) > 0, + ) + if (filtered.length > 0) { + results = filtered + } else { + // Fallback A: per-file git show vs current disk for in-worktree files + const perFileDiffs = yield* snapshot.diffFromDisk(from, [...agentFiles]).pipe( + Effect.catchCause((cause) => + Effect.gen(function* () { + yield* Effect.logWarning("diffFromDisk failed, falling through to summary fallback", { cause }) + return [] as Snapshot.FileDiff[] + }), + ), + ) + if (perFileDiffs.length > 0) results = perFileDiffs + } + } + + // External files: diff against HEAD in their own git repos + if (externalFiles.length > 0) { + const extDiffs = yield* snapshot.diffExternalFiles(externalFiles, sessionStartTime).pipe( + Effect.catchCause((cause) => + Effect.gen(function* () { + yield* Effect.logWarning("diffExternalFiles failed", { cause }) + return [] as Snapshot.FileDiff[] + }), + ), + ) + results = [...results, ...extDiffs] } - const allDiffs = yield* snapshot.diffFull(from, to).pipe( - Effect.catchCause((cause) => - Effect.gen(function* () { - yield* Effect.logWarning("diffFull failed, falling through to per-file fallback", { cause }) - return [] as Snapshot.FileDiff[] - }), - ), - ) - const filtered = allDiffs.filter( - (d: Snapshot.FileDiff) => d.file && agentFiles.has(d.file) && (d.additions ?? 0) + (d.deletions ?? 0) > 0, - ) - if (filtered.length > 0) return filtered - - // Fallback A: primary returned empty but from hash exists — per-file git show vs current disk - const perFileDiffs = yield* snapshot.diffFromDisk(from, [...agentFiles]).pipe( - Effect.catchCause((cause) => - Effect.gen(function* () { - yield* Effect.logWarning("diffFromDisk failed, falling through to summary fallback", { cause }) - return [] as Snapshot.FileDiff[] - }), - ), - ) - if (perFileDiffs.length > 0) return perFileDiffs + if (results.length > 0) return results } } diff --git a/packages/opencode/src/snapshot/index.ts b/packages/opencode/src/snapshot/index.ts index 0fcecd943..b8eca5ce4 100644 --- a/packages/opencode/src/snapshot/index.ts +++ b/packages/opencode/src/snapshot/index.ts @@ -43,6 +43,7 @@ export interface Interface { readonly diff: (hash: string) => Effect.Effect readonly diffFull: (from: string, to: string) => Effect.Effect readonly diffFromDisk: (ref: string, files: string[]) => Effect.Effect + readonly diffExternalFiles: (files: string[], startTime?: number) => Effect.Effect } export class Service extends Context.Service()("@opencode/Snapshot") {} @@ -801,6 +802,91 @@ const layer: Layer.Layer + formatPatch(structuredPatch(file, file, before, after, "", "", { context: Number.MAX_SAFE_INTEGER })) + + // Group files by their git root to avoid repeated rev-parse calls + const repoRoots = new Map() + + for (const absPath of files) { + const dir = path.dirname(absPath) + const topLevel = yield* appProcess + .run(ChildProcess.make("git", ["rev-parse", "--show-toplevel"], { cwd: dir, extendEnv: true }), {}) + .pipe(Effect.catch(() => Effect.succeed({ exitCode: 1, stdout: Buffer.from(""), stderr: Buffer.from("") }))) + if (topLevel.exitCode !== 0) continue + const repoRoot = topLevel.stdout.toString("utf8").trim() + if (!repoRoot) continue + + const relPath = absPath.startsWith(repoRoot + "/") + ? absPath.slice(repoRoot.length + 1) + : absPath.startsWith(repoRoot + "\\") + ? absPath.slice(repoRoot.length + 1).replaceAll("\\", "/") + : path.relative(repoRoot, absPath).replaceAll("\\", "/") + + const entry = repoRoots.get(repoRoot) ?? { root: repoRoot, files: [] } + entry.files.push({ abs: absPath, rel: relPath }) + repoRoots.set(repoRoot, entry) + } + + for (const [, { root, files: repoFiles }] of repoRoots) { + // Find the commit that was HEAD at session start time + let ref = "HEAD" + if (startTime) { + const isoTime = new Date(startTime).toISOString() + const revList = yield* appProcess + .run( + ChildProcess.make("git", ["-C", root, "rev-list", "-1", `--before=${isoTime}`, "HEAD"], { + extendEnv: true, + }), + {}, + ) + .pipe(Effect.catch(() => Effect.succeed({ exitCode: 1, stdout: Buffer.from(""), stderr: Buffer.from("") }))) + const foundRef = revList.exitCode === 0 ? revList.stdout.toString("utf8").trim() : "" + if (foundRef) ref = foundRef + } + + for (const { abs: absPath, rel: relPath } of repoFiles) { + // Get the file content at the session-start ref + const showResult = yield* appProcess + .run(ChildProcess.make("git", ["-C", root, "show", `${ref}:${relPath}`], { extendEnv: true }), {}) + .pipe(Effect.catch(() => Effect.succeed({ exitCode: 1, stdout: Buffer.from(""), stderr: Buffer.from("") }))) + const before = showResult.exitCode === 0 ? showResult.stdout.toString("utf8") : "" + + // Read current file from disk + const after = yield* fs.readFileString(absPath).pipe(Effect.catch(() => Effect.succeed(""))) + + if (before === after) continue + + // Compute line-level additions/deletions + const sp = structuredPatch(relPath, relPath, before, after, "", "") + let adds = 0 + let dels = 0 + for (const hunk of sp.hunks) { + for (const line of hunk.lines) { + if (line.startsWith("+")) adds++ + else if (line.startsWith("-")) dels++ + } + } + + const status: "added" | "deleted" | "modified" = before === "" ? "added" : after === "" ? "deleted" : "modified" + // Use absolute path so the client doesn't prefix it with the wrong workspace + const home = process.env.HOME + const displayPath = home && absPath.startsWith(home + "/") ? "~" + absPath.slice(home.length) : absPath + result.push({ + file: displayPath, + patch: patchFn(relPath, before, after), + additions: adds, + deletions: dels, + status, + }) + } + } + + return result + }) + yield* cleanup().pipe( Effect.catchCause((cause) => Effect.logError("cleanup loop failed", { cause: Cause.pretty(cause) })), Effect.repeat(Schedule.spaced(Duration.hours(1))), @@ -808,7 +894,7 @@ const layer: Layer.Layer s.diffFromDisk(ref, files)) }), + diffExternalFiles: Effect.fn("Snapshot.diffExternalFiles")(function* (files: string[], startTime?: number) { + return yield* InstanceState.useEffect(state, (s) => s.diffExternalFiles(files, startTime)) + }), }) }), )