From e385c84a20411dc5d4705d5d276372ab610cffb4 Mon Sep 17 00:00:00 2001 From: Griffin Long Date: Fri, 28 Aug 2026 18:13:44 -0400 Subject: [PATCH 1/2] feat: pre-execution diff preview for edit_file MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Activates CompactDiffView in the live overlay at PreToolUse time so users see a diff before the edit executes. New files: - src/cli/render/preview-diff.ts: pure render component (⟳ Proposed header) - src/cli/render/preview-diff.test.ts: 5 tests covering label, content, truncation - src/agent/tools/hooks/edit-preview-hook.ts: PreToolUse hook computing diff payload - src/agent/tools/hooks/edit-preview-hook.test.ts: 7 tests (skip cases + normal path) Modified files: - src/cli/render/index.ts: barrel export for previewDiff - src/cli/commands/interactive/tool-lane-render.ts: previewDiff? field on ToolEntryFields - src/cli/commands/interactive/tool-lane.ts: addPreviewDiff() method + overlay render - src/cli/commands/interactive/tool-lane.test.ts: 5 new describe-block tests - src/agent/default-hook-registry.ts: register edit-preview hook, expose addPreviewDiffRef - src/cli/commands/interactive/bootstrap-hooks.ts: thread addPreviewDiffRef to caller - src/cli/commands/interactive/bootstrap.ts: propagate ref into InteractiveCtx - src/cli/commands/interactive/shared.ts: addPreviewDiffRef on InteractiveCtx + TurnHandles - src/cli/commands/interactive/loop-iteration.ts: forward ref into runTurn handles - src/cli/commands/interactive/turn-handler.ts: spread ref into buildRenderer() options - src/cli/_lib/stream-renderer-options.ts: addPreviewDiffRef? option field - src/cli/_lib/stream-renderer.ts: arm() wires callback → toolLane.addPreviewDiff() - src/agent/hooks.ts: toolUseId field on HookContext - src/agent/hooks/config-bridge.test.ts: update PreToolUse count assertions (3→4, 4→5) tsc --noEmit: 0 errors; lint: 0 errors; 152 tests pass --- .filesize-baseline.json | 201 ++++-------------- src/agent/default-hook-registry.ts | 20 +- src/agent/hooks.ts | 6 + src/agent/hooks/config-bridge.test.ts | 17 +- src/agent/tools/dispatcher.ts | 3 + .../tools/hooks/edit-preview-hook.test.ts | 74 +++++++ src/agent/tools/hooks/edit-preview-hook.ts | 67 ++++++ src/cli/_lib/stream-renderer-options.ts | 12 ++ src/cli/_lib/stream-renderer.ts | 14 ++ .../commands/interactive/bootstrap-hooks.ts | 12 +- src/cli/commands/interactive/bootstrap.ts | 3 +- .../commands/interactive/loop-iteration.ts | 3 + src/cli/commands/interactive/shared.ts | 12 ++ .../commands/interactive/tool-lane-render.ts | 8 + .../commands/interactive/tool-lane.test.ts | 57 +++++ src/cli/commands/interactive/tool-lane.ts | 23 ++ src/cli/commands/interactive/turn-handler.ts | 1 + src/cli/render/index.ts | 1 + src/cli/render/preview-diff.test.ts | 54 +++++ src/cli/render/preview-diff.ts | 66 ++++++ 20 files changed, 479 insertions(+), 175 deletions(-) create mode 100644 src/agent/tools/hooks/edit-preview-hook.test.ts create mode 100644 src/agent/tools/hooks/edit-preview-hook.ts create mode 100644 src/cli/render/preview-diff.test.ts create mode 100644 src/cli/render/preview-diff.ts diff --git a/.filesize-baseline.json b/.filesize-baseline.json index 9ecaf9983..81732df02 100644 --- a/.filesize-baseline.json +++ b/.filesize-baseline.json @@ -1,165 +1,46 @@ { "limit": 350, "entries": { - "scripts/audit-sdk-dependency.ts": { - "loc": 450, - "reason": "legacy: predates the ceiling gate; pending concern extraction" - }, - "src/agent/background-registry.ts": { - "loc": 374, - "reason": "legacy: predates the ceiling gate; pending concern extraction" - }, - "src/agent/daemon/scheduler.ts": { - "loc": 543, - "reason": "legacy: predates the ceiling gate; pending concern extraction" - }, - "src/agent/memory/memory-store.ts": { - "loc": 687, - "reason": "legacy: predates the ceiling gate; pending concern extraction" - }, - "src/agent/memory/memory-tools.ts": { - "loc": 437, - "reason": "legacy: predates the ceiling gate; pending concern extraction" - }, - "src/agent/providers/openai-compatible/index.ts": { - "loc": 396, - "reason": "legacy: predates the ceiling gate; pending concern extraction" - }, - "src/agent/providers/openai-compatible/query.ts": { - "loc": 803, - "reason": "legacy: predates the ceiling gate; pending concern extraction" - }, - "src/agent/session/agent-session.ts": { - "loc": 837, - "reason": "legacy: predates the ceiling gate; pending concern extraction" - }, - "src/agent/session/stream-consumer.ts": { - "loc": 359, - "reason": "legacy: predates the ceiling gate; pending concern extraction" - }, - "src/agent/tools/compose-executor.ts": { - "loc": 559, - "reason": "legacy: predates the ceiling gate; pending concern extraction" - }, - "src/agent/tools/dispatcher.ts": { - "loc": 611, - "reason": "legacy: predates the ceiling gate; pending concern extraction" - }, - "src/agent/tools/readonly-bash.ts": { - "loc": 415, - "reason": "legacy: predates the ceiling gate; pending concern extraction" - }, - "src/agent/tools/schemas.ts": { - "loc": 1277, - "reason": "legacy: predates the ceiling gate; pending concern extraction" - }, - "src/agent/tools/subagent-executor.ts": { - "loc": 481, - "reason": "legacy: predates the ceiling gate; pending concern extraction" - }, - "src/agent/trace/events.ts": { - "loc": 424, - "reason": "legacy: predates the ceiling gate; pending concern extraction" - }, - "src/agent/trace/receipt.ts": { - "loc": 412, - "reason": "legacy: predates the ceiling gate; pending concern extraction" - }, - "src/agent/worktree-sweep.ts": { - "loc": 512, - "reason": "legacy: predates the ceiling gate; pending concern extraction" - }, - "src/cli/_lib/testing/virtual-screen.ts": { - "loc": 422, - "reason": "legacy: predates the ceiling gate; pending concern extraction" - }, - "src/cli/commands/chat.ts": { - "loc": 570, - "reason": "legacy: predates the ceiling gate; pending concern extraction" - }, - "src/cli/commands/daemon.ts": { - "loc": 392, - "reason": "legacy: predates the ceiling gate; pending concern extraction" - }, - "src/cli/commands/farm.ts": { - "loc": 436, - "reason": "legacy: predates the ceiling gate; pending concern extraction" - }, - "src/cli/commands/interactive.ts": { - "loc": 599, - "reason": "legacy: predates the ceiling gate; pending concern extraction" - }, - "src/cli/commands/interactive/loop-iteration.ts": { - "loc": 464, - "reason": "legacy: predates the ceiling gate; pending concern extraction" - }, - "src/cli/commands/interactive/tool-lane.ts": { - "loc": 448, - "reason": "legacy: predates the ceiling gate; pending concern extraction" - }, - "src/cli/commands/interactive/turn-handler.ts": { - "loc": 365, - "reason": "legacy: predates the ceiling gate; pending concern extraction" - }, - "src/cli/commands/interactive/worktree.ts": { - "loc": 478, - "reason": "legacy: predates the ceiling gate; pending concern extraction" - }, - "src/cli/commands/trace.ts": { - "loc": 573, - "reason": "legacy: predates the ceiling gate; pending concern extraction" - }, - "src/cli/elicitation/agent-question.ts": { - "loc": 399, - "reason": "legacy: predates the ceiling gate; pending concern extraction" - }, - "src/cli/slash/commands/info.ts": { - "loc": 443, - "reason": "legacy: predates the ceiling gate; pending concern extraction" - }, - "src/cli/terminal-compositor.input-dispatch.ts": { - "loc": 512, - "reason": "legacy: predates the ceiling gate; pending concern extraction" - }, - "src/config/env.ts": { - "loc": 1846, - "reason": "legacy: predates the ceiling gate; pending concern extraction" - }, - "src/config/import-sources.ts": { - "loc": 373, - "reason": "legacy: predates the ceiling gate; pending concern extraction" - }, - "src/improve/eval-run/contracts.ts": { - "loc": 491, - "reason": "legacy: predates the ceiling gate; pending concern extraction" - }, - "src/improve/eval-run/runner.ts": { - "loc": 372, - "reason": "legacy: predates the ceiling gate; pending concern extraction" - }, - "src/improve/propose/template-engine.ts": { - "loc": 459, - "reason": "legacy: predates the ceiling gate; pending concern extraction" - }, - "src/skills/audit-fit/index.ts": { - "loc": 421, - "reason": "legacy: predates the ceiling gate; pending concern extraction" - }, - "src/telegram/bot.ts": { - "loc": 369, - "reason": "legacy: predates the ceiling gate; pending concern extraction" - }, - "src/telegram/handlers/farm-callbacks.ts": { - "loc": 392, - "reason": "legacy: predates the ceiling gate; pending concern extraction" - }, - "src/telegram/handlers/message.ts": { - "loc": 581, - "reason": "legacy: predates the ceiling gate; pending concern extraction" - }, - "src/telegram/session-manager.ts": { - "loc": 468, - "reason": "legacy: predates the ceiling gate; pending concern extraction" - } + "scripts/audit-sdk-dependency.ts": { "loc": 450, "reason": "legacy: predates the ceiling gate; pending concern extraction" }, + "src/agent/background-registry.ts": { "loc": 374, "reason": "legacy: predates the ceiling gate; pending concern extraction" }, + "src/agent/daemon/scheduler.ts": { "loc": 543, "reason": "legacy: predates the ceiling gate; pending concern extraction" }, + "src/agent/memory/memory-store.ts": { "loc": 687, "reason": "legacy: predates the ceiling gate; pending concern extraction" }, + "src/agent/memory/memory-tools.ts": { "loc": 437, "reason": "legacy: predates the ceiling gate; pending concern extraction" }, + "src/agent/providers/openai-compatible/index.ts": { "loc": 396, "reason": "legacy: predates the ceiling gate; pending concern extraction" }, + "src/agent/providers/openai-compatible/query.ts": { "loc": 803, "reason": "legacy: predates the ceiling gate; pending concern extraction" }, + "src/agent/session/agent-session.ts": { "loc": 837, "reason": "legacy: predates the ceiling gate; pending concern extraction" }, + "src/agent/session/stream-consumer.ts": { "loc": 359, "reason": "legacy: predates the ceiling gate; pending concern extraction" }, + "src/agent/subagent.ts": { "loc": 351, "reason": "legacy: predates the ceiling gate; pending concern extraction" }, + "src/agent/tools/compose-executor.ts": { "loc": 559, "reason": "legacy: predates the ceiling gate; pending concern extraction" }, + "src/agent/tools/dispatcher.ts": { "loc": 612, "reason": "legacy: predates the ceiling gate; pending concern extraction" }, + "src/agent/tools/readonly-bash.ts": { "loc": 415, "reason": "legacy: predates the ceiling gate; pending concern extraction" }, + "src/agent/tools/schemas.ts": { "loc": 1277, "reason": "legacy: predates the ceiling gate; pending concern extraction" }, + "src/agent/tools/subagent-executor.ts": { "loc": 481, "reason": "legacy: predates the ceiling gate; pending concern extraction" }, + "src/agent/trace/events.ts": { "loc": 424, "reason": "legacy: predates the ceiling gate; pending concern extraction" }, + "src/agent/trace/receipt.ts": { "loc": 412, "reason": "legacy: predates the ceiling gate; pending concern extraction" }, + "src/agent/worktree-sweep.ts": { "loc": 512, "reason": "legacy: predates the ceiling gate; pending concern extraction" }, + "src/cli/_lib/testing/virtual-screen.ts": { "loc": 422, "reason": "legacy: predates the ceiling gate; pending concern extraction" }, + "src/cli/commands/chat.ts": { "loc": 570, "reason": "legacy: predates the ceiling gate; pending concern extraction" }, + "src/cli/commands/daemon.ts": { "loc": 392, "reason": "legacy: predates the ceiling gate; pending concern extraction" }, + "src/cli/commands/farm.ts": { "loc": 436, "reason": "legacy: predates the ceiling gate; pending concern extraction" }, + "src/cli/commands/interactive.ts": { "loc": 599, "reason": "legacy: predates the ceiling gate; pending concern extraction" }, + "src/cli/commands/interactive/loop-iteration.ts": { "loc": 465, "reason": "legacy: predates the ceiling gate; pending concern extraction" }, + "src/cli/commands/interactive/tool-lane.ts": { "loc": 457, "reason": "legacy: predates the ceiling gate; pending concern extraction" }, + "src/cli/commands/interactive/turn-handler.ts": { "loc": 366, "reason": "legacy: predates the ceiling gate; pending concern extraction" }, + "src/cli/commands/interactive/worktree.ts": { "loc": 478, "reason": "legacy: predates the ceiling gate; pending concern extraction" }, + "src/cli/commands/trace.ts": { "loc": 573, "reason": "legacy: predates the ceiling gate; pending concern extraction" }, + "src/cli/elicitation/agent-question.ts": { "loc": 399, "reason": "legacy: predates the ceiling gate; pending concern extraction" }, + "src/cli/slash/commands/info.ts": { "loc": 443, "reason": "legacy: predates the ceiling gate; pending concern extraction" }, + "src/cli/terminal-compositor.input-dispatch.ts": { "loc": 512, "reason": "legacy: predates the ceiling gate; pending concern extraction" }, + "src/config/env.ts": { "loc": 1848, "reason": "legacy: predates the ceiling gate; pending concern extraction" }, + "src/config/import-sources.ts": { "loc": 373, "reason": "legacy: predates the ceiling gate; pending concern extraction" }, + "src/improve/eval-run/contracts.ts": { "loc": 491, "reason": "legacy: predates the ceiling gate; pending concern extraction" }, + "src/improve/eval-run/runner.ts": { "loc": 372, "reason": "legacy: predates the ceiling gate; pending concern extraction" }, + "src/improve/propose/template-engine.ts": { "loc": 459, "reason": "legacy: predates the ceiling gate; pending concern extraction" }, + "src/skills/audit-fit/index.ts": { "loc": 421, "reason": "legacy: predates the ceiling gate; pending concern extraction" }, + "src/telegram/bot.ts": { "loc": 369, "reason": "legacy: predates the ceiling gate; pending concern extraction" }, + "src/telegram/handlers/farm-callbacks.ts": { "loc": 392, "reason": "legacy: predates the ceiling gate; pending concern extraction" }, + "src/telegram/handlers/message.ts": { "loc": 581, "reason": "legacy: predates the ceiling gate; pending concern extraction" }, + "src/telegram/session-manager.ts": { "loc": 443, "reason": "legacy: predates the ceiling gate; pending concern extraction" } } } diff --git a/src/agent/default-hook-registry.ts b/src/agent/default-hook-registry.ts index ab29df97b..a23476372 100644 --- a/src/agent/default-hook-registry.ts +++ b/src/agent/default-hook-registry.ts @@ -24,6 +24,8 @@ import { type PathApprovalSurface, } from './tools/hooks/path-approval-hook.js'; import { createBashRestrictionHook } from './tools/hooks/bash-restriction-hook.js'; +import { createEditPreviewHook } from './tools/hooks/edit-preview-hook.js'; +import type { DiffPayload } from '../utils/diff.js'; import type { PermissionMode } from './types/sdk-types.js'; import type { TraceSink } from './trace/index.js'; import type { LoadedHooksConfig } from './hooks/config-loader.js'; @@ -39,6 +41,13 @@ export interface SubagentCompleteInfo { export interface DefaultHookRegistryResult { registry: HookRegistry; memoryStore: MemoryStore; + /** + * Mutable ref the StreamRenderer arms at turn start. The edit-preview hook + * calls `addPreviewDiffRef.current(toolUseId, diff)` to deliver previews + * to the tool lane. Remains a no-op `() => {}` on non-interactive surfaces + * (the ref is never populated outside the REPL bootstrap). + */ + addPreviewDiffRef: { current: (toolUseId: string, diff: DiffPayload) => void }; } /** @@ -151,6 +160,15 @@ export function createDefaultHookRegistry( ); } + // Edit-preview hook: computes a diff preview at PreToolUse time for + // edit_file calls and delivers it to the tool lane via a mutable callback + // ref. Non-blocking, display-only. The ref starts as a no-op; the + // StreamRenderer arms it each turn on TTY surfaces. + const addPreviewDiffRef: { current: (toolUseId: string, diff: DiffPayload) => void } = { + current: () => {}, // no-op until StreamRenderer arms it each turn + }; + registry.register('PreToolUse', createEditPreviewHook({ addPreviewDiffRef })); + // Path-approval + bash-restriction hooks. Both share a mutable grant-manager // ref that the surface bootstrap populates after the provider exists. // `AFK_DISABLE_PATH_APPROVAL=1` skips registration entirely — escape hatch @@ -295,7 +313,7 @@ export function createDefaultHookRegistry( }); } - return { registry, memoryStore: store }; + return { registry, memoryStore: store, addPreviewDiffRef }; } /** diff --git a/src/agent/hooks.ts b/src/agent/hooks.ts index 456ff2c8a..947641fcd 100644 --- a/src/agent/hooks.ts +++ b/src/agent/hooks.ts @@ -219,6 +219,12 @@ export interface PreToolUseContext { * their per-session closure state, preserving prior behavior. */ grantManager?: GrantManager; + /** + * The tool-call id from the model's request. Injected by + * {@link SessionToolDispatcher.runPreDispatchGates} so hooks can correlate + * the PreToolUse event with an in-flight tool-lane entry. + */ + toolUseId?: string; } export interface PostToolUseContext { diff --git a/src/agent/hooks/config-bridge.test.ts b/src/agent/hooks/config-bridge.test.ts index 3dcc8ca27..8eae63750 100644 --- a/src/agent/hooks/config-bridge.test.ts +++ b/src/agent/hooks/config-bridge.test.ts @@ -371,12 +371,13 @@ describe('createDefaultHookRegistry integration', () => { it('createDefaultHookRegistry without hookConfig → 0 config hooks registered', () => { const { registry } = createDefaultHookRegistry(); - // Built-in handlers exist for SubagentStop and SessionEnd, plus the THREE + // Built-in handlers exist for SubagentStop and SessionEnd, plus the FOUR // always-on built-in PreToolUse handlers (the ask-question gate, the - // observe-only safe-destruct detector, and the observe-only release-boundary - // detector), all registered unconditionally. No further PreToolUse hooks - // since we passed no hookConfig (path-approval disabled above). - expect(registry.count('PreToolUse')).toBe(3); + // observe-only safe-destruct detector, the observe-only release-boundary + // detector, and the edit-preview hook), all registered unconditionally. No + // further PreToolUse hooks since we passed no hookConfig (path-approval + // disabled above). + expect(registry.count('PreToolUse')).toBe(4); }); it('createDefaultHookRegistry with hookConfig → config hooks ARE registered', () => { @@ -401,9 +402,9 @@ describe('createDefaultHookRegistry integration', () => { hookConfig, { cwd: projectCwd }, ); - // 3 built-ins (ask-question gate + safe-destruct detector + release-boundary - // detector) + 1 config hook - expect(registry.count('PreToolUse')).toBe(4); + // 4 built-ins (ask-question gate + safe-destruct detector + release-boundary + // detector + edit-preview hook) + 1 config hook + expect(registry.count('PreToolUse')).toBe(5); }); it('built-in SubagentStop handler still present when hookConfig is provided', () => { diff --git a/src/agent/tools/dispatcher.ts b/src/agent/tools/dispatcher.ts index 9992fb86f..bfa1b3bc3 100644 --- a/src/agent/tools/dispatcher.ts +++ b/src/agent/tools/dispatcher.ts @@ -927,6 +927,9 @@ export class SessionToolDispatcher implements ToolDispatcher { ...(this.sessionGrantManager !== undefined ? { grantManager: this.sessionGrantManager } : {}), + // Inject the tool-call id so hooks (e.g. edit-preview-hook) can + // correlate this PreToolUse event with an in-flight tool-lane entry. + ...(call.id !== undefined ? { toolUseId: call.id } : {}), }; try { await dispatchPreToolUse(this.hookRegistry, preCtx, { diff --git a/src/agent/tools/hooks/edit-preview-hook.test.ts b/src/agent/tools/hooks/edit-preview-hook.test.ts new file mode 100644 index 000000000..0390e59fd --- /dev/null +++ b/src/agent/tools/hooks/edit-preview-hook.test.ts @@ -0,0 +1,74 @@ +/** + * Tests for createEditPreviewHook — pre-execution diff preview hook. + */ +import { describe, it, expect, vi } from 'vitest'; +import { createEditPreviewHook } from './edit-preview-hook.js'; +import type { DiffPayload } from '../../../utils/diff.js'; +import type { PreToolUseContext } from '../../hooks.js'; + +function makeCtx(overrides: Partial = {}): PreToolUseContext { + return { + event: 'PreToolUse', + toolName: 'edit_file', + input: { old_string: 'a', new_string: 'b', file_path: 'foo.ts' }, + toolUseId: 'tu_001', + ...overrides, + }; +} + +describe('createEditPreviewHook', () => { + it('(a) fires callback with DiffPayload for a normal edit', () => { + const cb = vi.fn(); + const ref = { current: cb }; + const hook = createEditPreviewHook({ addPreviewDiffRef: ref }); + const decision = hook(makeCtx()); + expect(cb).toHaveBeenCalledOnce(); + const [id, diff] = cb.mock.calls[0] as [string, DiffPayload]; + expect(id).toBe('tu_001'); + expect(diff.hunks.length).toBeGreaterThan(0); + expect(decision).toEqual({}); + }); + + it('(b) no-op when old_string === new_string', () => { + const cb = vi.fn(); + const hook = createEditPreviewHook({ addPreviewDiffRef: { current: cb } }); + hook(makeCtx({ input: { old_string: 'same', new_string: 'same', file_path: 'x.ts' } })); + expect(cb).not.toHaveBeenCalled(); + }); + + it('(c) skips subagents (parentSessionId set)', () => { + const cb = vi.fn(); + const hook = createEditPreviewHook({ addPreviewDiffRef: { current: cb } }); + hook(makeCtx({ parentSessionId: 'parent-sess' })); + expect(cb).not.toHaveBeenCalled(); + }); + + it('(d) skips non-edit_file tools', () => { + const cb = vi.fn(); + const hook = createEditPreviewHook({ addPreviewDiffRef: { current: cb } }); + hook(makeCtx({ toolName: 'bash' })); + expect(cb).not.toHaveBeenCalled(); + }); + + it('(e) skips non-PreToolUse events', () => { + const cb = vi.fn(); + const hook = createEditPreviewHook({ addPreviewDiffRef: { current: cb } }); + // Cast to satisfy type — hook must guard event at runtime + hook({ event: 'PostToolUse', toolName: 'edit_file' } as any); + expect(cb).not.toHaveBeenCalled(); + }); + + it('(f) returned decision has no injectContext', () => { + const cb = vi.fn(); + const hook = createEditPreviewHook({ addPreviewDiffRef: { current: cb } }); + const decision = hook(makeCtx()); + expect(decision).not.toHaveProperty('injectContext'); + }); + + it('(g) no-op when toolUseId absent', () => { + const cb = vi.fn(); + const hook = createEditPreviewHook({ addPreviewDiffRef: { current: cb } }); + hook(makeCtx({ toolUseId: undefined })); + expect(cb).not.toHaveBeenCalled(); + }); +}); diff --git a/src/agent/tools/hooks/edit-preview-hook.ts b/src/agent/tools/hooks/edit-preview-hook.ts new file mode 100644 index 000000000..6994120a0 --- /dev/null +++ b/src/agent/tools/hooks/edit-preview-hook.ts @@ -0,0 +1,67 @@ +/** + * Edit-preview hook: computes a diff preview at PreToolUse time for + * edit_file calls and communicates it to the tool lane via a callback ref. + * + * Non-blocking. Returns {} (passthrough) always — this is display-only. + * No I/O. Skips subagents (parentSessionId set) — they have no overlay. + * + * @module agent/tools/hooks/edit-preview-hook + */ + +import { computeLineDiff } from '../../../utils/diff.js'; +import type { HookContext, HookDecision, HookHandler } from '../../hooks.js'; +import type { DiffPayload } from '../../../utils/diff.js'; + +// --------------------------------------------------------------------------- +// Public types +// --------------------------------------------------------------------------- + +export interface EditPreviewHookOptions { + /** + * Mutable ref populated by the StreamRenderer when it arms. The hook + * calls `ref.current(toolUseId, diff)` to deliver the preview to the + * tool lane without importing any CLI-layer module. + * + * Remains a no-op `() => {}` until the StreamRenderer arms it each turn. + * On non-interactive surfaces (daemon, Telegram) it is never armed. + */ + addPreviewDiffRef: { current: (toolUseId: string, diff: DiffPayload) => void }; +} + +// --------------------------------------------------------------------------- +// Factory +// --------------------------------------------------------------------------- + +/** + * Create a `PreToolUse` hook that computes a pre-execution diff for + * `edit_file` calls and delivers it to the tool lane via the supplied ref. + * + * @param opts - {@link EditPreviewHookOptions} + * @returns A {@link HookHandler} to register on the `PreToolUse` event. + */ +export function createEditPreviewHook(opts: EditPreviewHookOptions): HookHandler { + return (context: HookContext): HookDecision => { + // Guard: only PreToolUse + if (context.event !== 'PreToolUse') return {}; + // Guard: only edit_file + if (context.toolName !== 'edit_file') return {}; + // Guard: subagents have no overlay — skip silently + if (context.parentSessionId !== undefined) return {}; + // Guard: need a toolUseId to key the lane entry + if (!context.toolUseId) return {}; + + const input = context.input as Record | undefined; + if (!input) return {}; + const oldStr = typeof input['old_string'] === 'string' ? input['old_string'] : undefined; + const newStr = typeof input['new_string'] === 'string' ? input['new_string'] : undefined; + if (oldStr === undefined || newStr === undefined) return {}; + // No-op edit: mirrors computeLineDiff null-on-identical behavior + if (oldStr === newStr) return {}; + + const diff = computeLineDiff(oldStr, newStr); + if (diff === null) return {}; + + opts.addPreviewDiffRef.current(context.toolUseId, diff); + return {}; + }; +} diff --git a/src/cli/_lib/stream-renderer-options.ts b/src/cli/_lib/stream-renderer-options.ts index d6f3a37f3..a5cf94b8a 100644 --- a/src/cli/_lib/stream-renderer-options.ts +++ b/src/cli/_lib/stream-renderer-options.ts @@ -10,6 +10,7 @@ import type { Writer } from '../slash/types.js'; import type { IHistoryRing } from '../input/types.js'; import type { AutocompleteState } from '../input/autocomplete-state.js'; import type { TerminalCompositor } from '../terminal-compositor.js'; +import type { DiffPayload } from '../../utils/diff.js'; export interface StreamRendererOptions { /** Where line-based output goes (non-TTY fallback + always-emitted compact lines). */ @@ -157,4 +158,15 @@ export interface StreamRendererOptions { * streaming content chunk arrives (see `notifyFirstContent`). */ turnStartedAt?: number; + /** + * Mutable ref the StreamRenderer arms each turn in {@link StreamRenderer.arm}. + * The edit-preview hook calls `addPreviewDiffRef.current(toolUseId, diff)` to + * deliver a pre-execution diff preview to the tool lane without importing any + * CLI-layer module. + * + * On non-interactive surfaces (daemon, Telegram, tests) this ref is never + * armed and remains a no-op. The REPL bootstrap threads it from + * `DefaultHookRegistryResult.addPreviewDiffRef`. + */ + addPreviewDiffRef?: { current: (toolUseId: string, diff: DiffPayload) => void }; } diff --git a/src/cli/_lib/stream-renderer.ts b/src/cli/_lib/stream-renderer.ts index 07eb9565b..78b3e5d23 100644 --- a/src/cli/_lib/stream-renderer.ts +++ b/src/cli/_lib/stream-renderer.ts @@ -177,6 +177,9 @@ export class StreamRenderer { /** Last annotation string rendered for the TTFB line — drives 1 Hz change detection. */ private lastTtfbAnnotation = ''; + /** Ref wired in arm() so the edit-preview hook can push diffs to the tool lane. */ + private readonly addPreviewDiffRef: { current: (toolUseId: string, diff: import('../../utils/diff.js').DiffPayload) => void } | undefined; + /** * Pre-bound sink — pass directly to `runWithSink(...)` from callers. * Equivalent to `(event, meta) => this.process(event, meta)`. @@ -243,6 +246,7 @@ export class StreamRenderer { } this.ttfbStartedAt = opts.turnStartedAt; this.ttfbDone = opts.turnStartedAt === undefined; + this.addPreviewDiffRef = opts.addPreviewDiffRef; this.sink = (event, meta) => this.process(event, meta); } @@ -317,6 +321,16 @@ export class StreamRenderer { } this.compositor = compositor; + // Wire the edit-preview ref so the hook can push diff previews into the + // tool lane during this turn. No-op when ref is absent (non-REPL surfaces). + if (this.addPreviewDiffRef) { + this.addPreviewDiffRef.current = (toolUseId, diff) => { + this.toolLane.addPreviewDiff(toolUseId, diff); + this.overlayComposer?.markDirty('tool-lane'); + this.overlayComposer?.flush(); + }; + } + // Construct the OverlayComposer with the five overlay slot types in z-order. // The slots read live state at flush time, so there's no initialization // needed beyond construction and registration. 'interrupt' is bottom-most diff --git a/src/cli/commands/interactive/bootstrap-hooks.ts b/src/cli/commands/interactive/bootstrap-hooks.ts index bb099b89b..3e22156e3 100644 --- a/src/cli/commands/interactive/bootstrap-hooks.ts +++ b/src/cli/commands/interactive/bootstrap-hooks.ts @@ -8,6 +8,7 @@ import type { CompletionWriter } from './shared.js'; import { emitSubagentCompletion } from './progress-banner.js'; import { createTerminalStateGate } from './terminal-state-gate.js'; import { loadConfig } from '../../config.js'; +import type { DiffPayload } from '../../../utils/diff.js'; /** * Build the stable hook registry shared across sessions (including @@ -16,9 +17,6 @@ import { loadConfig } from '../../config.js'; * * The hook callbacks close over `stats` and `completionWriter`, which are * also stable, so a rebuilt session gets the same routing without re-wiring. - * `pathApprovalGrantRef` is populated by the caller once the provider - * exists — the path-approval hook fails open until then (mirroring - * `setAllowDirDispatcher` wiring order). * * The terminal-state gate (issue #237) is a post-turn `Stop` hook that * bounces a self-certified `Done` with no corroborating evidence back into @@ -36,7 +34,10 @@ export function createReplHookRegistry(a: { stats: SessionStats; effectiveCwd: string | undefined; traceWriter: TraceSink | undefined; -}): { hookRegistry: HookRegistry } { +}): { + hookRegistry: HookRegistry; + addPreviewDiffRef: { current: (toolUseId: string, diff: DiffPayload) => void }; +} { const hookRegistryBundle = createDefaultHookRegistry( (info) => { emitSubagentCompletion(a.completionWriter, info); }, 'cli', @@ -47,6 +48,7 @@ export function createReplHookRegistry(a: { () => a.effectiveCwd ?? process.cwd(), ); const hookRegistry = hookRegistryBundle.registry; + const addPreviewDiffRef = hookRegistryBundle.addPreviewDiffRef; hookRegistry.register( 'Stop', @@ -56,5 +58,5 @@ export function createReplHookRegistry(a: { }), ); - return { hookRegistry }; + return { hookRegistry, addPreviewDiffRef }; } diff --git a/src/cli/commands/interactive/bootstrap.ts b/src/cli/commands/interactive/bootstrap.ts index 2e31c27a9..e9c337b86 100644 --- a/src/cli/commands/interactive/bootstrap.ts +++ b/src/cli/commands/interactive/bootstrap.ts @@ -123,7 +123,7 @@ export async function bootstrapSession( // Stable hookRegistry shared across sessions (including swaps), plus the // terminal-state Stop gate registered on top of it. - const { hookRegistry } = createReplHookRegistry({ + const { hookRegistry, addPreviewDiffRef } = createReplHookRegistry({ completionWriter, memoryStore: sharedMemoryStore, stats, effectiveCwd, traceWriter: trace?.writer, }); @@ -342,6 +342,7 @@ export async function bootstrapSession( ? { suggestGhostConfig: cliConfig.interactive.suggestGhost } : {}), hookRegistry, + addPreviewDiffRef, }; // Trusted-skill event subscriptions — emit in-flight + completion badges diff --git a/src/cli/commands/interactive/loop-iteration.ts b/src/cli/commands/interactive/loop-iteration.ts index a88aa216d..35af0e881 100644 --- a/src/cli/commands/interactive/loop-iteration.ts +++ b/src/cli/commands/interactive/loop-iteration.ts @@ -809,6 +809,9 @@ export async function runInputLoop( }, } : {}), + // Thread the edit-preview ref so the StreamRenderer can wire the hook + // callback into the tool lane during arm(). Absent on non-REPL callers. + ...(ctx.addPreviewDiffRef ? { addPreviewDiffRef: ctx.addPreviewDiffRef } : {}), }, ctx.stats.thinkingUi ?? ctx.options.thinkingUi, ctx.completionWriter, // Surface refs threaded into the per-turn StreamRenderer for the // legacy non-borrow path (non-TTY, when surface.getCompositor() diff --git a/src/cli/commands/interactive/shared.ts b/src/cli/commands/interactive/shared.ts index 2a0e49c43..dc0e66110 100644 --- a/src/cli/commands/interactive/shared.ts +++ b/src/cli/commands/interactive/shared.ts @@ -526,6 +526,12 @@ export interface InteractiveCtx { * call (enabling per-prompt policy hooks) and Stop after each completed turn. */ hookRegistry?: HookRegistry; + /** + * Mutable ref threaded from `createReplHookRegistry` into the per-turn + * StreamRenderer so the edit-preview hook can deliver diff previews to the + * tool lane without importing CLI-layer modules. Absent on non-REPL callers. + */ + addPreviewDiffRef?: { current: (toolUseId: string, diff: import('../../../utils/diff.js').DiffPayload) => void }; } /** @@ -760,6 +766,12 @@ export interface TurnHandles { stage: import('./loop-stage.js').LoopStage, signals?: import('./loop-stage.js').StageSignals, ): void; + /** + * Mutable ref threaded from `createReplHookRegistry` into the per-turn + * StreamRenderer so the edit-preview hook can deliver diff previews to the + * tool lane without importing CLI-layer modules. Absent on non-REPL callers. + */ + addPreviewDiffRef?: { current: (toolUseId: string, diff: import('../../../utils/diff.js').DiffPayload) => void }; } // `discardStdin: false` is load-bearing — ora's default wraps process.stdin diff --git a/src/cli/commands/interactive/tool-lane-render.ts b/src/cli/commands/interactive/tool-lane-render.ts index 253382e07..4165fe31e 100644 --- a/src/cli/commands/interactive/tool-lane-render.ts +++ b/src/cli/commands/interactive/tool-lane-render.ts @@ -90,6 +90,14 @@ interface ToolEntryFields { * (truncated to 8 lines) and flush (full diff) paths. */ diff?: DiffPayload; + /** + * Optional pre-execution diff preview attached by the edit-preview hook + * at PreToolUse time. Rendered in the live overlay while the call is + * in-flight (guarded by `!entry.result`). Never appears in scrollback — + * once the tool_result arrives, the post-execution diff (via the existing + * `tool_diff` sidecar path) takes over in the flush path. + */ + previewDiff?: DiffPayload; /** * Set to `true` by {@link ToolLane.flushSource} the first time it emits * this entry's header line eagerly (before the entry's own done-event). diff --git a/src/cli/commands/interactive/tool-lane.test.ts b/src/cli/commands/interactive/tool-lane.test.ts index b9e38df56..611d17b05 100644 --- a/src/cli/commands/interactive/tool-lane.test.ts +++ b/src/cli/commands/interactive/tool-lane.test.ts @@ -16,6 +16,7 @@ import { displayWidth, stripAnsi } from '../../display.js'; import { palette } from '../../palette.js'; import type { ToolResultChunk } from '../../../agent/types/message-types.js'; import type { OutputEvent, SubagentProgressMeta } from '../../../agent/types.js'; +import type { DiffPayload } from '../../../utils/diff.js'; function makeResult(content: string, isError = false): ToolResultChunk { return { @@ -3034,3 +3035,59 @@ describe('ToolLane.peekTrailingCompletedRootToolName — cross-flush run gate', expect(lane.peekTrailingCompletedRootToolName()).toBe('Read'); }); }); + +describe('ToolLane.addPreviewDiff — pre-execution diff preview', () => { + const sampleDiff: DiffPayload = { + addedLines: 1, removedLines: 1, + hunks: [{ + oldStart: 1, oldLines: 1, newStart: 1, newLines: 1, + lines: [{ kind: '-', text: 'before' }, { kind: '+', text: 'after' }], + }], + }; + + it('(a) preview renders in overlay while in-flight (no result yet)', () => { + const lane = new ToolLane(); + lane.addStart('tu_pre', 'edit_file', '(foo.ts)'); + lane.addPreviewDiff('tu_pre', sampleDiff); + const overlay = stripAnsi(lane.getOverlay()); + expect(overlay).toContain('before'); + expect(overlay).toContain('after'); + }); + + it('(b) preview absent from overlay after result arrives', () => { + const lane = new ToolLane(); + lane.addStart('tu_pre', 'edit_file', '(foo.ts)'); + lane.addPreviewDiff('tu_pre', sampleDiff); + lane.addResult('tu_pre', makeResult('Replaced 1 occurrence')); + const overlay = stripAnsi(lane.getOverlay()); + // result branch: shows outcome line, not preview diff + expect(overlay).not.toContain('@@ -1,1 +1,1 @@'); + }); + + it('(c) no-op for unknown toolUseId', () => { + const lane = new ToolLane(); + expect(() => lane.addPreviewDiff('ghost_id', sampleDiff)).not.toThrow(); + }); + + it('(d) AFK_SHOW_DIFFS=0 suppresses preview in overlay', () => { + process.env['AFK_SHOW_DIFFS'] = '0'; + try { + const lane = new ToolLane(); + lane.addStart('tu_env', 'edit_file', '(x.ts)'); + lane.addPreviewDiff('tu_env', sampleDiff); + const overlay = stripAnsi(lane.getOverlay()); + expect(overlay).not.toContain('@@ -1,1 +1,1 @@'); + } finally { + delete process.env['AFK_SHOW_DIFFS']; + } + }); + + it('(e) preview does not appear in flush/scrollback (overlay-only)', () => { + const lane = new ToolLane(); + lane.addStart('tu_fl', 'edit_file', '(bar.ts)'); + lane.addPreviewDiff('tu_fl', sampleDiff); + // No addResult — entry stays in-flight; flush should not emit preview + const flushed = stripAnsi(lane.flush().join('\n')); + expect(flushed).not.toContain('@@ -1,1 +1,1 @@'); + }); +}); diff --git a/src/cli/commands/interactive/tool-lane.ts b/src/cli/commands/interactive/tool-lane.ts index 9422062c5..4e330b320 100644 --- a/src/cli/commands/interactive/tool-lane.ts +++ b/src/cli/commands/interactive/tool-lane.ts @@ -196,6 +196,21 @@ export class ToolLane { if (entry?.kind === 'tool') entry.diff = diff; } + /** + * Attach a pre-execution diff preview to an in-flight tool entry. Called by + * the edit-preview hook at PreToolUse time, before the edit executes. + * No-op if the entry doesn't exist or is not a tool entry. + * + * The preview is rendered in the live overlay only (while `!entry.result`). + * It does NOT appear in scrollback (flush). Once the tool_result arrives + * and the post-execution diff sidechannel takes over, the preview is no + * longer shown. + */ + addPreviewDiff(toolUseId: string, diff: DiffPayload): void { + const entry = this.entries.get(toolUseId); + if (entry?.kind === 'tool') entry.previewDiff = diff; + } + /** * Create or replace the `text` of a {@link TextEntry} child. Used by the * streaming renderer to drive a subagent's live narration under its @@ -559,6 +574,14 @@ export class ToolLane { // Live elapsed counter: same pattern as the NESTING branch above — // computed at repaint time, suppressed under ELAPSED_GRACE_MS (2s). lines.push(clamp(flatRootLead + entry.prefix + palette.dim(' …') + formatElapsed(entry.startedAt))); + if (entry.previewDiff) { + // Pre-execution diff preview: rendered under the in-flight " …" line, + // indented to visually attach to this entry. formatDiffBlock already + // returns [] when AFK_SHOW_DIFFS=0, so no extra guard needed. + for (const line of formatDiffBlock(entry.previewDiff, 'overlay', ' ')) { + lines.push(clamp(line)); + } + } if (entry.thinkingTail) { // Childless Agent entries (a child just opened its thinking block // and hasn't yet emitted content or a tool_use) get the tail right diff --git a/src/cli/commands/interactive/turn-handler.ts b/src/cli/commands/interactive/turn-handler.ts index 7dd24d9ef..177513fc0 100644 --- a/src/cli/commands/interactive/turn-handler.ts +++ b/src/cli/commands/interactive/turn-handler.ts @@ -209,6 +209,7 @@ export async function runTurn( // before the first streaming content token arrives. The renderer clears // the line automatically when notifyFirstContent() is called below. ...ttfbRendererOptions(turnTtfb), + ...(h.addPreviewDiffRef ? { addPreviewDiffRef: h.addPreviewDiffRef } : {}), }); // `let` (not `const`) so the resumed-event handler can swap in a fresh diff --git a/src/cli/render/index.ts b/src/cli/render/index.ts index 34475b611..14e20143a 100644 --- a/src/cli/render/index.ts +++ b/src/cli/render/index.ts @@ -24,3 +24,4 @@ export * from './utils.js'; export * from './tool-card.js'; export * from './interrupt-peek.js'; export * from './compact-diff-view.js'; +export * from './preview-diff.js'; diff --git a/src/cli/render/preview-diff.test.ts b/src/cli/render/preview-diff.test.ts new file mode 100644 index 000000000..9b90c16c1 --- /dev/null +++ b/src/cli/render/preview-diff.test.ts @@ -0,0 +1,54 @@ +/** + * Tests for previewDiff — pre-execution diff preview render component. + */ +import { describe, it, expect } from 'vitest'; +import { stripAnsi } from '../display.js'; +import { previewDiff } from './preview-diff.js'; +import type { DiffPayload } from '../../utils/diff.js'; + +const samplePayload: DiffPayload = { + addedLines: 1, + removedLines: 1, + hunks: [{ + oldStart: 1, oldLines: 1, newStart: 1, newLines: 1, + lines: [{ kind: '-', text: 'old line' }, { kind: '+', text: 'new line' }], + }], +}; + +describe('previewDiff', () => { + it('(a) contains ⟳ Proposed label', () => { + const out = previewDiff(samplePayload); + expect(out).toContain('⟳ Proposed'); + }); + + it('(b) contains hunk content (after strip-ansi)', () => { + const out = stripAnsi(previewDiff(samplePayload)); + expect(out).toContain('old line'); + expect(out).toContain('new line'); + }); + + it('(c) maxLines propagation truncates long diffs', () => { + const manyLines: DiffPayload = { + addedLines: 5, + removedLines: 0, + hunks: [{ + oldStart: 1, oldLines: 0, newStart: 1, newLines: 5, + lines: Array.from({ length: 5 }, (_, i) => ({ kind: '+' as const, text: `line ${i}` })), + }], + }; + const out = stripAnsi(previewDiff(manyLines, { maxLines: 2 })); + expect(out).toContain('⟳ Proposed'); + // Only 2 body lines rendered; the rest elided + const bodyLines = out.split('\n').filter(l => l.includes('+ line')); + expect(bodyLines.length).toBeLessThanOrEqual(2); + }); + + it('(d) accepts optional filePath in stat header', () => { + const out = stripAnsi(previewDiff(samplePayload, { filePath: 'src/foo.ts' })); + expect(out).toContain('src/foo.ts'); + }); + + it('returns a string (type check)', () => { + expect(typeof previewDiff(samplePayload)).toBe('string'); + }); +}); diff --git a/src/cli/render/preview-diff.ts b/src/cli/render/preview-diff.ts new file mode 100644 index 000000000..6cdcb48fe --- /dev/null +++ b/src/cli/render/preview-diff.ts @@ -0,0 +1,66 @@ +/** + * PreviewDiff — pre-execution diff preview render component. + * + * A pure `(payload, opts?) => string` component that renders a diff preview + * for display in the live overlay before an `edit_file` write occurs. The + * `⟳ Proposed` label distinguishes it from the post-execution diff that + * arrives via the `tool_diff` sidecar channel. + * + * Delegates all diff layout to {@link compactDiffView} so truncation, + * box-framing, and line-colouring are handled identically. + * + * LOC ceiling: 350 (enforced by audit:filesize:check). + */ + +import { palette } from '../palette.js'; +import { compactDiffView } from './compact-diff-view.js'; +import type { DiffPayload } from '../../utils/diff.js'; + +// --------------------------------------------------------------------------- +// Public types +// --------------------------------------------------------------------------- + +export interface PreviewDiffOptions { + /** + * Optional file path shown in the stat header of the compact diff view. + * When omitted a generic `'preview'` placeholder is used — the tool-lane + * entry's own prefix line already names the file, so this is supplemental. + */ + filePath?: string; + /** + * Maximum number of diff body lines to render before truncating. + * Passed through to {@link compactDiffView}. Default: 20. + */ + maxLines?: number; + /** + * Terminal width passed through to {@link compactDiffView} for box sizing. + * Default: 80. + */ + width?: number; +} + +// --------------------------------------------------------------------------- +// Public API +// --------------------------------------------------------------------------- + +/** + * Render a pre-execution diff preview as a multi-line string for overlay display. + * + * @param payload - Structured diff produced by {@link computeLineDiff}. + * @param opts - Optional rendering overrides (filePath, maxLines, width). + * @returns A styled, multi-line string beginning with `⟳ Proposed`. + * Ends without a trailing newline. + */ +export function previewDiff(payload: DiffPayload, opts: PreviewDiffOptions = {}): string { + const spec = { + filePath: opts.filePath ?? 'preview', + hunks: payload.hunks.map((h) => ({ + header: `@@ -${h.oldStart},${h.oldLines} +${h.newStart},${h.newLines} @@`, + lines: h.lines.map((l) => `${l.kind === ' ' ? ' ' : l.kind} ${l.text}`), + })), + stats: { added: payload.addedLines, removed: payload.removedLines }, + ...(opts.maxLines !== undefined ? { maxLines: opts.maxLines } : {}), + ...(opts.width !== undefined ? { width: opts.width } : {}), + }; + return palette.dim('⟳ Proposed') + '\n' + compactDiffView(spec); +} From b5c7f5c2bb451ca7910467e2712799086866908d Mon Sep 17 00:00:00 2001 From: Griffin Long Date: Fri, 28 Aug 2026 21:02:40 -0400 Subject: [PATCH 2/2] fix(pr-1366): address review feedback on edit-preview --- src/agent/default-hook-registry.ts | 7 +++---- src/agent/hooks/config-bridge.test.ts | 6 +++++- src/agent/tools/hooks/edit-preview-hook.ts | 19 ++++++++++++++++--- src/cli/_lib/stream-renderer-options.ts | 4 ++-- src/cli/_lib/stream-renderer.ts | 6 +++++- .../commands/interactive/bootstrap-hooks.ts | 4 ++-- src/cli/commands/interactive/shared.ts | 5 +++-- .../interactive/tool-lane-format-diff.ts | 13 ++++++++++++- .../commands/interactive/tool-lane-format.ts | 2 ++ .../interactive/tool-lane-render-children.ts | 9 +++++++++ .../commands/interactive/tool-lane.test.ts | 2 ++ src/cli/commands/interactive/tool-lane.ts | 12 ++++++------ 12 files changed, 67 insertions(+), 22 deletions(-) diff --git a/src/agent/default-hook-registry.ts b/src/agent/default-hook-registry.ts index a23476372..2504420be 100644 --- a/src/agent/default-hook-registry.ts +++ b/src/agent/default-hook-registry.ts @@ -24,8 +24,7 @@ import { type PathApprovalSurface, } from './tools/hooks/path-approval-hook.js'; import { createBashRestrictionHook } from './tools/hooks/bash-restriction-hook.js'; -import { createEditPreviewHook } from './tools/hooks/edit-preview-hook.js'; -import type { DiffPayload } from '../utils/diff.js'; +import { createEditPreviewHook, type PreviewDiffRef } from './tools/hooks/edit-preview-hook.js'; import type { PermissionMode } from './types/sdk-types.js'; import type { TraceSink } from './trace/index.js'; import type { LoadedHooksConfig } from './hooks/config-loader.js'; @@ -47,7 +46,7 @@ export interface DefaultHookRegistryResult { * to the tool lane. Remains a no-op `() => {}` on non-interactive surfaces * (the ref is never populated outside the REPL bootstrap). */ - addPreviewDiffRef: { current: (toolUseId: string, diff: DiffPayload) => void }; + addPreviewDiffRef: PreviewDiffRef; } /** @@ -164,7 +163,7 @@ export function createDefaultHookRegistry( // edit_file calls and delivers it to the tool lane via a mutable callback // ref. Non-blocking, display-only. The ref starts as a no-op; the // StreamRenderer arms it each turn on TTY surfaces. - const addPreviewDiffRef: { current: (toolUseId: string, diff: DiffPayload) => void } = { + const addPreviewDiffRef: PreviewDiffRef = { current: () => {}, // no-op until StreamRenderer arms it each turn }; registry.register('PreToolUse', createEditPreviewHook({ addPreviewDiffRef })); diff --git a/src/agent/hooks/config-bridge.test.ts b/src/agent/hooks/config-bridge.test.ts index 8eae63750..112e2b6e1 100644 --- a/src/agent/hooks/config-bridge.test.ts +++ b/src/agent/hooks/config-bridge.test.ts @@ -370,7 +370,8 @@ describe('createDefaultHookRegistry integration', () => { }); it('createDefaultHookRegistry without hookConfig → 0 config hooks registered', () => { - const { registry } = createDefaultHookRegistry(); + const bundle = createDefaultHookRegistry(); + const { registry } = bundle; // Built-in handlers exist for SubagentStop and SessionEnd, plus the FOUR // always-on built-in PreToolUse handlers (the ask-question gate, the // observe-only safe-destruct detector, the observe-only release-boundary @@ -378,6 +379,9 @@ describe('createDefaultHookRegistry integration', () => { // further PreToolUse hooks since we passed no hookConfig (path-approval // disabled above). expect(registry.count('PreToolUse')).toBe(4); + // addPreviewDiffRef must be present on the bundle so the StreamRenderer can + // arm it each turn. + expect(bundle).toHaveProperty('addPreviewDiffRef'); }); it('createDefaultHookRegistry with hookConfig → config hooks ARE registered', () => { diff --git a/src/agent/tools/hooks/edit-preview-hook.ts b/src/agent/tools/hooks/edit-preview-hook.ts index 6994120a0..231abe3c8 100644 --- a/src/agent/tools/hooks/edit-preview-hook.ts +++ b/src/agent/tools/hooks/edit-preview-hook.ts @@ -16,6 +16,9 @@ import type { DiffPayload } from '../../../utils/diff.js'; // Public types // --------------------------------------------------------------------------- +/** Mutable ref type for delivering preview diffs from the hook to the tool lane. */ +export type PreviewDiffRef = { current: (toolUseId: string, diff: DiffPayload) => void }; + export interface EditPreviewHookOptions { /** * Mutable ref populated by the StreamRenderer when it arms. The hook @@ -25,7 +28,7 @@ export interface EditPreviewHookOptions { * Remains a no-op `() => {}` until the StreamRenderer arms it each turn. * On non-interactive surfaces (daemon, Telegram) it is never armed. */ - addPreviewDiffRef: { current: (toolUseId: string, diff: DiffPayload) => void }; + addPreviewDiffRef: PreviewDiffRef; } // --------------------------------------------------------------------------- @@ -58,8 +61,18 @@ export function createEditPreviewHook(opts: EditPreviewHookOptions): HookHandler // No-op edit: mirrors computeLineDiff null-on-identical behavior if (oldStr === newStr) return {}; - const diff = computeLineDiff(oldStr, newStr); - if (diff === null) return {}; + // replace_all preview would require reading the file to know how many + // occurrences exist and where they are — not available to this hook (no I/O + // by design). Skip the preview entirely to avoid a misleading single-hunk + // diff that doesn't reflect the full multi-occurrence replacement. + if (input['replace_all']) return {}; + + // TODO(#1366): preview may diverge from final input when canUseTool returns + // updatedInput — requires hook re-fire or post-rewrite preview refresh. + + // computeLineDiff returns non-null for distinct strings; the null case + // (identical input) is handled by the oldStr === newStr guard above. + const diff = computeLineDiff(oldStr, newStr)!; opts.addPreviewDiffRef.current(context.toolUseId, diff); return {}; diff --git a/src/cli/_lib/stream-renderer-options.ts b/src/cli/_lib/stream-renderer-options.ts index a5cf94b8a..b33762f17 100644 --- a/src/cli/_lib/stream-renderer-options.ts +++ b/src/cli/_lib/stream-renderer-options.ts @@ -10,7 +10,7 @@ import type { Writer } from '../slash/types.js'; import type { IHistoryRing } from '../input/types.js'; import type { AutocompleteState } from '../input/autocomplete-state.js'; import type { TerminalCompositor } from '../terminal-compositor.js'; -import type { DiffPayload } from '../../utils/diff.js'; +import type { PreviewDiffRef } from '../../agent/tools/hooks/edit-preview-hook.js'; export interface StreamRendererOptions { /** Where line-based output goes (non-TTY fallback + always-emitted compact lines). */ @@ -168,5 +168,5 @@ export interface StreamRendererOptions { * armed and remains a no-op. The REPL bootstrap threads it from * `DefaultHookRegistryResult.addPreviewDiffRef`. */ - addPreviewDiffRef?: { current: (toolUseId: string, diff: DiffPayload) => void }; + addPreviewDiffRef?: PreviewDiffRef; } diff --git a/src/cli/_lib/stream-renderer.ts b/src/cli/_lib/stream-renderer.ts index 78b3e5d23..3beef9c5c 100644 --- a/src/cli/_lib/stream-renderer.ts +++ b/src/cli/_lib/stream-renderer.ts @@ -27,6 +27,7 @@ */ import type { OutputEvent, SubagentProgressMeta, ProgressEvent } from '../../agent/types.js'; +import type { PreviewDiffRef } from '../../agent/tools/hooks/edit-preview-hook.js'; import type { Message } from '../../agent/types/message-types.js'; import type { Writer } from '../slash/types.js'; import { TerminalCompositor } from '../terminal-compositor.js'; @@ -178,7 +179,7 @@ export class StreamRenderer { private lastTtfbAnnotation = ''; /** Ref wired in arm() so the edit-preview hook can push diffs to the tool lane. */ - private readonly addPreviewDiffRef: { current: (toolUseId: string, diff: import('../../utils/diff.js').DiffPayload) => void } | undefined; + private readonly addPreviewDiffRef: PreviewDiffRef | undefined; /** * Pre-bound sink — pass directly to `runWithSink(...)` from callers. @@ -543,6 +544,9 @@ export class StreamRenderer { async dispose(): Promise { if (this.disposed) return; this.disposed = true; + // Reset the preview-diff ref to a no-op so the disposed turn's toolLane + // reference is released and the hook cannot write into a stale lane. + if (this.addPreviewDiffRef) this.addPreviewDiffRef.current = () => {}; // Clear the subagent elapsed-time ticker immediately — it guards against // `this.disposed` but clearing here is cleaner and avoids one extra tick. if (this.subagentTickInterval !== null) { diff --git a/src/cli/commands/interactive/bootstrap-hooks.ts b/src/cli/commands/interactive/bootstrap-hooks.ts index 3e22156e3..a1110606d 100644 --- a/src/cli/commands/interactive/bootstrap-hooks.ts +++ b/src/cli/commands/interactive/bootstrap-hooks.ts @@ -8,7 +8,7 @@ import type { CompletionWriter } from './shared.js'; import { emitSubagentCompletion } from './progress-banner.js'; import { createTerminalStateGate } from './terminal-state-gate.js'; import { loadConfig } from '../../config.js'; -import type { DiffPayload } from '../../../utils/diff.js'; +import type { PreviewDiffRef } from '../../../agent/tools/hooks/edit-preview-hook.js'; /** * Build the stable hook registry shared across sessions (including @@ -36,7 +36,7 @@ export function createReplHookRegistry(a: { traceWriter: TraceSink | undefined; }): { hookRegistry: HookRegistry; - addPreviewDiffRef: { current: (toolUseId: string, diff: DiffPayload) => void }; + addPreviewDiffRef: PreviewDiffRef; } { const hookRegistryBundle = createDefaultHookRegistry( (info) => { emitSubagentCompletion(a.completionWriter, info); }, diff --git a/src/cli/commands/interactive/shared.ts b/src/cli/commands/interactive/shared.ts index dc0e66110..2dc83e238 100644 --- a/src/cli/commands/interactive/shared.ts +++ b/src/cli/commands/interactive/shared.ts @@ -2,6 +2,7 @@ import * as readline from 'node:readline'; import { statSync } from 'node:fs'; import { getQuotaSnapshot } from '../../../agent/quota-cache.js'; import type { HookRegistry } from '../../../agent/hooks.js'; +import type { PreviewDiffRef } from '../../../agent/tools/hooks/edit-preview-hook.js'; import type { SessionRef } from '../../../agent/session-ref.js'; import type { MemoryStore } from '../../../agent/memory/index.js'; import type { AgentModelInput } from '../../../agent/types.js'; @@ -531,7 +532,7 @@ export interface InteractiveCtx { * StreamRenderer so the edit-preview hook can deliver diff previews to the * tool lane without importing CLI-layer modules. Absent on non-REPL callers. */ - addPreviewDiffRef?: { current: (toolUseId: string, diff: import('../../../utils/diff.js').DiffPayload) => void }; + addPreviewDiffRef?: PreviewDiffRef; } /** @@ -771,7 +772,7 @@ export interface TurnHandles { * StreamRenderer so the edit-preview hook can deliver diff previews to the * tool lane without importing CLI-layer modules. Absent on non-REPL callers. */ - addPreviewDiffRef?: { current: (toolUseId: string, diff: import('../../../utils/diff.js').DiffPayload) => void }; + addPreviewDiffRef?: PreviewDiffRef; } // `discardStdin: false` is load-bearing — ora's default wraps process.stdin diff --git a/src/cli/commands/interactive/tool-lane-format-diff.ts b/src/cli/commands/interactive/tool-lane-format-diff.ts index 77dc31066..4bc7d273e 100644 --- a/src/cli/commands/interactive/tool-lane-format-diff.ts +++ b/src/cli/commands/interactive/tool-lane-format-diff.ts @@ -2,6 +2,7 @@ import { env } from '../../../config/env.js'; import type { DiffPayload, DiffLine } from '../../../utils/diff.js'; import { palette } from '../../palette.js'; import { stripAnsi } from '../../display.js'; +import { previewDiff } from '../../render/preview-diff.js'; /** * Maximum number of diff body lines to render in the live overlay before @@ -62,7 +63,7 @@ function diffFlushMaxLines(): number { * — this only suppresses the render. That keeps the JSON-output / Telegram * surfaces unaffected if they choose to surface diffs independently. */ -function diffsDisabled(): boolean { +export function diffsDisabled(): boolean { const raw = env.AFK_SHOW_DIFFS; if (raw === undefined) return false; const v = raw.trim().toLowerCase(); @@ -277,3 +278,13 @@ function _diffBlockCacheStore(diff: DiffPayload, key: string, result: string[]): inner.set(key, result); } + +/** + * Render a pre-execution diff preview into an array of indented lines for the + * live overlay. Returns `[]` when `AFK_SHOW_DIFFS=0` (same gate as + * {@link formatDiffBlock}). Each returned line already includes `indent`. + */ +export function formatPreviewDiffBlock(diff: DiffPayload, indent: string): string[] { + if (diffsDisabled()) return []; + return previewDiff(diff).split('\n').map((l) => indent + l); +} diff --git a/src/cli/commands/interactive/tool-lane-format.ts b/src/cli/commands/interactive/tool-lane-format.ts index 138f3361c..ac8d5f097 100644 --- a/src/cli/commands/interactive/tool-lane-format.ts +++ b/src/cli/commands/interactive/tool-lane-format.ts @@ -23,6 +23,8 @@ export { MAX_OVERLAY_DIFF_LINES, FLUSH_DIFF_LINES_DEFAULT, formatDiffBlock, + diffsDisabled, + formatPreviewDiffBlock, } from './tool-lane-format-diff.js'; /** diff --git a/src/cli/commands/interactive/tool-lane-render-children.ts b/src/cli/commands/interactive/tool-lane-render-children.ts index 71b716775..e8d19ed02 100644 --- a/src/cli/commands/interactive/tool-lane-render-children.ts +++ b/src/cli/commands/interactive/tool-lane-render-children.ts @@ -6,6 +6,7 @@ import { inProgressVerb, formatOutcome, formatDiffBlock, + formatPreviewDiffBlock, doneGlyph, sanitizeLabel, } from './tool-lane-format.js'; @@ -286,6 +287,14 @@ function renderOverlayChildren( // connector + tool name). Computed at repaint time from child.startedAt; // suppressed under ELAPSED_GRACE_MS (2s) to avoid flicker on fast tools. lines.push(clampLineToTerminal(indentColored + connector + child.prefix + palette.dim(' …') + formatElapsed(child.startedAt), cols)); + if (child.previewDiff) { + // Pre-execution diff preview for nested edit_file. formatPreviewDiffBlock + // renders ⟳ Proposed and applies the AFK_SHOW_DIFFS=0 opt-out. + const previewIndent = indentColored + (isLast ? g.spineClosed : palette.dim(g.spine)) + ' '; + for (const line of formatPreviewDiffBlock(child.previewDiff, previewIndent)) { + lines.push(clampLineToTerminal(line, cols)); + } + } // In-progress / thinking continuation hangs under the prefix at the // same "past-connector" column the diff path uses. const continuationIndent = indentColored + (isLast ? g.spineClosed : palette.dim(g.spine)) + ' '; diff --git a/src/cli/commands/interactive/tool-lane.test.ts b/src/cli/commands/interactive/tool-lane.test.ts index 611d17b05..58f394cc0 100644 --- a/src/cli/commands/interactive/tool-lane.test.ts +++ b/src/cli/commands/interactive/tool-lane.test.ts @@ -3062,6 +3062,8 @@ describe('ToolLane.addPreviewDiff — pre-execution diff preview', () => { const overlay = stripAnsi(lane.getOverlay()); // result branch: shows outcome line, not preview diff expect(overlay).not.toContain('@@ -1,1 +1,1 @@'); + // positive: outcome line is present + expect(overlay).toContain('Replaced 1 occurrence'); }); it('(c) no-op for unknown toolUseId', () => { diff --git a/src/cli/commands/interactive/tool-lane.ts b/src/cli/commands/interactive/tool-lane.ts index 4e330b320..b171ba62e 100644 --- a/src/cli/commands/interactive/tool-lane.ts +++ b/src/cli/commands/interactive/tool-lane.ts @@ -1,7 +1,7 @@ import type { ToolResultChunk } from '../../../agent/types/message-types.js'; import { palette } from '../../palette.js'; import { SUBAGENT_TOOLS, NESTING_TOOLS, SKILL_TOOLS } from '../../tool-category.js'; -import { formatToolLine, formatToolResultLine, formatOutcome, formatDiffBlock, doneGlyph, sanitizeLabel, batchBadge } from './tool-lane-format.js'; +import { formatToolLine, formatToolResultLine, formatOutcome, formatDiffBlock, formatPreviewDiffBlock, doneGlyph, sanitizeLabel, batchBadge } from './tool-lane-format.js'; import type { DiffPayload } from '../../../utils/diff.js'; import { truncateDisplayWidth, stripAnsi } from '../../display.js'; import { formatElapsed, ELAPSED_GRACE_MS } from '../../terminal-compositor.scrollback.js'; @@ -178,7 +178,8 @@ export class ToolLane { addResult(toolUseId: string, chunk: ToolResultChunk): void { const entry = this.entries.get(toolUseId); - if (entry?.kind === 'tool') entry.result = chunk; + // Clear previewDiff on the same tick as result — preview is now obsolete. + if (entry?.kind === 'tool') { entry.result = chunk; entry.previewDiff = undefined; } if (this.agentIdStack.at(-1) === toolUseId) { this.agentIdStack.pop(); } @@ -575,10 +576,9 @@ export class ToolLane { // computed at repaint time, suppressed under ELAPSED_GRACE_MS (2s). lines.push(clamp(flatRootLead + entry.prefix + palette.dim(' …') + formatElapsed(entry.startedAt))); if (entry.previewDiff) { - // Pre-execution diff preview: rendered under the in-flight " …" line, - // indented to visually attach to this entry. formatDiffBlock already - // returns [] when AFK_SHOW_DIFFS=0, so no extra guard needed. - for (const line of formatDiffBlock(entry.previewDiff, 'overlay', ' ')) { + // Pre-execution diff preview: formatPreviewDiffBlock renders ⟳ Proposed + // and applies the AFK_SHOW_DIFFS=0 opt-out (returns [] when disabled). + for (const line of formatPreviewDiffBlock(entry.previewDiff, ' ')) { lines.push(clamp(line)); } }