kai/web-terminal
1
0
Fork 0
Code Issues Pull Requests Actions 1 Packages Projects Releases Wiki Activity
Files
3ad8d8bebae68e11d5d2970809b7e1c6bf7a6911
web-terminal/docs/plans/w1-approval-preview.md

16 KiB
Raw Blame History

Approval preview — command/diff on the approval bar

Summary & grounding

Today /hook/permission (src/server.ts:423) reads the hook body, extracts only tool_name (server.ts:430), derives a gate (server.ts:454), parks the held res in pendingApprovals (server.ts:464), and broadcasts a bare waiting status via manager.handleHookEvent(sessionId, 'waiting', tool, true, gate) (server.ts:471). The rich tool_input — which parseHookEvent already knows how to pass through verbatim (src/http/hook.ts:104-105) — is dropped on this route. The status ServerMessage (src/types.ts:113-119) has no field to carry a preview, so the approval bar in public/tabs.ts:360-377 can only say "Claude wants to use Bash".

This feature derives a bounded, sanitized preview from tool_input server-side, threads it through the same broadcast + late-joiner re-send paths the gate already uses, and renders it above the Approve/Reject buttons — reusing the render-only diff renderer (public/diff.ts:139 renderDiffFile) and the sanitizeField pattern (src/session/timeline.ts:50).

Design decision: the derive logic lives in a new pure module src/http/approval-preview.ts (not inlined in hook.ts) so it is unit-testable in isolation and keeps hook.ts focused; hook.ts is cited only as the proof that tool_input is already available on the hook body. The /hook/permission route calls it directly (it does not go through parseHookEvent).

Contract

New message field — src/types.ts (coordination edit)

Add a new exported type and extend the status variant of ServerMessage (currently src/types.ts:113-119). Additive + optional → older clients ignore it; the frontend exhaustiveness check (terminal-session.ts:349-355) is unaffected.

/** A5-adjacent: compact, BOUNDED preview of what a held approval will run.
 *  Derived server-side from the hook tool_input, sanitized + byte-capped.
 *  Discriminated on `kind`:
 *   - 'command' → a shell command string (Bash). Newlines PRESERVED; all other
 *     control/ANSI chars stripped. Rendered in a <pre> via textContent.
 *   - 'diff'    → ONE synthetic DiffFile (Edit/Write/MultiEdit/NotebookEdit),
 *     rendered by public/diff.ts renderDiffFile (textContent-only, SEC-H4).
 *  `truncated` = the source exceeded the line/byte cap and was clipped. */
export type ApprovalPreview =
  | { kind: 'command'; text: string; truncated?: boolean }
  | { kind: 'diff'; file: DiffFile; truncated?: boolean };

Extend the status variant:

  | {
      type: 'status';
      status: ClaudeStatus;
      detail?: string;
      pending?: boolean;
      gate?: PermissionGate;
      preview?: ApprovalPreview;   // NEW — present only on a held (pending) waiting status
    }

Extend SessionManager.handleHookEvent (currently src/types.ts:331-339) with a trailing optional param (appended last so the existing positional /hook call at server.ts:412 is untouched):

  handleHookEvent(
    sessionId: string,
    status: ClaudeStatus,
    detail?: string,
    pending?: boolean,
    gate?: PermissionGate,
    eventClass?: string,
    toolName?: string,
    preview?: ApprovalPreview,   // NEW
  ): void;

New route behavior

No new route. POST /hook/permission (server.ts:423) gains: derive a preview from body['tool_input'] + tool, store it on the PendingApproval, pass it into handleHookEvent. GET/other routes unchanged.

New env vars — src/config.ts

None. The bounds are security limits, not user knobs (loosening them is a DoS/broadcast-bloat vector), so they are module constants in approval-preview.ts, not config. (Documented as a deliberate choice; if a knob is later wanted, APPROVAL_PREVIEW_BYTES slots into Config next to the existing previewBytes field.)

Bounds (module constants in src/http/approval-preview.ts)

