Add the local plan-indexed review screen (#9)

* Add local plan-indexed review screen and experiment setup

* Harden review actions and complete bounded file previews

* Require attribution resolution before item approval

* Confine demo fixtures and isolate helper Git environments

* Align planting path guards and accepted scope labels

* Reject fixture ancestor escapes and canonical planting collisions

* Keep experiment destinations outside the source checkout

* Bound preview dimensions and reject dangling SQLite sidecars

Author: mchwang

.gitattributes

@@ -0,0 +1,4 @@
+# These are hand-authored application sources, not build output.
+web/public/*.js linguist-generated=false linguist-vendored=false
+web/public/*.css linguist-generated=false linguist-vendored=false
+web/public/*.html linguist-generated=false linguist-vendored=false

.github/workflows/ci.yml

@@ -17,3 +17,5 @@ jobs:
       - run: npm ci --ignore-scripts
       - run: npm run typecheck
       - run: npm test
+      - run: npx playwright install --with-deps chromium
+      - run: npm run test:browser

.gitignore

@@ -2,3 +2,7 @@ node_modules/
 coverage/
 dist/
 .DS_Store
+
+.codeboost-local/
+playwright-report/
+test-results/

README.md

@@ -2,7 +2,7 @@
 
 Review agent-made Git changes one plan item at a time. The approved plan lists each item's files and acceptance checks; the review engine shows which item produced each change and flags foreign or overlapping work.
 
-**Status:** the first library slice is implemented. There is no application, server, agent runner, database, or merge command yet. Follow the [build order](docs/designs/codeboost-plan-indexed-review.md#build-order-and-the-gono-go-check); the read-only screen and real-PR go/no-go experiment come before agent execution.
+**Status:** the plan/linking library, SQLite store, and local read-only review screen are implemented. Run `npm run demo` and open its private local URL. There is no agent execution or merge command. The human go/no-go experiment is still pending; see [the local review guide](docs/implementation/read-only-review.md).
 
 ## Development
 
@@ -41,7 +41,7 @@ const history = readHistory(repoPath, baseCommit, headCommit);
 const segments = linkHistory(plan, history, trustedCommitLedger, checkoutPathKey);
 ```
 
-Inputs such as `planText` and the ledger must come from the caller. The future `runner/store` owns the database and ledger; this library does not infer them from commit messages. Before saving a suggested edit, the store must load its captured identity/revision binding by opaque suggestion ID, reject canceled or consumed IDs, and compare-and-swap the plan revision plus consume/invalidate old suggestions in one transaction. The pure `applySuggestion` function requires that trusted binding and validates a copy, but cannot lock storage or prevent replay by itself. Applying one card stales its siblings; refresh and review regenerated cards before the next Apply. Approvals and choices likewise require the stored plan identity.
+Inputs such as `planText` and the ledger must come from the trusted runner. `runner/store` owns the database, persistent request lifecycle, revision allocation, and atomic Apply. The browser sends review commands through `runner/review`; it cannot write ledger ownership or approval fingerprints. See [storage decisions](docs/implementation/persistent-review-store.md).
 
 ## Current limits and safety
 

core/linking.ts

@@ -1,7 +1,7 @@
 import { diffArrays } from 'diff';
 import type { Plan } from './plan.ts';
 
-export interface FileVersion { oid: string; mode: string; text: string | null }
+export interface FileVersion { oid: string; mode: string; text: string | null; byteSize?: number; preview?: string }
 export interface ContextRange { oldStart: number; oldCount: number; newStart: number; newCount: number; name: string }
 export interface FileDelta {
   oldPath: string | null; newPath: string | null;

docs/experiments/review-protocol.md

@@ -0,0 +1,40 @@
+# Plan-indexed review: go/no-go protocol
+
+Status: **not started; issue pairs and human reviewer still required**. This document does not claim a passed gate. Complete the pair table and commit its exact revision before any timed review. No pair or threshold may change once the first review begins.
+
+## Pair selection and order
+
+Choose four pairs of comparable, small real issues in a repository owned by the reviewer. Match each pair on scope, language, and estimated review complexity before implementing either issue. Record issue URLs, PR URLs, base/head SHAs, and which review method each gets. Do not reuse the demo fixture as an experimental issue.
+
+| Pair | Real issue A / PR | Real issue B / PR | A method | B method |
+|---|---|---|---|---|
+| 1 | Pending selection | Pending selection | Raw GitHub diff | codeboost |
+| 2 | Pending selection | Pending selection | codeboost | Raw GitHub diff |
+| 3 | Pending selection | Pending selection | Raw GitHub diff | codeboost |
+| 4 | Pending selection | Pending selection | codeboost | Raw GitHub diff |
+
+The reviewer must not implement the paired changes or inspect the plants before deciding on the PR. An operator prepares plans and commits by hand, records trusted ledger ownership, and supplies both the plan and code to each review method. The earlier one-issue manual assignment also remains required: record how every change maps to the attribution table and turn unclear cases into fixtures.
+
+## Planting
+
+The operator runs `node scripts/plant.ts review.json NEW_DIRECTORY plant-input.json`. The input contains `declaredText`, `undeclaredText`, and a fresh top-level `undeclaredPath`. The helper creates a separate local clone and database, randomly selects eligible owned commits, amends those commits while preserving readable messages/trailers, records old/new mappings and ownership, and writes the returned `review.json` for the local UI. It never pushes or changes the source repository. Both plants are owned through the ledger, so the undeclared-file plant is out of scope rather than foreign.
+
+The helper supports linear histories and regular, top-level declared files retained through the remaining history. It refuses unsupported targets and stops on cherry-pick conflicts; the operator must inspect a failed disposable clone and choose a fresh output path, never silently resolve a conflict or alter the frozen issue pair. Plant text must represent a plausible unrelated code change, not an obvious marker. The automated test uses obvious markers only to verify the helper.
+
+`sealed.json` outside the clone records locations and selected commits with owner-only file permissions. It is unblinding data, not encryption. The operator must withhold it from the reviewer until that PR's decision is recorded. Do not show CLI input or sealed data to the reviewer. Open PRs only after inspecting the prepared branch; PR publication is a separate action.
+
+## Measurement
+
+Time each review from first opening to the recorded decision. Record elapsed seconds, unexplained changes, the undeclared-file plant found (yes/no), and declared-file plant found (yes/no). Record decisions before unblinding. Also record interruptions and tool failures without silently dropping trials.
+
+Pass only when all are true:
+
+- codeboost catches at least 3 of its 4 undeclared-file plants and more than raw-diff review catches;
+- median codeboost time is no slower than median raw-diff time;
+- no change is left unexplained.
+
+Report declared-file catch rate for both methods without a pass threshold. Stop after four pairs. If within one catch of the bar, perform four more preselected pairs once, then decide. Do not add further trials. Record the outcome in a committed results document before merge/agent/planning/queue/learning implementation begins.
+
+## Current evidence
+
+Engineering tests prove browser interactions, persistence, attribution, and planting mechanics. They do **not** establish human review speed or catch rates. No real issue pairs, timed decisions, or catch-rate results exist yet.

docs/implementation/read-only-review.md

@@ -0,0 +1,52 @@
+# Read-only review screen (#3)
+
+Started from `dd6a1e7`, after merging the SQLite store PR. This delivers the local review surface and experiment tooling; issue #3 remains open until the human go/no-go work is complete.
+
+## Run it
+
+`npm ci --ignore-scripts`, then `npm run demo`. Open the private loopback URL printed by the command. The demo uses a real disposable Git repository and SQLite database under ignored `.codeboost-local/demo`; restart preserves approvals and notes. No source files in the codeboost checkout are edited by the demo. Stop with Ctrl+C.
+
+For an existing trusted store, use `npm start -- --config /absolute/path/review.json`. The JSON has `database`, `repository`, `identity` (`repositoryId`, `taskId`, `planId`), and `pathIdentity` (`caseSensitive` boolean, `unicodeNormalization` equal to `none` or `NFC`). Paths should be absolute. Supply actual checkout identity rules, not an OS guess. The demo probes its local filesystem; it uses only simple ASCII fixture paths. Unsupported or more complex filesystem equivalence must not be approximated by this adapter.
+
+The selected repository's current HEAD is compared to the stored base. Refresh observes new heads and creates an immutable snapshot; approvals are recomputed for the resulting changes. It does not fetch, rebase, run tests, invoke agents, or merge. Histories still obey the library's linear/complete/bounded-history constraints.
+
+## UI and storage
+
+The app follows the Evidence Desk tokens and self-hosts IBM Plex Sans/Mono. It has item and exception rows, four status checks, a provenance gutter, file metadata, shared-hunk labels, assignments, standalone acceptance, no-change confirmation, approval counts/staleness, before/after approval evidence, per-item questions/change requests, keyboard controls, resizable/collapsible side panes, desktop breakpoints, and loading/error/empty states. Questions are saved for discussion; no AI answer is fabricated. Tests/AI review are explicitly not run. Change requests remain pending for a future revision workflow.
+
+The server binds only 127.0.0.1 and requires its random private token for APIs. Host/origin checks, a restrictive CSP, bounded UTF-8 JSON bodies, and DOM escaping prevent another website from reading or writing local review state. Browser commands contain item/segment IDs and the reviewed state token, never approval fingerprints or ledger ownership. The runner derives those from Git/store data. A database review counter prevents concurrent review actions from approving unseen assignments; plan and snapshot CAS remain enforced.
+
+Store schema v2 adds the review counter and per-item notes through a transactional v1 migration. The test suite verifies existing revisions and ledger entries survive. This is an automatic local SQLite migration, not a migration against a shared environment.
+
+## Known limits and remaining gates
+
+- No agent answers, test execution, AI findings ingestion, send-to-agent action, or merge control is included. These belong after the go/no-go gate. Four checks distinguish unavailable evidence from success.
+- File cards show mode/path/object IDs and blob byte sizes. PNG/JPEG/GIF/WebP previews are bounded to 1 MiB each and 4 MiB across a history; unsupported/oversized images say unavailable. Gitlink byte sizes are not applicable. SVG/HTML is never embedded.
+- The protocol at `docs/experiments/review-protocol.md` must be filled with the real pairs and committed before the first timed review. The manual assignment, real paired PRs, human timing, and final result are pending. Do not mark issue #3 closed or claim the gate passed.
+- The planting helper is intentionally limited to disposable clones and supported regular top-level paths; it never publishes PRs.
+
+## Validation
+
+Baseline before this slice: 157 tests. Run `npm run typecheck`, `npm test`, and `npm run test:browser`. Browser tests use real Git and SQLite with Chromium and cover persistence, assignments, metadata, no-change approval, stale views, unsafe origins, keyboard/breakpoints, error display, and untrusted text. A browser regression exposed false stale reasons from JSON field order; structural comparison replaced that check. The plant test verifies source HEAD stays unchanged and both plants retain ledger ownership.
+
+A large-change regression reproduced HTTP 413 when the browser sent the full content-based choice key for a 20 KB segment. Browser segment IDs are now bounded SHA-256 identifiers; the runner reconstructs the original identity/content/copy key before saving the choice. This retains choice expiry semantics without sending source content back in a review command.
+
+## Review round 1
+
+Reproduced and fixed no-change approval with item-owned ambiguous segments; the runner refuses it and the UI directs the user to resolve attribution first. Previously approved no-change items become stale if ambiguous work appears. Reproduced malformed non-ASCII credentials returning a generic conflict instead of unauthorized; credentials now require the expected ASCII hex shape before constant-time comparison. Reproduced stale item controls surviving a failed refresh; errors now discard the loaded view and require refresh. Added acceptance persistence and whole-plan empty-state browser coverage. No findings declined.
+
+## Review round 2
+
+Extended the attribution guard to mixed items with both owned and ambiguous segments. The regression reproduced ordinary approval succeeding with unresolved attribution. Approval now rejects any item-specific ambiguity, previously approved affected items become stale, and the UI routes every such item to attribution resolution. Added browser coverage for this mixed case. No findings declined.
+
+A toolbar-focus browser regression reproduced review shortcuts being disabled while a button had focus. Only text-entry/select controls now suppress the single-letter shortcuts; native Enter/Space button behavior is unchanged. The plan-row click case already worked because rendering replaces the focused row.
+
+Review round 3 identified demo fixture config escapes and inherited Git environment redirection. Regression checks first reproduced the config, symlink, and environment failures. Existing demos now require exact fixture paths and ordinary config/database/repository objects (including `.git` and SQLite sidecars). Demo and plant Git commands share a case-insensitive environment scrub and disable inherited global/system Git configuration. Four helper tests cover these boundaries, including mixed-case variable names for Windows.
+
+Review round 4 had no inline findings but raised two summary concerns. Both were reproduced: planting accepted a non-ASCII path under case-insensitive identity even though the viewer refuses it; accepted cards retained an unplanned scope label. Planting now applies the same path guard, and accepted cards explicitly say “Accepted outside plan.” The underlying provenance remains intact. Unit/browser regressions cover both fixes; no concerns were declined.
+
+Review round 5 found a symlinked-ancestor escape and raw-spelling collisions in planting guards. Both regression cases failed before the fix. Demo creation/reuse now checks every ancestor (canonicalizing only the OS temporary-directory prefix); planting compares canonical identities for declared files and every path in the base/commit trees before cloning. The summary's status-styling concern had no additional concrete example; accepted cards already use the explicit label added in round 4.
+
+Review round 6 reproduced destination-inside-source mutation; planting now resolves the existing destination ancestor and rejects canonical source descendants before creating files, including `.git` and symlink aliases. The Git-environment finding was declined: `gitRaw` already passes `{cwd, env, ...}` to `execFileSync`. A new end-to-end test with inherited `GIT_DIR`/`GIT_WORK_TREE` passed before any production change. The summary's snapshot-race claim supplied no concrete interleaving; the service already checks revision/snapshot/review version before publishing a view and writes use atomic store CAS, covered by concurrent-write tests.
+
+Review round 7 reproduced oversized-dimension raster previews and dangling SQLite sidecar symlinks. Preview metadata is now parsed from the bounded buffer with image-size: at most 8,192 pixels per side, 4 million pixels per image and 16 million pixels across unique preview blobs. Unknown dimensions omit the preview; existing compressed/response byte limits remain. Sidecars use lstat so dangling links are rejected. Both new regressions failed before the fixes. The summary mentioned rename-scope/literal-path concerns without specific findings; no additional behavior was inferred from that shorthand.

git/history.ts

@@ -1,3 +1,4 @@
+import { imageSize } from 'image-size';
 import { execFileSync } from 'node:child_process';
 import { lstatSync, opendirSync } from 'node:fs';
 import { resolve as resolvePath, join } from 'node:path';
@@ -89,7 +90,8 @@ export function readHistory(repo: string, baseRef: string, headRef = 'HEAD', lim
     return { sha, parent, files: [] as FileDelta[] };
   });
   if (expectedParent !== head) throw new Error('The base must be an ancestor of the head.');
-  const blobs = new Map<string, string | null>();
+  const blobs = new Map<string, { text: string | null; byteSize: number; preview?: string }>();
+  let previewBytes = 0, previewPixels = 0;
   const version = (oid: string, mode: string): FileVersion | null => {
     if (/^0+$/.test(oid)) return null;
     if (mode === '160000') return { oid, mode, text: null }; // gitlink is not a local blob
@@ -100,9 +102,25 @@ export function readHistory(repo: string, baseRef: string, headRef = 'HEAD', lim
       blobBytes += data.length;
       let text: string | null = null;
       if (!data.includes(0)) { try { text = new TextDecoder('utf-8', { fatal: true, ignoreBOM: true }).decode(data); } catch { /* binary */ } }
