Commit Graph

694 Commits

Author SHA1 Message Date
Michael Ramos 2c98b3a159 chore(guides-show): regenerate viewer manifest pin for the touch selection changes 2026-08-16 21:38:16 -07:00
Michael Ramos cfd3907788 test(review): cover the compact-touch preserved range on both diff surfaces
DiffViewer.compactTouchSelection.test.tsx drives the FileDiff options the
component hands Pierre: a drag preserves the range instead of opening the
composer and paints it through selectedLines, a second drag repaints
through onLineSelectionChange, the gutter action opens the composer, a
cleared range still reaches the toolbar host, and desktop keeps routing
completed drags straight to the composer with no change handler at all.
The last three assertions fail against the pre-fix component.

The AllFilesCodeView compact test only checked what was published upward,
which a range nothing paints would also satisfy; it now asserts the range
reaches the CodeView props. That needs the App loop, so the mount feeds
published selections back down as pendingSelection: without it the
reconcile effect clears the highlight the preserve branch just painted.
2026-08-16 21:26:31 -07:00
Michael Ramos 50b9f13078 fix(review): 44px hit area for Pierre's gutter comment button on touch
With a dragged range now preserved instead of opening the composer, that
button is the only way to start writing about it, and it is roughly 20px
square: below the data-pn-touch-target standard the rest of the compact
shell holds. Grow its invisible ::before hit area to the 44px token,
leaving the glyph alone.

The rule ships through the same unsafeCSS both diff surfaces already
inject, which lands in Pierre's shadow root inside @layer unsafe (last in
the library's layer order, so no !important). It is injected only when
the shell is compact rather than gated in CSS: html:has() matches nothing
from inside a shadow root, and @media (pointer: coarse) would wrongly
claim a desktop with a touchscreen.
2026-08-16 21:26:21 -07:00
Michael Ramos 42e858b3fd fix(review): repaint a preserved mobile range on a second drag
A preserved range leaves pendingSelection non-null, so DiffViewer hands
Pierre a defined selectedLines and Pierre switches to controlled
selection: updateSelection then only records a proposed range and leaves
painting to the host. With no change handler wired, a second drag never
repainted, so the old highlight stayed put and the finger was untracked
until release. Wire onLineSelectionChange back into app state, only on
compact touch, so desktop keeps an options object with no such key.