Constant Value Purpose
PREVIEW_MAX_LINES 40 max diff/command lines emitted
PREVIEW_MAX_LINE_LEN 200 per-line char cap (= sanitizeField default)
PREVIEW_MAX_BYTES 4096 hard total-byte cap on the serialized preview payload
EDIT_TOOLS Edit,Write,MultiEdit,NotebookEdit reuse the set semantics from timeline.ts:17

Files to change

Path Change
src/types.ts Coordination edit. Add ApprovalPreview type; add preview? to the status ServerMessage variant (:113-119); add trailing preview? param to SessionManager.handleHookEvent (:331-339).
src/http/approval-preview.ts New file (pure, no DOM, never throws). deriveApprovalPreview(toolName: string | undefined, toolInput: unknown): ApprovalPreview | null. Bash→{kind:'command'}; Edit/Write/MultiEdit/NotebookEdit→{kind:'diff', file}; unknown tool / malformed input→null. Imports sanitizeField from ../session/timeline.js; defines a sanitizeLine wrapper and per-line splitting so \n/\t survive but ANSI/control chars don't.
src/server.ts In /hook/permission (:423): after computing tool (:430), call deriveApprovalPreview(tool, body['tool_input']). Add preview?: ApprovalPreview to the PendingApproval interface (:225-231); store it at creation (:464). Pass preview into handleHookEvent(...) (:471). In the late-joiner re-send (:858-864) include preview: heldApproval.preview. Import deriveApprovalPreview + ApprovalPreview.
src/session/manager.ts handleHookEvent (:223-249): accept trailing preview? param; set msg.preview = preview when defined, before broadcast(session, msg) (:245-249). (The Case-2 late-join re-send at :142 stays bare — the server layer owns pending/preview re-send, per the comment at :137-138.)
public/terminal-session.ts Add private pendingPreviewValue: ApprovalPreview | null = null near :98-106; getter get pendingPreview() near :185-207; in case 'status' (:313-322) set this.pendingPreviewValue = nextPending ? (msg.preview ?? null) : null; clear it in the two disconnect resets near :385-389. Import ApprovalPreview from ../src/types.js.
public/tabs.ts In updateApprovalBar (:360-377): after the label, if session.pendingPreview is set, build and insert a preview node before the buttons. Add private renderApprovalPreview(p: ApprovalPreview): HTMLElement — kind:'command'→<pre class="approval-cmd"> via textContent; kind:'diff'→renderDiffFile(p.file) wrapped in a scroll container; append a "… truncated" note when p.truncated. Import renderDiffFile from ./diff.js and ApprovalPreview from ../src/types.js.
public/style.css Add .approval-preview (scroll container: max-height, overflow:auto, overflow-x:auto), .approval-cmd (white-space:pre-wrap, monospace), .approval-truncated under the existing #approvalbar block (:1084). Reuse existing .df-* styling for the diff.
test/http/approval-preview.test.ts New (node). Unit tests for deriveApprovalPreview.
test/manager.test.ts Extend: handleHookEvent with a preview arg puts it on the broadcast status msg.
test/terminal-session.test.ts Extend (jsdom): status frame with preview sets pendingPreview; cleared when pending false / on disconnect.
test/tabs.test.ts Extend (jsdom): FakeTerminalSession gains pendingPreview; updateApprovalBar renders command / diff / truncated note; no-preview → name-only bar (regression).
test/integration/server.test.ts Extend: POST /hook/permission with tool_input → broadcast waiting status carries preview; a late-joining WS gets the preview on attach.

TDD steps (ordered)

