diff --git a/apps/examples/desktop-app/webview/hooks/use-chat-session.ts b/apps/examples/desktop-app/webview/hooks/use-chat-session.ts index 9543e7c26d..c3f0e2229b 100644 --- a/apps/examples/desktop-app/webview/hooks/use-chat-session.ts +++ b/apps/examples/desktop-app/webview/hooks/use-chat-session.ts @@ -625,6 +625,7 @@ export function useChatSession() { setPromptsInQueue(value as PromptInQueue[]); }, []); + const sessionDiffCwd = (config.cwd || config.workspaceRoot || "").trim(); const refreshSessionDiffSummary = useCallback( async (targetSessionId: string) => { try { @@ -632,7 +633,7 @@ export function useChatSession() { "read_session_hooks", { sessionId: targetSessionId, limit: MAX_MESSAGES }, ); - const diffState = buildSessionDiffState(events); + const diffState = buildSessionDiffState(events, sessionDiffCwd); setFileDiffs(diffState.fileDiffs); setDiffSummary(diffState.summary); setToolCalls( @@ -645,7 +646,7 @@ export function useChatSession() { // Ignore in non-Tauri mode. } }, - [], + [sessionDiffCwd], ); // ---- Message helpers ---- @@ -914,13 +915,13 @@ export function useChatSession() { if (events.length === 0) { return; } - const diffState = buildSessionDiffState(events); + const diffState = buildSessionDiffState(events, sessionDiffCwd); if (diffState.fileDiffs.length === 0) { return; } setFileDiffs(diffState.fileDiffs); setDiffSummary(diffState.summary); - }, [sessionId, messages, fileDiffs.length]); + }, [sessionId, messages, fileDiffs.length, sessionDiffCwd]); useEffect(() => { const activeSessionId = sessionId; diff --git a/apps/examples/desktop-app/webview/lib/session-diff.test.ts b/apps/examples/desktop-app/webview/lib/session-diff.test.ts new file mode 100644 index 0000000000..772924788b --- /dev/null +++ b/apps/examples/desktop-app/webview/lib/session-diff.test.ts @@ -0,0 +1,156 @@ +import { describe, expect, it } from "vitest"; +import { + buildSessionDiffState, + mergeToolDiffs, + type SessionHookEvent, +} from "./session-diff"; + +function editorCreateEvent(path: string, newText: string): SessionHookEvent { + return { + hookName: "tool_result", + toolName: "editor", + toolInput: { path, new_text: newText }, + toolOutput: { success: true, result: `File created: ${path}` }, + }; +} + +function editorReplaceEvent(path: string, diffBody: string): SessionHookEvent { + return { + hookName: "tool_result", + toolName: "editor", + toolInput: { path, old_text: "before", new_text: "after" }, + toolOutput: { + success: true, + result: `Edited ${path}\n\`\`\`diff\n${diffBody}\n\`\`\``, + }, + }; +} + +describe("mergeToolDiffs path canonicalization", () => { + it("merges relative and absolute spellings of the same file into one entry", () => { + // Regression: a file created with a relative path and later edited via + // its absolute path was listed twice, splitting the counts and showing + // stale content as a separate entry. + const events = [ + editorCreateEvent("journal.txt", "line one\nline two"), + editorReplaceEvent("/tmp/qa-ws/journal.txt", "+3: line three"), + ]; + + const merged = mergeToolDiffs(events, "/tmp/qa-ws"); + + expect(merged).toHaveLength(1); + expect(merged[0]).toMatchObject({ + path: "journal.txt", + additions: 3, + deletions: 0, + }); + expect(merged[0]?.hunks).toHaveLength(2); + }); + + it("collapses parent segments so escaped spellings merge with the absolute path", () => { + const merged = mergeToolDiffs( + [ + editorCreateEvent("../shared/config.txt", "x"), + editorReplaceEvent("/tmp/shared/config.txt", "+2: y"), + ], + "/tmp/qa-ws", + ); + expect(merged).toHaveLength(1); + expect(merged[0]).toMatchObject({ + path: "/tmp/shared/config.txt", + additions: 2, + deletions: 0, + }); + }); + + it("collapses redundant segments inside the workspace", () => { + const merged = mergeToolDiffs( + [ + editorCreateEvent("sub/../journal.txt", "one"), + editorReplaceEvent("/tmp/qa-ws/./journal.txt", "+2: two"), + ], + "/tmp/qa-ws", + ); + expect(merged).toHaveLength(1); + expect(merged[0]).toMatchObject({ + path: "journal.txt", + additions: 2, + deletions: 0, + }); + }); + + it("keeps canonicalizing when the cwd is the filesystem root", () => { + const merged = mergeToolDiffs( + [ + editorCreateEvent("notes.txt", "one"), + editorReplaceEvent("/notes.txt", "+2: two"), + ], + "/", + ); + expect(merged).toHaveLength(1); + expect(merged[0]).toMatchObject({ + path: "notes.txt", + additions: 2, + deletions: 0, + }); + }); + + it("merges Windows casing variants of the same file", () => { + const merged = mergeToolDiffs( + [ + editorCreateEvent("Journal.txt", "one"), + editorReplaceEvent("c:\\users\\saoud\\journal.txt", "+2: two"), + ], + "C:\\Users\\Saoud", + ); + expect(merged).toHaveLength(1); + expect(merged[0]).toMatchObject({ + path: "Journal.txt", + additions: 2, + deletions: 0, + }); + }); + + it("keeps files outside the cwd on their resolved path", () => { + const merged = mergeToolDiffs( + [editorCreateEvent("/etc/other/config.txt", "x")], + "/tmp/qa-ws", + ); + expect(merged).toHaveLength(1); + expect(merged[0]?.path).toBe("/etc/other/config.txt"); + }); + + it("keeps distinct files separate", () => { + const merged = mergeToolDiffs( + [ + editorCreateEvent("a.txt", "one"), + editorCreateEvent("/tmp/qa-ws/b.txt", "two"), + ], + "/tmp/qa-ws", + ); + expect(merged.map((entry) => entry.path).sort()).toEqual([ + "a.txt", + "b.txt", + ]); + }); + + it("falls back to raw path keys without a cwd", () => { + const merged = mergeToolDiffs([ + editorCreateEvent("journal.txt", "one"), + editorCreateEvent("/tmp/qa-ws/journal.txt", "two"), + ]); + // Without a cwd there is no way to know these are the same file. + expect(merged).toHaveLength(2); + }); + + it("sums the summary across canonicalized entries", () => { + const state = buildSessionDiffState( + [ + editorCreateEvent("journal.txt", "line one\nline two"), + editorReplaceEvent("/tmp/qa-ws/journal.txt", "+3: line three"), + ], + "/tmp/qa-ws", + ); + expect(state.summary).toEqual({ additions: 3, deletions: 0 }); + }); +}); diff --git a/apps/examples/desktop-app/webview/lib/session-diff.ts b/apps/examples/desktop-app/webview/lib/session-diff.ts index 02dcaa877d..265fe72cc3 100644 --- a/apps/examples/desktop-app/webview/lib/session-diff.ts +++ b/apps/examples/desktop-app/webview/lib/session-diff.ts @@ -1,3 +1,5 @@ +import { resolveWorkspaceFilePath } from "./workspace-paths"; + export type SessionHookEvent = { hookEventName?: | "tool_call" @@ -456,7 +458,90 @@ function parseApplyPatchFileDiffs(event: SessionHookEvent): SessionFileDiff[] { return parseApplyPatchInput(patchInput); } -export function mergeToolDiffs(events: SessionHookEvent[]): SessionFileDiff[] { +/** + * Collapses "." and ".." segments and rebuilds the path with the given + * separator. The webview has no `node:path`, so this is a string-level + * equivalent; both separator styles are treated as separators, matching + * `resolveWorkspaceFilePath`. ".." segments that would climb above an + * absolute root are dropped; on relative paths leading ".." is kept. + */ +function collapsePathSegments(path: string, separator: string): string { + const rootMatch = path.match(/^(?:\\\\|[A-Za-z]:[\\/]|[\\/])/); + const rawRoot = rootMatch?.[0] ?? ""; + const root = + rawRoot === "" + ? "" + : rawRoot === "\\\\" + ? "\\\\" + : /^[A-Za-z]:/.test(rawRoot) + ? rawRoot.slice(0, 2) + separator + : separator; + const segments: string[] = []; + for (const segment of path.slice(rawRoot.length).split(/[\\/]+/)) { + if (!segment || segment === ".") { + continue; + } + if (segment === "..") { + if (segments.length > 0 && segments[segments.length - 1] !== "..") { + segments.pop(); + } else if (!root) { + segments.push(".."); + } + continue; + } + segments.push(segment); + } + return root + segments.join(separator); +} + +/** + * Tool calls address the same file inconsistently across a session — one + * edit uses "journal.txt", a later one "/tmp/workspace/journal.txt". Keying + * diffs by the raw string would list that file twice with split counts, so + * paths are canonicalized against the session cwd first: resolved against + * the cwd with "." and ".." segments collapsed, so "../shared/file.txt" + * and its absolute spelling share one key, and a filesystem-root cwd such + * as "/" still canonicalizes. Windows sessions (drive-letter or UNC cwd) + * compare case-insensitively, matching `normalizeWorkspacePath`. Files + * inside the cwd display as workspace-relative paths; everything else + * displays as the resolved path, both in the first spelling seen. + */ +function canonicalizeDiffPath( + path: string, + cwd?: string, +): { key: string; display: string } { + const trimmed = path.trim().replace(/^\.\//, ""); + const baseRaw = (cwd ?? "").trim(); + if (!baseRaw) { + return { key: trimmed, display: trimmed }; + } + const separator = + baseRaw.includes("\\") && !baseRaw.includes("/") ? "\\" : "/"; + const base = collapsePathSegments(baseRaw, separator); + if (!base) { + return { key: trimmed, display: trimmed }; + } + const resolved = collapsePathSegments( + resolveWorkspaceFilePath(trimmed, base), + separator, + ); + const caseInsensitive = /^[A-Za-z]:/.test(base) || base.startsWith("\\\\"); + const comparable = (value: string) => + caseInsensitive ? value.toLowerCase() : value; + const basePrefix = base.endsWith(separator) ? base : base + separator; + const withinBase = + resolved.length > basePrefix.length && + comparable(resolved).startsWith(comparable(basePrefix)); + return { + key: comparable(resolved), + display: withinBase ? resolved.slice(basePrefix.length) : resolved, + }; +} + +export function mergeToolDiffs( + events: SessionHookEvent[], + cwd?: string, +): SessionFileDiff[] { const byPath = new Map(); for (const event of events) { @@ -471,13 +556,14 @@ export function mergeToolDiffs(events: SessionHookEvent[]): SessionFileDiff[] { } for (const diff of diffs) { - const existing = byPath.get(diff.path); + const { key, display } = canonicalizeDiffPath(diff.path, cwd); + const existing = byPath.get(key); if (!existing) { - byPath.set(diff.path, diff); + byPath.set(key, { ...diff, path: display }); continue; } - byPath.set(diff.path, { + byPath.set(key, { ...existing, additions: existing.additions + diff.additions, deletions: existing.deletions + diff.deletions, @@ -504,8 +590,9 @@ export function summarizeFileDiffs( export function buildSessionDiffState( events: SessionHookEvent[], + cwd?: string, ): SessionDiffState { - const fileDiffs = mergeToolDiffs(events); + const fileDiffs = mergeToolDiffs(events, cwd); return { fileDiffs, summary: summarizeFileDiffs(fileDiffs),