diff --git a/docs/TESTING.md b/docs/TESTING.md index 50be305..2558931 100644 --- a/docs/TESTING.md +++ b/docs/TESTING.md @@ -1441,6 +1441,34 @@ deferred to a later phase (see PLAN.md). - [ ] Select Claude CLI and LM Studio afterward → both still work and their existing settings remain intact. +## 26. Diff pop-out from Accomplished rows (issue #10) + +Written on Windows, where the app can't run — every box below is unverified +and needs a Mac pass. + +- [ ] Have an agent edit a tracked file in the tab's repo, then `git add` that + file (the pop-out reads `git diff --cached` only). The Accomplished row + "Edited ``" underlines on hover; clicking it opens the pop-out with + that file's unified diff, monospace, and **only** that file's section — + no other staged file bleeds in. +- [ ] Esc closes it; so does clicking the dimmed overlay and the Close button. + Clicking inside the diff (e.g. selecting text) does not close it. +- [ ] Stage a second file too → each row opens its own section, not the other's. +- [ ] Unstaged edit: agent edits a file, nothing staged → row still clicks, + pop-out shows the "No staged diff for this file" empty state, no crash + and no blank panel behind it. +- [ ] Row for a file outside the tab's repo (e.g. agent edits a file in another + checkout): staged there → its diff shows; unstaged → empty state. Either + way the tab's own panel is unchanged after closing. +- [ ] Non-file rows ("Ran …", Read/Grep rows) are **not** clickable — plain + text, no hover underline. +- [ ] Delete the file's directory (or prune the worktree) with the row still on + screen → clicking it falls back to the tab's repo and either shows the + diff or the empty state; never an unhandled error. +- [ ] Terminals: open/close the pop-out repeatedly while an agent is streaming + output — typing latency and PTY output unaffected, no input is ever sent + to the session. + ## Quality gates (machine-run, not manual) - [x] `npx tsc --noEmit` clean. *(rerun 2026-08-18, Phase 9)* @@ -1493,3 +1521,6 @@ deferred to a later phase (see PLAN.md). - [x] `npm run board:check` — extended with Now-set round-trip/cap assertions. *(Phase 20)* - [x] `npm run codex-transcript:check` — redacted real-shape Codex JSONL parser, ignored event types, and transcript-as-data assertions pass. +- [x] `npm run diff:check` — per-file diff slicing assertions pass, including + the same-basename-in-a-sibling-directory case that must NOT match. + *(new, issue #10; run 2026-09-06 on Windows via `npm run check`)* diff --git a/package.json b/package.json index 93e0f05..df57d26 100644 --- a/package.json +++ b/package.json @@ -10,7 +10,7 @@ "tauri": "tauri", "reinstall": "sh scripts/reinstall.sh", "golden": "tsx scripts/golden.ts", - "check": "npm run landing:check && npm run epoch:check && npm run bind:check && npm run dedupe:check && npm run reentry:check && npm run unclaimed:check && npm run notify:check && npm run spawn:check && npm run scope:check && npm run clock:check && npm run delta:check && npm run loop:check && npm run extractor-queue:check && npm run decisions:check && npm run codex-transcript:check && npm run board:check && npm run empty-state:check", + "check": "npm run landing:check && npm run epoch:check && npm run bind:check && npm run dedupe:check && npm run reentry:check && npm run unclaimed:check && npm run notify:check && npm run spawn:check && npm run scope:check && npm run clock:check && npm run delta:check && npm run loop:check && npm run extractor-queue:check && npm run decisions:check && npm run codex-transcript:check && npm run board:check && npm run empty-state:check && npm run diff:check", "landing:check": "tsx scripts/landing-check.ts", "epoch:check": "tsx scripts/epoch-check.ts", "bind:check": "tsx scripts/bind-check.ts", @@ -27,7 +27,8 @@ "decisions:check": "tsx scripts/decisions-check.ts", "codex-transcript:check": "tsx scripts/codex-transcript-check.ts", "board:check": "tsx scripts/board-check.ts", - "empty-state:check": "tsx scripts/empty-state-check.ts" + "empty-state:check": "tsx scripts/empty-state-check.ts", + "diff:check": "tsx scripts/diff-check.ts" }, "dependencies": { "@tauri-apps/api": "^2", diff --git a/scripts/diff-check.ts b/scripts/diff-check.ts new file mode 100644 index 0000000..97ae9b5 --- /dev/null +++ b/scripts/diff-check.ts @@ -0,0 +1,88 @@ +// Self-check for the Accomplished diff pop-out's file-section slicing. +// Run: npm run diff:check +import { strict as assert } from "node:assert"; +import { dirOf, extractFileDiff } from "../src/lib/diff"; + +const section = (path: string) => + [ + `diff --git a/${path} b/${path}`, + "index 1111111..2222222 100644", + `--- a/${path}`, + `+++ b/${path}`, + "@@ -1,2 +1,2 @@", + "-old", + "+new", + "", + ].join("\n"); + +const staged = section("src/a/foo.ts") + section("src/b/foo.ts") + section("src-tauri/src/pty.rs"); + +// --- dirOf --- +assert.equal(dirOf("/repo/src/foo.ts"), "/repo/src", "posix path should yield its parent dir"); +assert.equal(dirOf("C:\\repo\\src\\foo.ts"), "C:\\repo\\src", "windows path should keep its native separators"); +assert.equal(dirOf("README.md"), null, "a bare filename has no directory to point git at"); +assert.equal(dirOf("/README.md"), null, "a repo-root-relative path has no usable parent"); + +// --- extractFileDiff --- +const one = extractFileDiff(staged, "/Users/x/repo/src/b/foo.ts"); +assert.ok(one.startsWith("diff --git a/src/b/foo.ts"), "absolute path should match its repo-relative section"); +assert.ok(!one.includes("src/a/foo.ts"), "must not bleed into the neighbouring section"); +assert.equal(one.split("diff --git").length - 1, 1, "exactly one section should come back"); + +// Same basename in a sibling directory must not match — suffix matching is +// anchored at a separator, which is the whole point of the `/` in the check. +assert.equal( + extractFileDiff(staged, "/Users/x/repo/src/c/foo.ts"), + "", + "a same-named file in an unstaged directory must not borrow another's diff" +); + +// A file the agent touched but nobody staged: no section, empty state. +assert.equal(extractFileDiff(staged, "/Users/x/repo/src/unstaged.ts"), "", "unstaged file yields no diff"); +assert.equal(extractFileDiff("", "/Users/x/repo/src/a/foo.ts"), "", "empty diff (not a repo / git failed) yields no diff"); +assert.equal(extractFileDiff(staged, ""), "", "a row with no file path yields no diff"); + +// Windows-shaped file_path against git's always-forward-slash diff paths. +assert.ok( + extractFileDiff(staged, "C:\\repo\\src-tauri\\src\\pty.rs").startsWith("diff --git a/src-tauri/src/pty.rs"), + "backslashed agent path must still match git's forward-slash diff header" +); + +// Added file: the `--- /dev/null` half must never be treated as a path. +const added = [ + "diff --git a/src/new.ts b/src/new.ts", + "new file mode 100644", + "--- /dev/null", + "+++ b/src/new.ts", + "@@ -0,0 +1 @@", + "+hello", + "", +].join("\n"); +assert.ok(extractFileDiff(added, "/repo/src/new.ts").includes("+hello"), "a new file's section must be found"); +assert.equal(extractFileDiff(added, "/dev/null"), "", "/dev/null must never match a section"); + +// Rename with no hunks (pure rename): header-line fallback, either side matches. +const renamed = [ + "diff --git a/src/old.ts b/src/new-name.ts", + "similarity index 100%", + "rename from src/old.ts", + "rename to src/new-name.ts", + "", +].join("\n"); +assert.ok(extractFileDiff(renamed, "/repo/src/new-name.ts").startsWith("diff --git"), "rename destination should match"); +assert.ok(extractFileDiff(renamed, "/repo/src/old.ts").startsWith("diff --git"), "rename source should match"); + +// Hunk content that itself looks like a diff header must not split a section. +const nested = [ + "diff --git a/docs/x.md b/docs/x.md", + "--- a/docs/x.md", + "+++ b/docs/x.md", + "@@ -1 +1,2 @@", + " prose", + "+diff --git a/fake b/fake", + "", +].join("\n"); +assert.equal(extractFileDiff(nested, "/repo/fake"), "", "a header inside hunk content must not become its own section"); +assert.ok(extractFileDiff(nested, "/repo/docs/x.md").includes("+diff --git"), "hunk content stays with its own section"); + +console.log("diff-check: all assertions passed"); diff --git a/src/components/DiffModal.tsx b/src/components/DiffModal.tsx new file mode 100644 index 0000000..6de4e5f --- /dev/null +++ b/src/components/DiffModal.tsx @@ -0,0 +1,78 @@ +import { useEffect, useState } from "react"; +import { loadFileDiff } from "../lib/diff"; +import { basename } from "../lib/repo"; + +interface Props { + filePath: string; // agent-reported path off the Accomplished row + cwd: string; // active tab's project dir, the fallback repo to ask + onClose: () => void; +} + +/** Read-only diff pop-out for an Accomplished row. Raw unified diff, no + * highlighting and no editing — see docs/IDEAS.md for why the editable + * version is not wanted. A lookup that finds nothing renders the empty + * state rather than throwing into the panel tree. */ +export function DiffModal({ filePath, cwd, onClose }: Props) { + const [diff, setDiff] = useState(null); + + useEffect(() => { + let cancelled = false; + void loadFileDiff(filePath, cwd) + .catch(() => "") + .then((text) => { + if (!cancelled) setDiff(text); + }); + return () => { + cancelled = true; + }; + }, [filePath, cwd]); + + // Esc closes wherever focus sits — the overlay click alone strands a user + // who scrolled inside the diff. + useEffect(() => { + const handler = (e: KeyboardEvent) => { + if (e.key === "Escape") { + e.preventDefault(); + onClose(); + } + }; + window.addEventListener("keydown", handler); + return () => window.removeEventListener("keydown", handler); + }, [onClose]); + + return ( + // top-7 keeps the titlebar drag region reachable under the overlay +
+
e.stopPropagation()} + > +

