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#864Fixes#969
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* 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.
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.
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.
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.
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
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
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).
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.
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.
Add Bun's compile autoload guard to every release binary build so distributed executables do not inherit bunfig.toml from the caller's current directory.
Extend the binary smoke test to launch --help from a temporary directory with an invalid preload entry, matching the crash reproduction.
Update the Codex sandbox manual compile path so locally rebuilt binaries use the same guard.
Co-authored-by: Codex <codex@openai.com>
The Ask AI feature hardcodes a bare model alias ("claude-sonnet-4-6")
and force-passes it to the spawned claude binary as --model. Bare
aliases are valid against the first-party Anthropic API but are
rejected by Bedrock/Vertex, which require a full inference-profile id /
ARN, producing:
API Error (claude-sonnet-4-6): 400 The provided model identifier
is invalid.
Add resolveSDKModel(): when CLAUDE_CODE_USE_BEDROCK / _VERTEX is set,
map the picker's family aliases (opus/sonnet/haiku) to the matching
ANTHROPIC_DEFAULT_*_MODEL env var, pass through identifiers that are
already ARNs / inference profiles, and otherwise fall back to
ANTHROPIC_MODEL or omit --model so the SDK inherits the env default.
Off Bedrock/Vertex behavior is unchanged.
Co-authored-by: Pavan Purewal <pavandeep.purewal@ticketmaster.com>
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* feat(annotate): add live file tree workspace status
* fix(annotate): guard dirty feedback and rename stats
* fix(annotate): tighten live file tree status
* fix(annotate): surface deleted file browser roots
* fix(annotate): normalize file tree status paths
* fix(annotate): avoid optional git locks for workspace status
* fix(annotate): tighten workspace status git metadata
* fix(annotate): refresh file tree after reconnect
* fix(pi): expose file browser stream route
* fix(server): always show remote URL + close share-html symlink gap
Two pre-release fixes found by the QA sweep.
1. Remote URL stranding: handleServerReady only opened a browser (silent on
a headless box) and wrote a ready-file; the user-visible URL came solely
from writeRemoteShareLink, gated on sharingEnabled. So a remote/SSH user
with sharing disabled (PLANNOTATOR_SHARE=disabled or config.json
{share:"disabled"}, widened by #921) saw no URL and the agent hung on
waitForDecision() forever. Now handleServerReady prints the reachable
localhost URL whenever the session is remote, independent of sharing; the
share link stays an extra. Pi/OpenCode already printed the URL
unconditionally and are unaffected.
2. share-html symlink gap: the #927 symlink-containment fix hardened the
/api/html-assets asset sinks but missed packages/server/annotate.ts's
/api/share-html containment, which stayed lexical. A symlinked *.html
inside the doc directory pointing outside it leaked the target's contents
into the share payload. Now realpath-resolves both root and target like
the other sinks. Bun-only — Pi's copy was already hardened.
Adds regression tests for both (handleServerReady remote stderr; share-html
returns 403 on symlink escape).
* refactor(server): single shared isWithinDirectory for all asset/share sinks
The symlink-containment check existed as four byte-identical copies (Bun
html-assets route, share inliner, annotate /api/share-html, Pi server). That
duplication is exactly why the escape was missed in one sink before — #927
hardened three and missed the fourth, and the #929 fix added a fifth-in-waiting.
Export the single canonical isWithinDirectory from the shared (Pi-vendored)
html-assets-node module and have every sink import it; delete the three
duplicate bodies and their now-dead realpathSync/relative/isAbsolute imports.
A new sink can no longer silently diverge. Regression tests + 690 server/shared
tests pass; build:pi clean.
* fix(pi): surface the session URL in the in-turn 'opened' notice for remote
Remote Pi users never saw a review/annotate/last URL: it was emitted from
openBrowserForServer AFTER the command's turn ended (fire-and-forget), and
Pi only renders a notify during an active turn — so it silently dropped.
The 'X opened. You can keep chatting' notice fires in-turn and DOES show,
so fold the URL into it for remote sessions (single-line, matching the
notify convention). Local sessions are unchanged (browser auto-opens).
* feat(editor): clearer edit controls — Edits label, Save emphasis, Save/Cancel
Annotation panel: rename the pinned 'Direct edits' card label to 'Edits'
(it line-broke at narrow panel widths).
Save button (source-backed sessions): drive its state entirely from the
editableDocuments state machine instead of a parallel flag. Shows a dot +
emphasis only when there are unsaved disk changes (was inverted before,
lighting up when already saved), turns destructive on a failed save, and
no longer reflows — the dot slot is always reserved and the label width is
pinned to its widest value so Save/Saving/Saved swap without nudging
neighbors.
Edit exit control: a source-backed session with unsaved edits now shows a
two-step 'Cancel' (-> 'Discard?') that discards edits and exits, instead
of the ambiguous 'Done'. Plan mode and clean source sessions keep 'Done'
(commit edits + exit), so plan-mode keep behavior is unchanged.
* fix(editor): reset discard confirmation when switching files
In folder annotate mode the editor stays open across files, but the
'Cancel' -> 'Discard?' confirmation was a single app-level flag. Arming it
on one dirty file and switching to another dirty file carried the armed
state over, so the first click discarded the second file's edits with no
confirmation. Reset the confirmation whenever the active editable document
changes so each file's discard is independent.
* fix(annotate): add /api/save-notes POST endpoint to both servers
Copies the save-notes route from the plan review server into the
annotate server (Bun source and Pi extension copy), enabling Save to
Obsidian in annotation mode.
Fixes#844
* test(annotate): add saveToObsidian unit tests and HTTP endpoint tests
Verifies saveToObsidian writes files correctly and handles missing
vaults. HTTP endpoint tests cover success, empty integrations, and
integration-level error (not 500).
Imports consolidation from ./integrations into a single statement.
* fix(annotate): normalize server port fallback
* refactor(server): extract shared handleSaveNotes handler, fix catch-block bug
Move the /api/save-notes logic into shared handler modules
(shared-handlers.ts for Bun, handlers.ts for Pi) following the existing
pattern for handleImage, handleUpload, handleDraftSave. Replaces four
inline copies with two canonical implementations.
Fixes:
- Bun annotate catch block now correctly returns 500 (was logging only)
- Misindented brace in Pi serverAnnotate.ts resolved by extraction
- Revert unrelated port fallback change (keep server.port! for consistency)
- Static imports in integrations.test.ts
- Add /api/save-notes to CLAUDE.md Annotate Server API table
* fix(opencode): inject annotate server starter instead of global mock.module
commands.test.ts mocked the annotate server with
`mock.module("@plannotator/server/annotate", ...)`. Bun module mocks are
process-global and cannot be unset (oven-sh/bun#7823, #12823), so the stub
leaked into every suite that runs after it — in particular any test that boots
the real annotate server received a stub with no `.url`.
Make `startAnnotateServer` injectable through the existing CommandDeps
(defaulting to the real import, so production is unchanged) and have the test
pass its stub that way. This keeps the fake local to the opencode suite and
unblocks real annotate-server integration tests.
* test(server): cover save-notes — handler unit tests + annotate e2e wiring
- shared-handlers.test.ts: unit-test handleSaveNotes directly (Obsidian write,
empty integrations, integration-error reported not thrown, 500 on bad body).
- annotate.test.ts: boot the real annotate server and POST /api/save-notes,
asserting it is served as JSON (not the SPA HTML catch-all) — the regression
guard for #844. Now possible because the opencode suite no longer installs a
global annotate module mock.
---------
Co-authored-by: Michael Ramos <mdramos8@gmail.com>
* fix(annotate): resolve symlinks before HTML asset containment check
The /api/html-assets route and the share-payload inliner checked path
containment lexically, so an in-directory symlink pointing outside the
HTML's folder (e.g. evil.css -> ~/.ssh/id_rsa) passed the check and was
served or base64-inlined into the share payload. In remote mode the
inliner auto-fires at startup, so this could upload symlinked local
files to the paste service with no user action.
Resolve symlinks with realpathSync on both the asset and the root before
the relative-path check, in all three runtime copies (Bun route handler,
shared node inliner, Pi route handler). Non-existent assets fall back to
the lexical path and 404 on read. Adds regression tests for both sinks.
* test(pi): build rich git state in sandbox-pi.sh for review diff modes
Expand the Pi sandbox harness to create multiple commits, a feature
branch, and a rename+delete+modify commit so /plannotator-review can
exercise every diff mode (uncommitted, staged, branch, merge-base).
* docs: fix broken verification link in READMEs
The READMEs pointed at a non-existent anchor
(installation/#verifying-your-install); the verification guide is a
standalone reference page. Point to /docs/reference/verifying-your-install/
(and split the hook README's link so version pinning -> installation,
verification -> the reference page).
Allow disabling URL sharing through ~/.plannotator/config.json
({ "share": "disabled" }) in addition to the PLANNOTATOR_SHARE
env var. Adds a resolveSharingEnabled() helper (env var > config >
default enabled) and routes all sharing checks through it across the
hook server, OpenCode plugin, and Pi extension. Docs updated.
* feat(annotate): render html files by default
* fix(annotate): support raw html assets and sharing
* fix(annotate): address html first review followups
* fix(editor): avoid raw html sidebar init crash
* fix(annotate): support portable html shares
* fix(annotate): harden html share support
* fix(share): clear attachments when loading shared payloads
* fix(share): warn on remote share link failures
* perf(annotate): lazy-build html share payloads
* test(annotate): guard lazy html share generation
* test(annotate): drop flaky html share server test
When a comment popover scrolls out of view, show a small pill at the
top/bottom of the viewport pointing toward it; clicking scrolls back.
Only renders for tracked (anchored) popovers that can actually leave
the viewport; fixed/dragged and static-rect popovers stay silent.
Tagged data-popover-layer so clicking the pill doesn't trip the
click-outside handler and dismiss an empty popover.
Stop added annotations from forcing the annotation panel open, and keep dirty annotation comment popovers open on outside click so typed feedback is not lost.
Two QA follow-ups found while validating the release:
- Route SIGINT/SIGTERM through process.exit() so the existing "exit"
handlers actually run on Ctrl-C / termination. A signal death
previously skipped them, leaking background PR-checkout warmup
children and stale `git worktree` registrations. A second signal
still force-quits if cleanup hangs.
- Append the review-denied triage suffix for PR-mode feedback, not just
local diffs. The old `!isPRMode` gate suppressed it for every PR
review. Gate on whether the reviewer actually sent annotations
instead: genuine feedback always carries annotations, while platform
PR actions (approve/comment posted to the host) return an empty
annotation set + status message and correctly get no suffix. Applied
consistently across hook, OpenCode, and Pi.
* feat(review): large GitHub PR fallback + non-blocking PR checkout
Two PR-mode improvements:
1. Large GitHub PRs no longer fail to load. When `gh pr diff` is refused
(HTTP 406 for oversized diffs), fetchGhPR pages through the pulls files
API and stitches the per-file patches into a unified diff — mirroring
the existing GitLab raw_diffs fallback. Path quoting matches git's
exact rules (bare spaces unquoted) so downstream parsers round-trip;
truncation at the API's 3000-file cap is surfaced, never silent.
2. The --local worktree/clone no longer blocks startup. The review server
opens as soon as the platform diff arrives; the checkout warms in the
background as a seeded not-ready pool entry. Consumers that need real
files (agent jobs, full-stack diff, code-nav, semantic diff, AI
sessions) await pool.ensure(), with creations serialized so concurrent
fetches can't clobber the shared FETCH_HEAD. Cross-repo clone steps
converted from spawnSync to async spawns; warmup children are killed
on exit (plus `git worktree prune`) so aborted sessions can't leak
stale registrations; failed checkouts degrade honestly (no agent runs
in the wrong directory claiming local access) with a 30s retry
cooldown.
* fix(review): survive long PR checkout warmups + classify reconstructed renames
Stress-testing against oven-sh/bun#30412 (2,188 files) surfaced three bugs:
- Bun.serve's default 10s idleTimeout killed /api/semantic-diff while it
parked on the background checkout warmup (a clone that can take minutes).
Disable the idle timeout on all servers — AI SSE streams can also stall
>10s between bytes while a permission prompt waits.
- The file-badge hook memoized that failed fetch in a module-level cache
keyed by patch, pinning every badge to empty until a hard refresh. Never
cache failures; retry with backoff (5s/15s/30s).
- reconstructGhPatch/reconstructPatch omitted the `similarity index` line,
which Pierre's parser keys rename classification off — pure renames
rendered as blank plain changes with no old path. Emit 100% for
patch-less renames/copies (exactly accurate) and a synthetic 99% for
patched ones (consumers only branch on 100% vs not).
* feat(review): local full-diff upgrade for PRs whose API diff is truncated
On oversized PRs the platform APIs withhold per-file patch content entirely
(bun#30412: 1,066 of 2,188 files came back with status added/modified, zeroed
counts, and no patch). Those files rendered as empty stubs with no diff.
- fetchGhPR/fetchGlMR flag the result `patchIncomplete` when patch-less
non-rename entries exist or the 3000-file cap truncates the listing.
- New runPRLayerLocalDiff (pr-stack.ts) recomputes the exact layer diff in
the local checkout: platform merge-base + head SHA two-dot diff (three-dot
vs baseSha fallback), fetch-by-SHA for objects missing from shallow clones,
-l0 so rename detection doesn't silently degrade on huge PRs.
- The review UI shows a "Partial diff · Load full diff" notice in layer
scope; clicking re-requests the layer scope and the server swaps in the
recomputed full diff (waiting out the background clone if needed).
- PR scope/switch state writes are epoch-guarded: a request parked on the
checkout warmup can no longer overwrite a newer scope select or pr-switch.
- draftKey follows the upgraded patch so annotation drafts survive pr-switch
round-trips; recompute failures surface in the response error field.
- Pi server mirrors all of it, including an agentCwd fallback so the upgrade
works for PRs switched-to under a cross-repo clone pool.
* fix(review): use GitLab's too_large/collapsed flags for withheld-diff detection
External review caught a false negative: a too-large ADDED file comes back
new_file:true with an empty diff — indistinguishable from a legitimately
empty new file under the old heuristic, so the partial-diff upgrade was
never offered for exactly the files that matter most on big MRs.
The REST /diffs endpoint marks withheld content explicitly per entry
(verified against gitlab.com): too_large/collapsed are now authoritative in
both directions — withheld adds/deletes are flagged, binaries and empty
files are never misflagged. Older GitLab without the fields keeps the
empty-diff-on-modification heuristic.
* feat(prompts): unify review-denied suffix — triage first, no coding off raw feedback
The per-runtime defaults map (#627) gave OpenCode and Pi a different
review-denied suffix than every other runtime; updating one meant the
others silently kept "you must address all of them" — an instruction to
start coding immediately. Claude Code, Amp, Droid, Codex, Copilot, Gemini,
and Kiro were all still on it.
One default for every runtime now: triage the feedback, verify it against
the code, discuss before changing anything. Per-runtime customization
remains available via config (prompts.review.runtimes.<rt>.denied), which
resolves above the built-in default as before.
* fix(prompts): generalize review-denied suffix — 'from review', not 'external AI reviewers'
Review feedback isn't always from AI reviewers or agent jobs; often it's
the human reviewer's own annotations. Neutral wording covers both.
* fix(review): non-blocking 'Load full diff' + flag-handling hardenings
Self-review findings:
- The partial-diff upgrade reused the scope-switch handler, so clicking
"Load full diff" raised the full-screen PRSwitchOverlay — blocking the
entire UI, potentially for minutes behind a cold clone, with no text and
no cancel. The upgrade now has its own loading state: the notice shows a
spinner ("Loading full diff…") and the reviewer keeps working with the
partial diff while the request parks. Server-side epoch guards already
handle scope/PR changes made during the wait.
- GitLab too_large/collapsed: treat explicit null like absent (flags
inconclusive → legacy heuristic decides) instead of silently exonerating.
- Rename-limit lift uses -l100000 instead of -l0 ("0 = unlimited" only
holds on git >= 2.29; on older git it could disable detection outright).
* fix(review): stop scroll-driven sem stampede when semantic diff is failing
The badge retry change (a2d19a4e) cleared the client-side sem cache on
failure so transient errors could recover. But file-header badges mount and
unmount on every scroll in the virtualized all-files view, and each mount
re-requests /api/semantic-diff — and the server only cached SUCCESSFUL runs.
With sem erroring, scrolling spawned a continuous stream of sem processes,
pegging the CPU and making scrolling severely choppy.
Bound retry rate by time, not by mount events:
- client: keep the failed result memoized and expire it after a 60s
cooldown instead of clearing immediately
- server (Bun + Pi): memoize failed sem runs for 30s in
SemanticDiffResponseCache — request rate can no longer drive execution
rate
* fix(review): eliminate all-files scroll jank (pre-existing on main, from #885)
The CodeView migration introduced severe scroll chop; scrolling UP could
freeze the viewport entirely ("scrolling but nothing changes"). Three
compounding causes, diagnosed against Pierre 1.2.8 source:
1. Lazy full-content augmentation landed updateItem() mid-scroll-gesture:
the full-content parse counts collapsed-context regions the raw-patch
parse doesn't, so the item GROWS — re-render + re-tokenize hitches both
directions, and when the grown item sat above CodeView's scroll anchor,
its corrective scrollTo() killed wheel momentum (the up-scroll freeze).
Fetches still start as items enter the window; the item mutation now
waits for 150ms of scroll quiet (staleness re-checked at apply time).
2. reportVisibleFile read container.scrollTop/clientHeight/scrollHeight on
EVERY scroll event — a forced synchronous layout right after each
frame's DOM writes. Replaced with CodeView's cached accessors and
coalesced the handler to once per animation frame.
3. Missing containment CSS: Pierre's own production wrapper uses
contain:strict + will-change:scroll-position so forced layouts stay
scoped to the scroller instead of the whole document. Adopted.
Also: __devOnlyValidateItemHeights now requires explicit opt-in
(VITE_PIERRE_VALIDATE_HEIGHTS=1) — it runs getBoundingClientRect() per
rendered item per frame and made dev-server scrolling choppy by itself.
* feat(review): change-type status in headers + tree, diffshub CSS parity
Adopts two diffshub practices identified in the architecture comparison:
- DiffFile now carries a derived status (added/deleted/renamed/modified)
from the chunk's git metadata lines. FileHeader shows a status icon and
renders renames as "old/path → new/path" (dimmed old, arrow — diffshub's
treatment, including its rename blue); the file tree shows A/D/R markers.
'modified' is deliberately undecorated so the others pop. Works in both
the all-files surface and the single-file panel, including header-only
pure renames from the large-PR reconstruction.
- CodeView container gains diffshub's remaining perf CSS: overflow-anchor:
none (native scroll anchoring fights CodeView's own anchor resolution
whenever item heights change — exactly our augmentation applies),
overflow-x-clip, and overflow-clip containment on item elements.
* feat(review): worker-pool syntax highlighting (diffshub parity)
A performance trace of scrolling a small local diff attributed 2.2s of
2.6s main-thread CPU to findNextMatchSync — shiki's TextMate regex
scanner tokenizing on the main thread. diffshub avoids this entirely by
running tokenization in Pierre's worker pool; we never opted in.
Wires WorkerPoolContextProvider around the review app (pool size
min(cores-1, 3), 100-entry AST LRU, common languages preloaded), gates
the all-files surface on pool readiness with a 5s escape hatch (a dead
pool degrades to plaintext-then-highlight, never a blank view), and
syncs the UI theme pair into the long-lived pool.
Single-file build constraint solved with Vite's ?worker&inline (base64
blob worker) + worker.format 'es' with inlineDynamicImports — the
worker's lazy import("shiki/wasm") branch collapses into the bundle and
is never taken (shiki-js engine: the win is moving work off the main
thread, with no .wasm asset to smuggle into one HTML file). Bundle
+850KB.
* fix(review): un-poison worker-pool theme dedup on failed setRenderOptions
A failed round-trip recorded the theme as synced and never retried,
pinning the pool to the wrong palette for the session.
* fix(review): report partial diffs without a checkout; fail fast on missing checkout
Dogfood review of this PR (via plannotator itself) caught two valid issues:
- prPatchIncomplete was gated on the worktree pool, so a --no-local session
showed a truncated diff with no indication at all. Partiality is
information; upgradability is a capability. The flag is now always
reported, with a separate prPatchUpgradeAvailable — the UI shows the
amber notice either way, with the "Load full diff" button only when a
checkout can exist (otherwise a "re-run with --local" hint).
- After a FAILED checkout warmup, Ask AI sessions and agent jobs fell back
to process.cwd() (or a wrong revision on Pi) — running in the wrong tree
instead of failing. Both launch points now refuse with a clear "Local
PR checkout unavailable — retry shortly" error (503); the job handlers
surface buildCommand refusals instead of mislabeling them "Invalid
JSON". Bun and Pi mirrored.
A third finding (sem availability stuck after warmup) was triaged invalid:
the availability probe detects the sem binary, which is cwd-independent.
* fix(review): runtime-neutral copy for the no-checkout partial-diff hint
--local is a CLI remedy; OpenCode sessions have no such flag. Visible
text states the fact, the tooltip carries the CLI guidance.
* feat(readme): redesign README — hero banner, agent icons, simplified install
- New centered hero: banner, tagline, theme-aware agent icon row (picture/prefers-color-scheme), demo links
- Intro paragraph naming all nine supported agents
- Feature cards with screenshots; dedicated Annotate HTML Artifacts, Code review, and Sharing sections
- Install collapsed to one shared installer + per-agent table covering all nine agents, linking each app README
- New Try it section for manual invocation after install
- Fixed Codex plan-mode claim (supported via Stop hook), completed PLANNOTATOR_ORIGIN values, fixed demo links and typos
- Added Integrations, Remote/SSH, Security, and Configuration sections
* feat(readme): consolidate command examples into a Commands section
- Annotate HTML Artifacts is now screenshot-only
- Drop the duplicate code review screenshot (code-review-thumbnail.png removed)
- New Commands section: Annotate, Code review, and CLI groups
* fix(readme): point doc-review demo at Pi video, drop wrong code review demo link
* style(readme): plain-language pass
- No em dashes anywhere; split into sentences or swapped for colons/commas
- Rewrote the intro: integration with the agent session is now explicit (plugs into hooks/commands, feedback lands back in the live session)
- Varied repeated phrasing between the two feature cards
- Integration labels use colons instead of dashes
* style(readme): simplify feedback phrasing in hero and feature card
* feat(readme): add Plan mode note and sessions command to Commands section
* feat(readme): sharing beta link and Workspaces signup CTA badge
* style(readme): drop divider between feature table and HTML artifacts section
* style(readme): bump banner width 512 -> 640
* style(readme): fix agent icon sizing
- Crop Pi icon viewBox to glyph bounds (was 40% padding)
- Render Amp at 32px to offset its thin mark
* style(readme): bump Amp icon to 38px for visual parity
* fix(readme): strip em-based size attrs from Amp icon so it scales
The svg root had width/height of 1em plus an inline style, the only icon
in the set with relative sizing. SVG-as-image resolves em at a fixed 16px
and the drawing stayed pinned there regardless of the img height. Removed
the attrs, back to the uniform 28px row height.
* style(readme): width=100% on feature table images
* style(readme): cmux-style 40/60 feature table with middle-aligned text
* style(readme): split feature rows into separate tables so zigzag keeps 40/60
* style(readme): single feature table, text left 40 / image right 60, no zigzag
* style(readme): tighten intro and feature card copy
* style(readme): remove stray space before period
* style(readme): another round of copy edits
* chore: remove internal docs notes, move readme-assets to .github/assets
- Deleted docs/ (three internal handoff notes, nothing referenced them)
- README media now lives in .github/assets/ instead of a root-level folder
- All 18 asset links in README updated
* style(readme): split intro into two paragraphs
* feat(readme): live collaboration teaser in Sharing, tighten encryption copy
* style(readme): retitle Sharing & Multiplayer, rework CTA badge to banner palette with balloon
* feat(readme): updated annotate and review screenshots
* feat(readme): updated HTML artifact screenshot
* feat(readme): swap HTML artifact screenshot for v2
* feat(readme): restore share workflow sentence and Codex command note
* docs(agents): document bun link workflow for local plannotator command
* style(readme): trim Windows note from Codex command tip
* feat(readme): one-line AI features note under feature table
Replaces the all-files renderer with Pierre's virtualized CodeView (one scroller, identical look, scales to huge diffs); deletes the legacy AllFilesDiffView/LazyFileDiff and their flag; single-file panels stay on FileDiff. @pierre/diffs pinned exact 1.2.8 (1.2.9's tree is broken on npm — see #880).
Also ships: diff staleness detection ('Diff out of date · Refresh' toolbar notice backed by shared per-VCS fingerprints + GET /api/diff/fresh on both Bun and Pi servers), a stale-content guard so files edited mid-review degrade to raw-patch view instead of breaking virtualization, the Ask AI wrong-file fix (pre-existing), and toolbar/header UI polish (global settings cog, aligned sem badges, no all-files split dragger).
Hardened through five review waves; tested with 1384 passing tests incl. real-git fingerprint and patch-consistency suites. Known minor gaps documented in the PR.
- When a file exists but has an unsupported type (.cs, .ts, .py, etc.),
show 'File type not supported' instead of misleading 'File not found'
- Display supported file types (.md, .mdx, .html, .htm)
- Suggest using 'plannotator review' for code files
- Maintains backward compatibility for actual missing files
Fixes#757
Co-authored-by: ishowman <ishowman@users.noreply.github.com>
Upstream @pierre/diffs@1.2.x depends on @pierre/theming@0.0.1, which is
not published on npm, so any fresh install resolving our ^1.1.12 range
floated to 1.2.9 and failed with a 404. Our lockfile masked this for
repo builds, but npm consumers of @plannotator/pi-extension resolve
fresh and hit it (#880). Pin every declaration to exact 1.1.20, whose
dependency tree is fully resolvable (@pierre/theme@0.0.28).
The release/look-and-feel dialog previously rendered only in the plan
editor, so code-review-first users never saw the 0.20.0 announcement.
Render the same LookAndFeelAnnouncementDialog in review-editor, wired to
the shared gridEnabled config. Seen-state is a host-scoped cookie, so it
appears once across both apps. Gated ahead of the diff-type setup dialog
so the two never stack.
Aleksey Shelementev