Commit Graph

1173 Commits

Author SHA1 Message Date
Michael Ramos 9863e44aec Code review page: copy pass + inline guided-review demo (#1006)
* docs(marketing): tighten code-review page copy to match project voice

Rewrites the /code-review copy in a plainer, more technical register:
leads with the git/jj/Perforce diff viewer and the annotate → agent
feedback loop, names GitHub/GitLab PRs and worktrees, adds the local-API
and ~/.plannotator compound-engineering framing, and states the
MIT / Apache-2.0 license. Drops 'keep the final say' from the
agent-reviews heading — now just 'Run review agents'.

* feat(marketing): inline guided-review demo video

The Guided Review section now plays a muted, looping demo inline (~5 MB,
1440px), clickable to open a fullscreen player with controls. Other
feature screenshots keep the image lightbox. Ships guided-review-demo.mp4
+ poster (committed to public/assets and uploaded to the CDN).

* chore(marketing): update hero demo video with guided-review footage

Re-renders the code-review demo with the real Guided Review recording
swapped into the guided scene, and refreshes the CDN copy + poster.
2026-07-06 12:59:37 -07:00
Michael Ramos 5fdc3e4542 feat(marketing): code review landing page with demo video (#1003)
- New /code-review page: hero with a demo-video launcher + modal player,
  alternating feature sections (annotate, git status, commits, PRs, Ask AI,
  guided review, agent reviews), the external-annotations integration diagram,
  and a compound-engineering loop section.
- Feature screenshots open in a fullscreen lightbox (mobile-friendly); on
  mobile the screenshot sits above its prose.
- Ships the ~2-min product demo (code-review-demo.mp4) + poster, committed so
  marketing deploys re-ship them; also uploaded to the CDN.
- Footer 'Code Review' now links to the new page instead of the docs command.
2026-07-06 10:02:27 -07:00
Michael Ramos 82f5648ccb chore: bump version to 0.22.0 v0.22.0 2026-07-05 11:57:30 -07:00
Michael Ramos 42686f8f1e docs(marketing): bring the code-review reference up to the since-base era
The page still described the pre-#990 product: uncommitted default, no
Git status/Tree/Commits panel views, no Guided Review, an agent roster
without the marker engines, and a Server API table missing nine
endpoints. Rewritten to match what ships.
2026-07-05 11:50:41 -07:00
Michael Ramos 3ebb28a835 fix(review): avatar lookups can no longer delay /api/commits
The gh/glab runner has no subprocess timeout, and the commits endpoint
awaited avatar resolution before responding — a hanging network (proxy
black-hole, dead DNS) held the already-computed commit list hostage to
decoration. resolve() now races a 4s ceiling: on expiry the endpoint
returns with the initials fallback, the in-flight fetch finishes in the
background, and later calls pick its results out of the cache. Timed-out
attempts memoize no misses. Shared module, so both runtimes get it.
2026-07-05 11:50:41 -07:00
Michael Ramos b37d10a8e0 fix(review): anchor commit-diff annotations to their commit in feedback
Annotations made on a commit:<sha> diff carry line numbers from that
commit's diff-vs-parent, but the feedback export derived its one header
from the send-time diff — annotate a historical commit, switch back to
since-base, hit Send Feedback, and the agent reads those line numbers
against the wrong diff. Stamp annotations with the active commit (sha +
subject) at creation, mirroring the prUrl pattern, and label any
anchor/header mismatch in the export (both directions). The commit-diff
header itself now reads "Commit `abc1234` — subject" instead of the raw
mode string.
2026-07-05 11:50:27 -07:00
Michael Ramos 9fc2c6e9bd fix(review): keep the filename visible in git-status rows
Long paths in the Sections panel end-truncated, hiding the one part that
matters. TruncatedPath lets the directory absorb all truncation (ellipsis
lands right before the final slash); only a filename that alone exceeds the
row truncates at its own end. Full path stays on the row tooltip.
2026-07-05 11:50:13 -07:00
Michael Ramos ed4f2e82fe feat(review): Guided Reviews intro dialog + Guide button discovery hint (#1000)
* feat(review): one-time Guided Reviews intro dialog + Guide button hint

New GuideIntroDialog slots into the announcement chain (look-and-feel ->
guide intro -> review setup, never stacking): a hero screenshot of a
guided review with a brief plain-language description and a mock header
strip pointing at the real Guide button. Shows once per user (cookie),
and only when a Guide button actually exists (hasSearchableFiles) --
skipping the render does not consume the one-shot cookie.

The hero is CDN-hosted at https://plannotator.ai/assets/guided-review.webp
(committed to apps/marketing/public/assets so every marketing deploy
re-ships it past the --delete sync; uploaded to S3 now so it is already
live) instead of growing the single-file bundle; on load failure the
image container hides entirely for a clean text-only dialog.

Independently of the dialog, the header Guide button shimmers
(TextShimmer) with a pulsing dot until the guide is first opened --
acked on ANY open path (button, shortcut, job auto-open), not just the
button click.

* fix(review): guide intro first in the dialog chain, full-width panel

Chain is now guide intro -> look-and-feel -> review setup (was second);
panel widened to max-w-5xl to match the look-and-feel dialog. Intro seen
flag switched to the versioned-cookie pattern so future revisions can
re-show it.

* fix(review): latch guide-intro eligibility at first post-load render

hasSearchableFiles changes on mid-session diff switches; an
empty-to-non-empty switch would pop the intro over work in progress, or
yank an open look-and-feel dialog (its gate reads guideIntroVisible).
Eligibility is now decided once when loading clears -- dialogs only
mount after that, so the latch is always set before they render. A
skipped session keeps the cookie unconsumed. Also corrects AGENTS.md's
chain order (guide intro first).
2026-07-05 09:49:00 -07:00
Michael Ramos aa46dbb29a feat: guide per-file summaries + GitHub Copilot CLI agent engine (#997)
* feat(guide): per-file summary on guide diff refs

Each diffs[] entry now carries a required (schema-enforced) 1-2 sentence
summary of the semantic change in that file, written from the diff hunks
alone. Sanitizer passes it through when it's a non-blank string and omits
it otherwise -- a missing summary never drops the ref or fails the guide.
Rendered as a muted inline-markdown line above each diff in the guide;
marker-engine contract and demo data updated to match.

* fix(guide): repair prompt must not invent missing summaries

The guide schema now requires summary, so a schema-enforced repair of a
payload that lacks them (marker-engine output) would force the model to
fabricate captions with no diff in sight. Instruct it to fill missing
required fields with an empty string instead; the sanitizer already
renders nothing for blanks.

* feat(agents): GitHub Copilot CLI as a marker engine for review + guide jobs

Adds copilot as the fourth marker engine (no schema flag, so it uses the
nonce-tagged marker-block contract like Cursor/OpenCode/Pi):

- marker-review.ts: COPILOT_ENGINE — 'copilot --output-format json' JSONL
  stream (assistant.message carries the assembled text; deltas skipped),
  models discovered from 'copilot help config', live-log formatting for
  tool.execution_start/complete. Non-interactive posture: --no-ask-user
  auto-denies unallowed tools, --deny-tool=write, read-only-oriented shell
  allowlist (git/gh/glab/jj/wc), builtin MCPs and auto-update disabled.
- Exported MarkerEngineId and replaced every 'cursor'|'opencode'|'pi' cast
  with it (Bun server, Pi server mirror, guide-review) so the next engine
  is a two-edit change.
- agent-jobs (both runtimes): copilot in SERVER_BUILT_PROVIDERS; capability
  entry + model discovery come free from the MARKER_ENGINES loop.
- UI: copilot review/guide engine with per-surface model settings
  (useAgentSettings), AgentsTab launch + config rows, GuideEmptyState
  launcher, job-detail labels, CopilotIcon (currentColor, official mark).
- Tests: argv/read-only flags, help-config model parsing, stream reduction,
  full marker pipeline; profile-map expectations widened.

Verified live: composed review prompt through the real copilot binary,
marker block parsed, seeded bug found.

* fix(agents): structurally deny high-consequence verbs for Copilot jobs

git:*/gh:*/glab:* stay allowed for inspection ergonomics, but Copilot's
deny-precedence rules now block the verbs a prompt-injected background
job could abuse: git push/reset/clean/checkout/restore (local reviews run
in the user's real working tree), and PR/MR/issue comment/create/merge/
close/edit/review on gh and glab. Probe-verified: git log runs, git push
--dry-run is denied by the shell(git push) rule.

Addresses the one confirmed finding from the PR #997 review round; the
other two (untracked-file reads, git -C) were refuted by live probes.
2026-07-05 09:12:20 -07:00
Michael Ramos d6b98f0c0d feat(review): Guided Review + Pi agent-job provider (#993)
* docs(adr): Guided Review ADR/spec/research + agent-provider fit studies

ADR 006 (Guided Review as a first-class feature), four implementation
spikes, synthesis, spec (iterated through preflight and review fixes),
recap, and the Flue/Pi provider-fit research syntheses.

* feat(review): Guided Review takeover + Pi agent-job provider

Guided Review (ADR 006) — a Linear-Guides-style chaptered review:
- guide agent-job provider (packages/server/guide/guide-review.ts):
  schema-constrained sections (title/overview/file refs) over the live
  patch, coverage-validated server-side (every changed file exactly once,
  fabricated paths dropped, fail-closed on empty output), importance-first
  ordering prompt with speed discipline (diff-first, no repo exploration)
- runs on claude/codex natively and cursor/opencode/pi via the marker
  contract; guide-scoped low effort defaults for quicker generation
- takeover UI (packages/review-editor/components/guide/): Guide header
  badge + Mod+Shift+G, full-width screen that CSS-hides (never unmounts)
  the file tree/dock, empty state with inline model pickers, skeleton
  loading, generating state with collapsed activity log, two-column
  section cards (sticky prose column with internally-scrolling file list,
  content-height diffs, review-independent collapse, peek semantics)
- annotation parity: guide diffs mount the real DiffViewer with
  file-scoped handlers into the same CodeAnnotation state and feedback
  export; per-section reviewed state persisted via /api/guide/:jobId
- reviewed/tour prompt speed sections; renderMarkdownProse shared between
  tour and guide (fenced-block support, muted tone variant)

Pi agent-job provider:
- third MarkerEngine (pi --mode json --no-session --no-approve) with
  live model-catalog discovery, thinking-level control (--thinking),
  fail-closed marker parsing (Pi exits 0 on in-run errors by design)
- available for review + guide jobs from both launch surfaces

Shared UX:
- searchable, provider-grouped model pickers (auto over 12 options)
- full Claude version catalog with latest-resolving aliases labeled
- AgentControls primitives extracted from AgentsTab; cross-instance
  settings sync in useAgentSettings
- Pi extension server hand-mirrors + vendor.sh entries throughout

* fix(guide): PR-993 review fixes + failure recovery ladder

Review fixes:
- coerce marker-engine title/intent at ingest (prompt-enforced output
  could store non-strings and crash the takeover)
- keep placed files out of unplacedFiles (exactly-once guarantee)
- shared REVIEW_ENGINE_LABEL (fixes 'generated by pi' header; typed
  exhaustiveness prevents recurrence)
- key ActiveGuide by jobId so per-guide focus state resets
- formatModel handles marker-engine tour/guide jobs (empty model chip)
- drop dead guideCodexFast; memoize estimateDiffHeight
- store + display Pi thinking level on job cards

Failure recovery ladder (auto -> one click -> manual -> regenerate):
- mechanical JSON repair pass (fences/slice/trailing-commas/bracket
  balance) before any guide parse fail-closes
- failed payloads captured per job (200KB cap, per-engine extraction)
- 'Fix output' repair job: pure text transform on a schema-capable
  engine (claude/codex preferred), forced low effort, lands as a
  normal guide job; repairOf threaded through launch validation
- editable output panel: GET /api/guide/:jobId/output prefills a
  textarea, POST /api/guide/:jobId/submit runs the same server-side
  validation and opens the fixed guide; inline errors for iteration
- Pi extension mirrors + AGENTS.md endpoint/body docs

* fix(guide): string-aware mechanical JSON repair (self-review)

- terminate a dangling string literal before appending bracket closers
  (truncation mid-string is the most common shape; closers appended
  inside the string never parsed)
- strip trailing commas outside string literals only (the regex could
  rewrite overview content, violating the repair contract)

* fix(guide): launch-snapshot validation, codex strict schema, repair persistence

PR-993 round-2 review fixes + live-test findings:
- validate guide completion (and manual repair) against the LAUNCH-time
  changed-file set, snapshotted per job — switching diff/base/PR while a
  guide generates no longer destroys valid output (client already
  degrades stale refs per-file)
- codex strict structured output: unplacedFiles now required in the
  guide schema (OpenAI 400s schemas with optional properties under
  additionalProperties:false — live-confirmed, then live-verified fixed:
  5 sections / 83 files placed on a ChatGPT-account codex)
- manual repair survives reload: successful /submit flips the job to
  done via completeJobExternally + status guard on the endpoint
- codex model defaults -> gpt-5.5 (5.3-codex is deprecated; ChatGPT
  accounts reject it) with one-shot migration of saved picks; guide
  codex default reasoning stays low
- pi extension guide branch snapshots launch state (TOCTOU hygiene)
- paragraph collector no longer swallows an unspaced code fence
- tour hook backports: out-of-order fetch guard; save outside updater
- guide empty-state copy rewritten value-first

* fix(guide): PR-993 round-3 — repair-diff fidelity, failure surfacing, pi tool hardening

- repair jobs validate against the FAILED job's recorded file set (the
  payload under repair references that changeset; validating against the
  diff on screen at repair time re-introduced destroy-on-switch)
- takeover follows the NEWEST running guide (progress + Cancel), and a
  failure newer than the shown guide surfaces as a dismissible strip
  (error + Fix output + Details) instead of being silently masked
- pi jobs run with --exclude-tools edit,write (bash stays: the agent
  needs git inspection; full read-only would break generation)
- tour/guide result ingestion fails closed on unexpected throw (done-
  looking jobs no longer 404 their result)
- sidebar guide mode gated on file availability like the header badge
- drop orphaned DEFAULT_GUIDE_CODEX_FAST (fast mode intentionally not
  offered for guide); retry owns its fetch cancellation; final
  trailing-comma pass after bracket-closing in the repair ladder;
  collapsed-row checkbox/expand are sibling buttons (a11y)

* fix(guide): PR-993 round-4 — callout text, guide job detail, toggle races

- single-line callouts (> [!IMPORTANT] message) keep their message — the
  form our own prompt solicits rendered an empty labeled box; type now
  derives from the [!TAG] capture, not a whole-line scan that mistyped
  '> [!NOTE] this is important'
- job detail panel gets a guide status card (Open guide via a new
  ReviewStateContext.openGuide) instead of mislabeling 'Guide Generated'
  as a red Incorrect review
- marker-engine guide jobs get readable live logs (engine fallback in
  the formatter lookup; provider stays 'guide')
- repair prefers the failed job's own schema-capable engine (proven
  working on this machine) before falling back by binary presence
- toggleReviewed/toggleChecked: pure functional updaters + persistence
  effect with seed-skip — race-proof AND StrictMode-safe (resolves the
  tradeoff previous rounds accepted)
- dedups: formatDuration/ElapsedTime shared from AgentsTab; whichCmd
  exported in the pi extension; SEARCHABLE_THRESHOLD imported by
  InlinePicker
- new renderMarkdownProse tests (single-line/multi-line callouts,
  paragraph passthrough)

* fix(guide): PR-993 round-5 — keep engine-default model option, label Pi jobs

- GuideEmptyState's marker catalogs mirror AgentsTab's per-engine
  semantics: opencode/pi prepend the engine-managed Default ('' value)
  to the discovered list (dropping it left saved-default users a blank
  pill with no way back); cursor replaces (its list includes 'auto')
- job detail provider pill labels pi review jobs 'Pi' instead of the
  'Shell' fallback

* fix(guide): PR-993 round-6 — context scoping, stale models, repair engine, staging gate

- guide takeover + auto-open scoped to the current review context: a
  guide launched against PR A no longer shows over PR B, and a guide
  finishing for an away context defers its auto-open until the reviewer
  returns to that context (unmarked in the dedupe set on purpose)
- guide launcher reconciles saved cursor/opencode/pi model ids against
  the live catalog at read time (picker + launch share one effective
  value) — no more posting dead ids after an account switch
- repair prefers the failed job's OWN engine whenever its binary is
  present (provably runnable; marker binaries resolved via
  MARKER_ENGINES — cursor's CLI is 'agent'); claude/codex are fallback
  only, killing the broken-claude repair doom loop
- per-file staging gate (canStagePath) in guide diffs AND the
  pre-existing same gap in ReviewDiffPanel — committed-only files in
  since-base reviews no longer offer a no-op Git Add

* fix(review): scope tour auto-open to current context (self-review)

Same cross-context gap just fixed for guides: a tour finishing for PR A
popped its dialog over PR B. Both auto-open effects now share one
jobMatchesCurrentContext helper with the same deferred-open semantics.

* fix(guide): PR-993 round-7 — complete the context-scoping story

Round 6 scoped guides to their review context; this closes the two
surfaces that scoping left dangling, plus a focus gap:

- switching to a context that already HAS a completed guide now shows
  that guide: the takeover falls back to the context's newest done
  guide job when activeGuideJobId belongs elsewhere (previously landed
  on the empty state with the guide sitting unreachable)
- 'Open guide' affordances are context-gated everywhere they exist:
  job cards hide it for cross-context guides (opening can't switch
  PRs), and the job detail panel explains where the guide belongs
  instead of offering a dead button; one shared
  jobMatchesReviewContext predicate now backs App, GuideScreen,
  AgentsTab, and the detail panel
- file-chip navigation retargets the guide's focus arbiter (scrolling
  under a stationary pointer fires no pointerenter, so the annotation
  toolbar stayed bound to the previous diff)

* fix(tour): retry owns its fetch cancellation; document prUrl-only scoping

- useTourData retry converts to the nonce-driven effect re-run pattern
  (the round-4 backport carried the fetch guard but not the retry fix
  useGuideData got in the same batch — half a backport)
- jobMatchesReviewContext now documents WHY it matches by prUrl only
  and not diffScope/diffContext: guides/tours reference files and
  degrade per-file when the diff shifts; scope-strict matching would
  hide useful artifacts on layer/base/mode switches. Deliberate,
  do-not-tighten-without-UX-decision

* fix(guide): PR-993 round-9 — worktree-aware context, guide-scoped marker settings

- jobMatchesReviewContext now compares diffContext.worktreePath for
  local jobs: prUrl + worktree define WHERE the review is (matched
  strictly); mode/base/scope remain WHAT VIEW (deliberately loose) —
  a guide launched against worktree A no longer opens over worktree B.
  Jobs predating the snapshot coalesce to the main tree.
- Cursor/OpenCode/Pi get guide-scoped model/thinking settings
  (guideCursor/guideOpencode/guidePi), mirroring the existing
  guideClaude/guideCodex isolation: tuning a guide's marker model no
  longer silently changes the next code review with that engine.
  Defaults are each engine's natural default, not seeded from review
  settings (same precedent as guideClaudeEffort='low'). AgentsTab's
  reconcile effects snap both surfaces against the shared catalogs.

* fix(review): keep worktree parsing browser-safe (round-9 build fix)

parseWorktreeDiffType lives in shared/review-core, which imports
node:path at module top — pulling it into App.tsx broke the vite
browser build (typecheck and tests don't catch it; only the bundle
does). Context matching now reuses App's existing hand-parsed
activeWorktreePath memo — one parse, and it's the same one that
drives the sections/tree UI, so matching aligns with what's on
screen. Also fixes a deps-array reference to the removed memo that
bundled fine (treated as a global) but would have thrown a
ReferenceError at runtime.

* docs(review): mark worktree-subtype lockstep between server parser and App copy

* fix(review): PR-993 round-10 — guide shortcut gate, settings sync race, heading markdown

- bare a/v staging/viewed shortcuts suspend while the guide takeover is
  open (the dock is only CSS-hidden, so a diff panel stayed 'active'
  underneath and bare keys acted on an invisible file)
- cross-instance settings sync replaces the was-last-update-remote
  boolean with value comparison (lastSyncedJsonRef): the flag conflated
  'a commit happened' with 'the last change was remote', so a local
  edit batched with an incoming broadcast silently skipped its own
  cookie write and rebroadcast
- ### headings in guide/tour prose run through renderInlineMarkdown
  like every other block (backticked symbols rendered as raw tokens);
  regression test added

* fix(guide): PR-993 round-11 — reveal channel for sidebar jumps, repair exit code

- new guideRevealFile channel closes the documented round-7 gap and its
  AI sibling: sidebar jumps (annotation clicks, AI line citations) made
  while the guide takeover is open no longer mutate the hidden dock's
  active file; instead the GuideSectionCard containing the target file
  expands its collapsed (reviewed) section, focuses the diff, and
  scrolls to it — so jumps into collapsed sections stop silently
  no-opping. Cleared on guide close/switch so keyed remounts don't
  replay the last reveal.
- completeJobExternally resets exitCode to 0 (both servers): a
  successfully repaired guide kept showing the failed run's Exit 1 chip
  in the job detail panel

* fix(guide): clear reveal channel in-batch at guide-switch sites (self-review)

The clear-on-change effect fires after a switched guide's keyed cards
have already mounted (child effects run before parent effects), leaving
one commit where a stale reveal from guide A could expand+scroll a
same-named file's section in guide B. All three setActiveGuideJobId
sites now clear synchronously in the same batch; the effect stays as a
backstop for close and future set sites.

* fix(guide): PR-993 round-12 — titled sections, honest large-PR validation

- sanitizeGuideSection gives every surviving section a non-empty title
  ('Untitled section' fallback): a diffs-only section rendered as a
  blank chapter inside a 'Guide Generated' job — no parse failure, so
  no recovery flow. Keeping (titled) beats dropping: placed files are
  not in unplacedFiles, so dropping would orphan them silently.
- guide launches on large PRs (layerPatchIncomplete) recompute
  changedFiles from the local checkout (git diff --numstat
  origin/<base>...HEAD, rename/binary handling) when available — the
  PR-mode prompt tells the agent to read that full local diff, but the
  changed-files block and validation snapshot came from the truncated
  platform patch, under-listing files to the model and then dropping
  its valid refs (or failing the guide) at validation. Falls back to
  the partial list on any failure. Mirrored in the Pi extension server.

Round-12's P1 (Pi marker extraction reads message_end, 'should' read
text_delta) was refuted by live probe: Pi's assistant message_end
carries the fully-assembled content blocks, which is exactly what
piExtractText reads; the delta-reading code the finding cross-
referenced is the Ask-AI STREAMING provider, a different job.

* test(guide): first direct coverage for guide-review's pure logic (self-review)

The module's repair ladder, validation, and stream parsing had zero
unit tests — exercised only end-to-end through live agent runs. Pins
the behaviors the review rounds fixed: blank-title fallback (round 12),
first-placement-wins dedup, changedFiles filtering + fail-closed empty
guides, prose-only vs lost-diffs section handling, unplacedFiles
merge/dedup/fabrication-filtering, title/intent coercion, trailing-
comma and unbalanced-bracket repair, truncated stream-line recovery.
13 tests; server suite 374 → 387.

* fix(guide): PR-993 round-13 — full-stack recompute regression, flag snapshot, unplaced reveal

- the round-12 large-PR recompute now runs ONLY in layer scope: in
  full-stack scope launchPatch is already a local full recompute, and
  the layer diff (origin/<base>...HEAD) was the WRONG file set — it
  omits earlier stack layers' files, dropping their refs at validation
  or failing the guide. Introduced by round 12; caught before any
  release. Both servers.
- layerPatchIncomplete is snapshotted with the other launch locals
  (launchLayerPatchIncomplete): coherence with launchPatch was
  positional (same sync segment) — now structural, so a future await
  inserted upstream can't silently desync them. Both servers.
- the 'Everything else' bucket handles guideRevealFile: sidebar jumps
  to unplaced files now focus + scroll their diff (the section-card
  effect only covers placed files) — closes the residual gap noted in
  the round-11 self-review
2026-07-04 10:23:59 -07:00
Michael Ramos e8df06db7c feat(review): Commits panel — linear history rail with per-commit diffs (#994)
* docs(adr): spec for the commit-list review view (linear history rail)

* feat(review): commit:<sha> diff mode along the shared diff-type seams

A new git-only diff family for reviewing one historical commit against
its first parent (git-show style), threaded through every seam a diff
type crosses:

- runGitDiff: git diff <sha>^ <sha>; a root commit diffs against the
  empty tree. Label is 'Commit <shortsha> — <subject>'. The sha is
  validated as bare hex (parseCommitDiffType) before it reaches any
  argv position, on top of --end-of-options.
- getGitDiffFingerprint: anchored to the sha alone (present/gone), NOT
  headSha — new commits landing mid-review don't change this diff and
  must not raise the staleness banner. A vanished commit (rebase + gc)
  flips present→gone and fires it honestly.
- getFileContentsForDiff: old side <sha>^ (null on root), new side <sha>.
- parseWorktreeDiffType learns worktree:<path>:commit:<sha> (the
  sub-type contains a colon, so it needs its own split) — commit review
  composes with worktree sessions like every other mode.
- vcs-core: the git provider owns commit:*; staging stays gated off
  (canStageFiles allowlist unchanged — nothing in a historical commit
  is stageable).
- Ask AI context: a commit-mode inspect string ('git show <sha>', with
  an explicit 'historical commit, not the working tree' note).

Also adds listCommitHistory for the upcoming /api/commits endpoint:
one --first-parent page from HEAD (before=<sha> continues at its first
parent, +1 fetch for an honest hasMore), with isHead / isRepoUser /
isPastBase flags. isPastBase is reachability from the base — computed
as the complement of 'rev-list --first-parent HEAD ^base', which is a
prefix of the walk, so the client's single divider is exhaustive. An
unresolvable base degrades to no divider (matches since-base's posture
on such repos).

* feat(review): GET /api/commits — linear history endpoint (Bun + Pi)

One page of the branch's --first-parent history (?limit=&before=),
served by listCommitHistory against the active diff's cwd (worktree-
aware via resolveVcsCwd) and the active base (so the divider tracks
the same baseline the review compares against, including the startup
origin/<default> upgrade).

Gated to plain local git sessions — PR / workspace / jj / p4 return
400, mirroring the client's commitsCapable gate. Both runtimes; the
route-parity test covers the pair.

commit:<sha> switching needs no endpoint changes: /api/diff/switch
already dispatches through runVcsDiff (the git provider owns the new
family), the epoch guard applies as-is, the sections sidecar correctly
stays since-base-only, staging 400s via the canStageFiles allowlist,
and baseRelevantDiffType keeps the behind-GitHub banner suppressed for
commit diffs.

* feat(ui): Commits panel view — the linear history rail

Third left-panel view: 'Git status | Commits | Tree'. The Commits view
is a pure commit list (short sha, subject, relative age compacted to
'2h', author only when it isn't the repo user, HEAD dot, a divider
where the branch meets the resolved base, 'Show more' paging). It
never becomes a file list: clicking a commit switches the diff to
commit:<sha> and the existing needsInitialDiffPanel flow lands the
center dock on the all-files surface; re-clicking the active commit
just re-focuses that panel.

Persistence: reviewPanelView gains 'commits' with NO diff coupling —
the view persists, the diff opens on the user's normal default until a
commit is clicked, and a sha is never persisted (it may not survive a
rebase). The sections⟺since-base coupling is untouched, but the
classic-diff→Tree snap in Settings/ReviewSetupDialog now only fires
when the current view is 'sections', so picking a default diff no
longer stomps a Commits preference. The load-time self-heal ignores
'commits' by construction (it keys on panelView === 'sections').

Gates: commitsCapable = plain local git session (no PR / workspace /
jj / p4, matching the server's /api/commits gate); the segment is
hidden elsewhere. Staging is inert in commit diffs on both ends
(STAGEABLE_DIFF_TYPES and the server allowlist never match commit:*).
Worktree sessions compose — the client parses
worktree:<path>:commit:<sha> and handleDiffSwitch prefixes as usual;
the rail refetches on worktree/base changes but deliberately not on
commit clicks, so paging state survives selection.

Accepted edges (v1): no j/k selection in the rail (each move would run
a full diff switch); toggling Commits→Tree keeps a commit diff active,
where the tree's diff-type dropdown marks nothing — the gitRef header
still names the commit.

* fix(review): self-review cleanups on the commit-list feature

- listCommitHistory: a repo with no commits yet (unborn HEAD) returns
  an empty page instead of null — the panel showed a 500-backed 'Retry'
  error where 'No commits' is the truth. Every other review surface
  already degrades gracefully on a commit-less repo; now this one does.
  Only a genuinely unanswerable repo (not a repo at all) stays null.
- Hoist the bare-hex sha rule into one BARE_HEX_SHA_RE shared by
  parseCommitDiffType and the before-cursor validation (was written
  twice).
- The 'all' diff case now uses the getEmptyTreeSha helper the commit
  mode extracted, instead of keeping its own inline copy.
- Drop CommitsPanel's isLoadingDiff prop — declared and passed but
  never used.

* feat(ui): commit cards, labeled groups, avatars — review round 1

Four items from the live review round:

- Toggle: Commits moves to the far right (Git status | Tree | Commits)
  and the segment text goes 10px → 12px (padding trimmed to keep three
  segments inside the 256px header).
- The bare '── origin/main ──' divider confused more than it explained.
  Replaced with two labeled groups: 'On this branch' (commits not yet
  reachable from the base) and 'In <base>' (shared history), each with
  a tooltip spelling out what the boundary means.
- Rows become overview cards: subject (2-line clamp) on top; a meta row
  with author avatar, author name (only when it isn't the repo user),
  a HEAD badge, short sha, and compact age. Active card gets the
  primary border/tint.
- Author avatars, reusing the PR machinery rather than reinventing it:
  parseRemoteUrl/parseRemoteHost classify the origin forge; the gh/glab
  invocation shape (incl. the --hostname self-hosted convention) and
  GitLab's relative-avatar absolutization rule are mirrored from
  pr-github/pr-gitlab. What could NOT be reused directly: PR avatars
  are keyed by platform login, which local commits don't have — so
  GitHub resolves via one repos/{o}/{r}/commits?per_page=100 call per
  session building an author-EMAIL → avatar map (unpushed commits by
  the same author resolve through it; verified live), and GitLab uses
  its per-email /avatar endpoint (capped, deduped). Everything is
  memoized per session and fails closed to the initials fallback — a
  repo with no remote, an opaque self-hosted host, or an unauthenticated
  CLI never delays or errors the commits endpoint.

The Avatar component moves out of PRCommentsTab into its own module,
now shared by the PR timeline and the commit cards. New shared module
commit-avatars.ts is vendored to Pi (vendor.sh) and the enrichment runs
in both runtimes. CommitListEntry gains authorEmail (resolver key) and
avatarUrl (server-enriched).

* ui(review): flush commit rows, prominent base boundary — review round 2

- Cards are gone: rows stack flush (no borders, no margins), hover tint
  only. Subject is strictly one line, ellipsized (no clamp/wrap).
- Author name always shown (was: only when it differed from the repo
  user), with the avatar, on the compact meta line under the subject.
- The base boundary is a prominent labeled rule ('— In origin/main —',
  foreground-weight lines) instead of the faint border under a group
  header; the 'On this branch' header above stays quiet.

* docs(adr): evaluate local stack-parent detection — scenario matrix, no build decision

Empirical stress-test of the merge-base-reachability algorithm across 9
scenarios plus this repo itself. Findings: the stacked-ness test is graph
truth (never lied); parent attribution has one content-harmless failure
(same-merge-base sibling labels) and one dangerous, provably graph-
undetectable one (branches pointing inside B's own history win and
silently hide the user's commits). Determination: only viable as a
suggest-and-confirm affordance with per-branch persisted choice — never
silent auto-defaulting. Decision to build deliberately left open.

* fix(ui): Commits view is session-only — never the opening view

Live-review catch: the panel toggle inherited the sections/tree
'last choice becomes the default' cookie write, so clicking Commits in
one session made the NEXT review open on the history rail instead of
the diff. Ruling: a review always opens on the persisted sections/tree
default; Commits is entered via the toggle each session.

- reviewPanelView narrows back to 'sections' | 'tree'; a stale
  'commits' cookie is treated as unset (self-heals to the default).
- The live view is now a session overlay in App state: selecting
  Commits flips it without touching config; selecting Git status/Tree
  clears it and persists as before. Single choke point in
  selectPanelView, so every toggle site inherits the rule.
- Settings loses the Commits segment (it configures the OPENING view,
  which Commits can no longer be). Spec updated with the revision.

* ui(review): the panel toggle is session-only — it never writes the default

Second live-review ruling on persistence: the header toggle shouldn't
persist ANY choice, not just Commits. Looking at another view mid-review
must not silently change what the next review opens on.

- selectPanelView becomes a pure session override (local state layered
  over the persisted value); no configStore writes from the toggle path.
- handleSwitchToSections drops its defaultDiffType write — that write
  only existed to keep the persisted view/diff pair consistent, and the
  toggle no longer persists a view. Settings and the setup dialog remain
  the only reviewPanelView/defaultDiffType writers and keep enforcing
  the sections ⟺ since-base coupling; the load-time self-heal still
  repairs conflicted pairs those writers may leave.
- Spec + settings-registry comments updated to name the new rule.

* feat(ui): commit description card + collapsed all-files for commit diffs

Clicking a commit now answers 'what and why' before 'which lines':

- New commitInfo sidecar (Bun + Pi, same mode-conditional shape as
  sections): when a commit:<sha> diff is active, /api/diff and
  /api/diff/switch carry the commit's full metadata — subject, multiline
  body, author (+ avatar via the session resolver), sha, age — from a
  single 'git show -s' (getCommitDiffInfo in review-core, body as the
  trailing format field so newlines survive).
- CommitDescriptionHeader heads the all-files surface: subject, author
  row, and the full body rendered as markdown via the same MarkdownBody
  the PR viewer uses. Long bodies scroll inside the card (max-h) so the
  diff keeps the viewport.
- Commit diffs open FOLDED: AllFilesCodeView gains defaultCollapsed —
  items seed collapsed at build time, the seed is part of fileSetKey
  (CodeView seeds once per instance, so a seed change must remount),
  and the collapse-all toggle state initializes to match. Every other
  diff mode keeps opening expanded; leaving the commit diff clears the
  card and the folded default together (the sidecar is absent).

Verified live through the compiled binary: switching to a real commit
returns subject/body/avatar, switching back to since-base clears it.

* ui(review): commit description scrolls with the diff — no pinned card

Round feedback: the fixed header with its own inner scrollbar felt
embedded; the description should read as the top of the document and
scroll away with the files.

- AllFilesCodeView gains leadingContent: rendered via a portal INTO
  CodeView's scroll container (absolutely positioned at content top,
  so it participates in the scrollable overflow), with its measured
  height fed into layout.paddingTop so the virtualized items start
  below it. CodeView stays the single scroll authority — no nested
  scrollers, no wrapper flex row.
- CommitDescriptionHeader shows the full body inline (no inner
  OverlayScrollArea). Only genuinely huge bodies (>24 lines / >1800
  chars) get a CSS max-height clamp with a fade mask and a Show
  more/Show less toggle — CSS clamping keeps the markdown intact, and
  the ResizeObserver feeds the expanded height back into paddingTop.
  Keyed by sha so the toggle resets per commit.

* ui(review): entering the Commits view auto-opens the HEAD commit

Toggling to Commits used to leave the previous mode's all-files diff on
screen — a rail full of commits next to content that belonged to none
of them. Now entering the view selects the HEAD commit (top of the
rail) so the center immediately shows its diff + description card.

Once per entry, ref-guarded: an already-active commit diff survives a
toggle round-trip, a user click supersedes it, and a failed switch
doesn't retry-loop. When entry races the first log fetch, the effect
fires as soon as the log lands.

* perf/ui(review): fast commit navigation — instant veil + skip context recompute

Rail clicks felt laggy and jumpy: the old diff sat on screen for the
whole switch round-trip and then snapped, and the switch itself was
doing a full getVcsContext recompute (branch + worktree + recent-commit
enumeration — three git walks) that a commit click can never invalidate.

- Same-cwd commit:<sha> switches skip the context recompute in both
  runtimes (the client keeps its existing context when the field is
  absent — long-standing contract). Measured through the compiled
  binary: commit switches now ~40-60ms server-side.
- The center dock shows an immediate 'Loading commit…' veil whenever a
  commit switch is in flight (or the Commits view was just entered and
  HEAD auto-select hasn't landed) — the stale previous diff never shows
  and the click reads as instant. Errors surface through the normal
  empty-state, never trapped under the veil.

* fix(ui): PR-994 review round — toggle/settings separation + commit-nav fixes (App)

The headline fix: the load-time settings repair guarded on the live
panelView, which now carries the session toggle override — so clicking
'Git status' once mid-review (saved prefs tree + a classic diff) hit
the healer and silently persisted defaultDiffType='since-base'. The
toggle must never be a settings writer, directly or through a repair
path. Keyed to persistedPanelView now; audited the remaining
configStore.set call sites — no toggle path can reach a write.

Also in this file, same review round:
- Commit-navigation veil gains a real predicate: it drops on a commit-log
  fetch error (rail shows Retry; center was stuck under the spinner
  forever) AND on a genuinely empty history (zero commits — the second
  stuck-veil trigger the automated review missed), while still covering
  the log-loading and pre-auto-select frames.
- handleSelectCommit composes the worktree prefix once and calls
  fetchDiffSwitch directly (the equality check and the switch previously
  used two different composition paths).
- commitsCapable/showCommitsPanel hoisted above the global keyboard
  handler; Cmd+F is no longer intercepted in the Commits view (no search
  input exists there — the browser find takes over instead of a silent
  no-op).

* fix(review): PR-994 review round — commit-log staleness, loading flags, avatar cap

- useCommitLog clears its cached list when the history context changes
  (worktree/base switched while the view was away): the HEAD auto-select
  acted on the stale rows and opened the previous context's commit. Same
  contextKey still keeps the cache (no empty flash on re-entry).
- The hook's cleanup now resets both loading flags: a generation-skipped
  finally never cleared them, leaving 'Show more' stuck disabled as
  'Loading…' after leaving mid-page.
- commit-avatars memoizes misses only for emails an attempt actually
  covered — the GitLab per-call cap (10) was recording every email past
  it as permanently unresolvable for the session. Regression test added.
- GitLab host detection tightened from a bare substring to a structured
  match (gitlab.com / gitlab.* / *.gitlab.*) — 'mygitlabproxy.example.com'
  no longer classifies.
- Collapse-all mirror re-derives from live item state after per-file
  toggles: with commit diffs seeding all-collapsed, expanding one file
  left the dock button on 'Expand all', and clicking it re-collapsed the
  file the user just opened.
- The commit description card element is memoized on commitInfo identity
  so the measuring ResizeObserver stops churning on every context render.

* fix(ui): entering the Commits view ends any open search session (self-review)

The Cmd+F guard from the review round only blocked NEW searches in the
Commits view; a search opened in Git status/Tree survived the toggle as
hidden-but-live state — the query kept matching against the commit
diff, marks kept rendering in the all-files body, Enter/F3 kept
stepping matches, and the input to see or edit the query didn't exist
anywhere on screen. handlePanelViewSelect (the single choke point every
toggle site routes through) now clears and closes search on entry to
Commits.

Also re-traced the rest of the round under self-review — veil terminal
states (log error/empty/loaded, switch failure, background refresh with
a commit active), the hook's cleanup-before-rerun flag ordering, the
avatar attempted-set on broken platforms, and both collapse-mirror
paths — no further findings.

* fix(ui): PR-994 round 2 — live rail freshness, non-destructive errors

- The rail now keeps itself fresh: while the Commits view is visible, a
  quiet 10s poll head-compares page 1 and adopts it only when history
  actually moved. The commit DIFF's sha-anchored fingerprint stays as
  designed (a historical commit never goes stale, so the banner
  correctly stays quiet) — but the rail no longer freezes while an
  agent commits in the background. Adoption bumps the fetch generation
  so an in-flight 'Show more' from the old history can't append stale
  rows, and transient poll failures disturb nothing.
- A page-1 fetch resets isLoadingMore: a refresh superseding an
  in-flight paging request skipped that request's generation-guarded
  finally, leaving 'Show more' stuck on 'Loading…' — rare before, a
  live race once the poll exists.
- Errors are non-destructive when a list is on screen: a failed page or
  background refresh renders as an inline row with Retry under the
  list instead of replacing the whole rail; the full-panel error state
  is reserved for an empty rail.

* perf/refactor(review): parallel GitLab avatar lookups; drop dead isRepoUser

- The ≤10 per-call GitLab avatar lookups run in parallel instead of
  sequentially — they sit on /api/commits' critical path, and serial
  subprocess spawns added seconds to the rail's first paint on
  multi-author GitLab histories (PR-994 round 2 nit).
- CommitListEntry.isRepoUser removed: v1 showed the author only when it
  wasn't the repo user; the live-review pivot to always-shown names left
  the field computed (one git config subprocess per page), serialized,
  and tested but never read, with a doc comment describing behavior the
  panel no longer has. Spec updated to match the revised row design.

* fix(ui): commit-poll adoption clears all superseded fetch state (self-review)

Adopting a new history from the background poll bumps the generation,
which strands any in-flight fetch's generation-guarded finally — the
same stale-flag class just fixed for isLoadingMore, but for isLoading
(slow first load overtaken by the poll) — and a lingering inline error
from the replaced history would otherwise sit under the fresh list.
Adoption now clears isLoading, isLoadingMore, and error together.

* fix(ui)/docs: PR-994 round 3 — worktree switches drop commit diffs; API docs

- handleWorktreeSwitch treats commit:<sha> as non-portable: every other
  diff mode recomputes meaningfully against the target worktree, but a
  commit diff is context-bound content (worktrees share one object
  database — the reviewer's claimed git error doesn't exist, verified
  empirically), so 'preserving' it just re-rendered the OLD context's
  commit byte-for-byte. It now falls back to the session default (same
  option-availability rule resolveInitialDiffType applies). Swept every
  other activeDiffBase/diffType carrier: the whitespace toggle,
  staleness refresh, and fetch-base deliberately recompute the SAME
  diff (valid for commits); handleBaseSelect and the base-picker row
  already exclude commit mode; job diff-context labeling and the
  guarded diff-type dropdown are display-only. This was the single leak.
- AGENTS.md (CLAUDE.md symlink): /api/commits row added to the Review
  Server table; /api/diff and /api/diff/switch now document the
  commitInfo sidecar and the commit:<sha> diffType family; the
  since-main section describes the three-segment session-only toggle
  and the never-persisted Commits view.

* docs: normalize arrow glyphs in the /api/commits table row (self-review)

* fix/refactor(review): PR-994 round 4 — rebase-safe paging, locale-proof ages, shared parsing

- Pagination is rebase-safe: a 'Show more' cursor from a history that was
  rewritten mid-session (rebase/force-push) still resolves in the object
  store but is no longer on the branch — paging from it walked the
  orphaned pre-rewrite chain for the ≤10s window before the freshness
  poll adopts the new history. listCommitHistory now ancestor-checks the
  cursor (merge-base --is-ancestor) and returns an empty terminal page
  for orphaned AND vanished cursors alike; the poll replaces the list
  moments later. Regression test rewrites history between pages.
- Ages are locale-proof: git localizes %cr via gettext ('vor 2 Stunden'),
  which the English-only compactAge regex silently couldn't shorten.
  CommitListEntry/CommitDiffInfo now carry committedAt (epoch ms, %ct);
  the rail and description card format it with the existing
  formatRelativeTime — compactAge and the third formatter variant are
  gone.
- The %x1f over-split repair (fixed head/tail fields, rejoined free-text
  middle) was triplicated across listRecentCommits, listCommitHistory,
  and getCommitDiffInfo; one splitCommitFormatFields helper (head/tail
  counts, tail=0 covers the multiline-body shape) owns the edge case.
- Avatar resets its broken-image state when src changes — latent today
  (every caller keys by identity), but it's a shared component and must
  be safe for callers that update src in place.

Left alone from this round: hashString/hashFingerprintPart duplication is
pre-existing, and the suggested cross-package import would drag node:path
into the browser bundle — the documented reason client mirrors exist.

* fix(review): PR-994 round 5 — merge-commit prompts, boundary-aware poll, auto-select guard

- Agent prompts for commit:<sha> diffs instruct the exact on-screen
  command: git diff <sha>^ <sha> (first parent), explicitly steering
  away from git show — whose combined-diff presentation for a MERGE
  commit renders a different (often empty) changeset than the one under
  review. Root-commit fallback noted. One site covers Ask AI and every
  launched reviewer (and Pi via vendoring); prompt test pins the command.
- The rail's freshness poll now adopts on boundary/base movement, not
  just a new head: an agent running git fetch advances origin/<base>
  while HEAD stays put, which re-partitions isPastBase — previously the
  head-only compare skipped adoption and the 'In origin/main' divider
  stayed stale until the view was reopened. Boundary is compared over
  the page-1 overlap window (a divider paged deeper than the probe can
  see re-syncs on the next full reload — accepted micro-edge). The
  reviewer's proposed trigger (the Fetch banner) is unreachable in
  commit mode; the external-fetch trigger is the real one.
- The HEAD auto-select never fires while any diff switch is in flight —
  hardening only: the claimed first-click overwrite isn't reachable
  (isLoadingDiff was never a dependency of that effect, and auto-select
  settles before a human can click), but the guard makes the invariant
  explicit.

* docs(adr): spec the pre-merge commits-view structure refactor

Three behavior-preserving moves scoped for PR #994 before merge, motivated
by the audit finding that every recent review-round bug lived at the
App.tsx <-> useCommitLog seam: (R1) unify the commits-session state
machine (poll, auto-select, veil) in one hook; (R2) lift the commit-rail
block out of the 1.6k-line review-core into commit-history.ts; (R3) share
the isSameCwdCommitSwitch predicate both runtimes inlined. Ownership
tables name what deliberately stays put; execution runs smallest-first
with full gates per commit.

* refactor(review): share isSameCwdCommitSwitch across runtimes (spec R3)

The ~10-line parse-and-compare predicate both /api/diff/switch handlers
inlined moves to review-core beside its inputs (parseWorktreeDiffType /
parseCommitDiffType — the canonical home for diff-type string logic);
both call sites collapse to one line. Unit tests cover plain, worktree,
cross-worktree, and non-commit-target cases. Behavior identical.

* refactor(shared): lift the commit-rail block into commit-history.ts (spec R2)

review-core.ts (1.6k lines) was absorbing the ~230-line Commits-panel
data layer it doesn't need to own: CommitListEntry/CommitHistoryPage/
listCommitHistory and CommitDiffInfo/getCommitDiffInfo move verbatim to
a new commit-history.ts, matching the package's per-concept split
(jj-core, pr-stack, commit-avatars). The commit:<sha> DIFF plumbing
(parseCommitDiffType, the runGitDiff/fingerprint/file-contents cases)
stays in review-core with the other diff types — it participates in the
dispatch; the rail does not.

Wiring: review-core exports its three parsing primitives (COMMIT_FIELD_
SEP, splitCommitFormatFields, BARE_HEX_SHA_RE — listRecentCommits still
uses them, so they can't move); package exports + vendor.sh gain the
module (vendored siblings import relatively, same as pr-provider →
pr-github); types.ts re-exports split by source; both runtimes import
from the new module. History/metadata test suites move to
commit-history.test.ts with the package-style per-file git harness;
commit-DIFF tests stay in review-core.test.ts. Behavior identical —
verified live through the compiled binary (/api/commits + commit
switch + commitInfo sidecar).

* refactor(ui): useCommitsView owns the whole commits-session machine (spec R1)

The list cache, freshness poll, HEAD auto-select, and center-dock veil
were split between useCommitLog and App.tsx — and every sync bug found
across three review rounds (stuck flags, stale auto-select, adoption
races) lived at exactly that seam. The auto-select effect and the veil
derivation move into the hook verbatim (renamed useCommitsView, git mv),
so the machine's invariants are locally checkable in one file.

App now supplies only what it owns — visibility, the active commit,
switch state, and onOpenCommit (its handleSelectCommit: the SAME path
user clicks take, so auto- and user-selection cannot diverge by
construction) — and consumes panel props plus veilActive. Net ~45 lines
of App.tsx's most delicate logic deleted.

One equivalence note: the hook's single "enabled" (showCommitsPanel &&
origin) now gates auto-select/veil where App used bare showCommitsPanel;
the two only differ when origin is unset, which implies no gitContext
(demo mode) and therefore showCommitsPanel === false — identical in all
reachable states.

Deliberately still in App (each guards a flow App owns): capability
gating, handleSelectCommit, the Cmd+F / worktree-fallback /
staleness-refresh / search-clear guards, and render wiring. Behavior
identical; spec: adr/specs/refactor-commits-view-structure-20260703.md.
2026-07-04 10:13:11 -07:00
Michael Ramos 795f381ebe feat(review): "All changes" git-status review view (#990)
* feat(review): "Since main" git-status review view + correctness fixes

Adds a composite `since-base` diff — merge-base(origin/main, HEAD) vs the
working tree, plus untracked — as the default code-review view, rendered as a
three-section "git status" panel (Committed / Changes / Untracked) with a
Sections|Tree toggle, a first-run setup chooser, and a "baseline behind GitHub"
fetch banner. Everything normalizes to one git patch, so the diff viewer,
annotations, and feedback path are unchanged.

New diff type wired through runGitDiff / fingerprint / file-content / context /
staging / agent-context in packages/shared + both server runtimes (Bun +
Pi mirror). Default flipped from `unstaged` to `since-base`; the old
DiffTypeSetupDialog is replaced by ReviewSetupDialog (view + default-diff
chooser, screenshots, Settings access).

Includes a reviewed batch of correctness fixes:
- P0: baseBehindRemote was permanently true (rev-parse missing --verify)
- fingerprint blind to quoted/unicode untracked paths (unquote)
- sections parser: record both sides of a rename; staged-delete wins over
  untracked (rm --cached collision)
- graceful degrade when merge-base can't resolve (trunk/no-remote repos)
- /api/fetch-base re-queries remote (narrow-refspec honesty)
- /api/diff/switch concurrency guard (diffSwitchEpoch) + draft rekey
- decouple remote-staleness probe from initialBase (Pi parity)
- keyboard file nav in the sections view; banner gated to base-relative modes

See adr/decisions/005-since-base-github-view-default-20260701-223706.md

* fix(review): since-base review-round hardening (11 fixes, both runtimes)

Addresses the multi-agent + PR-990 review findings:

- Sections rename parser split ` -> ` on the raw quoted porcelain token, so a
  filename containing ` -> ` tore into garbage and a dirty file could render as
  "Committed". Now quote-aware (splitPorcelainRename + unit tests).
- /api/diff/switch captured the epoch AFTER await req.json(), letting a
  slow-body older switch overwrite a newer confirmed one. Epoch now captured
  before any await; hideWhitespace committed only on win. Both runtimes.
- Unresolvable base (trunk / no origin/HEAD) no longer auto-defaults to a
  degraded since-base that hides committed work — getGitContext only offers
  since-base when the base ref resolves, so the default falls through to
  uncommitted. Fingerprint + file-content degrade to HEAD to match the diff.
- git rm --cached no longer yields two diff entries for one path (untracked
  files already in the tracked patch are dropped) — fixes wrong-file-open and
  j/k nav looping in the Sections panel; also fixes latent uncommitted/unstaged.
- Settings' "Default Diff View" list now preserves the sections<->since-base
  coupling (can't leave an invalid sections+classic pair).
- "Behind GitHub" banner only treats origin/* as fetchable; a bare local base
  ("main") is upgraded to its tracking ref at startup so Fetch can clear it.
- hashUntracked resolves untracked paths against the repo toplevel, not cwd, so
  a review launched from a subdirectory isn't blind to untracked edits.
- Agent review instruction now tells agents to enumerate/inspect untracked files.
- Terminology: one "Committed changes" label; the live "Since <base>" label is
  dynamic (matches the header); "Since main" kept only as product copy.
- Docs: CLAUDE.md endpoints/fields + since-base view section; ADR recap.

Verified: typecheck (all projects), bun test 1793 pass, and empirical repro of
the rm --cached dedup, trunk->uncommitted fallback, and rename parser.

* fix(review): restore "(PR view)" on the Committed-changes label (unify toward it, not away)

* fix(review): close since-base coverage gaps from PR-990 review round 2

- First-run setup no longer forces since-base on repos where it isn't available
  (base ref can't resolve). getGitContext omits since-base there; the first-run
  block now checks the same availability before resetting the default or showing
  the chooser, so committed work isn't silently hidden on trunk/no-origin repos.
- reviewBase now includes 'since-base', so changing the base while in the
  git-status view passes the selected base to /api/file-content — expandable
  context is fetched from the right merge-base (was falling back to default).
- Settings now mirror ReviewSetupDialog's coupling exactly: Sections ⇒ force
  since-base; Tree ⇒ leave the diff (Tree + since-base is valid and now
  saveable); classic diff ⇒ Tree; since-base diff ⇒ leave the view. Removes both
  coercions that previously discarded a supported preference.

* fix(review): PR-990 review round 3 — fingerprint/dedup/base-canonicalization

- Fingerprint now uses `git status --porcelain -uall`, so editing a file inside
  a brand-new untracked directory changes the fingerprint and the "Diff out of
  date" banner fires (was collapsed to `?? dir/` and hashed as unreadable).
- extractTrackedPatchPaths pairs `---`/`+++` and excludes only the file's KEYED
  path (new side, or old side for a pure deletion) — no longer drops a recreated
  rename-source file (`git mv a b && touch a`). Verified rm --cached still
  dedups to one entry and the recreated file still shows.
- resolveReviewBase canonicalizes a bare local default name ("main") to its
  tracking ref ("origin/main") on every call, so a client that loaded before the
  startup upgrade resolved can't revert the server to the stale local base on the
  next refresh/switch. Both runtimes (new Pi resolveReviewBase helper).
- Removed the dead activeDiffLabel prop chain (FileTree -> DiffTypePicker).
- ADR recap: documented the staleness-banner scope limitation (default base only).

Verified: typecheck (all projects), bun test 23/23 review-core+fingerprint pass,
empirical repro of the untracked-dir fingerprint, rm --cached dedup, and
rename-recreate cases.

* fix(review): PR-990 review round 4 — dedup content, staging gate, banner timing

- Dedup rewrite: instead of dropping the untracked side of a same-path collision
  (which hid content), drop the tracked DELETION block and keep the untracked
  working-tree content. `git rm --cached f` + edit now shows the new content, not
  a phantom deletion — still one entry per path (no dock/nav collision). New
  stripHeaderPath also strips git's trailing-tab metadata, so space-named files
  dedup correctly. Verified: rm --cached+modify shows content (1 entry),
  space-named (1 entry), rename+recreate still shows both.
- All-files (and the `a` shortcut on the focused file) now gate staging per-file:
  committed files in since-base mode are not stageable, matching SectionsPanel /
  FileTreeNode. Shared isPathStageable helper threaded via ReviewStateContext →
  AllFilesCodeView. Stops the confusing `git add` no-op that still flipped local
  staged/viewed state.
- /api/diff/switch now awaits recomputeBaseBehindRemote before building the
  response, so the "Baseline behind GitHub" banner reflects the new base
  immediately (no ~5s lag switching in, no stale banner switching away). Both
  runtimes.
- ReviewSetupDialog re-applies the recommended default only on first-run dismiss,
  not when reopened from the header menu (was snapping a mid-session diff back).
- SectionsPanel "N added" header now counts sidecar-staged files too, so it
  matches the staged dots on rows.

Verified: typecheck (all projects), bun test 1830 pass, empirical dedup repro,
both bundles build.

* refactor(review): reuse parsePatchPathToken in removeTrackedDeletions (self-review)

Drop the duplicated stripHeaderPath helper — the shared diff-paths
parsePatchPathToken already strips a/|b/ prefix + C-quoting + git trailing-tab,
and verifies the prefix instead of blindly slicing two chars. Documents the
binary-deletion edge (no --- line, so a binary rm --cached is not deduped).

* fix(review): PR-990 review round 5 — mixed-base + staging/sort/flicker nits

- Atomic base upgrade: the startup origin/* canonicalization swapped currentBase
  without rebuilding the patch, so /api/diff could advertise origin/main while
  the served hunks came from local main (mixed-base review on origin/HEAD-absent
  repos). Now rebuilds the diff for the new base and commits base+patch+ref+
  fingerprint together (only if no user switch happened); the fingerprint change
  makes the client's freshness poll pick it up. Both runtimes.
- isPathStageable: gate staging OFF when since-base is active but the sidecar
  hasn't loaded (was falling through to true, allowing a git-add no-op on a
  committed file).
- SectionsPanel: session-staged files now float to the top of Changes — the sort
  key was `false ?? stagedFiles.has(...)` which short-circuits; now ORs them.
- /api/diff/fresh: early returns now carry baseBehindRemote, so a snapshot change
  mid-probe no longer clears the "behind GitHub" banner for one poll. Both runtimes.
- Removed the now-dead activeLabelFallback prop from DiffTypePicker (FileTree
  stopped forwarding it in round 3).

Skipped (agreed): the fetch-base input-validation nit (no realistic attacker)
and the diff-switch TOCTOU (concurrent switches aren't UI-reachable).

Verified: typecheck (all projects), bun test 1830 pass, both bundles build.

* refactor(review): drop redundant staged-OR in SectionsRow (self-review)

item.staged (the grouping key) already ORs in stagedFiles as of the round-5
sort fix, so the row-level `|| stagedFiles.has(...)` is dead weight.

* fix(review): surface the startup base upgrade to already-loaded clients

Review round 6 fixes:

- The startup main -> origin/main upgrade re-baselined the freshness
  fingerprint, so a client that fetched /api/diff before the rebuild kept
  the old patch and every /api/diff/fresh probe reported fresh (the probe
  compares server state to itself, never to what the client renders). Now
  the fingerprint is only re-baselined when no client has loaded the
  pre-upgrade snapshot; otherwise the stale baseline trips the normal
  "Diff out of date - Refresh" banner. Bun + Pi.
- preserveFile refreshes (staleness Refresh, post-Fetch) now adopt the
  server's returned base — exactly the paths where the server may have
  canonicalized main -> origin/main; keeping the old name sent
  /api/file-content and Ask AI context against the wrong base.
- ReviewSetupDialog: clamp the fixed 800px height to the viewport so the
  dialog fits on small laptop screens.
- Docs: /api/diff/fresh response also carries baseBehindRemote/agentCwd.
- Documented the committed-deletion + untracked-recreation dedupe edge as
  accepted (code comment + ADR) — fixing it needs two same-path diff
  entries, which the path-keyed UI cannot represent.

* fix(review): stop the Git-status default from silently reverting to tree

Users with reviewPanelView=sections could keep opening in the tree view on
a stale diff type despite their cookie saying Git status. Three causes:

- The header Sections toggle persisted the view but not defaultDiffType,
  creating a conflicted pair (sections + non-since-base default) that every
  UI writer is supposed to prevent. It now couples the diff default like
  the setup dialog and Settings do.
- configStore's debounced POST /api/config could be lost when a session
  closed within 300ms of a change, leaving cookie and config.json split.
  Pending writes now flush on pagehide with a keepalive fetch.
- configStore.init() applies config.json over the cookie without the UI
  coupling, so a stale server value re-corrupted the pair on every load.
  The app now self-heals at load: if the view says sections but the diff
  default isn't since-base, it repairs the default (cookie + config.json)
  and switches the live session to since-base.

* fix(ui): keep the stage (+) button border visible on the active file row

The button's --border border has no contrast against the active row's 30%
primary tint, so it vanished on the selected row until hovered. Tint the
border with the row's primary color on active rows (both sections and tree
views); the button's own hover border still applies.

* ui(review): spell out the panel view toggle — 'Git status | Tree' text instead of icons

* fix(review): PR-990 review round 7 — export label, diff snapshot race, unicode paths

- exportFeedback describeDiff(): add the missing since-base case — every
  feedback export in the new default mode read "**Diff:** since-base".
- /api/diff GET: snapshot patch/base/ref/error BEFORE the sections-sidecar
  await and pass the pinned base into buildSectionsSidecar. The startup base
  upgrade landing mid-await could pair a rebuilt patch with sections grouped
  against the old base, with initialDiffServed still false so no refresh
  banner ever came. Bun + Pi.
- SectionsPanel: a session-staged untracked file now moves to the Changes
  section immediately (anticipating the server's next sidecar) instead of
  sitting in Untracked with a staged dot until refresh.
- unquoteGitPath: real C-style unquoting with octal (UTF-8 byte) escape
  decoding. JSON.parse rejects octal escapes, so non-ASCII names kept their
  literal \303\251 form — an untracked "café.txt" was silently ABSENT from
  the review (git diff --no-index could not access the quoted name) and the
  deletion dedupe Set lookup could never match. getUntrackedFileDiffs now
  unquotes ls-files output. Unit tests: octal decoding + end-to-end unicode
  untracked file in a real repo.
- review-core.test: worktree subtype round-trip now covers since-base and
  all (was 6 of 8).

Parked (deliberate): SectionsPanel/FileTree keyboard-nav dedup refactor.

* fix(shared): unquoteGitPath keeps literal unicode intact (self-review)

The byte-collector treated literal non-ASCII code units as single bytes,
which would mojibake headers synthesized by our own quoteGitPath
(JSON.stringify leaves unicode unescaped inside quotes — workspace-mode
prefixed headers round-trip through the same parser). Literal chars now
append as string code units (surrogate-safe); only octal escapes go
through the UTF-8 byte decoder.

* fix(review): PR-990 review round 8 — pre-staged toggles, explicit base, AI context race

- useGitAdd: session Set replaced with a tri-state override map folded over
  the sections sidecar; stagedFiles is now the EFFECTIVE staged set (sidecar
  + session stages - session unstages). Fixes pre-staged files: the first
  `a` press actually unstages (was a git-add no-op), the sidebar dot clears
  after a real unstage (was stuck via sidecar OR), and the All-files header
  agrees with the sidebar on load. Overrides reset whenever a fresh sidecar
  arrives (switch, preserveFile refresh, PR response) so stale session
  intent can't fight new porcelain truth. SectionsPanel drops its own
  sidecar OR; stagedCount = effective size.
- Explicit base picks are honored verbatim: the picker sends explicitBase,
  and the server permanently disables local-name -> origin/* canonicalization
  once set (the local/remote groups are distinct choices). The behind-GitHub
  banner also exempts an explicitly-picked local name — Fetch advances
  origin/*, so the banner would be un-clearable nagging. Bun + Pi.
- buildCurrentAiReviewContext(patch, base): GET /api/diff builds Ask AI
  context from the same served snapshot as the patch — the startup base
  upgrade could hand Ask AI a different changeset than the screen. Bun + Pi.
- recomputeBaseBehindRemote: capture remoteDefaultInfo once — a concurrent
  refresh nulling it mid-await threw. Bun + Pi.
- Revert stray "Status Update / Updated!" edit to adr/0001 (test debris
  swept into the round-2 commit).
- splitPorcelainRename comment corrected (porcelain v1 does NOT quote plain
  spaces; a name containing " -> " is ambiguous without -z) + ADR notes for
  the index-only-changes semantics and the rename edge.

* fix(review): FileTreeNode uses the effective staged set — round 9

Round 8 made stagedFiles the effective set (sidecar + session overrides)
and removed SectionsPanel's sidecar OR, but missed the same OR in
FileTreeNode's since-base row (sectionStaged). In the Tree fallback a
pre-staged file unstaged this session kept its dot and the next toggle
re-staged it. Grep-swept: this was the last surviving sidecar-staged OR.

* refactor(review): make the staged-display invariant unrepresentable

The round-8/9 bug class (sidecar staged flag ORed over the effective set)
existed because surfaces had a second staging source to reach for. Remove
it: stagedFiles is now a REQUIRED prop on SectionsPanel/FileTree/
FileTreeNode and the optional-prop fallback branches reading
sectionEntry.staged for display are deleted — a future surface cannot
reintroduce the OR because the pattern no longer exists to copy. The
sidecar type's staged field and AGENTS.md now document the invariant at
the point of temptation. Grep for display reads of .staged now hits only
useGitAdd (owner) and the sidecar builder (producer).

* refactor(review): self-review cleanups on the staged-invariant hardening

- orderFilesBySections: document why its snapshot .staged read is safe
  (only called at sidecar-fresh moments where snapshot = effective) and
  that it must not be reused mid-session — the one remaining display-side
  snapshot read the invariant sweep surfaced.
- FileTreeNode: drop the now-pure sectionStaged alias; use isStaged.

* fix(review): PR-990 review round 10 — header staging gate, fingerprint cap, single-writer coupling

- Single-file diff header now uses the per-path staging gate (canStagePath),
  closing the last ungated staging trigger: committed-only files in
  since-base offered a no-op Git Add that flipped local staged/viewed state.
  Full trigger inventory swept: App `a` shortcut, all-files `a` + header,
  SectionsPanel rows, FileTreeNode rows, single-file header — all six now
  per-path gated or group-gated; no context-menu staging exists.
- Fingerprint circuit-breaker: the freshness poll's `git status --porcelain
  -uall` degrades permanently (per cwd, per process) to collapsed -unormal
  once its output exceeds 2MB — a forgotten node_modules/ no longer burns
  CPU every 5s for the whole session. Costs untracked-dir edit sensitivity
  only on such repos; one possibly-spurious staleness banner at the switch.
- The sections ⟺ since-base coupling now has a single writer:
  setReviewPanelView/setReviewDefaultDiffType in @plannotator/ui/config.
  All five call sites (setup dialog x2, Settings x2, header toggle,
  first-run reset, self-heal) converted; grep for direct writes of either
  setting now hits only reviewView.ts.
- /api/diff/switch responses pass snapshot args to the AI context builder
  (both branches, Bun + Pi) — correct today, now robust against future
  awaits between the epoch check and the response.
- Docs: explicitBase in the /api/diff/switch body.

Parked per discussion: SectionsPanel/FileTree nav+search+footer dedup
(extract when the commit-list view adds a third panel), querySelector row
measurement, fetch-base stderr passthrough (standing decision), Pi
hasAgentLocalAccess (pre-existing follow-up list).

* fix(review): PR-990 review round 11 — subdirectory launches, escape decode, settings note

- Repo-root-relative patch paths now resolve against the git toplevel
  everywhere they meet a filesystem path or pathspec, via a shared
  resolveRepoToplevel helper: file-content working-tree reads (hunk
  expansion returned null from a subdirectory launch) and gitAddFile/
  gitResetFile (stage/unstage failed with pathspec errors). Both bugs
  pre-existed for uncommitted/unstaged; since-base made them the default
  experience. The two existing inline toplevel resolutions (untracked
  diffs, fingerprint) now use the same helper. Shared code — Pi inherits
  via vendoring. Real-repo subdirectory tests for both.
- unquoteGitPath decodes \uXXXX (JSON.stringify emits it for control
  chars without a short escape; our synthesized workspace headers
  round-trip through this decoder). Malformed \u stays literal. Tests.
- Pi explicitBase guard matches Bun byte-for-byte on empty-string base.
- Settings Git tab notes when the CURRENT repo can't serve the Git-status
  view (base ref unresolvable) instead of letting the preference look
  silently broken — it's a global preference, so the options stay.

Same-class items left parked (pre-existing, untouched by this PR):
open-in root resolution and code-nav file reads from subdirectory
launches — on the follow-up list with the Pi divergences.

* fix(review): PR-990 review round 12 — per-client freshness, keyboard-operable row controls

- Freshness is now judged PER CLIENT: every patch-carrying response
  (/api/diff, /api/diff/switch, pr-switch, pr-diff-scope) includes
  snapshotId (the server's draftKey), and the client echoes it on
  /api/diff/fresh probes. A mismatch reports stale for THAT client
  regardless of the VCS fingerprint. This fixes the round-12 finding —
  reloads/second tabs after the startup base upgrade got a permanently
  bogus staleness banner from the shared pre-upgrade baseline — and
  deletes the round-6 conditional re-baseline hack entirely (the
  fingerprint recaptures unconditionally again; the old client's banner
  now comes from its snapshot mismatch, not a deliberately stale
  baseline). Also gives unfingerprintable modes (P4, PR layer) snapshot-
  level staleness for free. Bun + Pi + client hook.
- StageControl and ViewedControl (which had the identical gap) are
  keyboard-operable: tabIndex + Enter/Space activation + focus outline.
  They're spans inside the row <button> (nested real buttons are invalid
  HTML), so they need their own focus stop; the a/v shortcuts remain the
  power path.
- Docs: snapshotId on /api/diff, ?snapshot= on /api/diff/fresh.

* fix(review): PR-990 review round 13 — re-key snapshots on scope switch, PR-tab refresh

- The PR scope switch now re-keys draftKey (= snapshotId + draft storage
  key) at BOTH commit points, unconditionally — matching every other
  snapshot commit site. The full-stack branch previously kept the layer
  patch's key: stale layer tabs never got the banner after a cross-tab
  scope switch (the exact case snapshotId exists for), and full-stack
  drafts collided with layer drafts (pre-existing). The layer branch's
  !layerPatchIncomplete conditional is gone — it only stayed consistent
  because full-stack never re-keyed. Invariant now: every currentPatch
  commit is followed by a re-key. Bun + Pi.
- Refresh works for any stale PR tab: re-selects the CURRENT scope
  instead of no-opping for layer (only full-stack could go stale in the
  fingerprint-only world; snapshot mismatch changed that). Accepted
  residual (documented in code): after a cross-tab PR switch, refresh
  updates the patch but not prMetadata — the scope endpoint doesn't
  carry it, and the full-rehydrate refactor isn't worth the two-tab edge.

* fix(review): stale incomplete-layer Refresh uses the non-blocking upgrade path (self-review)

Round 13 made Refresh re-select the current scope for stale PR tabs; for
an INCOMPLETE layer patch that POST triggers the server's local recompute,
which can park for minutes behind checkout warmup — and
handlePRDiffScopeSelect renders the full-screen switch overlay the whole
time. Route that case through handleLoadFullDiff (same POST, progress
notice instead of modal), which exists for exactly this slow path.

* fix(review): round 14 — composite snapshot id, fully-pinned /api/diff, "All changes" label

- snapshotId is now content hash + diff type (+ PR scope), built by a
  single currentSnapshotId() helper used at every response site and the
  freshness compare. A cross-tab MODE switch with a byte-identical patch
  (layer vs full-stack on a single-PR stack) now flags old tabs; the base
  is deliberately excluded so a same-commit main -> origin/main
  canonicalization stays banner-silent (round-12 noise-avoidance kept).
  draftKey stays a pure content hash — drafts survive content-identical
  round-trips. Bun + Pi.
- GET /api/diff pins ALL served fields (diffType, hideWhitespace,
  prDiffScope join patch/base/ref/error/snapshotId) and the sidecar + AI
  context builders take the pinned type instead of reading globals — a
  concurrent tab switch during the sidecar await can no longer produce a
  since-base patch labeled with another mode. Bun + Pi.
- First-user feedback: the since-base label is now plain English —
  "All changes since origin/main" (dynamic, follows the picked base);
  dialog/Settings short form "All changes"; feedback exports match.
  uncommitted reverts to "Uncommitted" where it had borrowed "All
  changes", so the two stay distinguishable side by side.

* docs(review): finish the Since-main -> All-changes terminology sweep in comments (self-review)

* fix(review): PR-990 review round 15 — base-revert race, freshness reset, unstage regroup

- resolveReviewBase gains a second, probe-independent rule: a non-explicit
  echo of the bare local name of the CURRENT origin/* base stays on the
  tracking ref. The existing rule keys off remoteDefaultInfo, which comes
  from a second network probe that can lag the startup upgrade by seconds;
  in that window a diff-type/whitespace switch echoing "main" committed
  the session back onto the stale local branch and set baseEverSwitched,
  permanently blocking the upgrade. Bun + Pi.
- useDiffFreshness resets stale/dismissed state on snapshotId too — since
  round 14 a new snapshot can reuse identical patch text with a different
  id, and the old banner state wrongly carried over until the next poll.
- SectionsPanel: unstaging a PRE-staged add moves it back to Untracked
  (mirror of the round-7 stage regroup). Detection deliberately uses the
  sidecar's snapshot flag to recognize "was pre-staged"; staged
  modifications stay in Changes, staged renames remain a refresh-heals
  edge.

* fix(review): PR-990 review round 16 — rename staged count, guarded fetch-base replay

- "N added" no longer double-counts staged renames: the sidecar truthfully
  marks BOTH porcelain sides staged, but an above-threshold rename renders
  as ONE file — the hidden old path inflated the count and left a phantom
  effective-staged entry that unstaging the visible row couldn't clear.
  The client now filters sidecar-staged paths to rendered files, which is
  correct in both patch shapes (below-threshold renames render delete+add
  as two rows and both sides pass the filter). Client-only; the sidecar
  stays faithful to porcelain.
- The fetch-base completion only replays the diff refresh if the user's
  diff type/base selection is unchanged since the click — a slow fetch no
  longer yanks the review back to the view captured at click time.

Declined with reasons (in review thread): first-run persist-before-confirm
(approved forced default, second flagging), explicit-base flag ordering
(early-set preserves user intent; commit-after-win would leave picker and
server disagreeing), base-picker revert target (near-unreachable compound
race, deferred).

* fix(review): PR-990 review round 17 — find origin/main on feature-only clones

getDefaultBranch's chain (origin/HEAD -> local main -> blind "master")
skipped the fetched remote-tracking ref entirely. On checkouts with no
origin/HEAD symref and no local main/master — CI checkouts, `clone
--branch feature`, extra worktrees — it guessed "master", the base
didn't resolve, getGitContext suppressed since-base, and the flagship
"All changes" view silently disabled itself for the whole session even
though origin/main was fetched and diffable. The startup upgrade can't
rescue it either (its guard reads the bogus "master" as a deliberate
non-default base).

Chain is now: origin/HEAD (verified) -> origin/main -> local main ->
origin/master -> "master" — remote-tracking refs preferred, matching the
function's stated prefer-upstream intent. Shared core (Pi inherits);
real-repo test reproducing the exact clone shape.
2026-07-04 09:53:49 -07:00
Michael Ramos 9bded34324 chore: bump version to 0.21.4 v0.21.4 2026-07-01 11:32:47 -07:00
Michael Ramos 17b862339d chore(test): use loadable image URLs in the LaTeX/media fixture
The <img>/<picture> examples pointed at example.com placeholders that render
broken. Point them at picsum.photos / placehold.co so a manual run actually
shows images loading (and still proves the tags parse as media, not text).
2026-07-01 11:26:45 -07:00
Michael Ramos 16db952000 fix(ui): stop unclosed math from pairing with a stray delimiter in a later code fence
The unclosed-$$/\[ guard scanned ahead to EOF for the closing delimiter, so an
unterminated opener could match a $$ (or \]) that appears far below - e.g. inside
a code fence - and swallow every heading/paragraph in between into one broken
math block. The scan now stops at a blank line: real display math has no blank
line before its close, so a blank both confirms "unclosed" and keeps the search
from reaching distant delimiters.

Adds a regression test and a manual LaTeX + media test fixture.
2026-07-01 11:23:18 -07:00
Michael Ramos cd934e6de2 fix(ui): stop stray math delimiters from mangling plan/PR text
Two math-parser regressions from #878, both the same "greedy delimiter" class
as the <video> fix in ec551c83:

- Unclosed $$ / \[ swallowed every following block to EOF - a stray delimiter
  or informal "$$100k" hid headings/paragraphs from the TOC and made them
  unannotatable. The block parser now scans ahead for the close without
  committing; with no close it falls through and treats the line as ordinary
  text, matching the unclosed-HTML-tag policy.
- Inline $A$B where a digit abuts the closing $ ("$5-$10", "$50,000-$100,000",
  "$5/mo and tier B is $10/mo") rendered currency amounts as KaTeX. Left literal
  now - real inline math never abuts a digit across the closing delimiter.

Adds regression tests and updates the test that codified the old
swallow-to-EOF behavior as intended.
2026-07-01 11:15:55 -07:00
Michael Ramos d06b1a337a fix(review): scope PR description/comment notes to their PR
The two prose-annotation stores added in #981 carried no PR identity, so after
an in-place PR switch they still rendered, counted, and exported against the new
PR - leaking one PR's notes into another PR's agent feedback and GitHub/GitLab
review body. Stamp each note with the active prUrl at creation (mirroring
CodeAnnotation) and filter display/export/count to the active PR via a shared
proseAnnotationMatchesPr helper. Notes are never discarded - the full arrays and
the draft keep every PR's notes, so a switch back re-reveals them.
2026-07-01 10:57:38 -07:00
Michael Ramos ec551c83af fix(ui): stop unclosed <video>/<picture> from swallowing the document
An unclosed or self-closing <video>/<picture> ran the HTML-block scanner off
the end of the document, funneling every following heading/table/code block
into one opaque html block (gone from the TOC, unannotatable). The scanner now
only extends the block when it actually finds the matching close tag; otherwise
it keeps the block to the opening line. Also fixes multi-line <img> (was
truncated to a bare "<img" fragment) and allowlists srcset/media/sizes so
<picture>/<source> and responsive <img> render instead of rendering inert.

Reachable via GitHub PR description/comment media (#981) and plannotator
annotate on any .md file.
2026-07-01 10:57:38 -07:00
Devin d39bc871b7 feat(ui): render markdown math (#878)
* feat(ui): render markdown math

* feat(ui): support more markdown math delimiters

* feat(ui): renders spaced dollar-delimited TeX support

* chore(deps): update katex

* Update bun.lock

chore(deps): update

chore(deps): update

chore(deps): update

chore(deps): update

chore(deps): update

* feat(highlight): highlight math

* feat(ui): support formule & text highlight together

* fix(ui): make rendered math drag-selectable for annotation

Drag-selecting an inline formula did nothing: the browser normalizes the
selection focus to offset 0 of the following text node, so the strict
anchor/focus check in selectionHasNonMathContent treated every inline
drag as "mixed prose+math" and bailed without claiming the event. Control
fell to web-highlighter, which excludes .katex/.math-annotatable, so no
annotation was created and the selection just collapsed.

Drop the anchor/focus check and rely on the existing clone-and-strip logic
(leftover text => genuinely mixed), plus a guard for selections wholly
inside one formula. Add a regression test that reproduces the real
spilled-endpoint drag, which the prior empty-selection tests missed.

Also paint a preview highlight the moment a formula's toolbar/popover
opens (stripped on cancel, committed on submit) and show cursor: pointer
on hover so formulas read as annotation targets.

* chore(ui): reconcile lockfile and math styles after rebase onto main

Rebase reconciliation, not new behavior:
- Re-add katex to bun.lock. The lockfile conflicted on every replayed
  commit and was resolved to main's copy throughout; regenerate it here so
  package.json (katex) and the lockfile agree again.
- Move the math annotation styles (.math-inline-annotation /
  .math-block-annotation) into packages/ui/theme.css. main relocated all
  annotation-highlight rules there to share them with the code-review
  description annotations; the PR had added the math variants to the old
  editor/index.css location.

* test(ui): lazy-load highlighter hook so DOM-less bun test skips cleanly

The math-annotation test statically imported useAnnotationHighlighter, which
pulls in @plannotator/web-highlighter — a UMD bundle that reads `window` at
module-eval time. Under the default `bun test` (no DOM), that threw a
ReferenceError on import, failing CI before the skipIf(!hasDom) guards ever
ran. Import the hook lazily, gated on hasDom, so the file loads and its tests
skip cleanly in CI while DOM_TESTS=1 still exercises them.

* fix(ui): stop display math from swallowing the document

The `$$`/`\[` block parser only recognized the closing delimiter when it was
the last non-space text on its line. A line like `$$E=mc^2$$.` (trailing
period) or `$$E=mc^2$$ where E is energy.` therefore looked like an
unterminated opener and consumed every following line until the next fence or
EOF — silently hiding the rest of the plan.

Detect the closing delimiter anywhere on the line and re-process any trailing
text as its own line, so the equation renders and the following content is
preserved instead of dropped. Same fix for the multi-line closing path and the
`\[ ... \]` branch. Adds parser regression tests (these run in CI; no DOM).

---------

Co-authored-by: ishowman <ishowman@users.noreply.github.com>
Co-authored-by: Michael Ramos <mdramos8@gmail.com>
2026-07-01 10:48:53 -07:00
Michael Ramos e3de938914 feat(review): PR Overview panel + description/comment annotations + media (#981)
Combine the PR Summary/Comments/Checks tabs into one PR Overview panel, then
make the description and comments annotatable and render their media.

- PR Overview panel (one sidebar entry) + comment UI (avatars, filters, hide
  bots, live context, responsive stacking).
- Annotate the PR description (select → comment) and PR comments (Annotate
  button), with Ask AI; notes show in the Annotations sidebar and ship to the
  agent.
- Split/Unified diff toggle relocated into the dock tab strip.
- Render images + video in descriptions and comments (raw HTML + markdown),
  capped to the card so nothing bleeds.
- Review-flow fixes: copy-all feedback, prose-only feedback preamble, no image
  control on prose notes, GitHub review-body seeding; stronger review trailer.
- Add Claude Sonnet 5 as the default Ask AI model.

No server, endpoint, or Pi-runtime changes.
2026-06-30 23:05:43 -07:00
Michael Ramos ce5afdf94a feat(review): add "Copy agent instructions" for external review comments (#983)
Mirrors plan mode's agent-instructions feature in code review. Adds
buildReviewAgentInstructions() — the clipboard contract teaching an
external agent to read the diff via /api/diff and POST line/file/general
scoped comments to /api/external-annotations (accurate against
transformReviewInput). Wires a gated "Agent Instructions" item into the
review header menu (shown only in a live session).

Also drops the now-confusing cross-reference to code review from the
plan instructions module.
2026-06-30 11:32:21 -07:00
Michael Ramos 819b14c0d7 fix(review): show pending agent job launches (#980) 2026-06-29 21:43:19 -07:00
Michael Ramos f9ff55bfe0 feat(annotate): show annotation count badge in plan/annotate header (#979)
The plan-review and annotate apps share AppHeader, whose annotations
toggle had no count badge — unlike the code-review app's equivalent
button. Add a numeric badge (capped at 99+) driven by the existing
feedbackAnnotationCount, so both surfaces now surface how much feedback
is pending. Closes the parity gap with the code-review header.
2026-06-29 15:12:43 -07:00
Michael Ramos 1eee3eb313 chore(vscode): bump extension to 0.16.7 (clipboard/keyboard bridge #970, data-dir #795) 2026-06-28 12:51:07 -07:00
Michael Ramos ec8342c34a chore: bump version to 0.21.3 v0.21.3 2026-06-28 11:37:37 -07:00
Michael Ramos 8f77b4b0ad fix(review): default Ask AI per-file chat groups to expanded
The per-file Ask AI chat groups tracked an expandedFiles set that started
empty, so every file's chat rendered collapsed and had to be opened by hand.
Invert to a collapsedFiles set (default expanded) so chats are visible
without a click; manual collapse still works and sticks.
2026-06-28 11:33:04 -07:00
Michael Ramos 3467599167 fix(review): clear selection on out-of-scope sidebar annotation click
After an in-place PR switch, stale cross-scope annotations remain listed in
the sidebar. Clicking one routed through handleNavigateToAnnotation, which
silently returned for out-of-scope/deleted annotations, so the click did
nothing. Deselect instead, so the click gives visible feedback.
2026-06-28 11:33:04 -07:00
Michael Ramos a4a6b59d76 fix(ai): kill codex app-server if the initialize handshake fails
Previously a spawned app-server that completed spawn but stalled or timed
out on the JSON-RPC initialize handshake was left alive (_alive=true).
ensureThread() then saw a live process, skipped re-spawn, and every
subsequent Ask AI query hung on the uninitialized handshake until the idle
timer fired. Kill the process on initialize failure so the next query
spawns a fresh one.
2026-06-28 11:33:03 -07:00
renovate[bot] 903290e451 chore(deps): update github actions (#791)
Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
2026-06-28 11:08:06 -07:00
Michael Ramos cb6667e991 fix(ai): drive Codex Ask AI via codex app-server (#971)
* fix(ai): drive Codex Ask AI via app-server + answer-first review prompts (#971)

Codex Ask AI previously ran via @openai/codex-sdk (codex exec), which forces
approval_policy=never and breaks in enterprise-managed Codex environments that
ban it (#971). Replace the transport with a long-lived 'codex app-server'
process over JSON-RPC.

- New provider packages/ai/providers/codex-app-server.ts (registered as
  'codex-sdk' to preserve cookie/agents.ts/UI-gate); omits approvalPolicy so
  Codex resolves the user's + managed policy, pins read-only sandbox, and
  surfaces interactive approvals through the existing PermissionCard.
- Delete codex-sdk.ts and drop the @openai/codex-sdk dependency (and its 6
  prebuilt platform binaries); gate registration on 'which codex'.
- SessionManager: additive, optional dispose?() hook to kill the spawned
  process on evict/remove — a no-op for Claude/OpenCode/Pi (they don't
  implement it).

Also rework the Ask AI prompts (all providers, separate from the transport):

- Every mode now instructs the agent to answer the user's message directly and
  not launch an unprompted review of the context.
- Code review stops pasting the whole diff for git-reproducible diff types and
  instead tells the agent how to inspect it (git diff <base>..HEAD, three-dot
  for merge-base); non-git/PR/workspace types still paste.
- Claude gains the Bash tool so it can run git (still gated by approvals).
- The UI passes diffType/base (session) and what the user is viewing (per
  question) into the context.

Verified: full typecheck, full test suite (101 ai tests), and a live
end-to-end smoke against codex app-server.

* fix(review): pin AI approval card above the input/model bar

Render pending approval cards just above the input + provider/model bar in both the document chat (DocumentAIChatPanel) and code-review AI tab, instead of at the top of the scroll, so the user sees them where they act.

* fix(ai): harden Codex abort/cancel + add Ask AI Stop button

Addresses code-review findings on the codex app-server provider:

- turn/interrupt was sent as a notification (no id) so Codex ignored it and
  abort never took effect. It's now a proper JSON-RPC request.
- Filter turn events/approvals by turnId and reject an aborted turn's
  stragglers, so a stopped turn can no longer leak output into — or
  prematurely finish — the next turn (ask-stop-ask race).
- Guard listeners by query generation and end the drain loop on the abort
  signal, so a superseded/stopped turn can't touch the live one and abort
  returns promptly instead of waiting for turn/completed.
- Handle abort during startup: once the turn id is known, interrupt it
  instead of running it in the background.
- Add a sendAndWait timeout so a stalled (alive-but-unresponsive) process
  errors instead of hanging forever.
- Drain stderr (stdio 'ignore') to avoid a pipe-buffer deadlock.

Also add a Stop button to both Ask AI surfaces (plan/annotate
DocumentAIChatPanel and code-review AITab via ReviewSidebar). It replaces
Send while streaming and calls the hook's abort -> /api/ai/abort ->
session.abort(); the hook already exposed abort but nothing surfaced it.

* feat(ai): drive Codex models + reasoning levels from model/list

- Codex provider fetches model/list at startup (throwaway app-server, like
  Pi/OpenCode) and populates the real models plus each model's actual
  supportedReasoningEfforts + defaultReasoningEffort. Replaces the hardcoded
  model list and the static AI_REASONING_EFFORTS (which mislabeled xhigh as
  'Max' and omitted minimal).
- AIProviderBar + AIConfigBar now show the selected model's real efforts and
  hide the control when a model reports none. xhigh is shown verbatim.
- Fix: the Stop button now also appears in the populated code-review chat
  state (a prior edit missed the second GeneralInput due to indentation).

* fix(ai): scope Claude Ask AI Bash to read-only git; clear stale approval cards on Stop

- Claude Ask AI no longer auto-allows bare Bash (which ran arbitrary shell
  with no Allow/Deny prompt). Replace it with scoped read-only git rules
  (Bash(git diff:*), show, log, status, rev-parse, merge-base, ls-files).
  git reads auto-run so the agent can inspect large diffs itself; any other
  command (write git, arbitrary, or injected compound) falls through to the
  permission card. Keeps the git-inspect approach (large diffs don't fit in
  the prompt) while closing the auto-exec hole.
- useAIChat.abort() now drops still-undecided permission cards: abort cancels
  them server-side, so leaving them visible was a dead Allow/Deny.

* feat(ai): code-review Ask AI shares the agent-review prompt machine, delivered as user messages

Code-review Ask AI built its own diff description (gitInspectInstruction) in
the system prompt from just diffType+base, which was wrong for full-stack,
hide-whitespace, untracked files, and PR worktrees. Replace it with the same
machine the review jobs use, delivered on the user's messages.

- Server: buildCurrentAiReviewContext() reuses buildAgentReviewUserMessageForTarget
  (contextOnly) for the current view and ships it as aiReviewContext in every
  diff payload (/api/diff + switch/PR/scope). Mirrored in the Pi server.
- Client: review-editor latches aiReviewContext onto each question via the pure
  buildReviewContextPreamble (packages/ui/utils/aiPrompt.ts) and buildDefaultPrompt
  — full block on the first message / when the view changes (incl. after a
  provider switch via the !sessionId fresh-session check), a short reminder
  otherwise (never re-pastes a large diff).
- context.ts: delete the duplicate gitInspectInstruction; code-review system
  prompt is now role-only. Providers untouched (provider-agnostic user message).
- Tests: machine scenario gaps (plain PR, full-stack default, PR-worktree
  origin/<base> + stale-main warning, untracked mention, jj-evolog, workspace
  lines); composition (first/reminder, command/pasted, preamble ordering).

* fix(ai): agentic remote-PR context, UTF-8 stream decode, real Stop on supersede/disconnect

Addresses review findings on the Ask AI prompt work:

- Remote PR without a confirmed local checkout no longer gets URL-only. The
  agent is told it's in a PR worktree that's being prepared, to verify the PR
  files exist before relying on them, and to diff with git diff origin/<base>...
  HEAD (URL fallback). Inform + trust the agent rather than pasting. Shared
  machine, so review jobs get the same framing.
- Decode Codex stdout with a streaming TextDecoder instead of per-chunk
  toString(), so multi-byte UTF-8 split across chunks no longer corrupts into
  U+FFFD (matches the Pi provider).
- Stop now actually stops the server turn: ask() awaits /api/ai/abort when a
  new question supersedes a streaming one (awaiting avoids racing the new query
  into session_busy), and the /api/ai/query SSE stream gains a cancel handler
  so tab-close/navigation aborts the turn too. Both reuse the existing
  per-provider session.abort(); factored a shared postServerAbort helper.

* fix(ai): per-model reasoning effort, shared PR-checkout readiness, Stop-then-ask race, type hole

- Reasoning effort is now tracked per model (a map keyed by model) instead of
  one global value, so switching to a model that doesn't support the prior
  level (e.g. xhigh) no longer posts a stale/unsupported effort that Codex
  rejects. Each model keeps its own level; nothing leaks across.
- Extract resolvePoolCwd into packages/shared/worktree-pool.ts (ready/pending/
  absent) and use it from both servers' resolvePRLocalCwd so the readiness rule
  can't drift. Fix the Pi Ask AI helper to ready-check like Bun, so a warming
  PR checkout no longer claims 'checked out at PR head' and misdirects the diff.
- Stop-then-ask no longer races into session_busy: the abort promise (from Stop
  or a superseding question) is stashed and the next ask() awaits it before
  sending. Stop still kills the turn instantly; a follow-up just waits the one
  abort round-trip.
- Declare aiReviewContext on the initial /api/diff response type.

* fix(ai): surface real Codex error messages, skip transient retries, auto-deny permission escalations

Validated against codex-rs:

- Codex's ErrorNotification nests the text at params.error.message
  (TurnError.message); we read params.message → always 'Unknown error'. Read
  the nested field (top-level fallback) so auth/usage-limit/stream failures show
  their real, actionable text.
- Skip transient error notifications (willRetry=true): Codex retries on its own
  and the turn continues, so surfacing them flashed a spurious failure before
  the real answer.
- Handle item/permissions/requestApproval: respond {permissions:{}, scope:'turn'}
  — byte-for-byte Codex's own cancel response (codex_delegate.rs) — instead of
  'Unsupported request'. Ask AI is read-only, so denying the escalation lets the
  turn continue sandboxed rather than failing. Interactive grant deferred.

* refactor(ai): share AI provider/model config in one hook; aggregate paginated model/list

- Extract useAIProviderConfig (packages/ui/hooks): one home for provider/model/
  reasoning-effort selection — initial state, auto-resolve on capabilities load,
  per-model effort (no leak across models), and persistence. Both the plan and
  code-review apps now call it and only compose the session reset (the hook can't
  own reset without a cycle through useAIChat). Plan editor adapted to the
  effect-based resolve (adds aiDefaultProvider state). This fixes the plan
  editor's stale-effort-on-model-switch bug by construction and stops the two
  apps' copies from drifting again.
- fetchModels now follows model/list's nextCursor and aggregates every page into
  one list, so larger model catalogs aren't silently truncated (with a page
  guard against a misbehaving cursor).

Skipped per discussion: legacy v1 approval methods (we're a v2 client).

* feat(agents): per-agent review default; bound Codex model discovery so it can't stall the AI panel

- The selected review profile is now tracked per review engine (claude/codex/
  cursor/opencode) instead of one flat global value, so each agent keeps its own
  review default. Public hook API (reviewProfileId/setReviewProfileId) is
  unchanged — getter derives the current engine's value, setter writes it — so
  AgentsTab needs no changes. One-shot migration seeds every engine with any
  existing flat pick. Adds parseReviewProfileByEngine + tests.
- fetchModels (Codex model discovery) now uses a short 6s timeout for its
  initialize + model/list RPCs instead of the 30s default. /api/ai/capabilities
  awaits model discovery, so an installed-but-unauthenticated codex could
  otherwise block the AI panel for ~30s; it now falls back to the static model
  list fast. Authed codex is unaffected (discovery completes in tens of ms).
2026-06-28 09:58:09 -07:00
Michael Ramos 82acc4b0aa fix(cli): print per-subcommand help instead of launching the UI (#974)
* fix(cli): print per-subcommand help instead of launching the UI

`plannotator review --help` (and other subcommands) fell through to their
command branch because only top-level `--help` was handled. For `review`,
`--help` was parsed as a non-URL positional, dropping into local review
mode and opening a browser tab. When Claude Code probes the CLI with
`--help`, that stray tab's close injects a bogus "no feedback → proceed"
signal into the session.

Handle `--help`/`-h` for every user-facing subcommand (review, annotate,
annotate-last/last, setup-goal, archive, sessions) before any subcommand
branch can run: print command-specific usage on stdout and exit 0. Also
accept `-h` at the top level and advertise per-command help there.

Fixes #964

* fix(cli): handle `improve-context --help` too

The top-level help advertises `plannotator <command> --help`, but
`improve-context` (the only internal hook command listed there) had no
help entry, so `improve-context --help` fell through to the hook branch
and emitted additionalContext JSON instead of usage. Add a help entry so
every advertised command responds to --help.
2026-06-26 13:10:56 -07:00
Michael Ramos 6982d99925 fix(annotate): make Ask AI announcement provider cards clickable (#972) (#975)
The PlanAIAnnouncementDialog rendered provider cards as plain <div>s with
selection styling but no onClick, so clicking a card did nothing despite
the UI implying it switched the active AI provider.

Cards now render as buttons for actually-detected providers and call the
existing AI config change handler (same path as the Settings provider
selector). Providers that aren't installed are shown greyed out as
"not installed" rather than appearing selectable.

The dialog is fed the raw detected provider list (aiProviders) instead of
visibleAIProviders, so the agent-terminal substitution doesn't make every
card read "not installed".
2026-06-26 13:01:53 -07:00
Aleksey Shelementev 1e631ac62e fix(vscode): bridge clipboard and forward keystrokes in webview (#970)
The Plannotator app runs in a nested cross-origin iframe inside the VS Code
webview, where two input paths break:

- Clipboard: the iframe document never holds focus, so the async Clipboard
  API is rejected ("Document is not focused") and native copy/cut/paste
  events never fire. Copy, cut, and paste all silently fail.
- Keybindings: keydown events don't cross the iframe boundary, so VS Code
  never sees global shortcuts (Cmd+P, etc.) while a Plannotator tab is focused.

Fix both off the one input signal the iframe still receives — keydown:

- Route clipboard reads/writes through the extension host, which owns the
  system clipboard via vscode.env.clipboard. The injected script overrides
  navigator.clipboard and handles Cmd+C/X/V, relaying through the wrapper to
  an onDidReceiveMessage handler and back.
- Forward all other keystrokes to the wrapper, which re-dispatches a synthetic
  KeyboardEvent on the webview document so VS Code can resolve its keybindings.
  Undo/redo/select-all stay native.

Fixes #864
Fixes #969

Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
2026-06-26 12:54:19 -07:00
Michael Ramos e1efea8bd4 feat(review): file comments in the diff + unified click-to-highlight comment UX (#973)
* feat(review): file comments in diff + unified click-to-highlight comment UX

Render file-scoped comments inline in the code review UI and overhaul the
comment-card UX for consistency.

File comments
- Single-file view: a full-text banner below the file path.
- All-files view: rendered in the file header (expanded files only) — Pierre's
  virtualized custom header suppresses the body-prefix slot, and its measured
  item heights make a taller header safe. Guided-review external annotations can
  now drop the line-1 anchor hack.

Click-to-highlight
- Clicking a comment (inline card or sidebar) replays its stored line range as
  the controlled selection highlight; clicking it again clears it.
- Scroll/navigate is sidebar- and findings-list-only (token-based signal) so
  clicking a comment in the diff never moves the viewport.

Unified comment cards
- Shared CommentMeta identity row (badges + author + relative time) across the
  inline, sidebar, and file-banner cards (were three hand-rolled headers).
- Shared FileNameChip (basename, full path on hover) replaces the literal
  "file" badge everywhere.
- Force the app sans font on cards (inline was inheriting the diff's monospace),
  consistent selected ring, spacing, and absolute hover actions so timestamps
  align across surfaces.

Shared helpers: annotationScope (PR-scope / file-scope / line-range projection)
and fileName (basename). Plumbs createdAt through DiffAnnotationMetadata so the
inline cards can show timestamps.

* feat(review): unify comment action row (edit · copy · delete)

Shared CommentActions component used by the inline diff card, the file-comment
banner, and the sidebar — bottom-aligned, hover-revealed, order left→right
edit · copy · delete (delete/close furthest right).

- Inline + banner: actions moved from the top (where they overlapped the
  timestamp) to the bottom, and gained the copy button.
- Sidebar: now uses the same component (copy + delete) — same icons, order, and
  styling.
- Read-only (externally-sourced) file comments show copy only.
- Extract commentCopyText() so every card's copy produces identical,
  self-describing text (location prefix + body + reasoning); drops the format
  that was hand-built in the sidebar and the agent-job panel.
- Rename PRCommentsTab's local CommentActions -> PRCommentLinkActions to avoid
  the name clash with the shared component.
- Remove the now-dead .review-comment-action(s) CSS and CommentMeta's trailing
  slot.

* fix(review): address PR review findings

- all-files: let an active compose own the line highlight (pendingSelection
  guard) so clicking/deselecting a comment mid-compose no longer clobbers the
  toolbar's range — mirrors single-file's `pendingSelection ?? annotationRange`.
- clear scrollTargetAnnotation on inline select so a remount (Refresh / base
  switch) doesn't re-scroll back to a stale sidebar/findings navigate target.
- inline diff cards: hide edit/delete for external (source-set) findings
  (matches the file-comment card) and copy with the file:line location prefix
  via a precomputed metadata.copyText (commentCopyText), unifying the copy text.
- add decorations + createdAt to the file-comment refresh signature so an
  external PATCH changing only those repaints the all-files header.
- wrap finding reasoning text (overflow-wrap/word-break) so long file:line refs
  no longer bleed past the card edge.

* refactor(review): dedupe line-annotation projection + close signature gaps

Self-review follow-ups to the PR-feedback fixes:

- Extract lineAnnotationMetadata() — the all-files and single-file diff surfaces
  built the identical 14-field DiffAnnotationMetadata; now one source of truth.
- Add reviewProfileLabel + source + createdAt to the LINE-scope refresh
  signature: the unified CommentMeta now renders those on inline cards too, so
  an external PATCH changing only them must repaint the item (same gap that was
  just fixed for the file-comment tuple).
- Wrap the agent-job panel's finding text + reasoning (overflow-wrap) so long
  file:line refs don't bleed past the panel edge, matching the inline card fix.

* fix(review): zero inter-file gap + stop file-path descender clipping

- all-files: set CodeView layout gap to 0 (Pierre default was 8) so files sit
  flush; sticky headers already mark file boundaries. Keep default edge padding.
- FileHeader: path text leading-none -> leading-normal so the overflow-hidden
  ellipsis box has room for descenders (g/y/p were clipped by the box bottom).

* fix(review): address 2nd-pass review — header re-measure, delete, highlight

- all-files file comments: re-measure the CodeView item when a comment
  expands/collapses/edits (FileCommentCard -> onHeightChange -> refreshItem) so
  Pierre repositions following items instead of overlapping them. Fixes the
  dynamic header-height bug; the remaining static sticky-offset drift is bounded
  by the new banner cap and stays the accepted trade-off.
- cap the expanded comment body (max-h + overflow) so a long guided-review note
  doesn't fill the viewport / inflate the sticky header.
- delete is now available on every surface (inline, banner) including external
  source-set findings; edit stays gated on !source.
- restore the line highlight when a compose ends: the all-files pendingSelection
  effect now re-derives the selected comment's range instead of clearing
  (mirrors single-file's pendingSelection ?? annotationRange).
- add reasoning to the file-comment refresh signature (copyText includes it).
- guard handleNavigateToAnnotation with annotationMatchesPrScope so a stale
  sidebar/finding row can't navigate to an out-of-scope annotation.

* refactor(review): share annotation→highlight derivation + smoother re-measure

Self-review follow-ups:
- Extract lineSelectionForAnnotation() — the compose-end restore and the
  selection-replay effect built the same annotation→CodeViewLineSelection by
  hand; share it so they can't diverge.
- Re-measure file-comment height via useLayoutEffect (before paint) instead of
  useEffect, so an expand/collapse lands without a one-frame overlap.

* fix(review): ignore navigate to missing annotation + re-seed on scope switch

- handleNavigateToAnnotation: return early when the annotation id is gone
  (deleted) as well as out-of-scope, so a stale row can't leave an orphan
  selection (ring with nothing to scroll to). Dismissed agent-panel rows are
  already non-clickable; this is the belt-and-suspenders for any other path.
- fileSetKey now includes prUrl/prDiffScope so a pure layer<->full-stack scope
  switch (same file set) remounts and re-seeds annotations through the current
  scope filter — the incremental sync bails on an unchanged annotations ref and
  can't otherwise detect the filter change.

* fix(review): allow editing external/source findings inline again

Drop the editable = !source gate on inline and file-comment cards. main always
showed edit for external findings (it routes through updateExternalAnnotation),
so gating it was a regression. Edit, delete, and copy are now all available on
every comment surface regardless of source.

* refactor(review): drop redundant null-check after navigate guard

annotation is guaranteed non-null past the missing/out-of-scope early return.
2026-06-26 12:19:49 -07:00
Michael Ramos 3efd9962cd chore: bump version to 0.21.2 v0.21.2 2026-06-25 06:10:46 -07:00
Michael Ramos 842896a370 docs(review): fix Codex command flag (--full-auto, not --sandbox workspace-write)
The ai-code-review.md Codex example showed --sandbox workspace-write, but the
runtime (codex-review.ts) uses --full-auto. Align the doc with the actual
command.
2026-06-25 06:04:22 -07:00
Eric Clemmons e5e3fea38c Improve Codex App review URL discoverability (#966) 2026-06-24 21:10:43 -07:00
Eric Clemmons 63ed44e4a1 Allow Codex Ask AI outside git repos (#965) 2026-06-24 20:46:58 -07:00
Michael Ramos e138c824aa feat(review): Cursor + OpenCode review engines (unified marker-review) (#959)
Add Cursor CLI and OpenCode as background code-review engines alongside Claude/Codex/Tour, unified behind a shared marker-review module (prose marker-block parsing, since neither CLI exposes a schema flag). Includes multi-provider dynamic model discovery, review-profile support, fail-closed parse/ingest, Bun+Pi parity, and the review UI (Provider picker, brand icons).

Hardening from review: per-job nonce on marker tags (prevents echoed/quoted tags from corrupting extraction), truncation fail-closed, async model discovery (never blocks the event loop), nested-slash model ids, shared finding types.
2026-06-24 14:35:52 -07:00
Michael Ramos affeaa07fc feat(review): custom reviews as Agent Skills + whole-file/general findings (#955)
Custom reviews: pick an Agent Skill and its SKILL.md body becomes the review prompt (read live from global skill folders, never copied; default review byte-identical). Agent findings can be a line, a whole file, or a general review-level comment, and are never silently dropped. Plus: agent-icon engine picker, icon-only file-header opener, semantic diff restored as its own dock panel, and a dead-code sweep.
2026-06-23 20:49:48 -07:00
Michael Ramos e385ca90f5 feat(marketing): move capabilities section above demos on landing page (#953)
Surface "Everything the plugin does" immediately after the hero/social
proof so visitors see the full feature set before scrolling to demos.
Reorder commands to lead with annotate and review, and clarify the
review description.
2026-06-22 09:31:48 -07:00
Michael Ramos 567cf8050e fix(review): persist annotation deletions so they don't resurrect on refresh (#948) (#951)
The code-review autosave skipped saving when the annotation list was empty
(`if (annotations.length === 0 && viewedFiles.size === 0) return`). So
deleting the last annotation never overwrote the stale draft on the server;
on refresh the draft-recovery dialog offered the deleted annotations back.

Mirror the plan-side useAnnotationDraft pattern: when the list is cleared,
DELETE the draft with a generation tombstone (so it can't resurface and a
late save can't revive it) instead of skipping. Guarded so an empty state
only triggers a delete once the user has actually authored annotations this
session — keyed on user-authored (non-source) annotations only, because
viewedFiles are seeded from GitHub on mount and external/SSE annotations have
their own lifecycle; counting either would let a fresh load or external churn
wrongly delete an unrestored draft. Also clear the pending autosave timer in
restoreDraft/dismissDraft so a stale save can't fire after restore/dismiss.

Adds real-stack regression tests (hook + disk layer via an /api/draft shim):
delete-to-empty removes the draft, a fresh/unengaged session preserves an
unrestored draft, and external-annotation churn issues no delete.

Reported by @alexanderkreidich.

Closes #948
2026-06-22 07:05:38 -07:00
Michael Ramos caf8bb54bf chore: bump version to 0.21.1 v0.21.1 2026-06-22 05:26:21 -07:00
Michael Ramos f88fd8ab0e fix(editor): stop setState-during-render loop in multi-message annotate (#949) (#950)
buildMessageAnnotationEntries() is reachable on the render path via
currentFeedbackPayload (useMemo) -> getCurrentFeedbackPayload ->
buildFullAnnotationsOutput. It called saveCurrentMessageState(), which
writes React state (setCachedMessageAnnotationCounts) with a fresh Map
each call, so React's Object.is bail-out never fires. In multi-message
mode (annotateSource === 'message' && recentMessages.length > 1) this is
an infinite re-render — React throws #301 and the page renders blank.
This broke /plannotator-last on Pi for any conversation with 2+ assistant
messages.

Switch the render-path read to getMessageStatesWithCurrent(), which
returns the same merged states (cached + live current message) without
the setState side effect. Cache persistence still happens in event
handlers (handleSelectMessage). Exported feedback is unchanged.

Root cause introduced in 1fa60522. Reported and diagnosed by @emmaneugene.

Closes #949
2026-06-22 05:24:05 -07:00
Michael Ramos 90bc8f8db5 chore: bump version to 0.21.0 v0.21.0 2026-06-19 11:10:38 -07:00
Michael Ramos 1291ce5c63 fix(server): surface session URL in local mode when the browser can't open (#945)
handleServerReady only printed the reachable URL for remote sessions. In
local mode, if openBrowser() failed (headless box, devcontainer with no
display, broken open/xdg-open) nothing surfaced the URL and the agent hung
at waitForDecision with the user having no link. Capture openBrowser()'s
result and print the URL to stderr on a local failure (remote already
prints above; gated on !isRemote to avoid a double print). Mirrors the Pi
extension's existing behavior. Covers plan, review, and annotate via the
shared handler.

Also adds explanatory comments to the agent-terminal sidecar runtime (why
a Node sidecar exists; bundled-dev vs managed-release resolution).
2026-06-19 10:38:58 -07:00
Michael Ramos 740d6fb2eb Add WebTUI agent panel to annotate mode (#941)
* feat(annotate): add WebTUI agent terminal

* feat(annotate): wire WebTUI agent into annotate UI

* docs: recap annotate agent terminal work

* fix(annotate): harden agent terminal runtime

* docs: add annotate agent terminal runtime ADRs

* fix(annotate): polish agent terminal integration

* fix(ui): preserve comment draft on Ask AI failure

* fix(annotate): address terminal review findings

* fix(annotate): harden agent terminal runtime fallback
2026-06-19 09:04:15 -07:00
Michael Ramos 2a6fe5c457 Open files in external apps + code-review UX pass (#942)
Adds a server-side "open the current file in an external app" control to the code-review and annotate surfaces (split-button with host-detected apps; last-used becomes the default; cross-platform launch mirrored across the Bun and Pi runtimes), plus a code-review UX pass: file-header change letters and line counts, the diff-settings cog grouped with Split/Unified, an all-files collapse/expand-all toggle, and the semantic diff moved to a resizable sidebar accordion. Also fixes Cmd+click code navigation (worker token transformer).

Hardened over three adversarial review rounds: shared, tested path-containment for open-in (Bun + Pi), annotate scoping aligned to /api/doc reference roots, strict PR-checkout rooting (never the launch repo), deduped app-catalog and semantic-diff fetches, macOS bundle-only availability, Windows launch fixes (reveal + terminal off cmd's parser), and PR-checkout tracking across switch and pool warmup.
2026-06-19 08:56:06 -07:00
Michael Ramos f24bb435d3 Fix live file tree startup latency (#935) 2026-06-19 08:01:03 -07:00
Nam Le 2f4edfdd00 fix(ui): correct light-mode contrast in code hover preview (#934)
The code-file hover preview (CodeSnippetPreview) reuses the github-dark
hljs theme, whose default text color is tuned for a dark background. The
popover body hardcoded a generic --color-muted background that resolves
light in light mode, leaving light-gray text on a light surface (washed
out). Dark mode was unaffected.

The preview renders as a <div class="hljs"> and only appears in the plan
and annotate apps, which load editor/index.css. That stylesheet already
has unscoped .light .hljs-* rules (which color the preview's tokens) and
a default-color override — but the latter is scoped to pre code.hljs, so
the div misses it. Fix: switch the popover background to the theme-aware
--code-bg and add the matching light-mode default text color next to the
existing overrides.
2026-06-18 22:29:03 -07:00