Repo style: pure/back-end tests are plain vitest (node) — see test/hook.test.ts (import { describe, it, expect }, expect.objectContaining). Front-end tests carry // @vitest-environment jsdom and mock TerminalSession (test/tabs.test.ts) or mock WebSocket + stub xterm (test/terminal-session.test.ts). Diff render tests run in jsdom asserting textContent (test/diff.test.ts).

  1. RED — test/http/approval-preview.test.ts (node). Write, before any impl:
    • Bash: deriveApprovalPreview('Bash', { command: 'ls -la', description: 'x' }) → { kind:'command', text:'ls -la' }.
    • Bash multi-line: command:'a\nb' → text still contains the \n (newlines preserved).
    • Bash with ANSI/control injection: command:'\x1b[31mrm\x1b[0m\x07' → ESC/BEL stripped, no \x1b/\x07 in text.
    • Edit: { file_path:'/p/f.ts', old_string:'a\nb', new_string:'c' } → { kind:'diff', file } where file.newPath sanitized, hunk has removed lines a,b and added line c, file.removed===2, file.added===1.
    • Write: { file_path, content:'x\ny' } → all-added hunk, removed===0.
    • MultiEdit: { file_path, edits:[{old_string,new_string},{...}] } → one hunk per edit.
    • Truncation: content with >PREVIEW_MAX_LINES lines → truncated:true, ≤ cap lines emitted; a >PREVIEW_MAX_BYTES blob → truncated:true and serialized size ≤ cap.
    • Unknown tool ('WebFetch') → null; missing/null/array/number toolInput → null and never throws (mirrors hook.ts SEC-M7 style).
  2. GREEN — implement src/http/approval-preview.ts. Pure, unknown-narrowed, sanitizeField-per-line, byte-clamped. Run the suite to green.
  3. RED — test/manager.test.ts. Add: calling handleHookEvent(id,'waiting','Bash',true,'tool',undefined,undefined,{kind:'command',text:'ls'}) broadcasts a status msg whose preview deep-equals the arg (assert via the fake WS send capture already used in this file). Add: omitting preview → no preview key on the msg.
  4. GREEN — src/types.ts param + src/session/manager.ts. Add the trailing param and msg.preview assignment.
  5. RED — test/integration/server.test.ts. Add a test (follow the itPty pattern of ⑧ at :608-655): attach a WS, POST /hook/permission with { tool_name:'Bash', tool_input:{ command:'echo hi' } }, assert the broadcast pending===true status has preview.kind==='command' and preview.text containing echo hi. Add a late-joiner assertion: open a 2nd WS to the same sessionId while held → its first waiting/pending status includes preview.
  6. GREEN — src/server.ts. Wire deriveApprovalPreview into /hook/permission, store on PendingApproval, pass to handleHookEvent, include in the :858-864 re-send.
  7. RED — test/terminal-session.test.ts (jsdom). Feed a status frame with pending:true, gate:'tool', preview:{kind:'command',text:'ls'} → session.pendingPreview set; a follow-up status with pending:false clears it to null; disconnect clears it.
  8. GREEN — public/terminal-session.ts. Add field, getter, set/clear.
  9. RED — test/tabs.test.ts (jsdom). Extend FakeTerminalSession with pendingPreview. Assert updateApprovalBar: command preview → a .approval-cmd node whose textContent equals the command; diff preview → a .df-file present (renderDiffFile output); truncated:true → .approval-truncated present; pendingPreview:null → bar shows label + buttons only (regression, matches :373-374). Assert zero innerHTML — content via textContent only.
  10. GREEN — public/tabs.ts + public/style.css. Add renderApprovalPreview, insert before buttons, style the container.
  11. Coverage gate. The new pure module is branch-dense and fully unit-covered (helps the 80% gate); FE branches covered by jsdom tests. Run full npm test + coverage; confirm no regression in hook.test.ts/diff.test.ts/manager.test.ts.

