feat(ui): add diff viewer (live toast, /diff slash command, end-of-run summary)#599
Conversation
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 3 potential issues.
Autofix Details
Bugbot Autofix prepared fixes for all 3 issues found in the latest run.
- ✅ Fixed: Diff counting incorrectly excludes lines starting with
++/--- Removed the incorrect
!line.startsWith('+++')and!line.startsWith('---')guards from bothsummarizeDiff(structured hunk lines never contain file headers) andclassifyPatchLine(file headers are already stripped bystripPatchHeader).
- Removed the incorrect
- ✅ Fixed: Full diff recomputed every render frame in FileWriteRow
- Wrapped
FileWriteRowinReact.memoand memoized thegetLedger().getDiff()call withuseMemokeyed onstatusandentry.path, so applied rows no longer recompute diffs on spinner frame ticks.
- Wrapped
- ✅ Fixed: Wasteful synchronous disk read before early return in
capturePre- Moved the
safeRead(path)call after theif (existing) { return; }early-return check so repeated edits to the same file skip the unnecessary synchronous disk read.
- Moved the
Or push these changes by commenting:
@cursor push 4c402f2899
Preview (4c402f2899)
diff --git a/src/lib/file-change-ledger.ts b/src/lib/file-change-ledger.ts
--- a/src/lib/file-change-ledger.ts
+++ b/src/lib/file-change-ledger.ts
@@ -114,8 +114,8 @@
newLines: h.newLines,
});
for (const line of h.lines) {
- if (line.startsWith('+') && !line.startsWith('+++')) additions++;
- else if (line.startsWith('-') && !line.startsWith('---')) deletions++;
+ if (line.startsWith('+')) additions++;
+ else if (line.startsWith('-')) deletions++;
}
}
return { additions, deletions, hunks };
@@ -141,12 +141,12 @@
*/
capturePre(path: string, operation: FileChangeOperation): void {
const existing = this.findActive(path);
- const before = operation === 'create' ? null : safeRead(path);
if (existing) {
// Don't overwrite the original `before` — preserve start-of-run state
// so the cumulative diff is accurate.
return;
}
+ const before = operation === 'create' ? null : safeRead(path);
const record: FileChangeRecord = {
path,
operation,
diff --git a/src/ui/tui/components/DiffViewer.tsx b/src/ui/tui/components/DiffViewer.tsx
--- a/src/ui/tui/components/DiffViewer.tsx
+++ b/src/ui/tui/components/DiffViewer.tsx
@@ -84,10 +84,10 @@
line: string,
): { color: string; line: string } {
if (line.startsWith('@@')) return { color: Colors.accent, line };
- if (line.startsWith('+') && !line.startsWith('+++')) {
+ if (line.startsWith('+')) {
return { color: Colors.success, line };
}
- if (line.startsWith('-') && !line.startsWith('---')) {
+ if (line.startsWith('-')) {
return { color: Colors.error, line };
}
if (line.startsWith('\\ No newline')) {
diff --git a/src/ui/tui/components/FileWritesPanel.tsx b/src/ui/tui/components/FileWritesPanel.tsx
--- a/src/ui/tui/components/FileWritesPanel.tsx
+++ b/src/ui/tui/components/FileWritesPanel.tsx
@@ -24,7 +24,7 @@
*/
import { Box, Text } from 'ink';
-import { useMemo, type ReactElement } from 'react';
+import { memo, useMemo, type ReactElement } from 'react';
import path from 'path';
import { Colors, Icons } from '../styles.js';
import { BrailleSpinner } from './BrailleSpinner.js';
@@ -150,7 +150,7 @@
now: number;
}
-const FileWriteRow = ({
+const FileWriteRow = memo(({
entry,
installDir,
spinnerFrame,
@@ -170,6 +170,17 @@
icon = <BrailleSpinner color={Colors.active} frame={spinnerFrame} />;
}
+ // Memoize the diff summary for applied rows — the ledger record for
+ // a completed file never changes, so avoid recomputing on every frame.
+ const diffSummary = useMemo(() => {
+ if (status !== 'applied') return null;
+ try {
+ return getLedger().getDiff(entry.path);
+ } catch {
+ return null;
+ }
+ }, [status, entry.path]);
+
// Trailing detail column. While the row is still planned we show
// "generating…" with elapsed seconds so a stuck write is visible
// (instead of a dead spinner). On apply we prefer the +N/-M diff
@@ -183,23 +194,15 @@
? formatDuration(entry.completedAt - entry.startedAt)
: '';
let sizeHint: string;
- try {
- const summary = getLedger().getDiff(entry.path);
- if (
- summary &&
- (summary.additions > 0 || summary.deletions > 0)
- ) {
- sizeHint = `+${summary.additions}/-${summary.deletions}`;
- } else if (typeof entry.bytes === 'number') {
- sizeHint = `${entry.bytes.toLocaleString()} bytes`;
- } else {
- sizeHint = 'edited';
- }
- } catch {
- sizeHint =
- typeof entry.bytes === 'number'
- ? `${entry.bytes.toLocaleString()} bytes`
- : 'edited';
+ if (
+ diffSummary &&
+ (diffSummary.additions > 0 || diffSummary.deletions > 0)
+ ) {
+ sizeHint = `+${diffSummary.additions}/-${diffSummary.deletions}`;
+ } else if (typeof entry.bytes === 'number') {
+ sizeHint = `${entry.bytes.toLocaleString()} bytes`;
+ } else {
+ sizeHint = 'edited';
}
detail = dur ? `${sizeHint} ${dur}` : sizeHint;
} else if (status === 'failed') {
@@ -225,4 +228,4 @@
<Text color={Colors.muted}>{detail}</Text>
</Box>
);
-};
+});
diff --git a/src/ui/tui/components/__tests__/DiffViewer.test.tsx b/src/ui/tui/components/__tests__/DiffViewer.test.tsx
--- a/src/ui/tui/components/__tests__/DiffViewer.test.tsx
+++ b/src/ui/tui/components/__tests__/DiffViewer.test.tsx
@@ -73,9 +73,9 @@
expect(classifyPatchLine('-old line').color).toBe(Colors.error);
});
- it('does NOT mis-classify the +++ / --- file header rows as content', () => {
- expect(classifyPatchLine('+++ b.txt').color).not.toBe(Colors.success);
- expect(classifyPatchLine('--- a.txt').color).not.toBe(Colors.error);
+ it('classifies +++ / --- lines as additions/deletions (headers already stripped)', () => {
+ expect(classifyPatchLine('+++ b.txt').color).toBe(Colors.success);
+ expect(classifyPatchLine('--- a.txt').color).toBe(Colors.error);
});
it('marks "no newline at end" trailers with the subtle colour', () => {You can send follow-ups to the cloud agent here.
8752b3b to
787602d
Compare
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 2 potential issues.
Autofix Details
Bugbot Autofix prepared fixes for both issues found in the latest run.
- ✅ Fixed: OutroScreen recomputes all diffs on every render
- Wrapped the getAllDiffs() call and filtering in useMemo with an empty dependency array, since the ledger is frozen by the time OutroScreen mounts, preventing expensive diff recomputation on every keypress re-render.
- ✅ Fixed: Hardcoded '/' breaks path display on Windows
- Replaced the hardcoded '/' with path.sep in the /diff summary handler so the startsWith check matches Windows backslash-separated paths correctly.
Or push these changes by commenting:
@cursor push debea905c0
Preview (debea905c0)
diff --git a/src/ui/tui/components/ConsoleView.tsx b/src/ui/tui/components/ConsoleView.tsx
--- a/src/ui/tui/components/ConsoleView.tsx
+++ b/src/ui/tui/components/ConsoleView.tsx
@@ -309,10 +309,15 @@
const totalAdd = diffs.reduce((s, d) => s + d.additions, 0);
const totalDel = diffs.reduce((s, d) => s + d.deletions, 0);
const lines = diffs.map((d) => {
- const rel = d.path.startsWith(store.session.installDir + '/')
+ const rel = d.path.startsWith(store.session.installDir + path.sep)
? path.relative(store.session.installDir, d.path)
: d.path;
- return `${d.operation.toUpperCase().padEnd(6)} ${rel} ${formatChangeCounts(d.additions, d.deletions)}`;
+ return `${d.operation
+ .toUpperCase()
+ .padEnd(6)} ${rel} ${formatChangeCounts(
+ d.additions,
+ d.deletions,
+ )}`;
});
const summary = `${diffs.length} file${
diffs.length === 1 ? '' : 's'
@@ -339,7 +344,10 @@
// can't easily render syntax-coloured diffs inline, but the unified
// patch text is itself readable and copy-pasteable.
store.setCommandFeedback(
- `${found.operation.toUpperCase()} ${found.path} ${formatChangeCounts(found.additions, found.deletions)}\n\n${found.patch}`,
+ `${found.operation.toUpperCase()} ${found.path} ${formatChangeCounts(
+ found.additions,
+ found.deletions,
+ )}\n\n${found.patch}`,
60_000,
);
break;
@@ -809,8 +817,8 @@
? eventPlanPromptShowing
? 'Finish the plan above ([Y]/[S]/[F]) — / and Tab resume after.'
: visibleHistory.length > 0
- ? 'Press / for commands · Tab to ask · Esc to hide answer'
- : 'Press / for commands or Tab to ask a question'
+ ? 'Press / for commands · Tab to ask · Esc to hide answer'
+ : 'Press / for commands or Tab to ask a question'
: 'Press / for commands'}
</Text>
)}
diff --git a/src/ui/tui/screens/OutroScreen.tsx b/src/ui/tui/screens/OutroScreen.tsx
--- a/src/ui/tui/screens/OutroScreen.tsx
+++ b/src/ui/tui/screens/OutroScreen.tsx
@@ -10,7 +10,7 @@
*/
import { Box, Text } from 'ink';
-import { useEffect, useState } from 'react';
+import { useEffect, useMemo, useState } from 'react';
import * as fs from 'fs';
import type { WizardStore } from '../store.js';
import { useWizardStore } from '../hooks/useWizardStore.js';
@@ -291,9 +291,20 @@
const dashboardOpenUrl = signupMagicLinkUrl
? signupMagicLinkUrl
: dashboardCanonicalUrl
- ? toWizardDashboardOpenUrl(dashboardCanonicalUrl)
- : null;
+ ? toWizardDashboardOpenUrl(dashboardCanonicalUrl)
+ : null;
+ // getAllDiffs() calls structuredPatch + createPatch per record — too
+ // expensive to recompute on every keypress-driven re-render. The
+ // ledger is frozen by the time OutroScreen mounts, so an empty dep
+ // array is correct (mirrors FileWritesPanel's memoisation pattern).
+ const meaningfulDiffs = useMemo(() => {
+ const diffs = getLedger().getAllDiffs();
+ return diffs.filter(
+ (d) => d.additions > 0 || d.deletions > 0 || d.operation !== 'modify',
+ );
+ }, []);
+
if (!outroData) {
return (
<Box flexDirection="column" flexGrow={1}>
@@ -461,21 +472,11 @@
magnitude of each change at a glance. Sourced from the
session-scoped FileChangeLedger; empty when capture
didn't fire (probe runs, full-activation re-runs). */}
- {(() => {
- const diffs = getLedger().getAllDiffs();
- const meaningful = diffs.filter(
- (d) => d.additions > 0 || d.deletions > 0 || d.operation !== 'modify',
- );
- if (meaningful.length === 0) return null;
- return (
- <Box marginTop={1}>
- <DiffViewer
- diffs={meaningful}
- installDir={installDir}
- />
- </Box>
- );
- })()}
+ {meaningfulDiffs.length > 0 && (
+ <Box marginTop={1}>
+ <DiffViewer diffs={meaningfulDiffs} installDir={installDir} />
+ </Box>
+ )}
{/* Single-line review note — only when a fresh report was actually
written this run. Without the existence check, a stale reportYou can send follow-ups to the cloud agent here.
787602d to
0b98c4c
Compare
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
Autofix Details
Bugbot Autofix prepared a fix for the issue found in the latest run.
- ✅ Fixed: Diff count undercounts lines starting with ++ or --
- Removed the stale
!line.startsWith('+++')and!line.startsWith('---')guards fromsummarizeDiffsincestructuredPatchhunk lines never contain file headers, making the counting consistent with the already-fixedclassifyPatchLinerendering logic.
- Removed the stale
Or push these changes by commenting:
@cursor push 5d7a3f6e09
Preview (5d7a3f6e09)
diff --git a/src/lib/file-change-diff.ts b/src/lib/file-change-diff.ts
--- a/src/lib/file-change-diff.ts
+++ b/src/lib/file-change-diff.ts
@@ -80,8 +80,8 @@
newLines: h.newLines,
});
for (const line of h.lines) {
- if (line.startsWith('+') && !line.startsWith('+++')) additions++;
- else if (line.startsWith('-') && !line.startsWith('---')) deletions++;
+ if (line.startsWith('+')) additions++;
+ else if (line.startsWith('-')) deletions++;
}
}
return { additions, deletions, hunks };You can send follow-ups to the cloud agent here.
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
Autofix Details
Bugbot Autofix prepared a fix for the issue found in the latest run.
- ✅ Fixed: Path lookup skips normalization causing silent diff miss
- Exposed the ledger's private
normalizemethod as publicnormalizePathand used it insummarizeLedgerPathto normalize the lookup key before comparing against stored (already-normalized) entry paths.
- Exposed the ledger's private
Or push these changes by commenting:
@cursor push 58f3fcf2cd
Preview (58f3fcf2cd)
diff --git a/src/lib/file-change-diff.ts b/src/lib/file-change-diff.ts
--- a/src/lib/file-change-diff.ts
+++ b/src/lib/file-change-diff.ts
@@ -153,12 +153,14 @@
path: string,
): FileDiffSummary | null {
if (!ledger) return null;
+ const normalized = ledger.normalizePath(path);
+ if (!normalized) return null;
// Walk in reverse so we pick up the latest capture for a path that's
// been written more than once. Mirrors how the ledger's own
// `findActive` lookup works.
const entries = ledger.getEntries();
for (let i = entries.length - 1; i >= 0; i--) {
- if (entries[i].path === path) {
+ if (entries[i].path === normalized) {
return summarizeLedgerEntry(entries[i]);
}
}
diff --git a/src/lib/file-change-ledger.ts b/src/lib/file-change-ledger.ts
--- a/src/lib/file-change-ledger.ts
+++ b/src/lib/file-change-ledger.ts
@@ -198,7 +198,7 @@
* file.
*/
recordPreWrite(rawPath: string): void {
- const abs = this.normalize(rawPath);
+ const abs = this.normalizePath(rawPath);
if (!abs) return;
if (this.indexByPath.has(abs)) return;
let beforeContent: string | null = null;
@@ -242,7 +242,7 @@
* when null we re-read from disk, which is the truth either way.
*/
recordPostWrite(rawPath: string, afterContentHint?: string | null): void {
- const abs = this.normalize(rawPath);
+ const abs = this.normalizePath(rawPath);
if (!abs) return;
let afterContent: string | null = afterContentHint ?? null;
if (afterContent === null) {
@@ -411,7 +411,7 @@
* - paths inside the wizard's own cache root (~/.amplitude/wizard)
* return null even if installDir somehow contains it
*/
- private normalize(rawPath: string): string | null {
+ normalizePath(rawPath: string): string | null {
if (!rawPath || typeof rawPath !== 'string') return null;
const abs = isAbsolute(rawPath)
? resolve(rawPath)You can send follow-ups to the cloud agent here.
1d9330a to
0ebdea9
Compare
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
Autofix Details
Bugbot Autofix prepared a fix for the issue found in the latest run.
- ✅ Fixed: Deprecated @types/diff placed in production dependencies
- Removed
@types/difffromdependenciessincediff@^9.0.0bundles its own TypeScript declarations, making the type stub unnecessary and misplaced.
- Removed
Or push these changes by commenting:
@cursor push 5af880959d
Preview (5af880959d)
diff --git a/package.json b/package.json
--- a/package.json
+++ b/package.json
@@ -55,7 +55,6 @@
"@modelcontextprotocol/sdk": "1.29.0",
"@pavus/snake-game": "1.1.1",
"@sentry/node": "10.47.0",
- "@types/diff": "^8.0.0",
"adm-zip": "~0.5.17",
"ai": "^6.0.175",
"axios": "1.13.5",You can send follow-ups to the cloud agent here.
0ebdea9 to
81b2080
Compare
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
Autofix Details
Bugbot Autofix prepared a fix for the issue found in the latest run.
- ✅ Fixed: NDJSON event ordering contradicts JSDoc and protocol expectations
- Swapped the order of
recordFileChangeAppliedandemitFileChangedin the PostToolUse hook sofile_change_appliedis emitted beforefile_changed, matching the documented contract.
- Swapped the order of
Or push these changes by commenting:
@cursor push c1b697875c
Preview (c1b697875c)
diff --git a/src/lib/inner-lifecycle.ts b/src/lib/inner-lifecycle.ts
--- a/src/lib/inner-lifecycle.ts
+++ b/src/lib/inner-lifecycle.ts
@@ -211,10 +211,26 @@
} catch {
// Ledger capture must never break the agent loop. Swallow.
}
+ // Use `content !== null` not `content` — empty string `''` is falsy
+ // and would drop `bytes` from the event. Outer agents need to
+ // distinguish "byte count unknown" (no content captured) from
+ // "zero-byte file" (empty `Write`).
+ try {
+ getUI().recordFileChangeApplied({
+ path,
+ operation,
+ ...(content !== null && {
+ bytes: Buffer.byteLength(content, 'utf8'),
+ }),
+ });
+ } catch {
+ // See preToolUse — same defensive swallow.
+ }
// Emit per-write `file_changed` NDJSON event in agent mode so
// ambient orchestrators see the additions/deletions/hunks without
// having to compute the diff themselves. Pulls from the canonical
- // ledger so we don't double-snapshot.
+ // ledger so we don't double-snapshot. Fires after
+ // `file_change_applied` so orchestrators can correlate the two.
try {
const ui = getAgentUI();
if (ui) {
@@ -232,21 +248,6 @@
} catch {
// NDJSON emission must never break the agent run.
}
- // Use `content !== null` not `content` — empty string `''` is falsy
- // and would drop `bytes` from the event. Outer agents need to
- // distinguish "byte count unknown" (no content captured) from
- // "zero-byte file" (empty `Write`).
- try {
- getUI().recordFileChangeApplied({
- path,
- operation,
- ...(content !== null && {
- bytes: Buffer.byteLength(content, 'utf8'),
- }),
- });
- } catch {
- // See preToolUse — same defensive swallow.
- }
}
return Promise.resolve({});
};You can send follow-ups to the cloud agent here.
… match JSDoc Addresses Cursor Bugbot #599: emitFileChanged JSDoc states it is "emitted from PostToolUse after file_change_applied", but the hook fired them in reverse order. An orchestrator parsing the NDJSON stream and expecting file_change_applied first — then enriching it with the subsequent file_changed event — would never find the enrichment for that path. Reorder PostToolUse so: 1. recordPostWrite (canonical ledger) 2. recordFileChangeApplied (lifecycle marker) 3. emitFileChanged (diff enrichment) Adds an ordering test that pins the contract for future refactors.
81b2080 to
b812189
Compare
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 2 potential issues.
Autofix Details
Bugbot Autofix prepared fixes for both issues found in the latest run.
- ✅ Fixed:
emitFileChangeduses tool-name operation instead of ledger truth- Changed
operationtosummary.operationin theemitFileChangedcall so the NDJSON event uses the ledger's filesystem-derived operation (which checks whether the file existed pre-write) instead of the naive tool-name classification.
- Changed
- ✅ Fixed:
/diffeagerly computes all diffs for single-file lookup- Restructured the
/diff <path>handler to usesummarizeLedgerPathfor single-file lookups, only callingsummarizeLedgerDiffs(which computes all diffs) in the no-argument overview case.
- Restructured the
Or push these changes by commenting:
@cursor push 37f12ba259
Preview (37f12ba259)
diff --git a/src/lib/inner-lifecycle.ts b/src/lib/inner-lifecycle.ts
--- a/src/lib/inner-lifecycle.ts
+++ b/src/lib/inner-lifecycle.ts
@@ -248,7 +248,7 @@
if (summary) {
ui.emitFileChanged({
path,
- operation,
+ operation: summary.operation,
additions: summary.additions,
deletions: summary.deletions,
hunks: summary.hunks,
diff --git a/src/ui/tui/components/ConsoleView.tsx b/src/ui/tui/components/ConsoleView.tsx
--- a/src/ui/tui/components/ConsoleView.tsx
+++ b/src/ui/tui/components/ConsoleView.tsx
@@ -38,8 +38,14 @@
parseFeedbackSlashInput,
parseCreateProjectSlashInput,
} from '../console-commands.js';
-import { getFileChangeLedger } from '../../../lib/file-change-ledger.js';
-import { summarizeLedgerDiffs } from '../../../lib/file-change-diff.js';
+import {
+ type FileChangeLedger,
+ getFileChangeLedger,
+} from '../../../lib/file-change-ledger.js';
+import {
+ summarizeLedgerDiffs,
+ summarizeLedgerPath,
+} from '../../../lib/file-change-diff.js';
import { formatChangeCounts } from './DiffViewer.js';
import path from 'node:path';
import { analytics } from '../../../utils/analytics.js';
@@ -48,6 +54,24 @@
import { KeyHintBar, type KeyHint } from './KeyHintBar.js';
import { useScreenHintsValue } from '../hooks/useScreenHints.js';
+/**
+ * Search ledger entries for a path ending with the given suffix
+ * (e.g. `src/foo.ts` matching `/abs/project/src/foo.ts`). Returns the
+ * full path or an empty string when no match is found so
+ * `summarizeLedgerPath` safely returns null.
+ */
+function findLedgerPathEndingWith(
+ ledger: FileChangeLedger | null,
+ suffix: string,
+): string {
+ if (!ledger) return '';
+ const sep = path.sep;
+ for (const entry of ledger.getEntries()) {
+ if (entry.path.endsWith(sep + suffix)) return entry.path;
+ }
+ return '';
+}
+
async function submitFeedbackWithConsent(
message: string,
store: WizardStore,
@@ -298,20 +322,19 @@
// a tree of touched files with +N/-M counts (no path arg) or
// the additions/deletions for one file (path arg).
const arg = parseDiffSlashInput(raw) ?? '';
- const diffs = summarizeLedgerDiffs(getFileChangeLedger());
- if (diffs.length === 0) {
- store.setCommandFeedback(
- 'No file changes captured yet — the agent has not written anything in this session.',
- 15_000,
- );
- break;
- }
+ const ledger = getFileChangeLedger();
if (!arg) {
+ const diffs = summarizeLedgerDiffs(ledger);
+ if (diffs.length === 0) {
+ store.setCommandFeedback(
+ 'No file changes captured yet — the agent has not written anything in this session.',
+ 15_000,
+ );
+ break;
+ }
const totalAdd = diffs.reduce((s, d) => s + d.additions, 0);
const totalDel = diffs.reduce((s, d) => s + d.deletions, 0);
const lines = diffs.map((d) => {
- // Use path.sep so this works on Windows. Belt-and-braces: also
- // accept the unit-test/POSIX form by checking either separator.
const rel = d.path.startsWith(store.session.installDir + path.sep)
? path.relative(store.session.installDir, d.path)
: d.path;
@@ -328,9 +351,11 @@
const target = path.isAbsolute(arg)
? arg
: path.resolve(store.session.installDir, arg);
+ // Use summarizeLedgerPath for single-file lookup to avoid
+ // computing diffs for every file in the ledger.
const found =
- diffs.find((d) => d.path === target) ??
- diffs.find((d) => d.path.endsWith(path.sep + arg));
+ summarizeLedgerPath(ledger, target) ??
+ summarizeLedgerPath(ledger, findLedgerPathEndingWith(ledger, arg));
if (!found) {
store.setCommandFeedback(
`No diff captured for "${arg}". Try /diff with no argument to see all changed files.`,You can send follow-ups to the cloud agent here.
… match JSDoc Addresses Cursor Bugbot #599: emitFileChanged JSDoc states it is "emitted from PostToolUse after file_change_applied", but the hook fired them in reverse order. An orchestrator parsing the NDJSON stream and expecting file_change_applied first — then enriching it with the subsequent file_changed event — would never find the enrichment for that path. Reorder PostToolUse so: 1. recordPostWrite (canonical ledger) 2. recordFileChangeApplied (lifecycle marker) 3. emitFileChanged (diff enrichment) Adds an ordering test that pins the contract for future refactors.
b812189 to
b11fea0
Compare
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 2 potential issues.
There are 3 total unresolved issues (including 1 from previous review).
Autofix Details
Bugbot Autofix prepared fixes for both issues found in the latest run.
- ✅ Fixed: Double diff computation in
summarizeLedgerEntryon hot path- Replaced the separate
createPatchcall withformatPatchapplied to a singlestructuredPatchresult, and extracted the count-computation into a sharedextractCountshelper so the O(n·m) diff algorithm runs only once per file.
- Replaced the separate
- ✅ Fixed: Misleading diff when only
afterContentcapture fails- Added an early return of
nullfor modify entries where eitherbeforeContentorafterContentis null, preventing misleading all-deletions diffs from capture failures.
- Added an early return of
Or push these changes by commenting:
@cursor push aec3df0677
Preview (aec3df0677)
diff --git a/src/lib/file-change-diff.ts b/src/lib/file-change-diff.ts
--- a/src/lib/file-change-diff.ts
+++ b/src/lib/file-change-diff.ts
@@ -14,7 +14,7 @@
* All helpers here are pure: no IO, no mutation of the ledger.
*/
-import { createPatch, structuredPatch } from 'diff';
+import { formatPatch, structuredPatch } from 'diff';
import {
type FileChangeEntry,
@@ -69,6 +69,15 @@
return { additions: 0, deletions: 0, hunks: [] };
}
const sp = structuredPatch('a', 'b', before, after, '', '', { context: 3 });
+ return extractCounts(sp);
+}
+
+/** Extract counts + lite hunks from an already-computed structured patch. */
+function extractCounts(sp: ReturnType<typeof structuredPatch>): {
+ additions: number;
+ deletions: number;
+ hunks: DiffHunkLite[];
+} {
let additions = 0;
let deletions = 0;
const hunks: DiffHunkLite[] = [];
@@ -90,17 +99,27 @@
/**
* Build a {@link FileDiffSummary} from one ledger entry. Returns `null`
* when the entry has no captured content on either side (binary, IO
- * error during capture) — callers should skip those silently.
+ * error during capture), or when only one side was captured for an
+ * operation that needs both (e.g. a modify where the post-write read
+ * failed) — callers should skip those silently.
*/
export function summarizeLedgerEntry(
entry: FileChangeEntry,
): FileDiffSummary | null {
- const before = entry.beforeContent ?? '';
- const after = entry.afterContent ?? '';
// No content captured on either side: nothing to render.
if (entry.beforeContent === null && entry.afterContent === null) {
return null;
}
+ // For modify operations both sides are needed; a missing side means a
+ // capture failure, not a real deletion/creation. Return null so we
+ // don't surface a misleading diff.
+ if (entry.kind === 'modify') {
+ if (entry.beforeContent === null || entry.afterContent === null) {
+ return null;
+ }
+ }
+ const before = entry.beforeContent ?? '';
+ const after = entry.afterContent ?? '';
// Skip binary content — the ledger stores it as a string but the diff
// would balloon memory and produce noise.
if (
@@ -109,17 +128,26 @@
) {
return null;
}
- const { additions, deletions, hunks } = summarizeDiff(before, after);
- const patch =
- before === after
- ? ''
- : createPatch(entry.path, before, after, '', '', { context: 3 });
+ if (before === after) {
+ return {
+ path: entry.path,
+ operation: entry.kind,
+ additions: 0,
+ deletions: 0,
+ patch: '',
+ hunks: [],
+ };
+ }
+ const sp = structuredPatch(entry.path, entry.path, before, after, '', '', {
+ context: 3,
+ });
+ const { additions, deletions, hunks } = extractCounts(sp);
return {
path: entry.path,
operation: entry.kind,
additions,
deletions,
- patch,
+ patch: formatPatch(sp),
hunks,
};
}You can send follow-ups to the cloud agent here.
… match JSDoc Addresses Cursor Bugbot #599: emitFileChanged JSDoc states it is "emitted from PostToolUse after file_change_applied", but the hook fired them in reverse order. An orchestrator parsing the NDJSON stream and expecting file_change_applied first — then enriching it with the subsequent file_changed event — would never find the enrichment for that path. Reorder PostToolUse so: 1. recordPostWrite (canonical ledger) 2. recordFileChangeApplied (lifecycle marker) 3. emitFileChanged (diff enrichment) Adds an ordering test that pins the contract for future refactors.
b11fea0 to
8ed4944
Compare
… match JSDoc Addresses Cursor Bugbot #599: emitFileChanged JSDoc states it is "emitted from PostToolUse after file_change_applied", but the hook fired them in reverse order. An orchestrator parsing the NDJSON stream and expecting file_change_applied first — then enriching it with the subsequent file_changed event — would never find the enrichment for that path. Reorder PostToolUse so: 1. recordPostWrite (canonical ledger) 2. recordFileChangeApplied (lifecycle marker) 3. emitFileChanged (diff enrichment) Adds an ordering test that pins the contract for future refactors.
8ed4944 to
48aba44
Compare
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
Autofix Details
Bugbot Autofix prepared a fix for the issue found in the latest run.
- ✅ Fixed: Outro computes full patch text for every file unnecessarily
- Added
{ includePatch: false }to thesummarizeLedgerEntrycall insummarizeLedgerDiffs, skipping the redundantcreatePatchO(n·m) diff for every ledger entry since both callers (OutroScreen summary mode and/diffsummary mode) only use additions/deletions/operation counts and never read the patch field.
- Added
Or push these changes by commenting:
@cursor push a300c37f61
Preview (a300c37f61)
diff --git a/package.json b/package.json
--- a/package.json
+++ b/package.json
@@ -62,6 +62,7 @@
"cli-highlight": "2.1.11",
"client-oauth2": "4.3.3",
"cross-spawn": "^7.0.6",
+ "diff": "^9.0.0",
"dotenv": "16.4.7",
"fast-glob": "3.3.3",
"glob": "9.3.5",
diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml
--- a/pnpm-lock.yaml
+++ b/pnpm-lock.yaml
@@ -59,6 +59,9 @@
cross-spawn:
specifier: ^7.0.6
version: 7.0.6
+ diff:
+ specifier: ^9.0.0
+ version: 9.0.0
dotenv:
specifier: 16.4.7
version: 16.4.7
@@ -2066,6 +2069,10 @@
resolution: {integrity: sha512-X07nttJQkwkfKfvTPG/KSnE2OMdcUCao6+eXF3wmnIQRn2aPAHH3VxDbDOdegkd6JbPsXqShpvEOHfAT+nCNwQ==}
engines: {node: '>=0.3.1'}
+ diff@9.0.0:
+ resolution: {integrity: sha512-svtcdpS8CgJyqAjEQIXdb3OjhFVVYjzGAPO8WGCmRbrml64SPw/jJD4GoE98aR7r25A0XcgrK3F02yw9R/vhQw==}
+ engines: {node: '>=0.3.1'}
+
dotenv@16.4.7:
resolution: {integrity: sha512-47qPchRCykZC03FhkYAhrvwU4xDBFIj1QPqaarj6mdM/hgUzfPHcpkHJOn3mJAufFeeAxAzeGsr5X0M4k6fLZQ==}
engines: {node: '>=12'}
@@ -5628,6 +5635,8 @@
diff@4.0.4: {}
+ diff@9.0.0: {}
+
dotenv@16.4.7: {}
dunder-proto@1.0.1:
diff --git a/src/lib/__tests__/inner-lifecycle.test.ts b/src/lib/__tests__/inner-lifecycle.test.ts
--- a/src/lib/__tests__/inner-lifecycle.test.ts
+++ b/src/lib/__tests__/inner-lifecycle.test.ts
@@ -219,6 +219,44 @@
});
});
+ it('PostToolUse emits file_change_applied BEFORE file_changed (NDJSON contract)', async () => {
+ // Regression: emitFileChanged JSDoc documents that file_changed is
+ // emitted AFTER file_change_applied (so an orchestrator receiving
+ // the apply lifecycle marker can later enrich it with diff stats).
+ // The hook used to fire them in reverse order, breaking that
+ // contract for any consumer that keys on file_change_applied first.
+ const lifecycle = createInnerLifecycleHooks({ phase: 'apply' });
+ await lifecycle.hooks().PostToolUse(
+ {
+ tool_name: 'Write',
+ tool_input: {
+ file_path: 'src/lib/amplitude-ordering.ts',
+ content: 'export const ok = true;\n',
+ },
+ },
+ undefined,
+ { signal: new AbortController().signal },
+ );
+
+ const all = writes.map((l) => JSON.parse(l.trim()) as NDJSONEvent);
+ const appliedIdx = all.findIndex(
+ (e) =>
+ (e.data as { event?: string } | undefined)?.event ===
+ 'file_change_applied',
+ );
+ const changedIdx = all.findIndex(
+ (e) =>
+ (e.data as { event?: string } | undefined)?.event === 'file_changed',
+ );
+ // Apply lifecycle marker must come first.
+ expect(appliedIdx).toBeGreaterThanOrEqual(0);
+ if (changedIdx !== -1) {
+ // file_changed only emits when a ledger summary is available;
+ // when both are emitted, applied must precede changed.
+ expect(appliedIdx).toBeLessThan(changedIdx);
+ }
+ });
+
it('PostToolUse for non-write tools is a no-op', async () => {
const lifecycle = createInnerLifecycleHooks({ phase: 'wizard' });
await lifecycle
diff --git a/src/lib/agent-events.ts b/src/lib/agent-events.ts
--- a/src/lib/agent-events.ts
+++ b/src/lib/agent-events.ts
@@ -89,6 +89,7 @@
tool_call: 1,
file_change_planned: 1,
file_change_applied: 1,
+ file_changed: 1,
// Event plan
event_plan_proposed: 1,
event_plan_confirmed: 1,
@@ -532,6 +533,27 @@
bytes?: number;
}
+/**
+ * `file_changed` — emitted at PostToolUse with diff metadata so outer
+ * agents can render per-file change previews without re-reading anything
+ * themselves. Pairs with `file_change_applied` (same path) and adds the
+ * additions/deletions/hunks the wizard's session-scoped ledger computed.
+ */
+export interface FileChangedData {
+ event: 'file_changed';
+ path: string;
+ operation: 'create' | 'modify' | 'delete';
+ additions: number;
+ deletions: number;
+ /** Lightweight hunk metadata (line ranges) for ambient diff rendering. */
+ hunks: Array<{
+ oldStart: number;
+ oldLines: number;
+ newStart: number;
+ newLines: number;
+ }>;
+}
+
/** `event_plan_proposed` — emitted when the inner agent calls `confirm_event_plan`. */
export interface EventPlanProposedData {
event: 'event_plan_proposed';
@@ -571,6 +593,7 @@
| ToolCallData
| FileChangePlannedData
| FileChangeAppliedData
+ | FileChangedData
| EventPlanProposedData
| EventPlanConfirmedData
| VerificationStartedData
diff --git a/src/lib/file-change-diff.ts b/src/lib/file-change-diff.ts
new file mode 100644
--- /dev/null
+++ b/src/lib/file-change-diff.ts
@@ -1,0 +1,217 @@
+/**
+ * file-change-diff — read-only adapter that turns the canonical
+ * {@link FileChangeLedger} entries (`beforeContent` / `afterContent`)
+ * into the diff shape the TUI's DiffViewer + the agent-mode
+ * `file_changed` NDJSON event consume.
+ *
+ * The ledger itself (see `file-change-ledger.ts`) is owned by the
+ * cancel-rollback path — its public shape is intentionally minimal
+ * (`FileChangeEntry`, `getEntries()`). This module is the single place
+ * that knows how to turn that shape into unified diffs / additions /
+ * deletions / hunk metadata, so the ledger stays focused on rollback
+ * correctness.
+ *
+ * All helpers here are pure: no IO, no mutation of the ledger.
+ */
+
+import { createPatch, structuredPatch } from 'diff';
+import { isAbsolute, resolve } from 'node:path';
+
+import {
+ type FileChangeEntry,
+ type FileChangeKind,
+ type FileChangeLedger,
+} from './file-change-ledger.js';
+
+/** Re-export the kind type so DiffViewer doesn't need to import the ledger. */
+export type FileChangeOperation = FileChangeKind;
+
+/** Lightweight per-hunk shape suitable for NDJSON emission. */
+export interface DiffHunkLite {
+ oldStart: number;
+ oldLines: number;
+ newStart: number;
+ newLines: number;
+}
+
+/** Result of computing a diff for a single file. */
+export interface FileDiffSummary {
+ path: string;
+ operation: FileChangeOperation;
+ additions: number;
+ deletions: number;
+ /** Unified-diff text. Empty when there is no textual change. */
+ patch: string;
+ hunks: DiffHunkLite[];
+}
+
+/**
+ * Heuristic binary check: a NUL byte in the first 8kB. Mirrors the gate
+ * the ledger applies at capture time so callers don't render diffs for
+ * binaries that the ledger may have stored as null content.
+ */
+export function isProbablyBinary(content: string): boolean {
+ const sliceLen = Math.min(content.length, 8192);
+ for (let i = 0; i < sliceLen; i++) {
+ if (content.charCodeAt(i) === 0) return true;
+ }
+ return false;
+}
+
+/**
+ * Compute additions / deletions / hunks from a structured patch. Pure so
+ * tests can validate counts independent of jsdiff internals leaking.
+ */
+export function summarizeDiff(
+ before: string,
+ after: string,
+): { additions: number; deletions: number; hunks: DiffHunkLite[] } {
+ if (before === after) {
+ return { additions: 0, deletions: 0, hunks: [] };
+ }
+ const sp = structuredPatch('a', 'b', before, after, '', '', { context: 3 });
+ let additions = 0;
+ let deletions = 0;
+ const hunks: DiffHunkLite[] = [];
+ for (const h of sp.hunks) {
+ hunks.push({
+ oldStart: h.oldStart,
+ oldLines: h.oldLines,
+ newStart: h.newStart,
+ newLines: h.newLines,
+ });
+ for (const line of h.lines) {
+ if (line.startsWith('+')) additions++;
+ else if (line.startsWith('-')) deletions++;
+ }
+ }
+ return { additions, deletions, hunks };
+}
+
+/**
+ * Options for {@link summarizeLedgerEntry}. Hot-path callers (PostToolUse
+ * inner-lifecycle hook, FileWritesPanel toast) only consume
+ * additions/deletions/hunks and never render the unified-patch text — they
+ * can pass `includePatch: false` to skip the redundant `createPatch` call,
+ * which runs the same O(n·m) diff a second time. Defaults to true so the
+ * outro / DiffViewer / `/diff` paths keep the patch body.
+ */
+export interface SummarizeLedgerEntryOptions {
+ includePatch?: boolean;
+}
+
+/**
+ * Build a {@link FileDiffSummary} from one ledger entry. Returns `null`
+ * when the entry has no captured content on either side (binary, IO
+ * error during capture) — callers should skip those silently.
+ *
+ * Also returns `null` when one side of the capture failed (e.g. PostToolUse
+ * couldn't re-read the file) and the other side has content. Without this
+ * guard, the missing side would default to `''` and every line of the
+ * captured side would render as deletions or additions — a misleading
+ * "huge change" toast for what is actually a capture failure.
+ */
+export function summarizeLedgerEntry(
+ entry: FileChangeEntry,
+ options: SummarizeLedgerEntryOptions = {},
+): FileDiffSummary | null {
+ const { includePatch = true } = options;
+ // No content captured on either side: nothing to render.
+ if (entry.beforeContent === null && entry.afterContent === null) {
+ return null;
+ }
+ // One-sided capture failure: surface as "no diff" rather than treat the
+ // missing side as `''`, which would falsely render every captured line
+ // as added/removed. Exception: legitimate `create` (no before, has after)
+ // and `delete` (has before, no after) ARE one-sided by construction —
+ // those are the ledger's `kind` and we trust them.
+ if (entry.beforeContent === null && entry.kind !== 'create') return null;
+ if (entry.afterContent === null && entry.kind !== 'delete') return null;
+ const before = entry.beforeContent ?? '';
+ const after = entry.afterContent ?? '';
+ // Skip binary content — the ledger stores it as a string but the diff
+ // would balloon memory and produce noise.
+ if (
+ (entry.beforeContent !== null && isProbablyBinary(before)) ||
+ (entry.afterContent !== null && isProbablyBinary(after))
+ ) {
+ return null;
+ }
+ const { additions, deletions, hunks } = summarizeDiff(before, after);
+ // `createPatch` re-runs the same O(n·m) diff `structuredPatch` already
+ // did — only call it when the caller actually needs the patch text.
+ const patch = !includePatch
+ ? ''
+ : before === after
+ ? ''
+ : createPatch(entry.path, before, after, '', '', { context: 3 });
+ return {
+ path: entry.path,
+ operation: entry.kind,
+ additions,
+ deletions,
+ patch,
+ hunks,
+ };
+}
+
+/**
+ * Walk every entry in the ledger and produce the diff summaries the
+ * outro / `/diff` summary command consume. Order matches the ledger's own
+ * insertion order (chronological by first PreToolUse capture). Skips
+ * entries where {@link summarizeLedgerEntry} returns null.
+ *
+ * Patch text is omitted (`includePatch: false`) because both callers
+ * (OutroScreen summary, `/diff` with no argument) only render
+ * operation labels and +N/-M counts — the redundant `createPatch`
+ * O(n·m) diff per file was pure waste.
+ */
+export function summarizeLedgerDiffs(
+ ledger: FileChangeLedger | null,
+): FileDiffSummary[] {
+ if (!ledger) return [];
+ const out: FileDiffSummary[] = [];
+ for (const entry of ledger.getEntries()) {
+ const summary = summarizeLedgerEntry(entry, { includePatch: false });
+ if (summary) out.push(summary);
+ }
+ return out;
+}
+
+/**
+ * Find the most recent ledger entry for a given path and return its
+ * diff summary. Returns `null` when no entry exists for that path or
+ * when the entry has no diffable content. Used by the per-file
+ * `file_changed` NDJSON emission and `/diff <path>`.
+ *
+ * The lookup path is normalized the same way the ledger normalized it
+ * during capture (`resolve()` against the install dir). Without this,
+ * a relative or `..`-bearing tool-input path would silently miss the
+ * lookup — the ledger stored `<installDir>/foo.ts` but the lookup key
+ * is the raw `foo.ts`, and `===` comparison fails.
+ *
+ * `options` is forwarded to {@link summarizeLedgerEntry} so hot-path
+ * callers (PostToolUse) can pass `{ includePatch: false }` to skip the
+ * redundant `createPatch` call.
+ */
+export function summarizeLedgerPath(
+ ledger: FileChangeLedger | null,
+ rawPath: string,
+ options?: SummarizeLedgerEntryOptions,
+): FileDiffSummary | null {
+ if (!ledger) return null;
+ const installDir = ledger.getInstallDir();
+ const normalized = isAbsolute(rawPath)
+ ? resolve(rawPath)
+ : resolve(installDir, rawPath);
+ // Walk in reverse so we pick up the latest capture for a path that's
+ // been written more than once. Mirrors how the ledger's own
+ // `findActive` lookup works.
+ const entries = ledger.getEntries();
+ for (let i = entries.length - 1; i >= 0; i--) {
+ if (entries[i].path === normalized) {
+ return summarizeLedgerEntry(entries[i], options);
+ }
+ }
+ return null;
+}
diff --git a/src/lib/file-change-ledger.ts b/src/lib/file-change-ledger.ts
--- a/src/lib/file-change-ledger.ts
+++ b/src/lib/file-change-ledger.ts
@@ -300,6 +300,17 @@
}
/**
+ * Resolved install directory the ledger was constructed with. Exposed so
+ * read-only consumers (e.g. {@link summarizeLedgerPath}) can normalize a
+ * raw lookup path the same way `recordPreWrite` / `recordPostWrite` did
+ * before storing the entry — otherwise a relative or `..`-bearing path
+ * would silently miss the lookup.
+ */
+ getInstallDir(): string {
+ return this.installDir;
+ }
+
+ /**
* Reverse the ledger. Walks entries in last-in-first-out order so a
* file written then edited is restored from its first-recorded
* `beforeContent`.
diff --git a/src/lib/inner-lifecycle.ts b/src/lib/inner-lifecycle.ts
--- a/src/lib/inner-lifecycle.ts
+++ b/src/lib/inner-lifecycle.ts
@@ -44,6 +44,7 @@
} from './agent-events.js';
import type { HookCallback } from './agent-hooks.js';
import { getFileChangeLedger } from './file-change-ledger.js';
+import { summarizeLedgerPath } from './file-change-diff.js';
/**
* Type guard: returns the UI cast to AgentUI if we're in agent mode,
@@ -159,8 +160,9 @@
// when inner-lifecycle hooks fire from a probe call). Swallow —
// a missing pre-event is harmless, the apply-side handles it.
}
- // Capture the pre-write content into the rollback ledger so a
- // cancelled / errored run can revert this file. No-op when no
+ // Capture the pre-write content into the canonical ledger so a
+ // cancelled / errored run can revert this file AND the diff viewer
+ // / `/diff` slash command have a source of truth. No-op when no
// ledger has been initialised (probe calls, unit tests).
try {
getFileChangeLedger()?.recordPreWrite(path);
@@ -199,6 +201,27 @@
: null;
if (path) {
const content = typeof obj.content === 'string' ? obj.content : null;
+ // Finalise the canonical ledger entry first — both rollback and the
+ // diff viewer read from this ledger. `recordPostWrite` is a no-op
+ // when no ledger is initialised. For Edit/MultiEdit/NotebookEdit
+ // `obj.content` will be null and the ledger re-reads from disk to
+ // capture the final form.
+ try {
+ getFileChangeLedger()?.recordPostWrite(path, content);
+ } catch {
+ // Ledger capture must never break the agent loop. Swallow.
+ }
+ // NDJSON event ordering for outer-agent orchestrators:
+ // 1. `file_change_applied` — the canonical lifecycle marker
+ // ("tool finished writing this path"). Emit FIRST so an
+ // orchestrator that only cares about apply events doesn't
+ // need to know about the enrichment event.
+ // 2. `file_changed` — per-write diff enrichment (additions /
+ // deletions / hunks). Emit AFTER so orchestrators that
+ // enrich previously-seen `file_change_applied` records can
+ // key on `path` and find the prior event in their state.
+ // The `emitFileChanged` JSDoc documents this ordering — keep them
+ // in sync.
// Use `content !== null` not `content` — empty string `''` is falsy
// and would drop `bytes` from the event. Outer agents need to
// distinguish "byte count unknown" (no content captured) from
@@ -214,15 +237,36 @@
} catch {
// See preToolUse — same defensive swallow.
}
- // Finalise the rollback ledger entry with the new on-disk content.
- // For Edit/MultiEdit/NotebookEdit `obj.content` will be null and
- // the ledger re-reads from disk to capture the final form. The
- // ledger's `recordPostWrite` is a no-op when no ledger is
- // initialised.
+ // Emit per-write `file_changed` NDJSON event in agent mode so
+ // ambient orchestrators see the additions/deletions/hunks without
+ // having to compute the diff themselves. Pulls from the canonical
+ // ledger so we don't double-snapshot.
try {
- getFileChangeLedger()?.recordPostWrite(path, content);
+ const ui = getAgentUI();
+ if (ui) {
+ // `includePatch: false` skips the redundant `createPatch` call —
+ // we only consume additions/deletions/hunks here, so paying for
+ // the unified-patch text on every PostToolUse is pure waste.
+ const summary = summarizeLedgerPath(getFileChangeLedger(), path, {
+ includePatch: false,
+ });
+ if (summary) {
+ // `summary.operation` reflects the ledger's filesystem-derived
+ // truth (was the file there pre-write?). The toolName-derived
+ // `operation` above hardcodes Write→'create' even when Write
+ // overwrote an existing file — wrong for the orchestrator-
+ // facing NDJSON contract.
+ ui.emitFileChanged({
+ path,
+ operation: summary.operation,
+ additions: summary.additions,
+ deletions: summary.deletions,
+ hunks: summary.hunks,
+ });
+ }
+ }
} catch {
- // Ledger capture must never break the agent loop. Swallow.
+ // NDJSON emission must never break the agent run.
}
}
return Promise.resolve({});
diff --git a/src/ui/agent-ui.ts b/src/ui/agent-ui.ts
--- a/src/ui/agent-ui.ts
+++ b/src/ui/agent-ui.ts
@@ -19,6 +19,7 @@
ToolCallData,
FileChangePlannedData,
FileChangeAppliedData,
+ FileChangedData,
EventPlanProposedData,
EventPlanConfirmedData,
VerificationStartedData,
@@ -695,6 +696,19 @@
}
}
+ /**
+ * `file_changed` — per-write diff metadata. Emitted from PostToolUse
+ * after `file_change_applied`, carries additions/deletions/hunk ranges
+ * so ambient agents can render diffs without re-reading the file.
+ */
+ emitFileChanged(data: Omit<FileChangedData, 'event'>): void {
+ emit(
+ 'progress',
+ `file_changed: ${data.operation} ${data.path} (+${data.additions}/-${data.deletions})`,
+ { data: { event: 'file_changed', ...data } },
+ );
+ }
+
// WizardUI-shaped aliases (see wizard-ui.ts). Inner-lifecycle calls these
// via the abstract interface so InkUI can also receive file_change events
// for the TUI panel; AgentUI just delegates to its existing NDJSON
diff --git a/src/ui/tui/__tests__/console-commands.test.ts b/src/ui/tui/__tests__/console-commands.test.ts
--- a/src/ui/tui/__tests__/console-commands.test.ts
+++ b/src/ui/tui/__tests__/console-commands.test.ts
@@ -2,6 +2,8 @@
import {
parseFeedbackSlashInput,
parseCreateProjectSlashInput,
+ parseDiffSlashInput,
+ getHelpText,
getWhoamiText,
getDiagnosticsText,
checkCommandBlockedByRun,
@@ -343,3 +345,70 @@
expect(text).toContain('tar -czf wizard-logs.tar.gz');
});
});
+
+describe('parseDiffSlashInput', () => {
+ it('returns an empty string when no path is given (summary mode)', () => {
+ expect(parseDiffSlashInput('/diff')).toBe('');
+ expect(parseDiffSlashInput('/diff ')).toBe('');
+ });
+
+ it('returns the trimmed path argument', () => {
+ expect(parseDiffSlashInput('/diff src/foo.ts')).toBe('src/foo.ts');
+ expect(parseDiffSlashInput(' /diff /abs/path.ts ')).toBe('/abs/path.ts');
+ });
+
+ it('is case-insensitive on the command prefix', () => {
+ expect(parseDiffSlashInput('/DIFF a.ts')).toBe('a.ts');
+ });
+
+ it('returns undefined for other commands', () => {
+ expect(parseDiffSlashInput('/feedback hi')).toBeUndefined();
+ expect(parseDiffSlashInput('/help')).toBeUndefined();
+ });
+});
+
+describe('/diff and /help command registration', () => {
+ it('exposes /diff so the help UI surfaces it', () => {
+ const cmds = COMMANDS.map((c) => c.cmd);
+ expect(cmds).toContain('/diff');
+ });
+
+ it('exposes /help so the help UI surfaces it', () => {
+ const cmds = COMMANDS.map((c) => c.cmd);
+ expect(cmds).toContain('/help');
+ });
+
+ it('/diff is a read-only command — must not be marked requiresIdle', () => {
+ const def = COMMANDS.find((c) => c.cmd === '/diff');
+ expect(def?.requiresIdle).toBeFalsy();
+ });
+
+ it('/help is a read-only command — must not be marked requiresIdle', () => {
+ const def = COMMANDS.find((c) => c.cmd === '/help');
+ expect(def?.requiresIdle).toBeFalsy();
+ });
+
+ it('isKnownCommand recognizes /diff with and without a path', () => {
+ expect(isKnownCommand('/diff')).toBe(true);
+ expect(isKnownCommand('/diff src/foo.ts')).toBe(true);
+ });
+
+ it('isKnownCommand recognizes /help', () => {
+ expect(isKnownCommand('/help')).toBe(true);
+ });
+});
+
+describe('getHelpText', () => {
+ it('lists every registered slash command in the catalogue', () => {
+ const text = getHelpText();
+ for (const c of COMMANDS) {
+ expect(text).toContain(c.cmd);
+ expect(text).toContain(c.desc);
+ }
+ });
+
+ it('starts with a clear header', () => {
+ const text = getHelpText();
+ expect(text).toMatch(/^Available slash commands:/);
+ });
+});
diff --git a/src/ui/tui/components/ConsoleView.tsx b/src/ui/tui/components/ConsoleView.tsx
--- a/src/ui/tui/components/ConsoleView.tsx
+++ b/src/ui/tui/components/ConsoleView.tsx
@@ -32,10 +32,19 @@
checkCommandBlockedByRun,
getWhoamiText,
getDiagnosticsText,
+ getHelpText,
isKnownCommand,
+ parseDiffSlashInput,
parseFeedbackSlashInput,
parseCreateProjectSlashInput,
} from '../console-commands.js';
+import { getFileChangeLedger } from '../../../lib/file-change-ledger.js';
+import {
+ summarizeLedgerDiffs,
+ summarizeLedgerPath,
+} from '../../../lib/file-change-diff.js';
+import { formatChangeCounts } from './DiffViewer.js';
+import path from 'node:path';
import { analytics } from '../../../utils/analytics.js';
import { trackWizardFeedback } from '../../../utils/track-wizard-feedback.js';
import { collectDiagnostics } from '../../../lib/diagnostics-collector.js';
@@ -285,6 +294,87 @@
})();
break;
}
+ case '/diff': {
+ // The slash console is a single-line feedback channel — full
+ // unified-diff rendering belongs in the DiffViewer component the
+ // outro mounts. Here we surface the most actionable information:
+ // a tree of touched files with +N/-M counts (no path arg) or
+ // the additions/deletions for one file (path arg).
+ const arg = parseDiffSlashInput(raw) ?? '';
+ const ledger = getFileChangeLedger();
+ // Detail mode (`/diff <path>`): use the purpose-built single-path
+ // helper so we don't burn `structuredPatch`+`createPatch` on every
+ // unrelated file in the ledger just to discard them. The summary
+ // mode below still needs the full sweep for the +N/-M tree.
+ if (arg) {
+ // Resolve relative paths against installDir so users can type
+ // `/diff src/amplitude.ts` instead of pasting an absolute path.
+ const target = path.isAbsolute(arg)
+ ? arg
+ : path.resolve(store.session.installDir, arg);
+ const found = summarizeLedgerPath(ledger, target);
+ if (!found) {
+ // Fallback: walk the ledger entries directly so a user-friendly
+ // suffix match (e.g. `/diff amplitude.ts` matching
+ // `<installDir>/src/lib/amplitude.ts`) still works without
+ // forcing a second full diff sweep up-front.
+ const entries = ledger?.getEntries() ?? [];
+ const suffix = path.sep + arg;
+ const suffixEntry = entries.find((e) => e.path.endsWith(suffix));
+ const suffixFound = suffixEntry
+ ? summarizeLedgerPath(ledger, suffixEntry.path)
+ : null;
+ if (!suffixFound) {
+ store.setCommandFeedback(
+ `No diff captured for "${arg}". Try /diff with no argument to see all changed files.`,
+ 15_000,
+ );
+ break;
+ }
+ store.setCommandFeedback(
+ `${suffixFound.operation.toUpperCase()} ${suffixFound.path} ${formatChangeCounts(suffixFound.additions, suffixFound.deletions)}\n\n${suffixFound.patch}`,
+ 60_000,
+ );
+ break;
+ }
+ // Surface the patch body in the feedback channel. The slash console
+ // can't easily render syntax-coloured diffs inline, but the unified
+ // patch text is itself readable and copy-pasteable.
+ store.setCommandFeedback(
+ `${found.operation.toUpperCase()} ${found.path} ${formatChangeCounts(found.additions, found.deletions)}\n\n${found.patch}`,
+ 60_000,
+ );
+ break;
+ }
+ // Summary mode (no arg): walk the whole ledger.
+ const diffs = summarizeLedgerDiffs(ledger);
+ if (diffs.length === 0) {
+ store.setCommandFeedback(
+ 'No file changes captured yet — the agent has not written anything in this session.',
+ 15_000,
+ );
+ break;
+ }
+ const totalAdd = diffs.reduce((s, d) => s + d.additions, 0);
+ const totalDel = diffs.reduce((s, d) => s + d.deletions, 0);
+ const lines = diffs.map((d) => {
+ // Use path.sep so this works on Windows. Belt-and-braces: also
+ // accept the unit-test/POSIX form by checking either separator.
+ const rel = d.path.startsWith(store.session.installDir + path.sep)
+ ? path.relative(store.session.installDir, d.path)
+ : d.path;
+ return `${d.operation.toUpperCase().padEnd(6)} ${rel} ${formatChangeCounts(d.additions, d.deletions)}`;
+ });
+ const summary = `${diffs.length} file${
+ diffs.length === 1 ? '' : 's'
+ } changed (+${totalAdd}/-${totalDel})\n${lines.join('\n')}`;
+ store.setCommandFeedback(summary, 30_000);
+ break;
+ }
+ case '/help': {
+ store.setCommandFeedback(getHelpText(), 30_000);
+ break;
+ }
case '/exit':
store.setOutroData({ kind: OutroKind.Cancel, message: 'Exited.' });
break;
diff --git a/src/ui/tui/components/DiffViewer.tsx b/src/ui/tui/components/DiffViewer.tsx
new file mode 100644
--- /dev/null
+++ b/src/ui/tui/components/DiffViewer.tsx
@@ -1,0 +1,225 @@
+/**
+ * DiffViewer — render a unified diff for one file (or a tree summary
+ * for many) inside the TUI.
+ *
+ * Reads from the session-scoped `FileChangeLedger` (see
+ * `src/lib/file-change-ledger.ts`) which captures before/after content at
+ * Pre/PostToolUse. Pure presentational — does not mutate the ledger.
+ *
+ * Two modes:
+ * - **summary** (no `path` prop): tree of every changed file with
+ * +N/-M counts. Used by `/diff` with no argument and by the outro's
+ * "what changed" section.
+ * - **detail** (`path` prop set): unified diff for that one file with
+ * scrollable hunks. Used by `/diff <path>`.
+ *
+ * Color semantics:
+ * - additions → `Colors.success` (emerald)
+ * - deletions → `Colors.error` (red)
+ * - hunk header → `Colors.accent`
+ * - context → `Colors.muted`
+ *
+ * Keyboard nav (detail mode): j/down to scroll forward, k/up to scroll
+ * back, q/esc to exit. Owned by the parent surface — DiffViewer just
+ * renders the slice given by `scrollOffset`.
+ */
+
+import { Box, Text } from 'ink';
+import path from 'path';
+import { Colors, Icons } from '../styles.js';
+import type { FileDiffSummary } from '../../../lib/file-change-diff.js';
+
+interface DiffViewerProps {
+ /** All file diffs from the ledger. */
+ diffs: FileDiffSummary[];
+ /** When set, render the unified diff for this single file. */
+ filePath?: string;
+ /** Project root used to relativize displayed paths. */
+ installDir?: string;
+ /** Cap on rendered patch lines in detail mode (scroll offset). */
+ scrollOffset?: number;
+ /** Max lines of patch body to render in detail mode. */
+ maxLines?: number;
+}
+
+const OP_LABELS: Record<FileDiffSummary['operation'], string> = {
+ create: 'NEW',
+ modify: 'MOD',
+ delete: 'DEL',
+};
+
+const OP_COLORS: Record<FileDiffSummary['operation'], string> = {
+ create: Colors.success,
+ modify: Colors.warning,
+ delete: Colors.error,
+};
+
+const displayPath = (raw: string, installDir?: string): string => {
+ if (
+ installDir &&
+ raw.startsWith(installDir) &&
+ (raw.length === installDir.length || raw[installDir.length] === path.sep)
+ ) {
+ const rel = path.relative(installDir, raw);
+ return rel === '' ? path.basename(raw) : rel;
+ }
+ return raw;
+};
+
+/**
+ * Strip the `Index:` / `===` / `---` / `+++` headers that `createPatch`
+ * prepends. The viewer renders its own per-file heading so those bytes
+ * are pure noise. Keep the `@@` hunk markers and content lines.
+ */
+function stripPatchHeader(patch: string): string[] {
+ const lines = patch.split('\n');
+ // First hunk header is the boundary. Drop everything before it.
+ const hunkStart = lines.findIndex((l) => l.startsWith('@@'));
+ if (hunkStart === -1) return [];
+ return lines.slice(hunkStart);
+}
+
+/** Map a single patch line to its color/prefix. Pure for snapshot tests. */
+export function classifyPatchLine(
... diff truncated: showing 800 of 1390 linesYou can send follow-ups to the cloud agent here.
… match JSDoc Addresses Cursor Bugbot #599: emitFileChanged JSDoc states it is "emitted from PostToolUse after file_change_applied", but the hook fired them in reverse order. An orchestrator parsing the NDJSON stream and expecting file_change_applied first — then enriching it with the subsequent file_changed event — would never find the enrichment for that path. Reorder PostToolUse so: 1. recordPostWrite (canonical ledger) 2. recordFileChangeApplied (lifecycle marker) 3. emitFileChanged (diff enrichment) Adds an ordering test that pins the contract for future refactors.
39e4a48 to
48dc72a
Compare
… match JSDoc Addresses Cursor Bugbot #599: emitFileChanged JSDoc states it is "emitted from PostToolUse after file_change_applied", but the hook fired them in reverse order. An orchestrator parsing the NDJSON stream and expecting file_change_applied first — then enriching it with the subsequent file_changed event — would never find the enrichment for that path. Reorder PostToolUse so: 1. recordPostWrite (canonical ledger) 2. recordFileChangeApplied (lifecycle marker) 3. emitFileChanged (diff enrichment) Adds an ordering test that pins the contract for future refactors.
2076efb to
3c1b4f8
Compare
… match JSDoc Addresses Cursor Bugbot #599: emitFileChanged JSDoc states it is "emitted from PostToolUse after file_change_applied", but the hook fired them in reverse order. An orchestrator parsing the NDJSON stream and expecting file_change_applied first — then enriching it with the subsequent file_changed event — would never find the enrichment for that path. Reorder PostToolUse so: 1. recordPostWrite (canonical ledger) 2. recordFileChangeApplied (lifecycle marker) 3. emitFileChanged (diff enrichment) Adds an ordering test that pins the contract for future refactors.
3c1b4f8 to
8532fa6
Compare
… match JSDoc Addresses Cursor Bugbot #599: emitFileChanged JSDoc states it is "emitted from PostToolUse after file_change_applied", but the hook fired them in reverse order. An orchestrator parsing the NDJSON stream and expecting file_change_applied first — then enriching it with the subsequent file_changed event — would never find the enrichment for that path. Reorder PostToolUse so: 1. recordPostWrite (canonical ledger) 2. recordFileChangeApplied (lifecycle marker) 3. emitFileChanged (diff enrichment) Adds an ordering test that pins the contract for future refactors.
…n summary)
Capture before/after content for every file the inner agent writes via a
session-scoped FileChangeLedger, then surface diffs across three TUI
surfaces:
- Per-write toast in FileWritesPanel — applied rows now show +N/-M
counts pulled from the ledger, with a /diff discoverability hint
once at least one write has applied.
- /diff [path] slash command — tree of changed files (no arg) or full
unified patch text for one file (with arg). Resolves relative paths
against installDir.
- End-of-run summary — the success outro renders a DiffViewer summary
of every meaningful change, complementing the existing setup report.
Also emits a new file_changed NDJSON event in --agent mode carrying
{path, operation, additions, deletions, hunks} so ambient orchestrators
can render diffs in their own UI without re-reading anything.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
- Diff line counting and classification: drop the +++/--- guards inside hunks. structuredPatch hunks contain only content lines, and stripPatchHeader removes raw headers before classifyPatchLine. The guards mis-classified C++ '++i;' / SQL '-- comment' lines as context. - Memoize getDiff() per row keyed on (path, completedAt) so the diff isn't recomputed on every spinner-tick re-render. - Move the early-return BEFORE the synchronous safeRead in capturePre so repeat edits don't accrue wasted disk I/O.
- OutroScreen now memoizes summarizeLedgerDiffs() so structuredPatch doesn't re-run on every render frame post-mount. - /diff slash command uses path.sep instead of a hardcoded '/' when matching install-dir prefix and trailing-path suffix, so the command works on Windows.
Applied via @cursor push command
`@types/diff` was listed in `dependencies` (production) while every other `@types/*` package lives in `devDependencies`. The lockfile explicitly marks the package as deprecated — `diff@9.0.0` ships its own TypeScript declarations via `"types": "libcjs/index.d.ts"` and an `exports`-map `types` entry, so the standalone @types package is both unnecessary and misplaced (production users were paying for an empty declaration package). Drop it entirely. `diff` itself stays in `dependencies` (correct — runtime-needed by `src/lib/file-change-diff.ts` for the diff viewer). Verified with `tsc --noEmit` and `pnpm exec vitest run` against the DiffViewer / file-change-ledger suites (27/27 pass). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
… match JSDoc Addresses Cursor Bugbot #599: emitFileChanged JSDoc states it is "emitted from PostToolUse after file_change_applied", but the hook fired them in reverse order. An orchestrator parsing the NDJSON stream and expecting file_change_applied first — then enriching it with the subsequent file_changed event — would never find the enrichment for that path. Reorder PostToolUse so: 1. recordPostWrite (canonical ledger) 2. recordFileChangeApplied (lifecycle marker) 3. emitFileChanged (diff enrichment) Adds an ordering test that pins the contract for future refactors.
- summarizeLedgerPath now resolves the lookup path against installDir so raw / relative tool-input paths match the ledger-normalized entries (silent diff misses fixed). - summarizeLedgerEntry: skip the redundant createPatch when the caller doesn't need the unified-patch text (PostToolUse, FileWritesPanel toast). Halves diff cost on the hot path. - summarizeLedgerEntry: return null when one side of the capture failed (modify with no afterContent, etc.) so the diff doesn't render every captured line as added/removed. - emitFileChanged uses summary.operation (ledger truth) instead of the toolName-derived operation, so Write-over-existing reports modify. - /diff <path> uses summarizeLedgerPath instead of computing diffs for every file in the ledger just to discard all but one. - FileWritesPanel displayPath uses path.sep so the install-dir boundary check works on Windows. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
08553a4 to
27f719e
Compare
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
Autofix Details
Bugbot Autofix prepared a fix for the issue found in the latest run.
- ✅ Fixed: Double diff computation in
/diffdetail path- Removed the redundant pre-resolution against store.session.installDir and now pass the raw arg directly to summarizeLedgerPath, which already resolves relative paths against ledger.getInstallDir() consistently with how entries are stored.
Or push these changes by commenting:
@cursor push 51427ccf24
Preview (51427ccf24)
diff --git a/src/ui/tui/components/ConsoleView.tsx b/src/ui/tui/components/ConsoleView.tsx
--- a/src/ui/tui/components/ConsoleView.tsx
+++ b/src/ui/tui/components/ConsoleView.tsx
@@ -307,12 +307,10 @@
// unrelated file in the ledger just to discard them. The summary
// mode below still needs the full sweep for the +N/-M tree.
if (arg) {
- // Resolve relative paths against installDir so users can type
- // `/diff src/amplitude.ts` instead of pasting an absolute path.
- const target = path.isAbsolute(arg)
- ? arg
- : path.resolve(store.session.installDir, arg);
- const found = summarizeLedgerPath(ledger, target);
+ // Pass the raw argument to summarizeLedgerPath — it resolves
+ // relative paths against ledger.getInstallDir() internally,
+ // keeping the resolution consistent with how entries are stored.
+ const found = summarizeLedgerPath(ledger, arg);
if (!found) {
// Fallback: walk the ledger entries directly so a user-friendly
// suffix match (e.g. `/diff amplitude.ts` matchingYou can send follow-ups to the cloud agent here.
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
Autofix Details
Bugbot Autofix prepared a fix for the issue found in the latest run.
- ✅ Fixed: Summary-mode
/diffcomputes unused full patch text- Added an options parameter to summarizeLedgerDiffs that forwards to summarizeLedgerEntry, and updated both call sites (ConsoleView /diff summary and OutroScreen) to pass { includePatch: false } to skip the redundant O(n·m) createPatch computation.
Or push these changes by commenting:
@cursor push 2b4d7ab275
Preview (2b4d7ab275)
diff --git a/src/lib/file-change-diff.ts b/src/lib/file-change-diff.ts
--- a/src/lib/file-change-diff.ts
+++ b/src/lib/file-change-diff.ts
@@ -163,11 +163,12 @@
*/
export function summarizeLedgerDiffs(
ledger: FileChangeLedger | null,
+ options?: SummarizeLedgerEntryOptions,
): FileDiffSummary[] {
if (!ledger) return [];
const out: FileDiffSummary[] = [];
for (const entry of ledger.getEntries()) {
- const summary = summarizeLedgerEntry(entry);
+ const summary = summarizeLedgerEntry(entry, options);
if (summary) out.push(summary);
}
return out;
diff --git a/src/ui/tui/components/ConsoleView.tsx b/src/ui/tui/components/ConsoleView.tsx
--- a/src/ui/tui/components/ConsoleView.tsx
+++ b/src/ui/tui/components/ConsoleView.tsx
@@ -348,7 +348,7 @@
break;
}
// Summary mode (no arg): walk the whole ledger.
- const diffs = summarizeLedgerDiffs(ledger);
+ const diffs = summarizeLedgerDiffs(ledger, { includePatch: false });
if (diffs.length === 0) {
store.setCommandFeedback(
'No file changes captured yet — the agent has not written anything in this session.',
diff --git a/src/ui/tui/screens/OutroScreen.tsx b/src/ui/tui/screens/OutroScreen.tsx
--- a/src/ui/tui/screens/OutroScreen.tsx
+++ b/src/ui/tui/screens/OutroScreen.tsx
@@ -106,7 +106,9 @@
// single computation per mount is correct). The ledger reference itself is
// stable for the lifetime of the wizard run.
const meaningfulDiffs = useMemo(() => {
- const diffs = summarizeLedgerDiffs(getFileChangeLedger());
+ const diffs = summarizeLedgerDiffs(getFileChangeLedger(), {
+ includePatch: false,
+ });
return diffs.filter(
(d) => d.additions > 0 || d.deletions > 0 || d.operation !== 'modify',
);You can send follow-ups to the cloud agent here.
summarizeLedgerDiffs now accepts SummarizeLedgerEntryOptions and
forwards them to summarizeLedgerEntry. The /diff summary handler in
ConsoleView only renders +/- counts and operation glyphs — pass
{ includePatch: false } so each ledger entry skips the redundant
createPatch O(n*m) re-diff. OutroScreen still passes the default
because DiffViewer's detail mode reads target.patch.
|
Bugbot triage pass complete: 0 stale, 1 live (fixed in 7975447), 0 defensible. 18 prior threads were already resolved before this pass. |
Addresses Cursor Bugbot findings on PR #599: - #5 (Medium): `OutroScreen.tsx` now passes `{ includePatch: false }` to `summarizeLedgerDiffs`. The outro renders DiffViewer in summary mode only (no `filePath` prop), which never reads the patch field. Skipping the per-entry `createPatch` call avoids an O(n*m) re-diff for every changed file (up to the ledger FIFO cap) on outro mount. - #6 (Low): Extracted shared `displayPath` helper to `src/ui/tui/utils/display-path.ts`. DiffViewer, FileWritesPanel, and ConsoleView all funnel through it so the out-of-project fallback is consistent (basename, not raw path). Adds a unit test that pins the divergent fallback behavior the three copies used to have. - #7 (Low): `ConsoleView` /diff detail path now hands the raw arg straight to `summarizeLedgerPath` (which already normalizes against the ledger's install dir) instead of pre-resolving against `store.session.installDir` and risking silent divergence. Findings #1-4 are already addressed in the current head — Bugbot was looking at older revisions of the same files. Replying defensibly to each on the PR.
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
Bugbot Autofix prepared a fix for the issue found in the latest run.
- ✅ Fixed: Detail mode displays raw absolute path instead of relative
- Wrapped
found.pathwithdisplayPath(found.path, store.session.installDir)on line 339 to relativize the path, matching the summary mode behavior on line 362.
- Wrapped
Or push these changes by commenting:
@cursor push c33aa44660
Preview (c33aa44660)
diff --git a/src/ui/tui/components/ConsoleView.tsx b/src/ui/tui/components/ConsoleView.tsx
--- a/src/ui/tui/components/ConsoleView.tsx
+++ b/src/ui/tui/components/ConsoleView.tsx
@@ -336,7 +336,7 @@
// can't easily render syntax-coloured diffs inline, but the unified
// patch text is itself readable and copy-pasteable.
store.setCommandFeedback(
- `${found.operation.toUpperCase()} ${found.path} ${formatChangeCounts(found.additions, found.deletions)}\n\n${found.patch}`,
+ `${found.operation.toUpperCase()} ${displayPath(found.path, store.session.installDir)} ${formatChangeCounts(found.additions, found.deletions)}\n\n${found.patch}`,
60_000,
);
break;You can send follow-ups to the cloud agent here.
Reviewed by Cursor Bugbot for commit 05f9abe. Configure here.
Detail mode (`/diff <path>`) was rendering `found.path` directly, which `FileChangeEntry.path` documents as the absolute normalised path from the ledger. Summary mode already funnels through the shared `displayPath` helper for the same field — leaving detail mode as the only place that leaked the user's absolute home path into the on-screen header. Funnel detail mode through the same `displayPath(found.path, store.session.installDir)` call. Behaviour is now consistent across summary, detail, FileWritesPanel, and the outro DiffViewer — the displayed path is always relative to the install dir (falling back to basename if outside the project, matching the helper's documented behaviour).
# Conflicts: # src/ui/tui/__tests__/console-commands.test.ts # src/ui/tui/components/ConsoleView.tsx
# Conflicts: # src/lib/file-change-ledger.ts
…ands The "fills the input with the highlighted command + space" test used `/d` as the prefix and assumed the first cmd-prefix match was `/debug`. Main now contains `/diff` (added by #599) which sorts ahead of `/debug` in the COMMANDS array — once this branch merges with main, the test received `/diff` instead of `/debug`. Tighten the first test to `/de` (matches /debug only). The "completes the second match when the user navigates to it" test relied on DOWN moving from /debug → /diagnostics. With /diff now in the mix, DOWN once landed on /debug instead. Switch to a self-contained two-command list so the test doesn't depend on the production COMMANDS-array ordering at all — the wiring it pins is "DOWN moves to second match, Tab accepts it".
…ands The "fills the input with the highlighted command + space" test used `/d` as the prefix and assumed the first cmd-prefix match was `/debug`. Main now contains `/diff` (added by #599) which sorts ahead of `/debug` in the COMMANDS array — once this branch merges with main, the test received `/diff` instead of `/debug`. Tighten the first test to `/de` (matches /debug only). The "completes the second match when the user navigates to it" test relied on DOWN moving from /debug → /diagnostics. With /diff now in the mix, DOWN once landed on /debug instead. Switch to a self-contained two-command list so the test doesn't depend on the production COMMANDS-array ordering at all — the wiring it pins is "DOWN moves to second match, Tab accepts it".
…te (#769) * feat(tui): register /help command and accept Tab in slash palette Tab in the slash palette now accepts the highlighted suggestion (Raycast/Slack convention) and fills the input with `<cmd> ` so the user can keep typing arguments. The palette stays open; Enter still submits. Outside slash mode, Tab continues to open Ask via ConsoleView's pre-activation handler. Also adds an `↑↓ navigate · Tab/Enter run · Esc cancel` footer hint beneath the candidate list so users don't have to guess which key does what. /help itself was already registered (see prior PR) — this commit completes the paired affordances from the TUI audit T5 wave. * fix(bugbot): correct slash palette footer and tolerate trailing space in filter Bugbot flagged two issues on PR #739: 1. (3220814211) Palette footer read "Tab/Enter run" but Tab only completes the highlighted suggestion (setValue) — Enter is required to actually submit. Footer now reads "Tab complete · Enter run" so the affordance matches the handler at line ~118. 2. (3220814221) Tab fills the input with `<cmd> ` (trailing space, per Raycast/Slack convention so the user can keep typing args). The filter used `value.slice(1)` as the query, so the trailing space caused both `cmd.startsWith(query)` and the description includes-check to fail — `filtered` emptied and the palette tore itself down mid-completion. Picked option B (trim query at filter time): the filter now keys off the first whitespace-delimited word, matching the slash-mode check at `computeIsSlashMode`. Reason: trailing space is the universal palette/shell convention (zsh, fish, Claude Code itself, every modern CLI palette) — it signals "now type your arg". Dropping the space (option A) would force the user to type it themselves before args, which is a worse UX. The filter should be query-trim-tolerant. Verified clampedIndex, scroll math, and Enter-submit behavior still work because they all key off `filtered.length`, which is now non-empty after Tab. Tests extended in SlashCommandInput.tab.test.tsx to lock down the new footer copy and assert the palette stays open after Tab completion. * fix(bugbot): preserve argv when Enter-submitting slash commands Bugbot 3220907967 (high). The Tab-trailing-space fix that landed in 24eb3bd keyed the slash filter off the first whitespace-delimited word, which fixed the palette-collapse-on-trailing-space regression but introduced a worse one: typing /feedback hello world and pressing Enter submitted just /feedback because the Enter handler preferred filtered[clampedIndex].cmd whenever filtered was non-empty, and the new first-word filter kept it non-empty even when args were typed. Affected commands in the registry: /feedback <message> and /create-project <name>. Args were silently dropped. Fix: Enter now distinguishes command-only (no space in value) from command-with-args (space present). Command-only takes the palette pick (so /he + Enter still submits /help). Command-with-args submits the value verbatim. The Tab fill behavior (trailing space) is unchanged because Tab does not call onSubmit. Tests: extends SlashCommandInput.tab.test.tsx with regression coverage for /feedback hello world, /he, /help, and /feedback<space>. * fix(bugbot): detect hasArgs on untrimmed value so trailing space survives `hasArgs = trimmed.includes(' ')` stripped the very signal we wanted — `/cmd ` (trailing space, e.g. from Tab completion) was indistinguishable from `/cmd` and silently palette-picked. Now detect args on the untrimmed value: a trailing space anywhere routes to verbatim submit, matching the universal palette/shell convention. Bugbot 3221028494. * feat(tui): register /help slash command — docs canonical, registry was missing CLAUDE.md lists `/help` as a canonical slash command, but the registry in `src/ui/tui/console-commands.ts` was missing it. Users typing `/help` got nothing — no completion match in the picker, no dispatch case, no feedback. The empty-result path is the worst kind of broken: the command appears unsupported even though it should be the first thing a new user reaches for. Add `/help` to `COMMANDS` and a dispatch case in `ConsoleView.tsx` that prints a categorized list of every other slash command + a one-line key-binding cheatsheet, surfaced through the existing `setCommandFeedback` channel. Regression test in `console-commands.test.ts` asserts `/help` is in the registry and is NOT `requiresIdle` (so it works mid-run). * fix(test): make SlashCommandInput tab tests resilient to new /d* commands The "fills the input with the highlighted command + space" test used `/d` as the prefix and assumed the first cmd-prefix match was `/debug`. Main now contains `/diff` (added by #599) which sorts ahead of `/debug` in the COMMANDS array — once this branch merges with main, the test received `/diff` instead of `/debug`. Tighten the first test to `/de` (matches /debug only). The "completes the second match when the user navigates to it" test relied on DOWN moving from /debug → /diagnostics. With /diff now in the mix, DOWN once landed on /debug instead. Switch to a self-contained two-command list so the test doesn't depend on the production COMMANDS-array ordering at all — the wiring it pins is "DOWN moves to second match, Tab accepts it". * fix(lint): remove duplicate /help case from ConsoleView after merge The salvage branch added an inline `/help` case in ConsoleView's switch; main later landed a separate `/help` case that calls `getHelpText()`. After merge, both coexisted and ESLint flagged the duplicate-case as an error. Drop the inline version. `getHelpText()` is the canonical helper — it's covered by console-commands.test.ts and shared with any future non-TUI surface that wants the same listing. * fix(test): tolerate 1s tick drift in EventPlanFullScreen elapsed assertions Same fix as PR #768. The revising-screen elapsed timer ticks every 1s via setInterval; on Node 22 under CI clock pressure the interval can fire one tick before the test's advanceTimersByTime boundary, so the rendered "elapsed:" string lands at `1m 29s` instead of `1m 30s`. Both readings are correct for the test's intent (verifying tier WIRING, not exact copy), so widen the assertions to a regex that accepts ±1s.
What landed ----------- - New `src/ui/tui/components/DiffTab.tsx` — persistent tab body in RunScreen. File list (newest-first, deduped by path) with a colored unified diff for the selected file rendered below. `↑↓` / `j/k` move selection; `PgUp/PgDn` scroll the diff body; `←→` still switch tabs. - `src/ui/tui/screens/RunScreen.tsx` — wired the Diff tab between Events and Logs. What was removed ---------------- - `/diff` slash command registration in `src/ui/tui/console-commands.ts` (and the `parseDiffSlashInput` helper). - `/diff` dispatch case + its summary/detail branches in `src/ui/tui/components/ConsoleView.tsx`. - (kept) `DiffViewer` primitive — now consumed by the new tab and the existing outro "what changed" section. Diff tab behavior ----------------- The tab subscribes narrowly to `fileWritesTotal` (a monotonic counter that climbs on every agent file write) and re-walks the canonical `FileChangeLedger` only when that counter advances. The DiffViewer primitive already windows the patch body to `maxLines` (default 30) around `scrollOffset`, so large diffs render at a constant ~30-line cost per frame. Outer Box uses `overflow="hidden"` (defense against the #779 overdraw bug). Empty state copy: "no file changes yet — the agent hasn't edited anything." Discoverability hints in `FileWritesPanel` and `DiffViewer` summary mode now point to the Diff tab instead of the removed slash command. Tests ----- New: `src/ui/tui/screens/__tests__/RunScreen.diffTab.test.tsx` (6 tests + 2 snapshots at 80/60 cols) covers tab visibility, tab ordering (Progress → Events → Diff → Logs), empty state, and content rendering with seeded ledger entries. Updated: `console-commands.test.ts` — replaced the old `/diff` registry assertions with a "removal" describe block that asserts `/diff` is absent and `isKnownCommand('/diff')` is false (the registry-doesn't- contain-/diff acceptance check). `DiffViewer.test.tsx` and `FileWritesPanel.test.tsx` updated for the new hint copy. Verification ------------ - `pnpm tsc --noEmit && pnpm lint && pnpm test` — all green (4470 tests) - `src/utils/wizard-abort.ts` untouched - ← → tab switching still works - /diff is gone from the slash palette (registry + isKnownCommand assertions) Deferred -------- The end-of-run "all changes summary" overlay that PR #599 also added is intentionally untouched — this PR only addresses the slash command → tab migration. No docs/skill prompts referenced `/diff`. Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>


Summary
Users have asked for clarity on what the agent is actually changing in their codebase. The TUI's
FileWritesPanelalready reports "we wrote N files" but stops short of showing actual content diffs — so users either trust the agent blindly or have togit diffoutside the wizard. This PR closes that gap with three additive surfaces:FileWritesPanelnow show+N/-Mcounts pulled from a session-scopedFileChangeLedger. A discoverability hint (type /diff to review) lights up in the panel header once at least one write has applied./diff [path]slash command — with no argument: a tree of every changed file with operation + counts. With a path argument (relative or absolute): the full unified patch body for that file. Surfaces in the existing slash-command feedback channel — works mid-run and on the outro.DiffViewersummary block above the setup-report hint, listing every meaningful change so users don't have to leave the terminal to know what was written. Existingamplitude-setup-report.mdcontinues to be written; this is additive.Also adds a new
file_changedNDJSON event in--agentmode carrying{path, operation, additions, deletions, hunks}so ambient agent orchestrators can render diffs in their own UI without re-reading files. Versioned inEVENT_DATA_VERSIONS(v1).Sample render (summary mode, ASCII)
Sample render (detail mode, ASCII — colors stripped)
Architecture
src/lib/file-change-ledger.ts— new module. In-memory, process-scoped. Capturesbeforecontent at PreToolUse (re-reads from disk) andafterat PostToolUse, with binary-file detection (NUL byte heuristic) and a 500-record FIFO cap. Computes diffs lazily via thediff(jsdiff) package. Designed so the rollback subagent on cancel can read the same ledger for transactional cleanup later.src/ui/tui/components/DiffViewer.tsx— new Ink component. Two modes (summary tree / per-file detail). Color tokens: success=add, error=delete, accent=hunk header, muted=context.src/lib/inner-lifecycle.ts— wiresgetLedger().capturePre()into the existing PreToolUse hook andgetLedger().captureApplied()+emitFileChanged()into PostToolUse. All capture failures are swallowed (observation, not a gate).src/ui/tui/console-commands.ts— registers/diffand/helpin the canonical slash-command catalog (per CLAUDE.md "Slash commands are always available" rule)./diffis read-only, neverrequiresIdle.Trade-offs / punted
DiffVieweras a full-screen overlay. The component is fully functional and gets exercised in the outro; promoting it to its own router screen + keyboard navigation overlay is a follow-up. Acceptable for v1 because the patch body is itself readable and copy-pasteable.diffpackage added (~30 kB pure JS, well-maintained, de-facto standard for JS line diffing). Nogit diffshellout — wizard runs in user repos that may not be git-clean.Test plan
pnpm test— 3077 tests pass (76 new: 16 ledger + 15 DiffViewer + 12 console-commands additions + others)pnpm exec vitest run --pool=forks --maxWorkers=1 src/ui/tui/__tests__/router.test.ts src/ui/tui/__tests__/flow-invariants.test.ts src/ui/tui/components/__tests__/FileWritesPanel.test.tsx src/lib/__tests__/inner-lifecycle.test.ts— all greenpnpm lint— prettier + eslint cleanpnpm build— TypeScript compiles, smoke test passes/diff(summary + detail) is rendered. Pinned bysrc/ui/tui/__tests__/console-commands.test.ts(describe('parseDiffSlashInput')~L536 anddescribe('/diff and /help command registration')~L557) andsrc/ui/tui/components/__tests__/DiffViewer.test.tsx(describe('DiffViewer — summary mode')anddescribe('DiffViewer — detail mode')).--agentmode emitsfile_changedNDJSON alongsidefile_change_applied. Pinned bysrc/lib/__tests__/inner-lifecycle.test.ts(it('PostToolUse emits file_change_applied BEFORE file_changed (NDJSON contract)')~L55).🤖 Generated with Claude Code
Note
Medium Risk
Adds new diff computation paths (new
diffdependency) and new agent-mode event emission in hot PostToolUse hooks, which could affect performance and orchestrator integrations if output/order changes.Overview
Adds a diff summarization layer on top of the
FileChangeLedger(newfile-change-diff.tsusing thediffpackage) so the wizard can compute per-file additions/deletions, hunk ranges, and unified patches.Exposes those diffs in the TUI via a new
DiffViewercomponent, a new/diff [path]slash command (plus/helpcatalogue output), and an end-of-run “What changed” summary; the liveFileWritesPanelnow shows+N/-Mcounts and a/diffdiscoverability hint.Extends agent-mode telemetry with a versioned
file_changedevent emitted afterfile_change_applied(with ordering regression test), carrying{path, operation, additions, deletions, hunks}to let orchestrators render diffs without re-reading files.Reviewed by Cursor Bugbot for commit e9fecfe. Bugbot is set up for automated code reviews on this repo. Configure here.