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 | diff --git a/src/review-checkpoints.test.ts b/src/review-checkpoints.test.ts index 0c2aeb7b..b761950d 100644 --- a/src/review-checkpoints.test.ts +++ b/src/review-checkpoints.test.ts @@ -173,27 +173,143 @@ 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); +}); + +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 { diff --git a/src/review-checkpoints.ts b/src/review-checkpoints.ts index 467b80f4..9de55598 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,19 @@ 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 && 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 baseline was re-anchored to the latest commit.", + summary: { files: 0, additions: 0, removals: 0 }, + files: [], + patch: "", + }; + } + let effectiveSince = since; let usedWorkspaceOpenFallback = false; if (since === "last_shown" && !state.baselineRefAvailable) { @@ -108,39 +123,54 @@ 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, - })).stdout; - const files = parseNumstat(numstat); + const whitespaceArgs = ignoreWhitespace ? ["--ignore-all-space"] : []; + const patch = await diffOrDegrade( + state.gitRoot, + ["diff", ...whitespaceArgs, "--no-color", "--no-ext-diff", "--no-textconv", baseline, current.commit], + ); + 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) { - 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}`, + 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: patch.text, }; }, }; @@ -171,28 +201,36 @@ 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; + const head = await headSha(gitRoot); + if (head) { + state.headSha = head; + state.headTracked = true; + } } - state.gitRoot = eligibility.gitRoot; + state.gitRoot = gitRoot; } catch (error) { state.diagnostic = error instanceof Error ? error.message : String(error); } finally { @@ -222,22 +260,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,