Also route a null range to the toolbar host instead of swallowing it in
the preserve branch, mirroring AllFilesCodeView's early return so an open
composer (Ask AI included) closes with the selection it was written for.
2026-08-16 21:26:00 -07:00
Michael Ramos a371e545c0 feat: preserve mobile diff ranges before commenting 2026-08-16 20:21:50 -07:00
Michael Ramos bc2cf0988e revert: remove command-mediated touch range prototype 2026-08-16 19:58:05 -07:00
Michael Ramos 3c7bc81968 feat: add mobile touch range selection prototypes 2026-08-16 19:50:38 -07:00
Michael Ramos 94f8d45daa guide-viewer: readable on phones and tablets, desktop untouched (#1329)
* fix(guide-viewer): readable on phones and tablets, desktop untouched

Every change is behind a breakpoint; 1440px and 1024px renders of the same
guide are byte-identical before and after (screenshot MD5s match).

- Split diffs below lg (1024px) are forced unified in the portable viewer's
  diff renderer (matchMedia; the setting is untouched, so a wider window
  gets split back). A phone has ~350px of pane and a portrait tablet ~430px,
  so two columns were under 220px each.
- Padding scales: page px-3/sm:px-6/lg:px-10, chapter column px-4/md:px-6,
  diff column px-1.5/md:px-4. Code pane on a 390px phone: 276px → 352px.
- Tablets: the chapter column is proportional (minmax(260px,36%)) from md
  and the fixed 440px only from lg. Pane at 768px: 214px → 426px.
- Header actions (Download, theme) sit in a right-aligned row above the
  title below md instead of floating into it.

Viewer rebuilt and published (viewer.dWt7KCum.js), manifest synced.

* fix(guide-viewer): touch targets, labels, and no tap delay on coarse pointers

Only under `pointer: coarse` (Tailwind's `pointer-coarse:` variant), so mouse
layouts are unchanged:
- Reviewed checkbox and the collapse chevron get an invisible ::before hit
  area (visual 15–17px, hit ≥ 44px); the "Reviewed" text button and file
  chips get taller padding; the theme toggle and hosted Download button grow
  to a 44px hit box.
- `touch-action: manipulation` on controls in the portable viewer and the
  landing page (no double-tap-to-zoom delay; the page still pinch-zooms).
- `aria-label` on the two icon-only buttons (theme toggle, collapse chevron).
- Landing page: the GitHub link and the Copy button are 44px tall on touch.

Tailwind v4 already gates `hover:` behind `@media (hover: hover)`, so no
false hover states on tap. Viewer rebuilt and published, manifest synced.

* guide-viewer: manifest for the combined build (labels + mobile), viewer.sFtOnb1i.js published
2026-08-16 13:30:58 -07:00
Michael Ramos cb2b264303 guide-viewer: label agent harnesses in the generated-by line (#1328)
An authored guide's generator.engine is the harness the agent ran in
(claude-code, codex-cli, …), which the in-app engine map does not know, so
the header printed the raw id. Map the common ones; unknown ids still render
as written. Viewer rebuilt and published, manifest synced.
2026-08-16 13:15:26 -07:00
Michael Ramos 64062af9a1 feat: Portable Guided Reviews — export, share links, agent-authored guides, guides.show (#1324)
A Guided Review can now leave Plannotator: as a single self-contained HTML file that renders exactly like the in-app guide, as an encrypted-by-default share link on guides.show, or authored by any agent through the new guide CLI.

Highlights: packages/guide-viewer extracted from review-editor at the injection seam (read-only host, no third renderer); guides.show Worker with R2-backed share storage, per-IP rate limiting on creation, delete tokens hashed at rest, and 128-bit ids; portable exports pin the viewer by SRI hash with budget and manifest gates in PR CI and at deploy; two-runtime parity across Bun and Pi verified; v0.27.x saved guides load unchanged. Retention is indefinite by explicit decision, to revisit with the lean sharing refactor.

Decision record: adr/decisions/007-portable-guided-reviews-20260815.md
2026-08-16 12:17:13 -07:00
Graeme Folk e3091331a5 feat(review): jj support for Call Flow analysis (#1312)
Adds Jujutsu (jj) as a Call Flow analysis provider: jj-current/jj-last/jj-line/jj-all snapshot revsets with deterministic first-parent resolution across merge revisions, root-anchored filesets so results are cwd-independent, bounded snapshot materialization (base tree + changed-file delta) with a streamed 64MB output ceiling in both the Bun and Pi runtimes, and real-jj regression tests covering merges and subdirectory invocation.

Contributed by @graemefolk, who also built the original jj integration. Review fixes pushed in-branch: merge-parent resolution, root-glob filesets, bounded materialization and buffering, plus CI gating guards for runners without jj.
2026-08-15 10:47:09 -07:00
Ashish Huddar 59ef54be44 fix(comments): expose skill picker semantics to assistive tech (#1316)
Adds the ARIA autocomplete contract to the comment composer skill picker: aria-autocomplete=list + aria-haspopup=listbox on the textarea, aria-controls/aria-owns while the menu is open, role=option rows with aria-activedescendant tracking. Preserves the no-preselection keyboard state machine and the untouched plain composer when skill references are off.

Closes #1233

Co-authored-by: ashish921998
2026-08-14 10:17:35 -07:00
Michael Ramos aa0bf860d8 chore: bump version to 0.27.3 2026-08-13 16:06:13 -07:00
Michael Ramos 192b026073 fix(annotate): stop the folder watcher freezing the server (#1314)
* fix(annotate): stop the folder watcher freezing the server (#1313)

The file-browser content watcher built a chokidar scan over the whole
workspace synchronously on the request path. Under Bun that scan
monopolizes the event loop (a 780-directory nested tree measured 79
seconds), and because teardown was immediate on the last unsubscribe,
every EventSource reconnect paid the scan again: the reconnect the
freeze itself provoked made the hang self-sustaining.

The watcher engine now lives once in
packages/shared/file-browser-watch-core and both runtimes keep only
their transport:

- construction is deferred off the request path, so the SSE ready event
  and concurrent API requests are served before any scan starts
- teardown gets a 30s reconnect grace; a reload reuses the warm watcher
- on macOS and Windows the content watcher is the platform's native
  recursive fs.watch (measured ~0ms for the same tree); chokidar stays
  the Linux backend and the runtime fallback, with a forced catch-up
  refresh on the swap so no events are lost
- server stop tears every watcher down immediately in both runtimes

The responsiveness regression test reproduces the reported freeze on
the pre-fix implementation (79s, fails) and passes in under a second on
the fix.

* docs: folder annotate sessions do write per-file version history

The PLANNOTATOR_ANNOTATE_HISTORY row claimed URL, folder, and
annotate-last sessions never write to the data dir. The folder /api/doc
path deliberately runs the per-file version-history pipeline (lazily,
memoized per resolved path, gated on the same flag) to power the
per-file version diff, and has since it shipped. The code is the
intended behavior; the sentence was stale. URL and annotate-last
sessions remain fully stateless, and submit records remain single-file
only.

* fix(annotate): review follow-ups for the watcher engine

Applied from the independent review of #1314:

- contentWatchBackend gains a forced 'native' mode and the fallback
  tests use it, so the native-to-chokidar paths (creation failure and
  runtime error) genuinely execute on Linux CI; the runtime-error test
  is no longer macOS-only
- a platform-agnostic responsiveness test pins that SSE ready is served
  before the scan starts on the chokidar backend, via the runtime test
  hooks; the tight full-scan bound stays macOS-only
- watcher construction failures and the native-to-chokidar swap now log
  one console.error each instead of stranding subscribers silently;
  the swap also increments the diagnostics start counter honestly
- closeEntry guards both watcher close() calls; the Bun annotate stop
  chain got the same try/finally shape as the plan server; all four
  stop chains close watchers ahead of throwable disposals so a failing
  dispose cannot strand a watcher keeping embedded hosts alive
- a broadcast that empties the subscriber map by deleting dead
  subscribers now schedules the teardown grace instead of leaving the
  entry live until closeAll
- bun.lock drift reverted: only the chokidar edge and the workspace
  version corrections remain (27 unrelated esbuild resolution entries
  dropped; frozen-lockfile install verified)
- stale never-write comments in both annotate servers corrected to
  match the folder per-file history reality documented in AGENTS.md;
  the engine header now states plainly that chokidar is a correctness
  fallback, not a performance one
2026-08-13 16:02:41 -07:00
Michael Ramos 8b9dfe7e5f chore: bump version to 0.27.2 2026-08-13 11:36:16 -07:00
Michael Ramos 8e88dcec8c fix: v0.27.2 pre-release QA batch (mobile TOC, dialog bounds, seed guard) (#1311)
* fix(plan): make the compact TOC scroll the document again

The compact navigator overlay rendered outside App's ScrollViewportProvider,
so the TableOfContents it hosts resolved a null viewport and every "jump to
heading" tap was a silent no-op on phones. The provider is context-only, so
hoisting it above the overlay fixes the lookup without touching desktop DOM
structure or order.

* fix(plan): scope the permission-mode chooser to plan review and bound its card

The one-time chooser fired in every non-goal-setup Claude Code session, so
annotate, annotate-last, annotate-folder and archive reviewers got a blocking
dialog about what happens after plan approval. Gate it on plan review, which
is the absence of a mode field in the /api/plan payload.

The card itself was hand-rolled with no height cap and no internal scroll, so
on a short landscape phone it overflowed both edges of a modal that has no
dismiss control. Give it the same bounded shell the sibling one-time dialogs
use: safe-area padding, a visible-viewport max height, and the option list as
the only scrolling region. Content and cookie behavior are unchanged.

* fix(review): never seed Tree over a persisted panel view

The first-run initializer gated only on the setup-seen cookie, but sessions
that never reach it (non-git, workspace, PR, no since-base) still let Settings
persist a panel view. A reviewer could hold an explicit Git status choice with
"seen" unset, and the next plain git session seeded Tree over it. Treat a
persisted view as the decision: consume the one-time setup and write nothing.

* fix(comments): give the geometry-forced composer a working Escape

When the anchor has no room the position tracker forces dialog mode. On a
fine-pointer viewport Escape took the collapse branch, the tracker instantly
re-forced the dialog, and the keystroke was eaten; the Collapse button bounced
the same way. Track forced expansion separately from the preferred kind: in
that state Escape closes (draft-preserving) and Collapse is hidden, because
collapsing is geometrically impossible.

* fix(review): stop the compact Editor tab editing the desktop diff style

The dock's Split/Unified control returns null under the compact touch layout,
but the Settings copy of it kept rendering while the phone showed the
session-only unified diff. It looked dead and silently rewrote the persisted
desktop preference. Hide it on compact and state what the session is doing;
the prop defaults to false, so the plan editor and desktop are untouched.

* fix(portal): give the share portal the mobile app shell

The portal mounts the same plan editor App as the hook but kept the pre-mobile
entry document: no viewport-fit=cover (so every safe-area token was inert) and
a min-h-screen body without the shell's scroll ownership. Mirror the hook's
body class, root class, and viewport meta, and extend the entry-asset pin to
cover the portal alongside them.

* fix(plan): keep compact overlays out of the printed document

The compact plan stage and the compact navigator are full-viewport transient
surfaces with no print-hide marker, so printing on a touch device with
Annotations, Ask AI, Versions or Archive open clipped the document behind
them. Mark both with data-print-hide, which print.css already hides. The
desktop rail is untouched.

* fix(plan): give the selection toolbar real touch targets

Copy / Delete / Comment / quick label / looks-good / Cancel measured 28x28
with 2px gaps on a phone because the toolbar never got the touch-target
markers the rest of the stack uses. Stamp them on its buttons and add a
compact-scoped gap so adjacent destructive and comment actions are not a
mis-tap apart. Both are inert outside the compact scope, so desktop geometry
is unchanged.

* docs: keep the new QA-batch comments free of em dashes
2026-08-13 11:35:00 -07:00
Michael Ramos d2d2dba7fa feat(annotate): configurable extra markdown extensions (#1309)
* feat(annotate): configurable extra markdown extensions (#1307)

Adds a config-only `markdownExtensions` key to ~/.plannotator/config.json,
e.g. { "markdownExtensions": [".livemd"] } for Livebook notebooks. A listed
extension is accepted everywhere .md is on the annotate path: CLI target
resolution, folder discovery and the file browser, /api/doc plus relative and
wiki-link navigation between sibling docs, the 2MB size cap, and per-file
version history. Listed extensions render as markdown with frontmatter
stripped, never as raw HTML, and they only widen the accepted set.

Design:
- packages/core/annotatable.ts stays browser-safe and zero-dep. Its regexes
  and predicates now take an optional, defaulted-empty list of extra
  extensions, plus a normalizer and regex builders.
- packages/shared/markdown-extensions.ts is the node-side seam: it reads
  config.json once per process through the existing loadConfig() and threads
  the normalized list into those pure functions. resolve-file re-exports the
  config-aware predicates so both runtimes pick them up; the Bun server, the
  Pi mirror, the OpenCode plugin and the CLI all go through them.
- The annotate /api/plan payload ships the resolved list so the renderer can
  linkify links to sibling documents (module-level UI registry, empty by
  default, so nothing changes without config).

Validation: entries must be dot-led, lowercase-normalized, and free of path
separators, globs and whitespace. Invalid entries are dropped silently,
built-ins are deduplicated, and `.env` is denylisted so config can never
register it (annotate copies file contents into the data dir).

Deliberately unchanged: the Pi plan-write allowlist (ALLOWED_PLAN_EXTENSIONS
in tool-scope.ts) and Edit Mode source save (SOURCE_SAVE_FILE_REGEX), which
keep their own narrower allowlists.

* fix(annotate): deny the dotenv family and sandbox config-aware tests

Review follow-ups on #1309:
- deny the whole dotenv family (.prod.env, .env.local, ...) in
  normalizeMarkdownExtensions, not just the exact .env name
- resolve config.json path per call instead of at module scope so
  PLANNOTATOR_DATA_DIR sandboxing works in single-process test runs
- stop resolve-file.test.ts reading the real user config: pure
  predicate imports plus pinned empty extras on every resolve call
- add the config.json -> memo -> predicate integration test using
  resetMarkdownExtensionsCache under a temp data dir

* test(call-flow): make the stale-read advert test self-sufficient

The read-only GET only probes the node runtime while Call flow is
enabled. The stale-read test relied on earlier tests' settings POSTs
leaking callFlow=true through the process-frozen config path; with lazy
config resolution each sandbox is genuinely isolated, so the test now
enables Call flow in its own data dir. Locally the dependency was
masked by an fnm-shimmed sem sidecar spawning node coincidentally.
2026-08-13 09:47:18 -07:00
Michael Ramos 14e5c9ebd1 Fix folder watcher cold-start refs scan (#1306) 2026-08-13 09:45:59 -07:00
Leonardo Reis 1d4e490b8e fix(review): update Codex automatic approval flag (#1231) 2026-08-13 09:42:32 -07:00
Michael Ramos 27791a6fba Mobile Phase 2B: Plan shell and navigation (#1303)
* feat(editor): add compact plan navigator

* feat(editor): simplify compact plan chrome

* fix(editor): close compact navigator after file selection

* feat(editor): add compact plan review surfaces

* fix(editor): hold navigator through cold file loads

* fix(editor): preserve desktop diff activation
2026-08-13 09:38:14 -07:00
Michael Ramos 64e3fa7762 feat(review): add compact touch review shell (#1301)
* feat(review): add compact touch review shell

* fix(review): let submission dialog own initial focus

* fix(review): refine compact mobile review chrome

* fix(review): restore reliable mobile diff scrolling

* fix(review): preserve mobile file identity

* docs(mobile): specify phase 2b plan shell

* fix(review): close mobile shell regressions

* fix(review): restore narrow overview stacking

* test(review): preserve real syntax theme resolver
2026-08-13 09:29:24 -07:00
Michael Ramos f387cdabde feat(ui): add mobile-safe touch and dialog primitives (#1300)
* feat(ui): add mobile-safe touch and dialog primitives

* fix(ui): scope touch targets to compact shell
2026-08-13 09:06:51 -07:00
Michael Ramos e181b824cc Mobile-safe plan and code comment composition (#1297)
* feat: harden mobile comment composition

* fix(ui): keep mobile app inside Safari viewport

* docs: record physical mobile triage

* fix(ui): extend plan canvas behind Safari controls

* fix(ui): let mobile plans drive Safari chrome

* fix(ui): release Safari top edge on mobile plans

* docs: triage mobile feedback and close phase 1b

* fix(ui): harden compact touch behavior
2026-08-13 08:58:55 -07:00
Michael Ramos d3633c9c52 Mobile foundation and quieter first run (#1295)
* feat: establish mobile foundation and simplify onboarding

* fix(ui): finish mobile foundation cleanup
2026-08-13 08:45:00 -07:00
Michael Ramos 356b628b6f ci(security): add Semgrep CE and Trivy monitoring (#1294)
* ci(security): add Semgrep CE and Trivy monitoring

* fix(ci): diagnose Trivy coverage assertions

* fix(ci): accept Trivy repository scan metadata

* fix(ci): harden scanner failure diagnostics
2026-08-12 20:40:08 -07:00
Michael Ramos 28d6d55927 Default new Code Review users to Tree (#1292)
* fix(review): default new reviewers to Tree

* test(review): prove Tree seed preserves preferences
2026-08-12 20:30:36 -07:00
Michael Ramos ef49c701c2 chore: bump version to 0.27.1 2026-08-12 17:07:36 -07:00
Michael Ramos 1aaedf9330 fix(review): detach open-in-editor launches and bound the wait (#1289)
Open-in-app launchers are now spawned in their own process group and the
request waits only a short grace (2s) for instant failures, with stderr
drained concurrently from spawn time. A launcher that is still running at
the deadline is treated as launched and the request resolves ok; instant
failures keep the existing friendly error shape (not-found, exit code plus
stderr). Mirrored in the Pi server (two-runtime law).

Fixes two demonstrated defects: the request (and the UI button) was held
hostage until the launcher CLI exited, and a launcher in the session's
process group could be killed along with the session, taking a cold-started
editor down with it.
2026-08-12 17:06:22 -07:00
Michael Ramos 3282be673b fix(review): hide viewed and stage controls in file headers when toggled off (#1288) 2026-08-12 17:06:19 -07:00
Michael Ramos d0d971a3bf chore: bump version to 0.27.0 2026-08-12 14:14:35 -07:00
Michael Ramos ed6f44bf2e fix(release): tailscale gate exit codes and lease gating, conditional SIGHUP, informative guide validation error (#1286)
- annotate --tailscale publish failures now exit through
  annotateStartupFailureExitCode: exit 2 under a strict gate
  (--require-approval / --result-file), where exit 1 is reserved for "the
  reviewer did not approve, decision record published". Non-strict annotate
  and review keep the documented exit 1.
- the annotate client lease (auto-dismiss on abandonment) is forced off
  while tailnetPublished is set: --tailscale reads as local to the CLI
  predicate, but clients connect through the serve proxy, and a proxy
  disconnect longer than the grace would dismiss a live review. Same
  rationale as remote/shared sessions; decided at the single point both
  the /api/plan advert and the SSE endpoint read.
- the SIGHUP-to-process.exit route moved from an unconditional CLI-entry
  listener into enableTailscaleServe's success path, installed only once a
  serve mapping exists. Any SIGHUP listener overrides the ignored
  disposition nohup depends on, so plain sessions now keep zero listeners
  and "nohup plannotator review &" survives terminal close again;
  --tailscale sessions still tear their mapping down on HUP (exit 129).
- validateGuideOutput explains a fully-invalidated guide whose refs named
  files outside the changeset (count plus up to 3 example paths, with a
  pointer to the Commits panel) instead of the bare generic message; the
  generic message stays for genuinely structural emptiness. The informative
  error now flows through onJobComplete to the job failure card; Pi picks
  the change up via the vendored guide-review copy.
2026-08-12 14:04:59 -07:00
Michael Ramos 5f33b72b2f feat(remote): tailnet auto-advertise, ready QR code, and a first-class --tailscale mode (#1280)
* feat(remote): resolve urlHost auto from Tailscale for advertised URLs

PLANNOTATOR_URL_HOST=auto (or config urlHost: "auto") detects this
machine's tailnet host at first use in a remote session: MagicDNS name
from tailscale status --json, falling back to the single tailscale ip -4
CGNAT address. Detection is cached per process, never spawns in local
sessions, warns once and falls back to localhost on failure, and stays
strictly display-only: binding remains governed by PLANNOTATOR_REMOTE.

Pure parsers live in the new @plannotator/shared/tailscale module,
vendored to the Pi extension; both runtimes mirror the resolution.

* feat(remote): render a terminal QR code for remote-ready session URLs

Remote sessions print their advertised URL as the lifeline; the usual
next step is opening it on another device (iPad, phone, laptop off the
VPS). handleServerReady now also renders a compact unicode QR of that
URL via the zero-dependency uqr package, TTY-gated so piped stderr and
hook transcripts keep only the plain URL line.

Pi keeps URL-only parity: its ready surface is an in-chat notification,
not a TTY stream, so a QR block would not render there.

* feat(cli): first-class --tailscale mode for review and annotate sessions

plannotator review --tailscale (also annotate and annotate-last/last)
publishes the session over the user's tailnet: the server stays
loopback-bound and the CLI orchestrates tailscale serve --bg
--https=<port> http://127.0.0.1:<port>, then advertises the HTTPS
tailnet URL with a terminal QR code. Nothing listens beyond localhost
and nothing is ever public (serve, never funnel).

Guarantees: preconditions fail with actionable errors (CLI missing,
daemon down or logged out); a pre-existing serve mapping on the chosen
port aborts instead of being stolen and other ports are never touched;
every mapping the process creates is torn down on normal completion,
SIGINT/SIGTERM, and errors via the exit-routed cleanup handler. When
combined with PLANNOTATOR_REMOTE or SSH detection, --tailscale wins and
forces local mode with a stderr notice, which also restores the random
local port so simultaneous sessions get distinct serve mappings.

* fix(remote): await tailscale-ready failures, harden serve teardown and conflict detection

Review fixes for #1280 (external review plus internal security review).

Startup failures no longer hang the session: startReviewServer and
startAnnotateServer now await async ready handlers and stop the server
on rejection, and the CLI's --tailscale ready path resolves publishing
failures itself with an actionable stderr message and exit 1. Under the
bang-prefix skill a hanging loopback server blocked the whole Claude
Code prompt.

Serve teardown is checked, not assumed: a failed off retries once, then
warns with the exact manual command, and a port is only forgotten after
a successful off. SIGHUP (terminal close) is now routed through
process.exit like SIGINT/SIGTERM so exit-time cleanup runs. Docs no
longer claim guaranteed cleanup: --bg mappings survive SIGKILL and
reboots, and the manual removal command is documented.

Conflict detection sees foreground serve sessions (Foreground.*.TCP),
which Tailscale prefers over background mappings, and fails CLOSED on
unrecognizable serve status output instead of assuming the port is
free. The extracted serve URL must match the requested port, so a
version-dependent output shape cannot advertise another mapping's URL.

The annotate agent terminal is gated off by default under --tailscale
behind the existing PLANNOTATOR_AGENT_TERMINAL_REMOTE opt-in: the PTY
token is not an auth boundary against network peers, and tailnet
reachability implies terminal reachability.

Also: --tailscale is rejected with a clear error on unsupported
subcommands and documented in review/annotate/annotate-last and
top-level help; the remote-ready QR renders only for URLs actually
reachable off-machine (never localhost); urlHost is suppressed for
--tailscale runs so the local-session warning cannot mislead; the
duplicated auto-host resolution moved into the shared vendored module;
tailscale-serve tests restore module and process state via a reset
seam.
2026-08-12 12:07:30 -07:00
Michael Ramos 1bf90a2357 feat(editor): focus-mode shortcut to toggle both sidebars (#1279)
* feat(editor): focus-mode shortcut to toggle both sidebars (#1276)

Keyboard-first reviewers had to reach for the mouse to collapse the
Contents sidebar and the right-hand panel every time they wanted to read
a document. Bind that to one key.

Mod+. now toggles the existing `focus` view mode from the keyboard, on
both the plan review and annotate surfaces. The first press remembers
whatever was open and closes both panels; the next press restores that
exact arrangement, so a session with only one panel open comes back the
same way. Any manual open (a sidebar tab, the annotation panel, the
agent terminal) already exits the view mode and clears the memory, so
the shortcut cannot leave the layout stuck.

The binding was picked after auditing every plan-review and annotate
binding: Mod+B / Mod+Shift+B are the sidebar toggles, Mod+S, Mod+P,
Mod+Enter, Mod+C and Mod+Z are taken, and code review already uses
Mod+. for its own "collapse the chrome" toggle. It is unshifted on every
keyboard layout and claimed by no browser or OS default.

The dispatcher gate is shared with the annotate sidebar shortcuts, so
the key is inert while a dialog, an overlay, a submission, or a text
field owns the keystroke. HTML surfaces are excluded because they own
their own persisted chrome and never render the Focus control.

* fix(editor): keep focus-mode exit reachable on HTML surfaces; note the HTML caveat in help
2026-08-12 11:54:31 -07:00
Michael Ramos 4d2dc5772e fix(review): restore the prior diff when leaving the Commits view (#1278)
* fix(review): restore the prior diff when leaving the Commits view

Clicking a commit in the Commits panel (or its HEAD auto-select) switches
the single session-global diff to commit:<sha>, and switching the panel
back to Tree never switched it back, leaving the review stuck on a
historical commit (shipped as an accepted v1 edge in #994). A page reload
did not escape either, since GET /api/diff serves the current session
diff.

Client-only fix:
- preCommitDiffRef captures the pre-commit diff (full diff type + base)
  on the first non-commit to commit switch in handleSelectCommit.
- Leaving Commits for Tree restores it via fetchDiffSwitch; without a
  memo (post-reload) it falls back to the session default with the same
  resolution handleWorktreeSwitch uses. Exiting while the commit switch
  is still in flight also restores, superseding it server-side.
- Any applied non-commit switch clears the memo in fetchDiffSwitch.
- Reload un-trap: a load that is served a commit diff while the opening
  panel view is not Commits snaps back to the session default once.

Git status keeps its existing since-base reset; the server keeps its
single-active-diff model unchanged.

* test(review): cover the offered-but-not-first preferred default; document the Commits detour contract
2026-08-12 11:54:27 -07:00
Michael Ramos 8e7b5ce300 feat(review): refine Call Flow navigation and annotations (#1277)
* feat(review): refine Call Flow navigation and annotations

* fix(review): align viewed controls with panel navigation

* feat(review): add Call Flow path search controls

* fix(ui): wrap long tooltip identifiers

* feat(review): annotate raw Call Flow output

* feat(review): refine call flow lens context

* fix(review): keep call flow lens search accessible

* fix(review): scope call flow find shortcuts
2026-08-12 11:18:29 -07:00
Michael Ramos eb34960da9 fix(review): remember the last-used panel view; full-width toggle and cleaner panel chrome (#1273)
* fix(review): remember the last-used panel view across sessions

- Add cookie-only reviewPanelViewLastUsed config (sections | tree, never
  commits); the header toggle records it and new sessions resolve
  session state, then last-used, then the persisted reviewPanelView.
  Explicit Settings/setup-dialog writes through the reviewView setters
  sync last-used so a stale memo cannot shadow them.
- Wire the FileTree fallback PanelViewToggle to the real panel-view
  selection and route Tree clicks through the shared select funnel
  instead of rendering a hardcoded, no-op tree segment.
- Mark the review-setup-seen cookie when the first-run reset runs, not
  only on the Got it click, so closing the tab without dismissing no
  longer re-stomps preferences every session.

* fix(review): full-width panel toggle, relocated tree controls, click-to-copy diff count

- Give the Git status | Tree | Commits toggle the entire top row in all
  three left panels; segments stretch to fill the row width.
- Move the controls that shared that row (staged count, search,
  collapse-all, hide-viewed, viewed counter) into a shared
  PanelControlsRow rendered below the All files entry, directly above
  the file list; the Commits panel's loaded-commit count moves to its
  own sub-row.
- Replace the footer Copy button with the diff count itself as the copy
  trigger (shared CopyDiffFooter): a keyboard-reachable button with the
  "click to copy the entire diff set" tooltip and transient
  Copied/Failed feedback in the old button's slot.
- Add a tooltip to the file-row stage button using the same Tooltip
  mechanism as the viewed control ("Stage file (git add)").

* fix(review): put Tree first in the panel view toggle

* fix(review): resolve effective panel view for the toggle; keep self-heal off the last-used memo

- Add resolvePanelView() and pass the EFFECTIVE view to FileTree so a
  latent sections/commits selection the session cannot offer highlights
  Tree instead of a hidden or wrong segment; allFilesOrder reuses it.
  Unit test covers the no-segment and wrong-segment cases.
- setReviewPanelView() gains { recordLastUsed?: boolean } (default
  true); the App self-heal passes false so repairing a conflicted
  persisted pair no longer overwrites the user's last-used memo.
- Update the AGENTS.md since-base section to the new toggle semantics
  (last-used memo, resolution order, setter sync, one-time first-run).
- Hardening: fold the diff counts into the copy button's aria-label,
  skip the tooltip while the copy button is disabled (hover never fires
  on a disabled native button in several browsers), let toggle segments
  shrink (min-w-0 + truncate), and rework the config test onto a fresh
  ConfigStoreForTest instead of resolving the singleton against a
  throwaway backend.
2026-08-11 23:54:22 -07:00
Michael Ramos fc348687bf fix(review): contain /api/call-flow analysis throws as JSON error responses (#1272)
* fix(review): contain /api/call-flow analysis throws as JSON error responses

A hard VCS failure during patch materialization escaped the handler in
both runtimes. On Pi the unhandled rejection reached the process-level
handler and killed the user's session; on Bun it surfaced as a non-JSON
500 the client's quiet-failure UX could not parse. Both handlers now
return the standard { status: "error", reason: "analysis-failed" }
envelope.

* fix(review): cut the Call Flow consent copy down to the three facts that matter

Six sentences of disclosure read as noise. The dialog and Settings now
say: what it does, what it installs (languages + size), Node 22+, and
that other languages install as needed. Nothing consent-relevant was
removed.

* test(review): pin consent-copy facts, not prose

The presentation test now asserts the server-derived facts (languages,
size, Node floor); the dialog and Settings tests assert only that the
disclosure prop renders, via a sentinel string. Copy edits no longer
break three test files.

* docs: add Testing Rules to AGENTS.md (no prose-pinning, no round-trip prop tests)

* docs: refine copy-pinning rule — deliberate locks allowed, incidental snapshots banned

* fix(review): use the maintainer's Call flow description in the intro dialog and Settings

* fix(review): Call flow description is the maintainer's exact copy; remove the dynamic disclosure plumbing

The intro dialog and Settings now show only: 'Diffs for function call
stacks across git commits. 22 languages supported (AST-based, built
using Tree-sitter).' The callFlowEnableDescription prop, its App wiring,
and getCallFlowEnableDescription are removed; install size and Node
requirements remain visible in the Call Flow panel itself.

* fix(review): reject empty-path worktree diff types; clean up QA findings

- parseWorktreeDiffType returns null for a worktree diff type with no path.
  An empty path resolved to an empty cwd, and Bun.spawn({ cwd: "" }) runs
  git in the server's own directory instead of the target repo, so a
  malformed 'worktree:' switch returned an unrelated checkout's diff.
  Fail closed to the caller's real cwd. (Pre-existing; surfaced by QA.)
- Remove an orphaned JSDoc comment left by the callFlowEnableDescription
  prop removal in Settings.tsx.
- Add useCallFlowAnalysis.test.tsx to the CI DOM_TESTS list; its two
  tests were silently skipping on every run.
2026-08-11 22:55:18 -07:00
Michael Ramos caf7ce1ccd feat(review): install Call Flow automatically in the background on opt-in (#1271) 2026-08-11 17:48:18 -07:00
Michael Ramos 9ee2e83287 feat(review): make the CallDiff runtime a strictly opt-in, in-UI install (#1270)
* feat(review): make the CallDiff runtime a strictly opt-in, in-UI install

The merged CallDiff integration eagerly installed a ~784MB runtime for
every user at install time, for a feature that is off by default. The
runtime is now strictly opt-in and the opt-in lives in the review UI:
toggle Call flow, click Install in the panel, watch staged progress, and
use the analysis in the same session.

Installers: the default sequence no longer installs the runtime. Opt in
with --with-call-flow (PowerShell: -WithCallFlow),
PLANNOTATOR_INSTALL_CALLDIFF=1, or { "installCallFlow": true } in
config.json (flag > env > config). PLANNOTATOR_SKIP_CALLDIFF_INSTALL is
deleted; --minimal keeps excluding the runtime; the installer prints an
honest note pointing at the in-app install. The headless CLI path
(plannotator install-runtime call-flow) is unchanged.

Server (both runtimes, contract-identical): POST /api/call-flow/install
starts installCallFlowRuntime() in the background via a single-flighted
coordinator (concurrent POSTs join the in-flight install), runs a
Node 22+ preflight before any download (distinct node-unavailable
error), and rejects cross-origin POSTs with 403. GET
/api/call-flow/install-status reports idle/running/done/error with
stage: downloading, verifying, installing-deps, building. Install
completion invalidates the 30s runtime probe cache so the next
capability advert resolves available without a server restart.

Client: the Call flow Dock's runtime-missing state is now the opt-in
funnel with an honest disclosure (about 800 MB on disk, Node 22+,
one-time), staged reduced-motion-safe progress, and error + retry with
a no-node hint. On done the advert is refreshed through
POST /api/review-analysis and the existing available-change refetch
starts the analysis for the current snapshot with no reload. The intro
dialog and Settings toggle note the separate first-use runtime.

Docs: AGENTS.md env table + Review Server API table, marketing
environment-variables / installation / ui-settings / code-review /
api-endpoints pages, and the CallDiff ADR runtime-boundary and server
contract sections.

* test(review): stop leaking PLANNOTATOR_DATA_DIR from the install endpoint tests

The call-flow install endpoint tests overrode PLANNOTATOR_DATA_DIR at
module-eval time and never restored it. bun runs CI's full suite in one
process and evaluates every test file's module before running tests,
while Pi's generated/storage.ts caches its data dir at import time; the
override therefore made storage's cached dir and later files' live
getPlannotatorDataDir() calls disagree, failing the Pi annotate-history
unwritable-dir test and both durable-submit-record tests.

An afterAll restore alone is not enough: it reproduces the same three
failures with the mismatch inverted (storage caches the leaked dir at
module eval, tests then run against the restored one). The env var is
now never touched at module-eval time at all; it changes only inside
tests and is restored to its original value in afterEach, exactly like
the PORT/PATH pattern. The config writes the advert tests persist
through the process's frozen config module are snapshotted at load and
restored in afterAll so a standalone run never flips a real
config.json setting, and the process-global scope of the mock.module
seams is documented.

Regression proof (previously failing in either mismatch direction, now
green in both orderings):

  bun test packages/server/call-flow-install-endpoint.test.ts \
    apps/pi-extension/server/annotate-history.test.ts \
    apps/pi-extension/server/annotate-submission.test.ts

* feat(review): install CallDiff grammars selectively

* fix(review): harden CallDiff worker environment

* fix(review): close CallDiff verification gaps
2026-08-11 16:28:08 -07:00
Michael Ramos 3245310aa8 feat(review): add optional CallDiff call-flow analysis (#1268)
* feat(review): add optional CallDiff call-flow analysis

* fix(review): harden CallDiff integration
2026-08-11 13:18:35 -07:00
Michael Ramos 98113182b5 feat(guide): reviewer-supplied extra instructions for Guided Review (#1267)
* feat(guide): reviewer-supplied extra instructions for Guided Review (#1265)

Adds a quiet, collapsed-by-default Custom instructions affordance to the
guide launch page. The text is APPENDED to the built-in organizer
methodology as a clearly delimited section (composeGuideMethodology) and
never replaces it; absent or blank instructions produce byte-identical
prompts to before. Persisted in a dedicated cookie
(plannotator-guide-instructions) so a standing team preference survives
sessions without bloating the plannotator.agents blob past the browser's
per-cookie limit.

Server side, the launch body gains an optional guide-only instructions
field (both the Bun and Pi node:http agent-jobs handlers accept and
thread it); prompt composition lives in the shared guide-review.ts that
vendor.sh already vendors to Pi, so both runtimes compose identically.
Text is capped at GUIDE_EXTRA_INSTRUCTIONS_MAX_CHARS (2000) server-side
and mirrored by the textarea maxLength. Repair launches deliberately
ignore instructions: a repair is a mechanical JSON fix, not a rewrite.

Tests pin the regression contract (empty input keeps prior prompt bytes),
appended-not-replacing composition, the length cap, repair isolation, and
the cookie round-trip via the storage backend seam.

* refactor(guide): store standing instructions server-side, not in a cookie

Review findings on the cookie approach (silent write failure past the
encoded 4KB per-cookie limit for multi-byte text) pointed at the real
design problem: the instructions are consumed by the SERVER at launch
time, so they belong in the data dir like review-skills.json, where no
size ceiling or encoding inflation exists and the preference follows
the machine instead of one browser profile.

New GET/PUT /api/agents/guide-instructions in both runtimes backed by
shared guide-instructions-store (vendored to Pi). Guide launches apply
the stored text when the body carries none; the launch page still sends
its live textarea value (explicit wins), so a just-typed preference can
never race the debounced save. The sidebar surface sends nothing and
inherits the stored text server-side. All cookie machinery removed.

Also folds in the review fixes: marker-tag-shaped strings in
instructions are defanged so first-match nonce recovery cannot be
hijacked by pasted examples.
2026-08-11 10:24:39 -07:00
Michael Ramos ab8d2581eb feat(ui): onUnanchoredChange report + readOnly keeps the host footer slot (0.30.0) (#1263)
The bridge now names WHICH annotations have no live representation on
the page (every target dead, or the restore never resolved), reported
on change through a new validated message and the HtmlViewer
onUnanchoredChange prop, delivered in readOnly mode too. Fail-closed
anchors previously hid markers silently.

AnnotationPanel readOnly no longer suppresses the renderCardFooter
slot: its contents are host-owned and may be read affordances (replies,
links), so the host gates what belongs in it. Built-in delete/edit and
direct-edit discard stay hidden.
2026-08-10 22:46:55 -07:00
Michael Ramos 96ad52cd90 fix(ui): null-guard vim focus reassert + publish/migration doc corrections (#1262)
* fix(ui): null-guard vim focus reassert + publish/migration doc corrections

The vim focus-reassert guard compared a nullable iframe ref to the
nullable document.activeElement: with both null the branch runs and
iframe.contentWindow throws. Also flagged by the first strict-TS
consumer of @plannotator/ui 0.29.0. HANDOFF gains the CI-only
provenance note, the highlight.js bundler-alias removal warning, and
the known @pierre/theming peer-range warning.

* chore(ui): bump to 0.29.1 for the null-guard fix
2026-08-10 22:10:22 -07:00
Michael Ramos acb4979c59 chore(ui): prep @plannotator/ui 0.29.0 + core 0.23.0 publish (#1222) (#1261)
Bless components/html-viewer as supported host surface, document the
.hljs to pn-code migration, pin the readOnly view-only contract with
tests, and bump both published packages for the next manual publish.
Core must publish first: ui 0.29.0 imports @plannotator/core/annotatable,
which is absent from published core 0.22.0.

Closes #1222
2026-08-10 20:21:45 -07:00
Michael Ramos 2fff8756d9 chore: bump version to 0.26.8 2026-08-10 17:00:33 -07:00
Michael Ramos 7d7654fcda feat(annotate): minimal-by-default HTML sessions with stale-preference decay (#1260)
* feat(annotate): minimal-by-default HTML sessions with stale-preference decay

Raw-HTML annotate sessions now open minimal by default: Pinpoint input, tools
hidden, left sidebar closed, and the right annotations drawer closed (the
drawer joins the persisted HTML chrome state, previously it always opened on
desktop). Explicit user choices still persist between HTML sessions, but the
records now carry a timestamp and expire after 7 days without a refresh, so
users who have not changed anything or annotated HTML in a while come back to
the product defaults. Explicit changes and annotation activity both re-stamp
the records, so active users keep their setup. Legacy untimestamped cookies
are treated as expired (a one-time reset to the new defaults). The markdown
surface keeps its own preference with no TTL, unchanged.

Mutation-verified: disabling the TTL fails 4 tests.

* test(annotate): stamp seeded chrome cookies and compare semantic fields

The App-level chrome suite seeded legacy untimestamped cookies, which the
stale-preference decay now treats as expired by design, and one assertion
compared cookie bytes that re-stamping legitimately changes. Seeds carry a
fresh savedAt and the write-integrity assertion compares the chrome fields.
2026-08-10 16:22:41 -07:00
Michael Ramos c4fc79f0aa fix(annotate): neutralize document-authored CSP meta tags so the bridge can run (#1259)
A document carrying its own <meta http-equiv=Content-Security-Policy> (including
Plannotator's own portable guided-review exports, which embed default-src 'none')
blocked the injected inline bridge script, disabling annotation entirely for that
file. The iframe sandbox attribute is the annotate surface's security boundary;
the page's CSP was authored for its standalone context, so injectIntoHead now
strips CSP meta tags (order/quote/case tolerant) before splicing the bridge.

Pre-existing bug, surfaced during v0.26.8 manual QA. Mutation-verified test.
2026-08-10 16:00:45 -07:00
Michael Ramos 121082430e fix: QA-gate hardening for the v0.26.8 feature set (overlay perf, numbering, OpenCode 2 parity) (#1258)
* fix(opencode): consolidate V2 system parts into one composed prompt (#1114)

The OpenCode 2 adapter still shipped the pre-#1114 multi-part system
injection: replacePlanningSystemParts kept one part per source and the
generic reminder pushed a separate part, so Qwen3.x Jinja template
corruption persisted for OpenCode 2 users. Mirror the V1 entry exactly:
compose the stripped existing text plus additions into a single system
part via composeSystemPrompt, and compose the generic reminder into the
existing text instead of appending a second part.

Also adds the regression tests for the bug class flagged in #1114's
review: both helpers must read/compose the existing system text BEFORE
truncating the array (a reorder to 'system.length = 0' first drops the
host prompt and goes red here).

* perf(annotate): harden the raw-HTML overlay reconcile (dead-target backoff, cull, batching)

Bridge-script hardening for mutation-heavy pages and large annotation
sets, plus the lost click-to-select hover affordance:

- A: dead-target re-search now carries a wall-clock backoff (300ms
  doubling to a 5s cap, reset on success) ON TOP of the generation gate,
  plus a 2-searches-per-reconcile-pass budget with a scheduled follow-up
  pass for budget-skipped eligible targets. A page that mutates every
  frame advances domGeneration every frame, so the generation gate alone
  re-ran the whole-document TreeWalker sweep (and anchor re-resolution)
  per frame forever for permanently unresolvable targets.
- B1: early viewport cull (64px margin) for element and range targets:
  wholly offscreen targets skip targetStyleHidden / getComputedStyle /
  clipBoundsFor / client-rect collection entirely and just omit their
  markers, which is what the visible pipeline produced anyway.
- B2: read/write batching in renderAnnotationOverlay: highlight rects are
  queued during the read phase and flushed as one write phase, so the
  pass no longer forces a synchronous layout per record.
- B3: restoreAnnotation defers its render through the existing
  rAF-coalesced reconcile scheduler; restoring N annotations now renders
  once instead of N full passes (searches stay synchronous for the
  mark-applied reply). DOM tests flush the frame via the suite's
  standard macrotask flush.
- B4: zero-work observer gate: page mutations with no records, no
  pending draft, and pinpoint inactive still bump domGeneration but no
  longer schedule a reconcile frame.
- D: hover affordance for click-to-select: the rAF-throttled mousemove
  hit-tests the pointer against the CACHED rendered committed rects and
  toggles a brightness class on that annotation's rect divs inside the
  shadow root. No page-DOM writes, rects stay pointer-transparent, and
  shadow-root writes are unobserved so there is no reconcile loop.
- G: while a text drag is in progress in drag mode, placed markers yield
  pointer input (data-pn-hittest) so the 25px bubble cannot capture a
  selection drag; armed only by a >4px primary-button move from a
  non-overlay mousedown, so marker clicks and click-to-select paths are
  untouched. withMarkersYielded now restores (not clears) the attribute.

New regression tests for A, B1, B3, B4, D; A/B1/B3 mutation-verified
(fix reverted, test observed failing, fix restored).

* fix(annotate): make on-page marker numbers match exportAnnotations numbering

The HtmlViewer sync excluded GLOBAL_COMMENT annotations before numbering
while exportAnnotations numbers '## N.' sections across the FULL list
including globals — so an on-page 'Comment 2' could be '## 3.' in the
feedback the agent reads. The sync now derives each marker's number from
its position in the full createdA-sorted list (globals occupy a number
but ship no entry, leaving the correct gaps on-page). Export format is
unchanged.

New buildSyncNumbering helper + tests asserting a mixed list yields
identical numbers between the sync payload and exportAnnotations output
(mutation-verified against the pre-fix ordering).

* chore: sync stale workspace versions in bun.lock (0.26.1 -> 0.26.7)

* docs: document raw-HTML overlay model, multi-target types, and known limitations

- Data Types: add htmlAdditionalTargets to the Annotation listing plus
  the HtmlElementAnchor (including the optional normalized point used by
  placed markers) and HtmlAnnotationTarget shapes.
- Annotation System: describe the post-#1257 raw-HTML surface (placed
  comment markers + overlay-projected highlights, no inline mark
  mutation; durable anchors persisted, disposable markers projected) and
  the print-parity limitation.
- URL Sharing: note that share links intentionally drop HTML element
  anchors and additional targets (restore is text-search based, per
  sharing.multiTarget.test.ts).

* test: fix Range.getClientRects stub typing in the B1 cull test

* fix(annotate): hover-race teardown and unbounded one-shot dead-search passes

Polish round on the overlay hardening:

- Hover race (1): switching into pinpoint mode (or opening a draft) now
  tears hover down fully via clearHoverHighlight() — cancels the pending
  rAF hit test and clears the tracked position and id — and the rAF
  callback itself refuses to paint outside drag mode / with an open
  draft. Previously the pending callback re-applied the class after the
  mode switch and every flushQueuedHighlights re-painted it from the
  stale hoverHighlightId, leaving a permanent phantom hover.
- One-shot budgets (3): beginDeadSearchPass takes a per-pass budget.
  Reconcile passes keep 2 (they repeat, skipped targets get follow-up
  frames); print and scroll-to are user-initiated one-shots with no
  follow-up and now run unbounded (backoff and generation gates still
  apply), so printing with 3+ dead-but-recoverable targets no longer
  silently prints fewer highlights.

Both changes carry new regression tests, mutation-verified (fix
reverted, test observed failing, fix restored).

* fix(annotate): number markers by array position and cap entries after dropping globals

The createdA sort made the export-match invariant false with external
annotations: exportAnnotations' sort keys tie for every raw-HTML
annotation (blockId '', startOffset 0), so its stable sort numbers the
combined [...local, ...external] list in ARRAY order — and external
annotations arrive appended with server-stamped createdA values that can
interleave with local timestamps. buildSyncNumbering now numbers by
array position of the input (verified to be the same combined list both
consumers receive from packages/editor/App.tsx allAnnotations; the
viewerAnnotations diffContext filter is order-preserving and vacuous on
the raw-HTML surface).

Also reorders the cap: number the full list, drop globals, THEN slice
512 entries — globals no longer waste sync capacity and a non-global the
export numbers past position 512 still syncs while slots remain. Numbers
may now exceed 512 (array positions); the bridge's own bound (100000)
accepts them and its 512-entry cap still agrees with the sender.

Tests updated: interleaved-external agreement with exportAnnotations
(mutation-verified against the createdA sort) and slice-after-filter
capacity.

* docs(opencode): note the accepted cache-hint flattening trade-off in V2 consolidation
2026-08-10 15:27:04 -07:00
Yohai Wiener 50092d7634 feat(review): add file header hover state (#1256) 2026-08-10 13:16:30 -07:00
Michael Ramos be0b1f185d feat(annotate): placed comment markers for raw-HTML annotation (#1257)
* feat(annotate): overlay-projected placed comment markers for raw HTML

Annotation state is no longer written into the visited page's DOM. A
fixed, pointer-transparent, shadow-rooted overlay host (appended to the
root element, outside page layout) now owns every committed annotation
visual:

- numbered placed-marker buttons (product-owned SVG speech bubble,
  accent-token colors, accessible 'Comment N' labels) projected at the
  user's selected relative point, re-resolved from durable anchors and
  reprojected on scroll/resize/mutation/animation via rAF-coalesced
  invalidation (never polled)
- persistent highlight rectangles for text-range annotations (built
  from Range.getClientRects), replacing inline <mark> wrapping
- the focused (blue) treatment as overlay rects covering EVERY rect of
  EVERY target, replacing the .focused class that querySelector applied
  to only the first mark of a multi-paragraph selection
- the draft selection highlight as overlay rects from the live pending
  range

Markers omit rather than guess: unresolved anchors, zero-size targets,
viewport/clip-scrolled-away targets, and clamped points no longer
visibly associated with their target all hide the marker. Viewport-edge
clamping keeps the full marker reachable; coincident markers spread
horizontally and deterministically. Numbering is parent-authoritative:
HtmlViewer syncs the ordered saved-annotation list (panel order,
index+1) to the bridge; grouped multi-select targets all carry their
one annotation's number. The anchor DTO additively gains the normalized
selected point (validated and clamped at the trust boundary), captured
from pinpoint/shift/data-annotate clicks.

Fixes the partial/auto blue highlight (focus-mark and scroll-to touched
only the first mark) and page layout breakage (surroundContents plus
padding/negative-margin marks mutated author content).

* test(annotate): migrate bridge DOM suite to the overlay marker contract

Mark/badge assertions become overlay assertions: markers are queried
through the shadow overlay host, restoration binding is proven via
scroll-to targets instead of inline mark containment, and page-DOM
purity is asserted byte-for-byte across restores.

* test(annotate): regression coverage for the placed-marker overlay contract

Bridge-side (srcdoc.test.ts): layout neutrality (byte-identical page DOM
across restore/sync/focus/scroll, host outside body, pointer-transparent),
relative-point capture and reprojection with fresh geometry, unresolved-
anchor and scrolled/clipped omission, viewport edge clamping with
visually-detached omission, deterministic coincident spreading, parent-
synced numbering override and renumbering, malformed sync rejection,
full-coverage focus rects (partial-blue regression), overlay draft
highlight, and hit-test yielding beneath markers.

Parent-side (htmlPinpointProtocol.test.tsx): anchor-point validation
(clamping, hostile-point dropping without losing the anchor), point
propagation onto committed annotations, multi-target anchor points, and
the ordered saved-annotation number sync (createdA order, index+1,
globals excluded, never before bridge-ready).

* test(annotate): fix strict typings in new overlay tests

* fix(annotate): clip-test overlay highlights, gate dead-target re-search, honest edge clamping

- M1: every painted highlight rect (committed comment/deletion, focus flash,
  draft selection) is now intersected with the target's clip-ancestor chain
  via shared clipBoundsFor()/clipRect(); rects with no visible remainder are
  dropped, so inner-scroll-container content scrolled out of its box no
  longer paints stripes over unrelated content.
- M2: computed-style visibility gate (visibility:hidden/collapse,
  display:none, opacity:0) treats targets as unresolved-for-display, so
  markers/focus rects stop rendering over visible content stacked in the
  same box (e.g. visibility-toggled carousel slides).
- M3: dead-target re-search (whole-document findTextRange + anchor
  re-resolution) is generation-gated: the generation advances only on
  text-capable signals (page mutations, settle events, frame loads), never
  on scroll/resize, and each target caches its last failed generation.
- m3: clipBoundsFor skips plain static overflow clippers for position:fixed
  targets until a fixed containing block (transform/perspective/filter/
  backdrop-filter/will-change) is reached.
- m4: marker association is tested against the UNCLAMPED point; the 29px
  viewport inset is rendering-only, so fully visible edge-flush elements
  keep their markers (dead band removed; sliver test updated — it conflated
  viewport-edge clamping with clip-container omission).
- m5: reconcile on document.fonts.ready and capture-phase subresource load
  (geometry-only: they never unlock dead-target re-search).
- m7: Range.getClientRects containment filter drops border boxes that
  duplicate their own line rects (redline double-paint).
- m9: settle events from viewer overlay nodes are identity-filtered out.
- m10: one marker per resolved element per record during refresh.
- m12: painting caps at 48 rects but the marker anchors to the TRUE last
  client rect.

* fix(annotate): restore print parity for committed highlights (M4)

The branch's '@media print { .pn-layer { display:none } }' hid ALL
annotation visuals in print, but pre-overlay the inline highlight marks
stayed visible in print on purpose (only pin badges were print-hidden).
On beforeprint (plus a matchMedia('print') mirror for Safari), committed
comment/deletion rects are re-projected into a temporary absolute-positioned
light-DOM layer in document coordinates so it paginates with content, and
torn down on afterprint. Markers remain print-hidden (parity: highlights
print, markers don't). A screen media guard keeps the layer from ever
flashing on screen, and any build error fails safe into printing without
visuals.

* fix(annotate): restore highlight click-to-select; marker-consistent pinpoint hover (M5, m6, m8, m11)

- M5: clicking anywhere on a committed range highlight posts mark-click
  again (pre-overlay parity). Rects stay pointer-transparent — the document
  bubble click handler hit-tests the point against the painted committed
  rects (smallest wins on overlap, ties to the topmost/later annotation).
  Coexistence matches the old '.annotation-highlight' handler: capture-phase
  pinpoint annotate clicks stopPropagation() first, marker buttons stop
  propagation, shift-clicks and drag-selection tails are skipped, and
  [data-annotate] elements defer to a highlight under the click point.
- m6: when the raw (pre-yield) pinpoint hit is a placed marker, the hover
  advertises the MARKER's identity (label 'Comment N', no annotate box)
  instead of labeling the element beneath — click and hover now agree.
  Annotating beneath still works by moving off the 25px bubble.
- m8: the pinpoint hover label is kept AFTER the overlay host on the root
  element (re-appended when not last), so marker bubbles can never occlude
  it at equal z-index.
- m11: clear-marks also clears the parent-synced number map so stale
  numbers cannot leak onto future records reusing an id.

* fix(annotate): parent-side bounds — cap mark-click ids, truncate the sync feed at 512 (m1, m2)

- m1: parseBridgeMessage rejects mark-click ids longer than 256 chars — the
  one page-controlled string in the changed path that lacked a length cap
  (the bridge's own sync validation already caps ids at 256).
- m2: the HtmlViewer sync-annotations effect slices the ordered collection
  to 512 entries AFTER the stable sort, mirroring the bridge-side
  MAX_SYNC_ANNOTATIONS bound so both sides agree on the first 512 numbers.

* test(annotate): committed-range extent assertions + fix-round regression coverage

- M6: restores the migration-dropped EXTENT assertions via a test-only
  bridge introspection hook (committedRanges): the vim Visual commit binds
  exactly 'Alpha ', the visual-block commit exactly 'Whole block target',
  and the pin-restore scoped range binds inside the anchored element
  covering exactly 'Anchor target text' (the scroll proxy only checked the
  element target).
- Regression tests for every behavioral fix: clip-tested highlight rects +
  focus flash (M1), style-hidden visibility gate (M2), generation-gated
  dead-target re-search (M3), print-parity layer lifecycle (M4),
  highlight click-to-select with smallest-wins overlap (M5), fixed-position
  clip exemption (m3) plus preserved clip-container omission, containment
  filter (m7), refresh dedup (m10), clear-marks numbering reset (m11),
  true-last-rect marker anchoring past the paint cap (m12), and
  marker-consistent pinpoint hover with label paint order (m6/m8).
- Parent-side: sync feed truncation at 512 after the stable sort (m2) and
  the 256-char mark-click id cap (m1).
- Test honesty: the hit-test-yield test now string-asserts the exact
  ':host([data-pn-hittest]) .pn-marker' pointer-events rule, since the
  elementFromPoint mock implements the yield itself.
- Tests advance the re-search generation via the settle-event signal:
  happy-dom stops delivering body MutationObserver callbacks once the
  overlay host holds an SVG marker button (environment bug; isolated repro
  without any bridge code — real browsers are unaffected).

* fix(annotate): bound range-rect collection at 48 with a by-index true-last read

The m12 fix had removed the collection cap: rangeClientRects materialized
EVERY client rect and the O(n^2) containment filter ran over the full list,
per range target per rAF reconcile and synchronously per click hit-test. A
large drag-selection or redline (the Range extent is uncapped — only the
selection text is capped at 10k chars) yields thousands of rects, i.e. tens
of millions of iterations per scroll frame.

Collection now breaks at MAX_HIGHLIGHT_RECTS again, so the zero-size and
containment (m7) filters operate on at most 48 entries, and the m12
requirement is met by reading the DOMRectList's final entry directly by
index (with the marker-association union extended to that tail rect, and a
zero-size tail falling back to the last paintable rect). Regression test:
60 mocked rects with a containing border box paint 47 (cap + containment)
while the marker anchors at the true 60th rect; mutation-verified against
both an uncapped collection and a capped-prefix last-rect read.

* fix(annotate): drop the opacity:0 display gate; contain-aware fixed clipping; dedup among placed markers

- The M2 gate's opacity:0 leg hid markers for the legitimate
  invisible-hit-target pattern (transparent input stretched over a styled
  control — the pinpoint hit resolves the input and pre-overlay the marker
  rendered exactly over the visible control), and failed its own carousel
  motivation anyway: computed opacity does not inherit, so a container
  faded to 0 leaves descendants at computed 1. visibility:hidden/collapse
  and display:none remain the gate.
- establishesFixedContainingBlock also treats contain layout/paint/strict/
  content and container-type size/inline-size as establishing a fixed
  containing block, so such clippers correctly apply to fixed targets.
- The per-record element dedup now runs among PLACED (visible) markers
  only: a target whose stored point is clipped away no longer consumes the
  element's slot and suppresses a sibling target whose point is visible.

Tests updated/added and mutation-verified: opacity keeps the marker,
contain:layout re-applies the clipper, and the visible sibling survives
the dedup.

* fix(annotate): watch documentElement for page mutations; print layer on the root element

- The M3 generation gate could lock out re-search forever when a page swaps
  the <body> element itself: the observer watched document.body, so
  documentElement.replaceChild(newBody, oldBody) produced no record, no
  generation bump, and a dead target whose one free retry ran against the
  interim skeleton never retried again. The observer now watches
  document.documentElement (same config), so body swaps and the new body's
  content are in-subtree.
- Consequence handled: childList mutations on the root/body whose
  added/removed nodes are ALL overlay-registered (host append, hover-label
  re-append, print-layer insert/remove) are filtered out via
  isOverlayOnlyMutation, so overlay writes neither bump the generation nor
  schedule the reconcile frame that caused them. The print layer stays
  overlay-registered through its async removal record (retired-layer
  deregistration is deferred to the next lifecycle step).
- The print layer is appended to documentElement instead of body: a page
  styling body { position: relative } made body's padding box the containing
  block and shifted every stripe by body's document offset. On <html> the
  containing block is the ICB, matching the viewport+scroll coordinates; a
  positioned documentElement is accepted as out of scope (commented).

Tests: the bridge's observer is captured at load (happy-dom stops
delivering records once the overlay host holds an SVG marker — environment
bug, so scope tests assert the observed target and drive the callback with
synthetic records): body-swap unlock, overlay-only no-bump, and the
print-layer parent are all covered and mutation-verified.
2026-08-10 13:16:27 -07:00