Commit Graph

11 Commits

Author SHA1 Message Date
Michael Ramos 44611e5300 feat(ui): publish the HTML annotation seams hosts were hand-rolling (#1395)
Parity seams for hosts of @plannotator/ui, requested by Workspaces after the HTML annotation handoff: projectHostThreads and buildPersistedHtmlAnchor in @plannotator/core; HtmlViewer onUnanchoredChange completed over the annotations prop with a restore-keyed report so hosts can drop their mark-applied listeners; published useHtmlRefresh with a fetchSnapshot adapter; published HtmlSurfaceControls (eye, refresh, pen) with label overrides; AnnotationPanel unanchoredIds chip (wired for Plannotator too); HtmlViewer maxAdditionalTargets and scrollBehavior carried on the bridge; shortcuts and utils/inputMethod blessed as consumer exports. Plannotator's behavior is unchanged apart from the new Unanchored chip, verified by a real-browser A/B including the orphan and re-anchor cycle and by a combined cross-surface verification with #1394.

AI-assisted (Claude) under maintainer direction.
2026-08-27 07:35:22 -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 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
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
Michael Ramos f8951cd3c6 feat(annotate): shift-click multi-element selection for raw-HTML pinpoint (#1254)
* feat(annotate): shift-click multi-element selection for raw-HTML pinpoint drafts

One comment covering multiple targets: shift-clicking elements while a
pinpoint draft composer is open toggles them in/out of the SAME draft.

- bridge: pendingMultiTargets registry with per-target pinned outline boxes,
  DOM-identity + anchor-equality toggle dedup, primary promotion on removal,
  draft cancel on last removal, shift-hover preview, rAF pointer relay for
  the composer yield, remove-target/flash-target parent messages, capped at
  16 additional targets at the source
- pins: registerPin now allows several elements per annotation id (deduped
  by (id, element)/(id, anchor)); badges number by first-seen id so all
  targets of one annotation share one number; find-and-mark restores
  additionalAnchors as same-numbered pins (anchor-only, fail-closed)
- parent hook: multi-target-added/removed/pointer messages validated and
  capped at the trust boundary (key<=64, label<=64 truncated, text via the
  10k surrogate-safe cap, anchors via parseHtmlElementAnchor, array cap 16);
  draftTargets state, chip removal with deterministic promotion mirrored on
  both sides, composerFocusToken for focus return
- types: additive Annotation.htmlAdditionalTargets (label/text/anchor per
  extra target; anchor optional so fail-closed targets still export)
- CommentPopover (all seams optional, default off): horizontally scrollable
  target chips with remove buttons and hover-to-flash, refocusToken,
  captureStrayKeys first-keystroke guard, yieldState fade/click-through with
  180ms transition and prefers-reduced-motion fallback
- HtmlViewer: composer-yield state machine (composerYield.ts) fed by parent
  mousemoves plus bridge-relayed pointer positions, with 48px/96px hysteresis
- export: multi-target comments append an 'Also applies to N more elements'
  block (label + excerpt per target); single-target output byte-identical
- share URLs: additional targets are dropped exactly like htmlAnchor (the
  compact tuple format never carried anchors); drafts carry them verbatim

* test(annotate): cover shift-click multi-select across bridge, DTO, composer, export, drafts, sharing

- srcdoc.test.ts (bridge DOM): shift-click add + toggle-off with echoed
  removals and per-target pinned boxes; create-mark commits all targets under
  one id with one badge number (second annotation numbers 2); primary
  promotion and last-removal cancel; parent remove-target mirrors without
  echo; flash-target; 16-target cap at the source; find-and-mark restores
  additionalAnchors as same-numbered pins with stale anchors failing closed
- htmlPinpointProtocol.test.tsx: multi-target-added/removed/pointer DTO
  validation (key/label/text/anchor caps, hostile payloads), selection
  targetKey/targetLabel validation; mounted-composer flows — primary chip,
  shift-adds into ONE submitted comment carrying htmlAnchor + 2 additional
  targets, single-target submit shape unchanged, promotion onto the comment,
  last-removal closes the composer, chip removal, 16-cap at the trust
  boundary, drag selections never arm multi-select
- CommentPopover.multiTarget.test.tsx: chips render primary-first with
  remove/hover handlers, refocusToken focus return, captureStrayKeys stray
  keydown routing (and non-interference when focused), yieldState classes +
  reduced-motion-aware 180ms style; default composer renders none of it
- composerYield.test.ts: distance + hysteresis state machine (48px over-exit,
  80/96px near enter/exit)
- parser.test.ts: multi-target export block (labels, excerpt clipping,
  fail-closed targets) and byte-identical single-target output
- useAnnotationDraft.seam.test.tsx: multi-target annotations round-trip the
  draft transport verbatim (save body + restoreDraft)
- sharing.multiTarget.test.ts: share tuples drop anchors AND additional
  targets while the comment itself still shares

* fix(annotate): keep pinpoint drafts alive when the pinned element scrolls out of view

Found by the real-browser signoff harness: reaching a second element to
shift-click often scrolls the pinned primary out of the viewport BEFORE any
additional target exists, and the scroll-out teardown then cleared the
bridge's pin state mid-compose — the shift-click landed on a dead draft and
started a new one instead of adding to it (and even a single-target pinpoint
draft silently lost its visual pin on commit after scrolling).

Pinpoint drafts (pendingPinViaPinpoint) now survive scroll-out; drag
selections keep the existing close-on-scroll-out behavior unchanged.

* fix(annotate): address adversarial review of multi-select (arm handshake, label sanitization, iframe shift relay, removal resync)

D1 (blocker): the bridge accepted shift-toggles for ANY pinpoint draft while
the parent only mirrors targets when the comment composer owns it — in
quickLabel mode the user could pin elements the saved annotation would never
carry. Multi-select is now ARMED EXPLICITLY: the parent posts
arm-multi-select (keyed to the primary, so a stale arm can never arm a new
draft) only from the composer flow, and the bridge refuses the toggle —
shift-click behaves as a plain click — until armed.

D2: target labels derive from page-controlled attributes (aria-label), so
newlines could smuggle real markdown structure (fake headings) into
agent-read feedback. parseTargetLabel now collapses all whitespace at the
trust boundary, and the exporter collapses again (defense in depth for
persisted pre-fix drafts).

D3: the composer yield armed Shift only from parent-window keydown/mousemove,
but window blur (focus entering the iframe) cleared it and modifier keydowns
don't reach the parent from the sandbox — from the second shift-click on,
the composer never yielded. The bridge pointer relay now carries the
observed shiftKey (validated strict boolean) and arms/disarms yield directly.

D4: a forged multi-target-removed desynced parent (promotes) from bridge
(keeps original). applyTargetRemoval now ALWAYS echoes remove-target —
idempotent for legit bridge-side removals, forcing convergence after forgery.

D5: the 'Also applies to N more elements' block gains a leading blank line so
markdown lazy continuation cannot fold it into the preceding blockquote.

D6: the stray-key guard registers in capture phase (a bubbling global
shortcut can no longer both fire and have its character appended), inserts at
the textarea's remembered caret instead of end-of-text, and refocusToken now
preserves the caret rather than jumping to the end.

D7: the expanded dialog no longer carries the dead yield class/style — its
data-comment-popover wrapper spans the viewport, making proximity
meaningless; the dialog deliberately does not yield.

Also reverts the incidental bun.lock version-catch-up churn.

Tests: unarmed/stale-arm refusal, quickLabel non-arming and non-mirroring,
newline-label collapse at both layers, bridge-shift-driven yield, forged
removal echo + bridge-side idempotent resync, caret-preserving stray keys,
blockquote separation. Signoff harness re-run green (14/14) on
rules-ui-signoff.html including the arm handshake.

* fix(annotate): reset multi-select arm on every new pinpoint draft (D1-R)

The re-review caught that multiSelectArmed was never cleared when
annotateElement started a fresh draft — only clearPendingPin reset it.
So a comment-mode draft (armed) followed by a mode switch the parent
doesn't mirror (quick label posts no arm) and a new pinpoint click left
the stale arm live: the bridge accepted shift-clicks and pinned elements
the saved annotation would never carry. Reset the flag at the top of
annotateElement alongside clearMultiTargets.

Regression test reproduces the exact sequence (armed draft -> new
unarmed draft -> shift-click must not add a target); mutation-verified
that removing only this reset fails it.

* docs(annotate): correct the first-keystroke guard comment reasoning

The comment claimed capture phase prevents a global shortcut from also
firing; preventDefault does not stop the dispatcher (it ignores
defaultPrevented by design). Restate the actual invariant: the guard is
safe only because no bare printable single-key binding exists on this
surface, and flag that as a constraint for future bindings. Comment only.
2026-08-10 10:19:21 -07:00
Michael Ramos 608a8003aa feat(annotate): hit-test pinpoint targeting for raw-HTML sessions (#1251)
* feat(annotate): hit-test pinpoint hover instead of the semantic whitelist

Pinpoint hover in raw-HTML sessions now resolves the real element under
the cursor via deep elementFromPoint (piercing open shadow roots) instead
of the SEMANTIC_* tag-whitelist graph, so styled div/span prototypes
(chips, icon buttons, cards) are individually targetable. Scope control
stays mouse-only and geometric, matching the markdown surface: the
deepest element under the pointer wins; pointing at a container's padding
or any area not covered by a child selects the container.

- Exclusions are identity-based: html/body, script/style, and an identity
  Set of viewer overlay nodes (pin badges, pinpoint box/label, vim UI) so
  page markup cannot spoof its way out of targeting. Annotation marks are
  transparent (resolve to their parent).
- Tiny-element promotion: a leaf under 16px on both axes climbs to the
  nearest ancestor at least 16px on one axis (MIN_CAPTURE_SIZE floor,
  not a whitelist). SVG shape primitives still promote to their <g>.
- Hover no longer builds the semantic target graph per pointer frame
  (graphForPointerFrame removed); it is per-event hit-testing with a
  2px/16ms last-position cache that the scroll reconcile invalidates.
  The graph survives untouched as the vim-navigation vocabulary.
- Hover labels for generic containers use a cascade: aria-label -> role
  -> 1-2 meaningful class tokens (hash-ish tokens stripped) -> own text
  when <= 40 chars -> 'container'. Known semantic tags keep their names.
- Click annotates exactly what the hover box shows; text-less elements
  post '[element: <label>]' instead of being untargetable.
- Anchors stay fail-closed: data-test-id/data-cy/data-qa join the
  author-controlled identity attrs, and text-less elements store a
  '[[pn-shape]]' signature (tag + sorted classes + child count + 16px
  size buckets) verified whole-string on restore.

* test(annotate): cover the pinpoint hit-testing contract

New DOM regression tests modeled on the signoff prototype pages:
chip/button targeting on styled div/span markup, container selection via
uncovered-area pointing, tiny-element promotion (mocked 8x8 dot promotes
to its 200x100 card), the generic-container label cascade, the text-less
shape-signature anchor round trip (verify + fail-closed rejection on
shape drift), data-cy identity-attr trust, and an instrumented assertion
that hover hit-testing performs zero document-wide querySelectorAll
calls across 30 mousemoves. Also reattaches the cached hover label
element when the body was replaced (parity with the hover box), and
accepts the shape-prefixed snapshot through the parent-side DTO test.

* fix(annotate): drop shape-signature anchors; fail closed on text-less weak selectors

Review findings on the pinpoint hit-testing branch:

- D1 (blocker): structure-derived shape signatures are identical across
  identical siblings by construction, so a positional selector whose
  sibling was removed resolved the WRONG element while the signature
  still verified — the exact wrong-binding failure the anchor system
  exists to prevent. The mechanism is removed entirely: text-less
  elements now anchor ONLY when the element itself carries a
  stable-identity rung (#id or the data-* identity attrs); otherwise no
  anchor ships and the pin simply does not restore (same as before the
  branch, fail closed). A wrong-binding anchor is worse than no anchor.
  D2-D4 (in-band prefix discriminator, uncapped class-list component,
  viewport-dependent size bucket) die with it.
- D5: the pinpoint click handler gated pin-badge ownership on a
  [data-plannotator-pin-badge] selector match, contradicting the
  identity-only overlay rule. It now gates on the overlay identity set:
  a page element spoofing the attribute hovers and annotates like any
  other element, while real badges keep owning their clicks.
- D6: class-token hover labels now route through the 40-char label cap
  (two long tokens could reach 73 chars), and the '[element: ...]'
  posted text goes through capSelectionText for consistency.
- D7: the pointer-hit cache invalidation moved to the top of the rAF
  reconcile, ahead of its guards, so every reconcile pass (scroll,
  resize, and body ResizeObserver — i.e. layout-changing mutations)
  clears it; the 2px/16ms TTL bounds any remaining staleness to one frame.

Tests updated to the new contract, with regressions for D1 (identical
text-less siblings ship no anchor; crafted positional anchors with empty
snapshots resolve nothing after sibling removal), D5 (spoofed badge is
annotatable, real badge posts mark-click), and the D6 label cap.
2026-08-09 23:01:31 -07:00
Michael Ramos 2d61b76c44 fix: address the three pre-release sweep findings (#1246)
Three real, new-in-range bugs from the free-hunt regression sweep:

- The pinpoint anchor builder ran a document-wide uniqueness query per
  ancestor against a growing selector with no depth cap, so one click on
  a deeply wrapped document froze the tab synchronously (measured 58s at
  depth 800). The walk now abandons the anchor past 40 ancestors (fail
  closed: text-search restoration takes over), bounding the work to
  interactive time. Regression test uses two identical depth-60 chains
  so uniqueness cannot short-circuit before the cap.

- The 10k selection-text cap sliced UTF-16 code units and could split a
  surrogate pair at the boundary, silently corrupting the annotation
  tail into U+FFFD downstream. Both sides of the bridge now back the cut
  off one unit when it would land mid-pair.

- The old-git sparse fallback matched git's "unknown option" error
  literally, which localized git builds translate, so non-English users
  on old git hit the exact hard failure #1239 was written to fix. All
  three installers now pin LC_ALL=C around the probe clone (saved and
  restored; kept single-line in install.ps1 for the scoped-call
  scanner, whose expectation is updated to the new prefix).
2026-08-09 17:48:05 -07:00
Michael Ramos 9ab5392bad feat(annotate): pinpoint-first raw-HTML sessions with element anchors and a minimal-first render (#1243)
* feat(annotate): rebuild HTML pinpoint mode with element anchors, pin badges, and a minimal-first render

Raw-HTML annotate sessions now open as just the page: first-ever run hides
all annotation chrome (toolstrip, tongue tabs, action cluster, sidebar) and
the user's last chrome state is restored on every later session via the
plannotator-html-chrome cookie. Pinpoint becomes the default input method on
the HTML surface, persisted separately from the markdown preference.

Pinpoint mechanics are rebuilt in the sandbox bridge, modeled on
app-notes-extension and agentation: a fixed-position outline box + label
replaces class writes on author elements, hover is identity-gated and
re-hit-tested on scroll/resize via a rAF-coalesced reconcile pass, clicks
pin the element (crosshair cursor, pinned outline while composing) and go
straight to the comment popover. Each pin serializes a verified-unique CSS
selector anchor (id > identity attrs > meaningful classes > nth-of-type
path) with a text-snapshot fingerprint; restoration is anchor-first with
fail-closed validation and falls back to document-wide text search. Element
pins that cannot take an inline mark (SVG etc.) get numbered pin badges that
track their element and re-acquire it after re-renders.

All DOM inspection stays inside the bridge; only validated, size-capped DTOs
cross postMessage. Annotation model is extended additively (htmlAnchor), so
exported feedback, drafts, and share links keep working.

* fix(annotate): harden HTML element anchors per adversarial review

Four fixes from the pre-merge adversarial review of the pinpoint round:

- Stable identity is now only the element's own #id or data-* rung,
  re-derived from the resolved element and compared whole instead of
  parsed out of the selector string. Behavioral attributes (role, href,
  aria-label, name, alt) no longer exempt an anchor from the text check,
  so a regenerated page can no longer bind an annotation to the wrong
  element. A weak anchor with a missing or empty snapshot is rejected,
  not exempted.
- Restoration order: the document-wide text search now runs before the
  pin-badge fallback, so text that moved elsewhere in a regenerated page
  is followed rather than badging the stale container. Resolved SVG
  anchors still pin directly.
- Selection text is capped at 10k chars on both sides of the bridge
  (truncated, not rejected); one pinpoint click on a huge pre/table no
  longer ships an unbounded page-controlled string into drafts,
  feedback, or share URLs.
- buildAnchorSelector returns null instead of a terminal fallback path
  already proven non-unique; no anchor beats a known-ambiguous one.

Also moves the HTML chrome restore out of the one-shot mount effect into
a surface-transition effect, so a linked .html doc opened from a
markdown session gets the minimal-first chrome and persists its state,
and a markdown surface's sidebar use never leaks into the HTML cookie.

* fix(annotate): skip the chrome save in the restore commit itself

The reviewer's residual finding: htmlChromeRestoredRef flips synchronously
inside the restore effect, but the restored state lands a commit later, so
the save effect's run in the restore commit wrote pre-restore values over
the remembered state (self-corrected next flush, but a page ending between
the two writes would freeze the inverted value). The restore now arms a
skip-one flag the writer consumes, so the stale write never happens. A
follow-up save fires from the changed deps when the restore changed
anything; when it changed nothing the cookie already holds those values.

Regression test instruments every chrome cookie write on a returning-user
mount and asserts none ever differs from the remembered state (mutation
tested: removing the skip fails it).
2026-08-09 16:17:06 -07:00
Michael Ramos 47157e7a55 feat(editor): add Vim keyboard annotation controls and live HUD (#1127)
* feat(editor): add Vim keyboard annotation controls

* feat(ui): add optional live Vim HUD

* feat(ui): finish Vim HUD experience

* feat(ui): promote Vim to dedicated settings panel

* feat(ui): make Vim document focus automatic

* feat(ui): let Vim HUD hide its key panel

* fix(ui): harden Vim selection UX
2026-07-26 22:08:35 -07:00
Michael Ramos 1047539801 Render arbitrary HTML in HtmlViewer without altering it (#1023)
HtmlViewer injected host-app state into rendered documents: ~26 bare
theme tokens (--muted, --background, ...) written into :root and
re-applied inline on the author's documentElement on every host theme
flip, a `light` class toggled on the author's root, an asymmetric
color-scheme:light injection, and unconditional ins/del diff styles.
Documents defining the same token names rendered with wrong colors in
both Plannotator and @plannotator/ui consumers.

Arbitrary documents now render exactly as in a standalone tab:

- Host tokens are pushed only under the viewer-owned --pn-* prefix;
  annotation CSS and the bridge read only var(--pn-*, fallback). The
  bridge refuses non---pn- writes and never touches the root class
  list unless the document opts in.
- Host theme-following is opt-in per document via
  <meta name="plannotator-theme" content="host">, which restores the
  bare-token push, the light class, and a symmetric color-scheme sync.
  The visual-explainer skill now emits the tag in generated artifacts.
- Diff CSS is injected only while the version-diff view is active and
  scoped to ins/del.plannotator-diff, which htmlDiff now emits on its
  generated wrappers; author <ins>/<del> markup is never restyled.

The srcdoc injection logic moves to a pure module (srcdoc.ts) with
tests pinning the neutrality contract, including a DOM test that runs
the actual bridge script and asserts a theme flip lands nothing on the
author's root except --pn-* properties.

Claude-Session: https://claude.ai/code/session_01MDqD8jdgTbdjiXcVVigzV2
2026-07-08 13:11:01 -07:00