{basename(filePath)}

+

+ {filePath} +

+ {diff === null ? ( +

Loading diff…

+ ) : diff === "" ? ( +

+ No staged diff for this file. The app can only read staged changes + (git diff --cached), so an edit the agent hasn't + staged — or a file outside this project's repo — shows nothing here. +

+ ) : ( + 
+            {diff}
+
+ )} +
+ +
+
+
+ ); +} diff --git a/src/components/SidePanel.tsx b/src/components/SidePanel.tsx index 7f12759..4d5ea7e 100644 --- a/src/components/SidePanel.tsx +++ b/src/components/SidePanel.tsx @@ -22,6 +22,7 @@ import { gitPush, gitUntrackedFiles, } from "../lib/pty"; +import { DiffModal } from "./DiffModal"; import { HUES, RainbowText } from "./RainbowText"; import type { AgentState, Blocker, Commit, Decision, FanOutRollup, Note, ToolEvent } from "../types"; @@ -144,6 +145,7 @@ export function SidePanel({ const [landing, setLanding] = useState(null); // active project, momentum const [notes, setNotes] = useState([]); // active project, open notes & reminders const [context, setContext] = useState(null); + const [diffPath, setDiffPath] = useState(null); const [expandedSessions, setExpandedSessions] = useState>(new Set()); const [plannedCard, setPlannedCard] = useState(null); const seededCwdRef = useRef(null); @@ -1109,9 +1111,19 @@ export function SidePanel({
  • {ago(e.ts)} - - {e.plain} - + {e.filePath ? ( + + ) : ( + + {e.plain} + + )} {e.detail && ( {e.tool} {e.detail} @@ -1166,6 +1178,8 @@ export function SidePanel({ )} + {diffPath && setDiffPath(null)} />} + {context && ( // top-7 keeps the titlebar drag region reachable under the overlay
    setContext(null)}> diff --git a/src/lib/diff.ts b/src/lib/diff.ts new file mode 100644 index 0000000..4080aeb --- /dev/null +++ b/src/lib/diff.ts @@ -0,0 +1,72 @@ +import { gitDiffCached } from "./pty"; + +/** Diff pop-out for an Accomplished row (issue #10). + * + * The only diff the app can ask for is `git_diff_cached(cwd)` — a whole + * directory's *staged* diff — but a row hands us one file path that may sit + * in a different repo than the tab's cwd. Bridge: run the existing command in + * the file's own directory (so a row pointing at another checkout diffs that + * checkout, not this tab's) and slice the one file's section out of the + * unified diff here. No new Tauri command, and nothing is staged on the + * user's behalf — an unstaged file simply has no section, which is the empty + * state. Fails open throughout: every failure path is an empty string. */ + +/** Parent directory of a path, either separator. Null when the path carries + * no directory at all — nothing to point `git -C` at. */ +export function dirOf(path: string): string | null { + const i = path.replace(/\\/g, "/").lastIndexOf("/"); + if (i <= 0) return null; + return path.slice(0, i); +} + +/** Paths a `diff --git` section is about. `--- a/`/`+++ b/` are unambiguous + * where they exist; the header line is the fallback for sections that have + * none (a pure mode change), where a path containing a space can't be split + * reliably — a miss there costs an empty state, never a wrong file. */ +function sectionPaths(section: string): string[] { + const out: string[] = []; + for (const line of section.split("\n")) { + if (line.startsWith("@@")) break; // past the header, into hunk content + const m = /^(?:---|\+\+\+) [ab]\/(.*)$/.exec(line); + if (m) out.push(m[1]); + } + if (out.length === 0) { + const m = /^diff --git a\/(.+?) b\/(.+)$/.exec(section.split("\n")[0] ?? ""); + if (m) out.push(m[1], m[2]); + } + return out; +} + +/** The one file's section of a unified diff, or "" if it isn't in there. + * Diff paths are repo-relative and `file_path` is absolute, so they're + * matched by path suffix — anchored at a separator, so `src/b/x.ts` never + * matches a row for `src/a/x.ts`. A rename matches on either side. */ +export function extractFileDiff(diff: string, filePath: string): string { + if (!diff || !filePath) return ""; + const target = filePath.replace(/\\/g, "/"); + const sections = diff.split(/^(?=diff --git )/m).filter((s) => s.startsWith("diff --git ")); + for (const section of sections) { + const hit = sectionPaths(section).some( + (p) => p !== "/dev/null" && (target === p || target.endsWith(`/${p}`)) + ); + if (hit) return section.trimEnd(); + } + return ""; +} + +/** Staged diff for one Accomplished row's file. `fallbackCwd` is the tab's + * project dir, used when the row's path has no directory of its own and as a + * second try when the file's own directory is gone (worktree pruned, dir + * deleted) but the tab's repo still holds the staged change. + * The path reaches git as a `Command::arg`, never a shell string, so an + * agent-supplied path is inert here — see `git_commit`'s note in pty.rs. */ +export async function loadFileDiff(filePath: string, fallbackCwd: string): Promise { + const dir = dirOf(filePath) ?? fallbackCwd; + const tries = dir === fallbackCwd ? [dir] : [dir, fallbackCwd]; + for (const at of tries) { + const staged = await gitDiffCached(at).catch(() => ""); + const own = extractFileDiff(staged, filePath); + if (own) return own; + } + return ""; +} diff --git a/src/lib/repo.ts b/src/lib/repo.ts index 6997828..cb6d252 100644 --- a/src/lib/repo.ts +++ b/src/lib/repo.ts @@ -110,6 +110,10 @@ export async function listToolEvents(cwd: string, limit = 50): Promise = { Edit: "Edited", Write: "Wrote", @@ -130,6 +134,7 @@ export async function listToolEvents(cwd: string, limit = 50): Promise; tool = typeof p.tool_name === "string" ? p.tool_name : "?"; @@ -138,6 +143,7 @@ export async function listToolEvents(cwd: string, limit = 50): Promise