-      blobs.set(oid, text);
+      let preview: string | undefined;
+      // Only bounded raster images; never embed SVG/HTML or fetch external references.
+      const mime = data.subarray(0,8).equals(Buffer.from([137,80,78,71,13,10,26,10])) ? 'image/png' :
+        data.subarray(0,3).equals(Buffer.from([255,216,255])) ? 'image/jpeg' :
+        /^GIF8[79]a$/.test(data.subarray(0,6).toString('ascii')) ? 'image/gif' :
+        data.subarray(0,4).toString('ascii') === 'RIFF' && data.subarray(8,12).toString('ascii') === 'WEBP' ? 'image/webp' : null;
+      if (mime && data.length <= 1024 * 1024 && previewBytes + data.length <= 4 * 1024 * 1024) {
+        try {
+          // Parse metadata from the already bounded buffer; never allocate decoded pixels.
+          const { width, height } = imageSize(data), pixels = width * height;
+          if (Number.isSafeInteger(pixels) && width > 0 && height > 0 && width <= 8192 && height <= 8192 && pixels <= 4_000_000 && previewPixels + pixels <= 16_000_000) {
+            preview = `data:${mime};base64,${data.toString('base64')}`;
+            previewBytes += data.length; previewPixels += pixels;
+          }
+        } catch { /* Unknown or malformed dimensions: metadata card only. */ }
+      }
+      blobs.set(oid, { text, byteSize: data.length, ...(preview ? { preview } : {}) });
     }
-    return { oid, mode, text: blobs.get(oid)! };
+    return { oid, mode, ...blobs.get(oid)! };
   };
   const diff = (from: string, to: string, contexts: boolean): FileDelta[] => {
     const raw = accountDiff(run('diff', '--ignore-submodules=none', '--no-relative', '--raw', '-z', '--no-abbrev', '--no-ext-diff', '--no-textconv', '-M', from, to, '--'));