From dc5a3312e0aabb76607aabeabf451c2e0354a920 Mon Sep 17 00:00:00 2001 From: Waishnav Date: Mon, 10 Aug 2026 01:03:25 +0530 Subject: [PATCH 1/4] fix: harden git review snapshots Review checkpoints were commits created with -p HEAD from an index in the system temp directory, and the whole path failed on repositories with no commits yet. Snapshots are now root commits built from a temporary index inside the git common directory with fsmonitor and the untracked cache disabled, so capture is isolated from the real index, immune to HEAD ancestry, and works on unborn repositories. The checkpoint manager records the HEAD used at capture and re-anchors both refs to fresh snapshots when the repository's HEAD moves, so reviews never diff across unrelated histories. Existing checkpoints created before this change are left untouched. Diffs now ignore whitespace-only changes by default, use the standard 10 MB output cap, and degrade to a file list with a note instead of failing when a patch exceeds it. --- src/review-checkpoints.ts | 113 +++++++++++++++++++++++++++----------- 1 file changed, 82 insertions(+), 31 deletions(-) diff --git a/src/review-checkpoints.ts b/src/review-checkpoints.ts index 467b80f4..6e9c10f0 100644 --- a/src/review-checkpoints.ts +++ b/src/review-checkpoints.ts @@ -1,6 +1,5 @@ import { mkdtemp, rm } from "node:fs/promises"; -import { tmpdir } from "node:os"; -import { join } from "node:path"; +import { isAbsolute, join } from "node:path"; import { git, getGitEligibility, safeWorkspaceRefSegment } from "./git.js"; export type ReviewSince = "last_shown" | "workspace_open"; @@ -33,6 +32,8 @@ interface WorkspaceReviewState { baselineRef: string; openRefAvailable: boolean; baselineRefAvailable: boolean; + headSha?: string; + headTracked?: boolean; diagnostic?: string; } @@ -43,6 +44,7 @@ export interface ReviewCheckpointManager { root: string; since?: ReviewSince; markReviewed?: boolean; + ignoreWhitespace?: boolean; }): Promise; } @@ -78,7 +80,7 @@ export function createReviewCheckpointManager(): ReviewCheckpointManager { } }, - async reviewChanges({ workspaceId, root, since = "last_shown", markReviewed = true }) { + async reviewChanges({ workspaceId, root, since = "last_shown", markReviewed = true, ignoreWhitespace = true }) { let state = states.get(workspaceId); assertWorkspaceRoot(state, workspaceId, root); if (!isReadyState(state)) { @@ -91,6 +93,22 @@ export function createReviewCheckpointManager(): ReviewCheckpointManager { throw new Error(state?.diagnostic ?? "show_changes requires a Git workspace in this version."); } + const currentHead = await headSha(state.gitRoot); + if (state.headTracked && state.headSha !== currentHead) { + const snapshot = await createWorkingTreeSnapshot(state.gitRoot); + await git(state.gitRoot, ["update-ref", state.openRef, snapshot.commit]); + await git(state.gitRoot, ["update-ref", state.baselineRef, snapshot.commit]); + state.headSha = snapshot.headSha; + state.openRefAvailable = true; + state.baselineRefAvailable = true; + return { + result: "The repository HEAD moved since the last review, so the review checkpoints were re-anchored to the latest commit. No changes since then.", + summary: { files: 0, additions: 0, removals: 0 }, + files: [], + patch: "", + }; + } + let effectiveSince = since; let usedWorkspaceOpenFallback = false; if (since === "last_shown" && !state.baselineRefAvailable) { @@ -108,39 +126,38 @@ export function createReviewCheckpointManager(): ReviewCheckpointManager { const baselineRef = effectiveSince === "workspace_open" ? state.openRef : state.baselineRef; const baseline = (await git(state.gitRoot, ["rev-parse", "--verify", `${baselineRef}^{commit}`])).stdout.trim(); const current = await createWorkingTreeSnapshot(state.gitRoot); - const patch = (await git(state.gitRoot, [ - "diff", - "--no-color", - "--no-ext-diff", - "--no-textconv", - baseline, - current, - ], { - maxBuffer: 50 * 1024 * 1024, - })).stdout; - const numstat = (await git(state.gitRoot, ["diff", "--numstat", "-z", baseline, current], { - maxBuffer: 50 * 1024 * 1024, + const whitespaceArgs = ignoreWhitespace ? ["--ignore-all-space"] : []; + const patch = await diffOrDegrade( + state.gitRoot, + ["diff", ...whitespaceArgs, "--no-color", "--no-ext-diff", "--no-textconv", baseline, current.commit], + ); + const numstat = (await git(state.gitRoot, ["diff", ...whitespaceArgs, "--numstat", "-z", baseline, current.commit], { + maxBuffer: 10 * 1024 * 1024, })).stdout; const files = parseNumstat(numstat); const summary = summarizeFiles(files); if (markReviewed) { - await git(state.gitRoot, ["update-ref", state.baselineRef, current]); + await git(state.gitRoot, ["update-ref", state.baselineRef, current.commit]); state.baselineRefAvailable = true; + state.headSha = current.headSha; } const fallbackNote = usedWorkspaceOpenFallback ? ` The last-shown checkpoint was missing, so changes were compared from workspace open${markReviewed ? " and the baseline was re-established" : ""}.` : ""; + const degradedNote = patch.degraded && summary.files > 0 + ? " The diff was too large to render, so a file list is shown instead." + : ""; return { result: `${ summary.files === 0 ? `No changes since ${effectiveSince === "workspace_open" ? "workspace open" : "last shown changes"}.` : `Changed ${summary.files} ${summary.files === 1 ? "file" : "files"} (+${summary.additions} -${summary.removals}).` - }${fallbackNote}`, + }${fallbackNote}${degradedNote}`, summary, files, - patch, + patch: patch.text, }; }, }; @@ -171,28 +188,31 @@ async function initializeWorkspaceState( try { const eligibility = await getGitEligibility(root); - if (!eligibility.ok || !eligibility.gitRoot) { + if (!eligibility.gitRoot) { state.diagnostic = eligibility.message ?? "show_changes requires a Git workspace in this version."; return; } + const gitRoot = eligibility.gitRoot; const [openCommit, baselineCommit] = await Promise.all([ - commitForRef(eligibility.gitRoot, state.openRef), - commitForRef(eligibility.gitRoot, state.baselineRef), + commitForRef(gitRoot, state.openRef), + commitForRef(gitRoot, state.baselineRef), ]); if (!openCommit && !baselineCommit) { - const initialCommit = await createWorkingTreeSnapshot(eligibility.gitRoot); - await git(eligibility.gitRoot, ["update-ref", state.openRef, initialCommit]); - await git(eligibility.gitRoot, ["update-ref", state.baselineRef, initialCommit]); + const initialCommit = await createWorkingTreeSnapshot(gitRoot); + await git(gitRoot, ["update-ref", state.openRef, initialCommit.commit]); + await git(gitRoot, ["update-ref", state.baselineRef, initialCommit.commit]); state.openRefAvailable = true; state.baselineRefAvailable = true; + state.headSha = initialCommit.headSha; + state.headTracked = true; } else { state.openRefAvailable = openCommit !== undefined; state.baselineRefAvailable = baselineCommit !== undefined; } - state.gitRoot = eligibility.gitRoot; + state.gitRoot = gitRoot; } catch (error) { state.diagnostic = error instanceof Error ? error.message : String(error); } finally { @@ -222,22 +242,53 @@ function reviewRefs( }; } -async function createWorkingTreeSnapshot(gitRoot: string): Promise { - const tempDir = await mkdtemp(join(tmpdir(), "devspace-review-index-")); +async function createWorkingTreeSnapshot(gitRoot: string): Promise<{ commit: string; headSha?: string }> { + const head = await headSha(gitRoot); + const commonDir = (await git(gitRoot, ["rev-parse", "--git-common-dir"])).stdout.trim(); + const commonDirPath = isAbsolute(commonDir) ? commonDir : join(gitRoot, commonDir); + const tempDir = await mkdtemp(join(commonDirPath, "devspace-review-index-")); const indexPath = join(tempDir, "index"); const env = checkpointEnv(indexPath); + const fsFlags = ["-c", "core.fsmonitor=false", "-c", "core.untrackedCache=false"]; try { - await git(gitRoot, ["read-tree", "HEAD"], { env }); - await git(gitRoot, ["add", "-A", "--", "."], { env }); + if (head) { + await git(gitRoot, ["read-tree", "HEAD"], { env }); + } else { + await git(gitRoot, ["read-tree", "--empty"], { env }); + } + await git(gitRoot, [...fsFlags, "add", "-A", "--", "."], { env }); const tree = (await git(gitRoot, ["write-tree"], { env })).stdout.trim(); - const parent = (await git(gitRoot, ["rev-parse", "--verify", "HEAD^{commit}"])).stdout.trim(); - return (await git(gitRoot, ["commit-tree", tree, "-p", parent, "-m", "DevSpace review snapshot"], { env })).stdout.trim(); + const commit = (await git(gitRoot, ["commit-tree", tree, "-m", "DevSpace review snapshot"], { env })).stdout.trim(); + return { commit, headSha: head }; } finally { await rm(tempDir, { recursive: true, force: true }); } } +async function headSha(gitRoot: string): Promise { + try { + return (await git(gitRoot, ["rev-parse", "--verify", "HEAD^{commit}"])).stdout.trim(); + } catch { + return undefined; + } +} + +async function diffOrDegrade( + gitRoot: string, + args: string[], +): Promise<{ text: string; degraded: boolean }> { + try { + const patch = await git(gitRoot, args, { maxBuffer: 10 * 1024 * 1024 }); + return { text: patch.stdout, degraded: false }; + } catch (error) { + if (String(error).includes("maxBuffer")) { + return { text: "", degraded: true }; + } + throw error; + } +} + function checkpointEnv(indexPath: string): NodeJS.ProcessEnv { return { GIT_INDEX_FILE: indexPath, From 9959ce485dbd03826c997fdf07aba1b27982af67 Mon Sep 17 00:00:00 2001 From: Waishnav Date: Mon, 10 Aug 2026 01:03:25 +0530 Subject: [PATCH 2/4] test: cover snapshot hardening, HEAD re-anchor, and diff degradation The unborn-repository test now asserts the improved behavior: an empty repository is reviewable immediately, re-anchors after the first commit, and continues reviewing from the new baseline. New tests cover whitespace-only changes being ignored by default but visible on request, and an oversized diff degrading to a file list. --- src/review-checkpoints.test.ts | 85 ++++++++++++++++++++++++++++++---- 1 file changed, 76 insertions(+), 9 deletions(-) diff --git a/src/review-checkpoints.test.ts b/src/review-checkpoints.test.ts index 0c2aeb7b..d29de7d3 100644 --- a/src/review-checkpoints.test.ts +++ b/src/review-checkpoints.test.ts @@ -173,27 +173,94 @@ test("a concurrent review rejects a different root after initialization", async } }); -test("an unborn repository becomes reviewable after its first commit", async (t) => { +test("an unborn repository is reviewable and re-anchors after its first commit", async (t) => { const root = await unbornRepository(t); const manager = createReviewCheckpointManager(); await manager.initializeWorkspace({ workspaceId: "ws_unborn", root }); - await assert.rejects( - () => manager.reviewChanges({ workspaceId: "ws_unborn", root }), - /repository has no HEAD commit/, - ); + const initial = await manager.reviewChanges({ workspaceId: "ws_unborn", root, markReviewed: false }); + assert.equal(initial.summary.files, 0); + + await writeFile(join(root, "README.md"), "working tree only\n"); + const beforeCommit = await manager.reviewChanges({ + workspaceId: "ws_unborn", + root, + markReviewed: false, + }); + assert.equal(beforeCommit.files.length, 1); + assert.equal(beforeCommit.files[0]?.type, "new"); - await writeFile(join(root, "README.md"), "first commit\n"); await git(root, ["add", "README.md"]); await git(root, ["commit", "-m", "Initial commit"]); - const afterFirstCommit = await manager.reviewChanges({ + const reanchored = await manager.reviewChanges({ + workspaceId: "ws_unborn", + root, + markReviewed: false, + }); + assert.equal(reanchored.summary.files, 0); + assert.match(reanchored.result, /re-anchored/); + + await writeFile(join(root, "notes.txt"), "after commit\n"); + const afterCommit = await manager.reviewChanges({ workspaceId: "ws_unborn", root, markReviewed: false, }); - assert.equal(afterFirstCommit.summary.files, 0); - assert.equal(afterFirstCommit.patch, ""); + assert.deepEqual(afterCommit.files.map((file) => file.path), ["notes.txt"]); +}); + +test("whitespace-only changes are ignored by default and visible when requested", async (t) => { + const root = await committedRepository(t); + const manager = createReviewCheckpointManager(); + await manager.initializeWorkspace({ workspaceId: "ws_whitespace", root }); + + await writeFile(join(root, "README.md"), "hello \n"); + + const ignored = await manager.reviewChanges({ + workspaceId: "ws_whitespace", + root, + markReviewed: false, + }); + assert.equal(ignored.summary.files, 0); + assert.equal(ignored.patch, ""); + + const visible = await manager.reviewChanges({ + workspaceId: "ws_whitespace", + root, + markReviewed: false, + ignoreWhitespace: false, + }); + assert.equal(visible.summary.files, 1); + assert.equal(visible.summary.additions, 1); +}); + +test("an oversized diff degrades to a file list instead of failing", async (t) => { + const lineCount = 1_500_000; + const root = await committedRepository(t); + await writeFile(join(root, "big.txt"), `${"aaa\n".repeat(lineCount)}`); + await git(root, ["add", "big.txt"]); + await git(root, ["commit", "-m", "Add big file"]); + const manager = createReviewCheckpointManager(); + await manager.initializeWorkspace({ workspaceId: "ws_oversized", root }); + + await writeFile(join(root, "big.txt"), `${"bbb\n".repeat(lineCount)}`); + + const review = await manager.reviewChanges({ + workspaceId: "ws_oversized", + root, + markReviewed: true, + }); + assert.equal(review.files.length, 1); + assert.equal(review.patch, ""); + assert.match(review.result, /file list/); + + const after = await manager.reviewChanges({ + workspaceId: "ws_oversized", + root, + markReviewed: false, + }); + assert.equal(after.summary.files, 0); }); async function committedRepository(t: TestContext): Promise { From e93bb1d0d35dfa217f7ed87665210ecdb171331e Mon Sep 17 00:00:00 2001 From: Waishnav Date: Mon, 10 Aug 2026 01:03:25 +0530 Subject: [PATCH 3/4] docs: document change review modes and review checkpoints Describe the journal-based review, the DEVSPACE_REVIEW_MODE=git fallback, and how checkpoints are stored and re-anchored. --- docs/configuration.md | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/docs/configuration.md b/docs/configuration.md index 3502a98b..6bdee29c 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -119,6 +119,29 @@ sessions. | `changes` | Enables the aggregate `show_changes` tool and attaches widget UI to `open_workspace` and `show_changes`. | | `off` | Disables widget UI. | +## Change review + +`show_changes` (enabled by `DEVSPACE_WIDGETS=changes`) reviews the changes +made in the current work session. By default it diffs a change journal that +records the original state of every file the write, edit, and apply_patch +tools first touch, so review works in any workspace, git or not, and never +scans the repository. + +Set `DEVSPACE_REVIEW_MODE=git` to use the git-backed review instead. It +compares the working tree against persisted review checkpoints: + +- Checkpoints are commits created from a temporary index inside the git + common directory and stored under the + `refs/devspace/review//open` and + `refs/devspace/review//baseline` refs. +- The baseline advances whenever changes are shown. +- If the repository's HEAD moves between reviews, checkpoints are re-anchored + to the latest commit rather than diffed across different histories. +- Diffs ignore whitespace-only changes and degrade to a file list when the + patch would exceed 10 MB. + +The git-backed review is the fallback path; the journal is the default. + ## Skills | Variable | Purpose | From a371fb3a573ca9502d99886f702891e6783a396a Mon Sep 17 00:00:00 2001 From: Waishnav Date: Mon, 10 Aug 2026 11:28:08 +0530 Subject: [PATCH 4/4] fix: keep pending edits visible across HEAD moves and degradation Re-anchoring used to bake the whole working tree, unreviewed edits included, into the new baseline snapshot, which made pending changes vanish from every subsequent review. The baseline now points at the new HEAD commit itself, so unreviewed working-tree edits stay visible. HEAD tracking is restored when a restart finds persisted review refs, so a HEAD move after a restart is absorbed by re-anchoring instead of reporting committed changes as pending workspace changes. The degraded diff path no longer runs the 10 MB-capped numstat call that could overflow and fail the whole review; oversized diffs now fall back to a name-only file list, and a review that cannot list files either reports the degradation instead of claiming there were no changes. --- src/review-checkpoints.test.ts | 49 +++++++++++++++++++++++++++++++++ src/review-checkpoints.ts | 50 +++++++++++++++++++++++----------- 2 files changed, 83 insertions(+), 16 deletions(-) diff --git a/src/review-checkpoints.test.ts b/src/review-checkpoints.test.ts index d29de7d3..b761950d 100644 --- a/src/review-checkpoints.test.ts +++ b/src/review-checkpoints.test.ts @@ -263,6 +263,55 @@ test("an oversized diff degrades to a file list instead of failing", async (t) = assert.equal(after.summary.files, 0); }); +test("re-anchoring after a HEAD move keeps unreviewed pending edits visible", async (t) => { + const root = await committedRepository(t); + const manager = createReviewCheckpointManager(); + await manager.initializeWorkspace({ workspaceId: "ws_reanchor_edits", root }); + + await writeFile(join(root, "pending.txt"), "uncommitted work\n"); + await writeFile(join(root, "committed.txt"), "committed later\n"); + await git(root, ["add", "committed.txt"]); + await git(root, ["commit", "-m", "Move HEAD"]); + + const reanchored = await manager.reviewChanges({ + workspaceId: "ws_reanchor_edits", + root, + markReviewed: false, + }); + assert.equal(reanchored.summary.files, 0); + assert.match(reanchored.result, /re-anchored/); + + const after = await manager.reviewChanges({ + workspaceId: "ws_reanchor_edits", + root, + markReviewed: false, + }); + assert.deepEqual(after.files.map((file) => file.path), ["pending.txt"]); + assert.match(after.patch, /uncommitted work/); +}); + +test("HEAD moves after a restart are absorbed by re-anchoring", async (t) => { + const root = await committedRepository(t); + const manager = createReviewCheckpointManager(); + await manager.initializeWorkspace({ workspaceId: "ws_restart_track", root }); + await manager.reviewChanges({ workspaceId: "ws_restart_track", root, markReviewed: true }); + + const restartedManager = createReviewCheckpointManager(); + await restartedManager.initializeWorkspace({ workspaceId: "ws_restart_track", root }); + + await writeFile(join(root, "committed.txt"), "new\n"); + await git(root, ["add", "committed.txt"]); + await git(root, ["commit", "-m", "Move HEAD after restart"]); + + const review = await restartedManager.reviewChanges({ + workspaceId: "ws_restart_track", + root, + markReviewed: false, + }); + assert.equal(review.summary.files, 0); + assert.match(review.result, /re-anchored/); +}); + async function committedRepository(t: TestContext): Promise { const root = await mkdtemp(join(tmpdir(), "devspace-review-checkpoints-test-")); t.after(() => rm(root, { recursive: true, force: true })); diff --git a/src/review-checkpoints.ts b/src/review-checkpoints.ts index 6e9c10f0..9de55598 100644 --- a/src/review-checkpoints.ts +++ b/src/review-checkpoints.ts @@ -94,15 +94,12 @@ export function createReviewCheckpointManager(): ReviewCheckpointManager { } const currentHead = await headSha(state.gitRoot); - if (state.headTracked && state.headSha !== currentHead) { - const snapshot = await createWorkingTreeSnapshot(state.gitRoot); - await git(state.gitRoot, ["update-ref", state.openRef, snapshot.commit]); - await git(state.gitRoot, ["update-ref", state.baselineRef, snapshot.commit]); - state.headSha = snapshot.headSha; - state.openRefAvailable = true; + if (state.headTracked && currentHead && state.headSha !== currentHead) { + await git(state.gitRoot, ["update-ref", state.baselineRef, currentHead]); + state.headSha = currentHead; state.baselineRefAvailable = true; return { - result: "The repository HEAD moved since the last review, so the review checkpoints were re-anchored to the latest commit. No changes since then.", + result: "The repository HEAD moved since the last review, so the review baseline was re-anchored to the latest commit.", summary: { files: 0, additions: 0, removals: 0 }, files: [], patch: "", @@ -131,10 +128,24 @@ export function createReviewCheckpointManager(): ReviewCheckpointManager { state.gitRoot, ["diff", ...whitespaceArgs, "--no-color", "--no-ext-diff", "--no-textconv", baseline, current.commit], ); - const numstat = (await git(state.gitRoot, ["diff", ...whitespaceArgs, "--numstat", "-z", baseline, current.commit], { - maxBuffer: 10 * 1024 * 1024, - })).stdout; - const files = parseNumstat(numstat); + let files: ReviewFile[] = []; + if (patch.degraded) { + const names = await diffOrDegrade( + state.gitRoot, + ["diff", ...whitespaceArgs, "--name-only", "-z", baseline, current.commit], + ); + if (!names.degraded) { + files = names.text + .split("\0") + .filter((path) => path.length > 0) + .map((path) => ({ path, type: "change", additions: 0, removals: 0 })); + } + } else { + const numstat = (await git(state.gitRoot, ["diff", ...whitespaceArgs, "--numstat", "-z", baseline, current.commit], { + maxBuffer: 10 * 1024 * 1024, + })).stdout; + files = parseNumstat(numstat); + } const summary = summarizeFiles(files); if (markReviewed) { @@ -150,11 +161,13 @@ export function createReviewCheckpointManager(): ReviewCheckpointManager { ? " The diff was too large to render, so a file list is shown instead." : ""; return { - result: `${ - summary.files === 0 - ? `No changes since ${effectiveSince === "workspace_open" ? "workspace open" : "last shown changes"}.` - : `Changed ${summary.files} ${summary.files === 1 ? "file" : "files"} (+${summary.additions} -${summary.removals}).` - }${fallbackNote}${degradedNote}`, + result: patch.degraded && summary.files === 0 + ? "The diff was too large to render, so a file list is shown instead." + : `${ + summary.files === 0 + ? `No changes since ${effectiveSince === "workspace_open" ? "workspace open" : "last shown changes"}.` + : `Changed ${summary.files} ${summary.files === 1 ? "file" : "files"} (+${summary.additions} -${summary.removals}).` + }${fallbackNote}${degradedNote}`, summary, files, patch: patch.text, @@ -210,6 +223,11 @@ async function initializeWorkspaceState( } else { state.openRefAvailable = openCommit !== undefined; state.baselineRefAvailable = baselineCommit !== undefined; + const head = await headSha(gitRoot); + if (head) { + state.headSha = head; + state.headTracked = true; + } } state.gitRoot = gitRoot;