mirror of
https://github.com/cline/cline.git
synced 2026-10-30 18:12:49 +08:00
fix(desktop): canonicalize diff panel paths against the session cwd (#13092)
* fix(desktop): canonicalize diff panel paths against the session cwd Tool calls address the same file inconsistently across a session: one edit uses a workspace-relative path (journal.txt), a later one the absolute path (/tmp/ws/journal.txt). mergeToolDiffs keyed entries by the raw string, so the same file was listed twice in the diff panel with split +/- counts and inconsistent naming, most visibly after git was initialized mid-session and the model switched to absolute paths. Diff paths are now canonicalized against the session cwd before merging: entries for the same file collapse into one, files inside the cwd display as workspace-relative paths, and files outside it display their resolved path. Without a cwd the previous raw-key behavior is kept. * style: collapse editorReplaceEvent signature per biome format Co-authored-by: Saoud Rizwan <saoudrizwan@users.noreply.github.com> * fix(desktop): collapse dot segments and keep root cwd in diff path keys * fix(desktop): compare Windows diff path keys case-insensitively --------- Co-authored-by: Saoud Rizwan <saoudrizwan@users.noreply.github.com>
This commit is contained in:
co-authored by
Saoud Rizwan
parent
b0bba2e5d6
commit
4f25692d70
@@ -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;
|
||||
|
||||
@@ -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 });
|
||||
});
|
||||
});
|
||||
@@ -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<string, SessionFileDiff>();
|
||||
|
||||
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),
|
||||
|
||||
Reference in New Issue
Block a user