Edge cases & failure modes

  • Unknown / non-preview tool (WebSearch, Task, MCP tools, ExitPlanMode): deriveApprovalPreview returns null → status carries no preview → updateApprovalBar falls back to today's name-only bar (:373). Plan-gate (gate:'plan', :369-371) also gets no preview (ExitPlanMode has no reviewable command/diff) — unchanged.
  • Missing/partial tool_input: Bash without command, Edit without old_string, tool_input present but null (the 'tool_input' in b passthrough at hook.ts:104 can yield null) → return null, never throw.
  • Huge command / whole-file Write: clipped at PREVIEW_MAX_LINES then PREVIEW_MAX_BYTES; truncated:true shows the "… truncated" note. Prevents a multi-MB status frame from being broadcast to every client and retained in pendingApprovals.
  • Newlines vs control chars: sanitizeField (timeline.ts:50) strips \x00-\x1f which includes \n/\t/\r — so it is applied per line after splitting, never to the whole multi-line blob, preserving structure while still killing ANSI ESC (\x1b) and BEL.
  • MultiEdit with many edits: hunks accumulate until the line/byte cap, then stop + truncated:true.
  • Late joiner after approval already resolved: pendingApprovals.get() returns undefined (server.ts:858) → no preview re-sent (correct; nothing is held).
  • Approve/reject clears preview: handleHookEvent(...,'working',...,false) (server.ts:887,890) broadcasts a non-pending status → terminal-session sets pendingPreview = null → bar hides (existing :362 guard).
  • Multi-device: preview broadcasts to all clients via broadcast and re-sends to each new attach — every mirror shows the same preview.
  • Binary/odd content in Write: rendered as literal text via textContent (no interpretation), binary:false on the synthetic file (we don't attempt binary detection — out of scope).

Security

  • Trust boundary: tool_input is Claude-controlled content arriving on the loopback-only /hook/permission route (isLoopback guard at server.ts:424). It is untrusted for content: treat as unknown, narrow every field (typeof x === 'string'), never index without a guard. deriveApprovalPreview must never throw (SEC-M7 discipline, mirroring parseHookEvent).
  • Sanitization (SEC-H6 reuse): every emitted string passes through sanitizeField/sanitizeLine — strips \x00-\x1f (incl. ANSI ESC \x1b), truncates to PREVIEW_MAX_LINE_LEN. This neutralizes terminal-escape / cursor-hijack payloads in a filename or command before they reach the DOM.
  • XSS (SEC-H4 reuse): the frontend renders only via textContent / el() / renderDiffFile (which is already innerHTML-free — diff.ts:9, :191-194). No innerHTML anywhere in the new FE code; asserted in tests. <script>, &, <img onerror> appear as literal characters.
  • DoS / resource containment: PREVIEW_MAX_LINES + PREVIEW_MAX_BYTES cap the payload that is (a) broadcast to N clients and (b) retained in pendingApprovals for the held-decision lifetime. No unbounded growth from a hostile/huge tool_input.
  • No new route, no new capability token: the existing per-decision token (server.ts:457), Origin/loopback guards, and rate limiters are untouched. Preview is pure display data attached to an already-authorized held decision.
  • No path egress / no argv: file paths from tool_input are only sanitized + displayed as text; they are never passed to execFile, fs, or a shell. No path-traversal surface is added.

Effort & dependencies

  • Effort: ~1.5–2 days. Breakdown: pure approval-preview.ts + its tests ~0.5d (the bulk of logic + coverage); server/manager/types threading ~0.25d; FE (terminal-session field + tabs render + CSS) + jsdom tests ~0.5d; integration test + polish ~0.25d.
  • Depends on (all already shipped): H3 held-approval gate + pendingApprovals (server.ts:423-472); B4 gate re-send plumbing to late joiners (server.ts:858-864, manager.ts:137-142) — this feature rides the exact same rails; B1 public/diff.ts renderDiffFile + the DiffFile/DiffLine types (reused, not modified); A4 sanitizeField (timeline.ts:50, reused).
  • Unlocks / synergizes: the diff-render path here is the same one W3 "Diff against a base branch" and W4 "Stage/commit/push from the diff viewer" extend — a shared, security-reviewed DiffFile render surface. Also complements A1 lock-screen approvals: a future enhancement can put a one-line preview summary into the push detail (PushPayload.detail, types.ts:381) so the phone shows what is being approved (not planned here, but the derive function is the reusable source).
  • Isolation: src/http/approval-preview.ts is a new owned file (no conflict); src/types.ts is the one coordination edit (additive-optional, low collision risk); server.ts/manager.ts/terminal-session.ts/tabs.ts edits are localized to the cited line ranges.