From 1c1be5182981a9d6a0c4994295fc2ead4f254048 Mon Sep 17 00:00:00 2001 From: Radwuan Abouzeid Date: Sun, 6 Sep 2026 04:26:51 -0500 Subject: [PATCH] feat(panel): read-only diff pop-out from Accomplished rows MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Reading what an agent actually changed meant leaving the app: the Accomplished panel names the file ("Edited src/bar.rs") but not the change. Clicking a Wrote/Edited row now opens a read-only pop-out with that file's unified diff. The two halves already existed but did not line up: `git_diff_cached(cwd)` returns a whole directory's staged diff, while a row carries one file path that may live in a different repo than the tab's cwd. Bridged entirely in TypeScript, with no new Tauri command: run the existing command in the file's own directory, then slice that file's section out of the unified diff in `src/lib/diff.ts`. Diff paths are repo-relative and the row's path is absolute, so they are matched by separator-anchored suffix — src/b/x.ts never answers for a row about src/a/x.ts. Staged-only is a real limit, and it is surfaced rather than worked around: nothing is staged on the user's behalf (that would mutate the repo behind a human who is mid-commit), so an unstaged edit, or a file outside any repo git can see, renders an empty state naming the constraint. Every failure path — git absent, directory gone, path unparseable — resolves to that same empty state; the lookup can never throw into the panel tree. Which rows are diffable is decided in `listToolEvents`, not in the component: `ToolEvent.filePath` is set only for tools that write the file they name, so Read/Grep/Bash rows stay plain text and panels stay dumb. Read-only by design — see docs/IDEAS.md for why the editable code view is rejected. No schema change, no migration, no new dependency. `diff:check` covers the slicing and rides the existing `npm run check` gate. Closes #10 --- docs/TESTING.md | 31 +++++++++++++ package.json | 5 +- scripts/diff-check.ts | 88 ++++++++++++++++++++++++++++++++++++ src/components/DiffModal.tsx | 78 ++++++++++++++++++++++++++++++++ src/components/SidePanel.tsx | 20 ++++++-- src/lib/diff.ts | 72 +++++++++++++++++++++++++++++ src/lib/repo.ts | 8 +++- src/types.ts | 1 + 8 files changed, 297 insertions(+), 6 deletions(-) create mode 100644 scripts/diff-check.ts create mode 100644 src/components/DiffModal.tsx create mode 100644 src/lib/diff.ts diff --git a/docs/TESTING.md b/docs/TESTING.md index 839f1c8..9f6de7d 100644 --- a/docs/TESTING.md +++ b/docs/TESTING.md @@ -1108,6 +1108,34 @@ All passed 2026-09-05. within 5s → tagged `human` (paste counts as input). - [ ] Terminals: throughout, typing latency and PTY output unaffected. +## 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)* @@ -1153,3 +1181,6 @@ All passed 2026-09-05. - [x] `npm run reentry:check` — one row per tether, latest wins on resume. *(new, Phase 6)* - [x] `npm run unclaimed:check` — flag/claim predicate assertions pass. *(new, Phase 6)* - [x] `npm run notify:check` — nudge fire predicate assertions pass. *(new, Phase 6)* +- [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 287981d..f16d131 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", + "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 diff:check", "landing:check": "tsx scripts/landing-check.ts", "epoch:check": "tsx scripts/epoch-check.ts", "bind:check": "tsx scripts/bind-check.ts", @@ -22,7 +22,8 @@ "scope:check": "tsx scripts/scope-check.ts", "clock:check": "tsx scripts/clock-check.ts", "delta:check": "tsx scripts/delta-check.ts", - "loop:check": "tsx scripts/loop-check.ts" + "loop:check": "tsx scripts/loop-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 fb5954e..e528357 100644 --- a/src/components/SidePanel.tsx +++ b/src/components/SidePanel.tsx @@ -20,6 +20,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"; @@ -108,6 +109,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 [draft, setDraft] = useState(""); const [noteDraft, setNoteDraft] = useState(""); const [showAllTools, setShowAllTools] = useState(false); @@ -979,9 +981,19 @@ export function SidePanel({
  • {ago(e.ts)} - - {e.plain} - + {e.filePath ? ( + + ) : ( + + {e.plain} + + )} {e.detail && ( {e.tool} {e.detail} @@ -1036,6 +1048,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 1fdc8fb..55d262b 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", @@ -122,6 +126,7 @@ export async function listToolEvents(cwd: string, limit = 50): Promise; tool = typeof p.tool_name === "string" ? p.tool_name : "?"; @@ -130,6 +135,7 @@ export async function listToolEvents(cwd: string, limit = 50): Promise