111 Commits

Author	SHA1	Message	Date
renovate[bot]	269f7d02ae	chore(deps): update github actions	2026-09-10 17:30:31 +00:00
Michael Ramos	91d05511b8	feat(review): hover card trigger settings and first-run introduction (#1462) ... * docs(review): design the token hover card trigger settings and announcement * feat(review): token hover card trigger settings and first-run announcement Hover-for-everyone is a good default and a bad only-option. Two cookie-only settings now cover the three complaints it produces, and a one-time dialog puts the choice in front of the reviewer once. - @plannotator/core/token-hover (shimmed to shared): TokenHoverTrigger (hover | modifier | off), TokenHoverDelay (200 | 350 | 700), and the migration that reads the old tokenHoverCards boolean once. - settings.ts: tokenHoverTrigger REPLACES tokenHoverCards rather than sitting beside it, so there is no unreachable enabled-but-off state. A legacy `false` resolves to `off`; the legacy cookie is read, never written, so a downgrade cannot silently re-enable cards for someone who turned them off. - useTokenHover: an options argument, { mode, delayMs }. In modifier mode the Alt gate sits ahead of the dwell timer, the cache read and every piece of state, so a hover with the key up costs one boolean read. Pressing Alt while already parked on a token opens the card (a key press fires no pointer event, so the last enter is remembered); releasing it starts the same leave grace, except while the pointer is inside the card; window blur clears a stale held flag; and typing owns the key, since Alt+Backspace and Alt+arrow are word-editing chords. The three listeners exist only in modifier mode, so hover mode is byte-for-byte what shipped. - The diff views are untouched: their prop signatures are unchanged, which matters because both are compiled into the portable guide viewer. `off` is still enforced in App by withholding the handler props entirely. - TokenHoverAnnouncementDialog: last in the first-run chain, compact, with a radio group that applies the trigger immediately so Done and Escape both mean "accept what is selected". It skips a session where cards cannot run without consuming its cookie, and never shows to a reviewer who already has a non-default trigger. - Settings > Review Display: the toggle becomes a trigger segment plus a delay segment, the delay inert while the trigger is off. Click behavior is deliberately untouched. Cmd/Ctrl+click and Alt+click both still open the References panel; every gesture on a token is already spoken for, and the modifier mode delivers what a click mode would be wanted for. Alt rather than Cmd because Cmd/Ctrl+hover already paints the navigable-target affordance, and Alt is spelled the same on every platform. Reasoning and the rejected controls are in DESIGN_hover-settings.md. The guides.show viewer manifest moves with this: the settings registry is in the portable viewer's graph. * fix(review): hover card review findings, M1/M2 and minors M1 stale pointerInCardRef. onCardEnter set "the pointer is inside the card" and only onCardLeave cleared it, but a scroll-close or a window blur unmounts the card UNDER the pointer, so no leave ever arrives. The flag then stayed true for the rest of the session and every later Alt release was ignored as "they are reading the card", leaving cards stuck open. close() now resets it, which covers the blur path too since onBlur closes. M2 Alt+click double surface. handleCodeNavRequest closes the hover first, so EVERY route into the References panel is covered by one line: Cmd+click, Ctrl+click, the Alt+click alias, and the card's own location links. It sits ahead of the no-checkout early return, because a click that cannot resolve must still dismiss the card it was made on top of. This also repairs the overlap #1461 shipped with, where a click on a token whose card was open or mid-dwell left both surfaces on screen; close() cancels the pending dwell and aborts an in-flight request, so a click during the dwell can no longer resolve into a card behind the panel. Alt+click is where this is routine rather than incidental: in modifier mode the alias and the trigger share the key. Minors: - The announcement's featureAvailable is LATCHED at the first post-load render, like guideIntroEligibleRef. canUseLiveWorkspaceActions changes on mid-session diff switches, so an unlatched read could pop the dialog over work in progress or yank an open one away mid-read. - resolveTokenHoverAnnouncementPending is pure again, like its editMode sibling. The cookie write that retires the announcement for an already-decided reviewer moved to shouldConsumeTokenHoverAnnouncement, called from an App effect: a state initializer can run more than once, and a getter that writes storage goes unnoticed until StrictMode double-invokes it. - The migration is documented as what it is. fromCookie RE-READS the legacy key on every load until the user touches the setting, because a migrating read returns a value and so never triggers the registry's default-seeding write. Resolution is pure and identical every time, so the repeated read costs one lookup and cannot drift; seeding inside a getter was rejected as more code for no behavioral gain. DESIGN §7, the settings comment and AGENTS.md all say this now instead of claiming a one-time seed. - SegmentedControl builds its classNames from a filtered array (no double or trailing spaces) and only puts `disabled` on the button when true. Nits: the Cmd-only copy in the dialog note and the Settings row is now platform-aware through a new modKeyWord export ('Cmd' / 'Ctrl'), which is the prose spelling modKey's glyph cannot serve; and the dialog's radio group follows the WAI-ARIA roving-tabindex pattern, so Tab enters and leaves it as one stop while arrows move the selection and wrap. Tests: M1's exact reviewer sequence (hold Alt, open, pointer into card, scroll-close, hover another token, release, assert it closes after the grace) fails without the reset; the References-handoff mechanism (open card gone, pending dwell cancelled, in-flight request aborted) is behavioral in the hook suite, and codeNavHoverHandoff.test.ts pins the App wiring at source level the way iframeIsolation does, failing without the closeTokenHover call. Also: blur clears the held state, arrows move the radio selection, and the announcement latch no longer writes. The dialog suite additionally seeds its trigger cookie: configStore is a process-global singleton whose loadFromBackend keeps the in-memory value when the new backend is silent, so an unseeded map inherited another file's last write. The new roving-focus test is what surfaced that. * fix(review): dialog footer names the real Settings tab (Editor, not Review Display) * feat(review): jumbo hover card announcement, and rename the feature to Hover cards The announcement was too plain for a first-use dialog. It now uses the repo's big-format shell (max-w-5xl, badge/title/description header, a 1.1fr/1fr body that collapses to one column under 820px, footer with the Settings pointer and one action), matching EditModeAnnouncementDialog and LookAndFeelAnnouncementDialog. The left column, where the Edit Mode dialog plays its recording, is a worked example: a three-line strip of diff with `charge` shown under the pointer, and below it a hover card reproducing the real card's anatomy in the order TokenHoverCard renders it (name plus kind badge, approximate signature with its matched-line marker, doc line, Defined at, a reference sample ending in the overflow line). It is JSX over the existing theme tokens rather than a bitmap, so it is crisp at any DPI, follows the active palette in both light and dark, and cannot go stale against a card whose anatomy changes. It is decorative: the whole block is aria-hidden, carries nothing focusable, and every fact it shows is also stated in the prose beside it. The right column is unchanged in behavior: the same three-option radio group, the same immediate apply, the same roving-tabindex keyboard model, and the click-behavior note (now also carrying the ripgrep requirement, which the Settings row already stated). The footer keeps the Editor-tab destination d04cfce5 corrected. Rename: the user-facing name is "Hover cards" everywhere a reader meets it, in the dialog title and the Settings heading. "Token" described the implementation's unit rather than the thing the reviewer points at, and in an app full of annotations "token hover card" invites reading it as a kind of comment card. Every internal identifier is untouched: tokenHoverTrigger, tokenHoverDelay, the cookie keys, the hook and component names, and the data-token-hover-* test ids all keep their spelling, so there is no migration. Tests: the behavioral assertions are unchanged (once-ness, radio applies, dismiss and Escape keep the choice, arrows move selection), plus one that the example block is aria-hidden and holds nothing focusable, because read aloud it is a wall of invented identifiers that says nothing about the choice on offer. The guides.show CSS bundle hash moves with this: Tailwind scans source for class names, and the example introduces utilities the shared stylesheet did not carry. The JS bundle is unchanged, as expected for an app-side dialog. * feat(review): make the announcement example a live try-it on the real card The example was a hand-drawn impression of a hover card. A fork of that markup drifts from the shipped card the first time either changes, which is exactly what an example is supposed to prevent. It is now the real thing, and it is interactive: the `withRetry` token in the strip is genuinely hoverable, and resting on it opens the REAL TokenHoverCard through the REAL useTokenHover, so the reviewer feels the actual dwell, leave grace and Alt gate before committing to a setting. Nothing is reimplemented. Three thin seams carry it: - useTokenHover gains `resolve`, defaulting to the POST /api/code-nav/hover it always did. The try-it returns a hardcoded CodeNavHoverResponse instead. This is the ONLY fixture: a demo must not search the reviewer's repository for a symbol they never asked about, and a second copy of the dwell/supersession/ grace logic would drift. The resolver is read through a ref so an inline literal cannot re-arm the hook's callbacks every render. - TokenHoverCard gains `layerClassName` (default `fixed z-[60]`, unchanged). The card portals to <body> like every instance, so the one thing its host has to be able to say is which stacking context it belongs in; the try-it puts it at z-[110], above the z-[100] modal it is demonstrated inside. - TokenHoverCard gains `inert`, which drops the location buttons out of the tab order. The try-it has no References panel behind it, so they lead nowhere. The card body was extracted to TokenHoverCardContent so the positioned wrapper and any other host share one copy; every class, section and separator is the same markup the live card renders, not a duplicate. The hovered token wears the diff pane's own underline treatment, now defined once in tokenHoverStyles.ts: usePierreTheme serializes it into Pierre's shadow stylesheet as `.pn-token-hover`, the dialog applies the same declarations as a style object. Two consumers, one definition. The try-it reads the LIVE setting rather than a prop, so flipping the radio to "While holding Alt" makes the demo behave that way immediately and "Off" makes it do nothing and closes any open card. That is the honest preview of each choice, which is the point of putting the control next to the demo. Because it is interactive it is labeled rather than hidden: the region carries role="group" and an aria-label, the visible prompt line ("Try it: rest your pointer on withRetry") carries the meaning, and only the mock code lines stay aria-hidden, since read aloud they are a wall of invented identifiers. Tests: hovering the token opens the real card (asserted through the component's own data-token-hover-card marker) with the fixture's symbol and definition path after the dwell and not before; the try-it obeys the live trigger, so under "Hold Alt" a rest opens nothing until Alt goes down, and choosing Off takes an open card away; and the region is labeled while its mock code is hidden. React synthesizes onPointerEnter from pointerover, which is what the tests dispatch. The guides.show viewer hashes move on both halves: usePierreTheme is in the viewer's graph and now imports the shared style module (JS), and the try-it introduces Tailwind utilities the shared stylesheet did not carry (CSS). Budgets are unaffected (395.7 KB gz of 400). * fix(review): the try-it fixture names the token the prompt asks you to hover Two defects the browser capture caught, both invisible in the DOM tests. The prompt said "rest your pointer on withRetry" and the card that opened was titled `charge`: the fixture still described the symbol from the earlier static example. It now describes withRetry, and takes its name from EXAMPLE_SYMBOL so the prompt and the card cannot disagree again. The hovered token sat mid-line, and the card anchors at the token's left edge and is 400px wide, so it opened across the radio column the reviewer is meant to be reading. The token now leads the line, which keeps the card inside the example's own column. * feat(review): the hover card modifier gate is Cmd (Ctrl), not Alt Maintainer ruling. Two reasons, both recorded in DESIGN_hover-settings.md §4.1 in place of the old Alt argument: - Alt is very commonly bound to push-to-talk dictation, so an Alt-held gate would open cards every time the user starts speaking, with the pointer wherever they left it. That is the exact failure this setting exists to prevent. Alt-hold is also already taken inside Plannotator, by the plan editor's temporary input-method switch. - Cmd+hover is already VS Code's "tell me about this symbol" gesture: the navigable-target underline plus a definition peek under one held key. Gating on Cmd rides that muscle memory instead of competing with it. The old anti-Cmd argument is moot. The underline and the card appearing together under a held Cmd is not a collision, it IS that composite gesture; and handleCodeNavRequest already dismisses the hover surface on every References invocation, so Cmd+click cleanly supersedes an open card or a pending dwell rather than stacking with it. The key is platform-aware through two new helpers beside the existing ones in packages/ui/utils/platform.ts: `isModKeyHeld` (metaKey on macOS, ctrlKey elsewhere) and `modEventKey` ('Meta' / 'Control'). Labels come from the existing modKeyWord, so the key the code checks and the key the copy names cannot disagree. Labels updated in the dialog radio and its description, the Settings segmented control, the design doc and AGENTS.md. Only the modifier ALONE arms. Any other key going down while it is held (Cmd+C, Cmd+V, Cmd+S) disarms and takes an open card with it, so a copy performed with the pointer parked over the diff cannot pop a card mid-copy. This was cheap: the keydown handler already had to compare event.key. The typing-target guard stays as belt and braces. Blur handling stays and matters more now: on macOS the app switcher is the same key this gate arms on, so Cmd+Tab is the common way to leave with it held. Internal ids are untouched. The stored trigger value is still `modifier`, because the setting names the shape of the gate and not which key fills it, so nothing migrates. The #1461 Alt+click References alias is unrelated and is left alone. The try-it feels the new gate for free, since it drives the same hook: choosing the hold option and holding Cmd over the demo token opens the demo card. Tests: the gate suites now build their events from the platform helpers, so they exercise Cmd on macOS and Ctrl on the Linux runner rather than pinning one platform. All behavioral assertions are unchanged. Two new ones cover the chord rule, and both fail without it: a copy chord with the pointer parked on a token opens nothing, and a chord while a card is open closes it. * fix(review): pointer cursor wins over Pierre's I-beam on hovered and Cmd-held tokens * fix(review): the intro dialog's demo token gets the pointer cursor too * feat(review): hover delay default is 300ms matching VS Code; tiers respaced 150/300/700 * test(review): restore the 700-delay test's summed advances The 350 -> 300 sweep in b5538350 rewrote both advances in this test, but they SUM to the configured dwell rather than each naming it: 300+300 is 600, so a 700ms delay never elapsed and the suite went red at the tip. Back to 350+350 with the pairing spelled out, so the guard stays meaningful in both directions: the first advance is past the 300 default (a hook that ignored delayMs would have fired already) and the pair reaches exactly 700. Verified fail-closed by pinning the timeout back to DEFAULT_TOKEN_HOVER_DELAY_MS, which fails this test and nothing else. The dialog test's two advances become DEFAULT_TOKEN_HOVER_DELAY_MS itself: its dwell IS the registry default (the seeded backend says nothing about tokenHoverDelay), so naming the constant is both correct and immune to the next respacing. * fix(review): the hover gate's typing guard pierces the shadow root Two defects in one branch of the modifier gate's keydown handler. The guard read event.target, which a window-level listener receives RETARGETED to the shadow host. Pierre's edit-session editor is a contenteditable inside that shadow root, so a bare Cmd pressed while editing code in place read as a plain <div>, armed the gate, and popped a card over the text being edited: verbatim the failure "typing owns the key" exists to prevent. The three other window-level typing guards in this package (AllFilesCodeView, FileTree, SectionsPanel) already read composedPath()[0] for exactly this reason and say so in comments; this one is no longer the exception. The guard also sat ahead of the chord branch, so it suppressed DISARMING as well as arming. Focus reaches a composer between the two routinely (a card is open, the reviewer clicks into the comment box and pastes), and that left the card standing over the diff with the gate still armed behind it. The chord branch now runs first and typing only gates the arm path. keyup deliberately keeps no typing guard: if the gate never armed the branch is already a no-op, and if it did arm the release has to be honored wherever focus has since travelled. Tests: the old "modifier chords in a comment box" case exercised the chord branch, not the guard, so it passed with isTypingTarget deleted. Replaced by a bare-modifier press from a textarea and one from a shadow-root editor, plus a chord-while-typing case for the asymmetry; the two pre-existing chord-disarm tests stay, so that coverage is unchanged. The shadow event is synthesized rather than dispatched from a real shadow root because happy-dom does not retarget, which would have made the test vacuous. Each of the three fails closed against its own defect: reverting composedPath fails only the shadow case, deleting the guard fails both typing cases, and moving the guard back above the chord branch fails only the asymmetry case. * feat(review): the modifier gate paints the navigable-target affordance The ruling that settled Cmd over Alt was that the underline and the card appearing together under one held key IS the composite gesture rather than a collision. The code did not deliver it: the diff views paint pn-token-nav from the pointer ENTER event, and neither half of the gesture this mode exists for fires one. Parking on a symbol and THEN holding the key opened a card on a token wearing no affordance, and releasing closed the card while leaving the affordance painted until the pointer eventually left. useTokenHover now reports the arm and disarm transitions through an optional onModifierGate(armed, tokenElement), always carrying the token the pointer is parked on RIGHT NOW so a disarm also unpaints a neighbour the pointer drifted onto while the key was down. Every disarm route is covered: chord, release, blur, and the effect cleanup when the reviewer switches trigger with the key still held. The release notifies ahead of the reading-the-card early return, because the key being up ends the affordance whether or not the card is being read. App owns the painting, not the hook: the diff views are compiled into the portable guides.show viewer and their prop signatures must not move, and the class is a review-app concern. Enter-time painting is untouched, so plain Cmd+hover in the default hover mode behaves exactly as before. Also gates pn-token-hover on a hover handler actually being wired, and paints it in AllFilesCodeView. That class carries a cursor: pointer that became !important in eebab519, so painting it unconditionally started promising clickability on every token in a session with hover cards switched off; and leaving it out of the all-files view meant the affordance the announcement's try-it demonstrates was missing from the default review surface. A read-only or portable host passes no hover handler and still gets neither class. Fail-closed: dropping notifyGate(true) fails both new hook tests, dropping the App wiring fails the App pin, and making the DiffViewer paint unconditional fails the gating pin. * fix(review): hover card cleanups, and the design's delay tiers Comments first, because several of them had become false and this repo treats them as contract. The dialog cited a StaticTokenHoverCard that no longer exists and a card that "lives inside an aria-hidden decorative block" (it does not; the try-it region is a labelled role=group), the try-it still named "the Alt gate", and two hook comments plus four test comments still described the trigger key as Alt or claimed the Alt+click alias and the trigger share a key, which stopped being true when the gate became Cmd. The focus trap's rationale claimed the example card's buttons were in its query; the card portals to <body>, so they never were. tokenHoverStyles now single-sources what it says it does. Only thickness and offset were actually shared; the decoration line and the cursor were written twice and could drift. Both forms read one object, and the two genuinely per-form bits (the resolved color, and the !important the shadow form needs to beat Pierre's I-beam) are named as such. The display-only card stops advertising clicks it swallows: its own footer says "Click a location to jump", so under `inert` the locations lose the hand cursor and hover underline as well as the tab order, and the card gets aria-hidden — it portals outside the aria-modal dialog it belongs to, where a screen reader would meet it as a stray tooltip. The announcement's availability latch moves from the render body into an effect: a ref written during render is a side effect React may discard. The gate independently requires !isLoading, so the ordering is unchanged. Both announcement test files now re-hydrate configStore in afterEach. They re-resolve a process-global singleton from a two-entry fake map, and the pure lane runs in one process, so they were handing their own defaults to whatever file ran next. The dialog test gains the two assertions that were missing: the demo card sits at z-[110] (the layerClassName seam's entire purpose is not being buried under the modal it is demonstrated inside) and its locations are non-interactive. Both fail closed against dropping the respective prop. Design doc: §4.2 still listed 200/350/700 with a 350 default, which the maintainer ruling in §6 had already replaced; §3's 350 now reads as the history it is; §6 records how the composite gesture is delivered and §9 how the hovered-token treatment is gated. * chore(core): re-pin the guide viewer build The manifest was synced before the last three commits of this branch and went stale: usePierreTheme (which serializes the token-hover CSS into Pierre's stylesheet), the token-hover default constant, and now AllFilesCodeView are all inside the viewer bundle, so the JS hash moved while the CSS hash did not — the .pn-token-hover rule is a JS template string, not a stylesheet entry, which is exactly why the drift was easy to miss by eye. check:manifest was failing at the tip. Rebuilt and synced last, so this pins the branch's final source. Budgets after: js 395.8 KB gz against a 400 KB budget, css 50.9 against 64, worker still import-free.	2026-09-03 13:28:27 -07:00
Michael Ramos	91ad7c95fc	feat(review): token hover cards, Tier 0 (#1461) ... * feat(review): add POST /api/code-nav/hover in both runtimes Tier 0 of the token hover card: the hover pipeline is the ripgrep search /resolve already runs, plus three cheap enrichments read off the same machinery. - packages/shared/code-nav.ts: definition patterns carry the kind they prove (alternations split one-per-kind, so definition-vs-reference classification is byte-identical and classifyMatch becomes a thin wrapper over classifyMatchDetailed); scanDocComment, buildSignature and resolveCodeNavHover; an additive timeoutMs option on resolveCodeNav so hover can ask for 3s while /resolve keeps its 5s; an optional readFile member on CodeNavRuntime so /resolve callers stay unchanged. - Both review servers gain /api/code-nav/hover behind the same guard stack as /resolve. /resolve itself is untouched. The doc scan is conservative by construction: per-language, blank-line separated, capped, and null for an unknown language. Returning nothing always beats returning garbage. * feat(review): token hover cards in the code-review diff Resting the pointer on a symbol opens a card with where it is defined, an approximate signature, its doc comment if the scan found a real one, and a sample of its references. Every location on the card routes into the same References panel Cmd+click opens. - utils/stitchTokenIdentifier: rebuilds one identifier from the token spans Shiki fragmented it into, using each span's data-char column to prove adjacency. It stops at dots (rg searches with --word-regexp, where a dotted path matches nothing) and refuses keywords and one-character names, which is what keeps most hovers off the wire entirely. - hooks/useTokenHover: 350ms dwell before any request exists, one in-flight request aborted by its successor, a 30-entry LRU flushed whenever the diff snapshot changes, a 250ms leave grace so the card's own links are reachable, and a scroll/wheel cancel because the anchor rect is stale the moment the pane moves. An unavailable backend, a failure, a timeout and a thin answer all render nothing, silently: a hover is an idle gesture and must never nag. - components/TokenHoverCard: portaled to body so it escapes the Dockview overflow and stacking context; anchored below the token, flipped above when the viewport would clip it. It shows what the search found and nothing it did not, so uncertainty is a second location line rather than a description of the ranking. - Wiring: two optional props beside onCodeNavRequest in both diff views, passed only when the existing live-workspace gate AND the new cookie-only "Token hover cards" setting are on. Off means no listeners, no requests and no card in the tree. Alt+click joins Cmd+click as an unadvertised alias into the References panel; the meta/ctrl branch is unchanged. The guides.show viewer manifest moves with this: AllFilesCodeView is in the portable viewer's graph, so its new optional props shift the bundle hash. * fix(review): correct token hover supersession, scroll and doc-scan defects Review findings, each with the regression test that fails without the fix. - An open card could be rewritten by a NEIGHBOUR's answer: drifting onto an adjacent token launched its request, and returning to the open card took the same-key early return without reclaiming the active key, so the neighbour's answer still passed the landing check. The early return now reclaims the key, kills the pending dwell, and aborts a foreign request. - Re-entering a token inside the leave grace re-armed the dwell while that token's own request was still in flight, spawning a second ripgrep for an answer already on its way. The dwell now joins the in-flight request, and the answer anchors to the span the pointer is on now. - Scrolling INSIDE the card closed it, which made the signature block's horizontal scroller unreadable by the gesture meant to read it. The cancel now ignores events originating in the card's own subtree; a pane scroll still closes it. - A below-threshold answer for a different token left the previous token's card standing over a symbol the reviewer had already left. - The doc scan rendered tooling directives as documentation. Directives are dropped from BOTH ENDS of the comment run — eslint-disable, @ts-*, prettier-ignore, biome-ignore, istanbul ignore, noqa, type: ignore and triple-slash references — because the commonest real position is the line immediately above the definition, which is the trailing end of the run as collected. Never from the middle: a directive surrounded by prose sits inside documentation we would have to interpret to cut safely. A run that is nothing but directives returns null, and prose that merely mentions a directive is untouched. Also: an answer whose token has been recycled out of the DOM opens no card (a detached rect is 0,0 and would pin it to the viewport corner); a flipped card is clamped to the top edge; the card is a tooltip, not a dialog; a location click describes the CLICKED location rather than forwarding the hover's charStart and language into another file; definition.preview stays declared but unpopulated until a consumer exists; the overflow line regains its leading ellipsis and now renders under the banned-vocabulary sweep. Portable viewer: the hover prop is inverted to (props, filePath) so the two diff views import nothing new and stitching lives in App. The read-only guide viewer bundle no longer carries the stitcher or the request builder.	2026-09-02 19:00:39 -07:00
Michael Ramos	36d59d3bbd	fix(ui): vim/popover Escape ownership, ladder defaultPrevented, doc-flow accuracy (PR7 review) ... F1: with vim enabled, Escape on an open ActionMenu/ApproveDropdown/ DecisionControl was consumed by useVimDocumentFocus (registered first on document, preventDefaults while reclaiming focus; the popover hook then skips the defaultPrevented event) and the menu stayed open. Open popovers now stamp data-pn-dismissable-popover and the vim hook's BLOCKING_OVERLAY_SELECTOR treats them as blocking overlays, so vim defers. Pinned in ActionMenu.test.tsx (verified failing without the selector entry). F2: the plan editor's diff-exit Escape listener gains the defaultPrevented early-return so a popover-consumed Escape no longer also exits the diff view. Audit of the remaining document-level Escape listeners found only component-internal ones (Settings theme preview, which already consumes; dialog focus traps handle Tab, not Escape) — left alone. F7: onDismiss callbacks memoized in ActionMenu/ApproveDropdown so useDismissablePopover stops re-registering listeners every render while open. F8: ActionMenu.test.tsx, PlanHeaderMenu.mobile.test.tsx, and ReviewHeaderMenu.mobile.test.tsx were DOM-gated but never registered in the CI DOM lane; added to the test.yml file list so they actually run (the F1 pin lives in a lane-registered file). F3-F5 (docs): test-opencode-review.sh stdout describes the decision control (Approve at zero) instead of the removed always-on Send Feedback button; UI-TESTING.md's platform-mode flow uses the real positional PR URL; the AGENTS.md annotate flow diagram ends on the adaptive primary like the review diagram.	2026-09-02 13:10:47 -07:00
Michael Ramos	f9b650ba6b	test(review): sidebar general-comment coverage — reachability, flip, PR-switch survival ... Spec §7 PR4 hunt list + §8 test 17: - ReviewSidebar.generalComment.test.tsx (DOM, registered in test.yml's DOM lane so it cannot silently skip): reachable at totalCount === 0; the General section header offers the button with only line comments; a commit delivers the trimmed text exactly once (button and Mod+Enter); an empty note never commits; Escape dismisses but keeps the draft. - App.decisionControl.test.tsx: end-to-end flip proof — a sidebar general comment flips the header primary from Approve to Send Feedback · 1 and rides the posted /api/feedback body with the sentinel shape and no prUrl. Guards the wiring a component test cannot see (prop dropped from the sidebar mount, handler stamping PR context). - reviewDecision.test.ts (pure lane): createGeneralReviewComment shape — trimmed scope:'general' with sentinels, randomUUID ids that cannot collide in one millisecond (spec §9), whitespace-only returns null, and the PR-switch survival pin: no prUrl/diffScope, passes annotationMatchesPrScope for any PR + diff scope.	2026-09-02 10:08:37 -07:00
Michael Ramos	97718772ec	test(review): decision-control payload, exhaustiveness, and App DOM coverage ... - reviewDecision.test.ts (pure lane): every id the spec can emit resolves to a route, routes fork only on approved, compact row ids stay unique - the runtime stand-in for the missing app typecheck (spec 8C). - review-note-payload.test.ts, ported as-is from the held branch: the scope:'general' note survives waitForDecision unmodified on BOTH runtimes and archives as a feedback decision counting the note. - exportFeedback.reviewNote.test.ts, ported as-is: the note renders under '## General', co-exists with placed annotations, and is real feedback. - App.decisionControl.test.tsx (DOM lane): mounts the real review App (workerPool's Vite-only ?worker&inline import stubbed per the AllFilesCodeView.lifecycle idiom) and asserts through the posted body: empty-state Approve posts the legacy LGTM shape, Send Feedback posts the live annotations, Request changes... delivers the sentinel general note in the array and the export, discard posts empty arrays only after its confirm, Mod+Enter equals the visible primary, approve-carrying items are absent while the advert is off, and compact touch offers a positive row at zero that posts (E16-review). Registered in the CI DOM lane so it cannot silently skip. - shortcuts.test.ts now asserts the decision-control scope is registered in BOTH adopting registries (the PR2-era compose-cleanly probe became real).	2026-09-02 09:27:31 -07:00
Michael Ramos	f8bcf600b7	test(annotate): decision-control payload, exhaustiveness, compact and Esc-ladder coverage ... - decisionHandlers.test.ts (pure, spec §8C): every id buildDecisionSpec can emit resolves to a transport route; note/discard routes follow the gate matrix; compact row ids stay collision-free. - App.decisionControl.test.tsx (DOM, spec §8D/E16, adapted from the held App.submitNote.test.tsx harness): Done posts the byte-identical legacy /api/feedback body, Mod+Enter equals the visible primary, the two note items do not collapse into one payload, gate approves on /api/approve, the discard confirm drops annotations, composer Escape keeps the note, compact touch has a working positive row at zero, and the closed popover never consumes the HTML Esc ladder's Escape. - Registered the DOM file in the CI seam-contract step so it cannot silently skip.	2026-09-02 08:28:38 -07:00
Michael Ramos	90dd3577e1	feat(ui): DecisionControl split pill + note field/dialog + dismissable-popover hook ... The joined split control: incumbent primary segment (never opens the popover, never fades while it is open — the popover holds only alternates, so the primary keeps its meaning), caret popover with role=menu rows and roving arrow-key focus, in-place composer morph (Esc ladder consumes on exactly two rungs, drafts kept; Mod+Enter submits trimmed; plain Enter is a newline; empty note refocuses instead of graying), and the single discard confirm through ConfirmDialog. useDismissablePopover is the shared pointerdown-outside + Escape effect with the framed-surface strategy: window blur to an iframe dismisses, since iframe clicks never reach the parent document. ActionMenuItem gains additive role/className props and ExitButton an additive appearance='ghost' form; defaults are byte-identical for existing consumers. DOM tests registered in the CI seam-contract step so they cannot silently skip.	2026-09-01 23:21:03 -07:00
Michael Ramos	352f880ebc	feat(ui): add Viewer-owned document header (#1450) ... * feat(ui): add Viewer-owned document header * chore(guides): refresh viewer integrity manifest * fix(ui): harden Viewer document header transitions * fix(ui): exclude document header from annotations	2026-09-01 17:25:15 -07:00
Michael Ramos	f4d0682418	feat(ui): add sticky header lane host seams (#1448) ... * feat(ui): add sticky header lane host seams * test(ui): align intersection observer fake with DOM types * fix(ui): preserve sticky lane defaults	2026-09-01 16:25:33 -07:00
Michael Ramos	79afb25e55	fix(ui): publish exact core dependency (#1446)	2026-09-01 11:44:46 -07:00
Michael Ramos	17883f8cbf	feat(ui): allow hosts to hide Quick Label (#1442) ... * feat(ui): allow hiding the quick label tool * docs(ui): review follow-ups for the hideQuickLabel seam Independent review verdict was merge-with-minor-changes; this closes the three findings: - StickyHeaderLane now forwards hideQuickLabel to its compact toolstrip, so a host's opt-out survives the pinned scroll header instead of the Label button reappearing on scroll. - HANDOFF.md gains the AnnotationToolstrip row on the supported-imports table (the strict-consumer gate's convention): the showHelpLink warning (the help modal embeds Plannotator's own videos), the hideQuickLabel / hideInputMethodSwitch opt-outs, and the caveat that hiding the button does not clamp 'quickLabel' mode - host mode state must stay out of it, including preferences restored through utils/editorMode. - README.md gains the matching 'Toolstrip host props (0.35.0)' section.	2026-09-01 11:11:00 -07:00
Michael Ramos	1cab9dd9a8	feat(review): mark files viewed as you scroll past them (#1430) ... * feat(review): mark files viewed as you scroll past them Reviewers reading the all-files diff top to bottom had to check every file off by hand. Now a file marks itself viewed when the reviewer MOVES ON from it, after its content was actually on screen long enough to have been read. Arriving at a file never marks it; leaving it downward does. - All-files surface: a file marks when the reader scrolls past it (its successor has reached the viewport top, so it genuinely scrolled out above) and has accumulated at least 1000ms as the reported reading file. Dwell is cumulative per diff snapshot, so bouncing between two files still accrues, while a momentum flick to the bottom marks nothing. The last file, which can never scroll out above, marks on reaching the end of the diff. - Single-file panel: opening a file never marks it; navigating away after the same dwell floor does. Keyboard file navigation drives the same panel switches, so keyboard-only parity is automatic. - Collapsed cards never mark. Generated files seed collapsed, so nobody reviews a lockfile by scrolling past its folded header. - Un-viewing a file suppresses auto-view for it until it is marked viewed by hand again. That set rides the review draft as an additive optional field. - Inert inside the Guided Review takeover and on a commit detour, where the files on screen are not the change under review. - A viewed file whose patch changes under a refresh loses its checkmark, but only while auto-view is on, so the off state stays byte-identical to today. - PR sessions batch the marks into one /api/pr-viewed request rather than one per file. The setting is reviewAutoViewed, cookie-only and on by default, with two off switches: Settings > Git and a row in the file-list gear popover. The first time auto-view actually fires, a toast says so and offers Turn off; using either switch consumes that one-time notice. The decision core is pure and clock-injected (utils/autoViewed.ts), the binding is a hook (hooks/useAutoViewed.ts), and AllFilesCodeView only gains one optional emission callback on the rAF path it already runs. No server changes in either runtime. AI-assisted (Claude) under maintainer direction. * fix(review): scope auto-mark-viewed to the transitions it was meant for Four review findings on the auto-mark-viewed branch. Rule 5 fired on EVERY applied diff switch, not just the staleness refresh. The review app funnels every transition through one apply path, so entering the Commits detour (the rail auto-opens HEAD), switching base branch, and toggling hide-whitespace all un-viewed files whose per-path patch text legitimately differs, which contradicts both Rule 4's "a commit detour is inert" and Rule 5's own rationale. The apply path now goes through resolveDiffSwitchUnviews, which requires the caller to opt in (`contentRefresh`) and re-checks the identity of the diff on top of that: same selection, same base, and never a commit-family type on either side. Only the staleness refresh and the post-fetch base refresh opt in. The pure delta resolver is unchanged. A source-level test pins which call sites may opt in, since that is where the guarantee actually lives. The at-bottom branch fired on the mount tick. A diff shorter than the viewport is at-bottom from the very first report, and that report is the mount seed, so the file on screen marked itself about a second later with zero interaction and fired the first-time toast at a motionless page. It now requires a real scroll event on the current file set. Staging a file marked it viewed without clearing auto-view suppression, unlike v, the header button and the tree row, so a file the reviewer un-viewed and later staged stayed permanently off-limits to auto-view. Dwell accrued while the setting was off, so enabling mid-read could mark the current file instantly on time the reviewer spent with the feature deliberately disabled. Disabled is now fully inert: the clock does not accrue, and enabling starts a fresh one rather than replaying the gap. AI-assisted (Claude) under maintainer direction. * chore: refresh pinned guide viewer manifest after merging main	2026-08-31 09:13:49 -07:00
Michael Ramos	4465950f0c	feat: add bounded annotation undo and redo (#1426) ... * feat: add bounded undo and redo history * fix: address undo redo review feedback * chore: refresh guide viewer manifest * fix undo history review regressions * chore: refresh guide viewer manifest	2026-08-31 09:12:13 -07:00
Michael Ramos	42978fe847	fix(share): invalidate stale short links (#1425) ... * fix(share): invalidate stale short links * fix(share): address short-link review feedback * test(ci): isolate short-link lifecycle coverage * test(ci): isolate registered DOM suites	2026-08-31 09:11:35 -07:00
Michael Ramos	c2950e709f	fix: pre-release QA findings for 0.27.9 (#1405) ... Fixes from the 0.27.9 pre-release review. Servers: an unreadable rendered-HTML root falls back to the startup snapshot on both runtimes with a once-per-process warning instead of hanging (Pi) or answering 500 (Bun); the version diff is recomputed against current bytes on reload and carried through the in-app Refresh instead of being dropped, with no history write on a GET. Client: a Refresh action on the compact touch shell; HtmlSurfaceControls renders Refresh independently of the eye; the dead HtmlSurfaceActions removed. Threading: one linear, cycle-safe reply resolution shared by the annotations panel, its sort, and the export (5,000-chain tests), PATCH ingest on both runtimes rejects self-references and cycles, nothing is ever dropped from feedback. WebMCP and viewer hygiene: bounded tombstone and request memories, per-instance minted ids, nudge id caps, waiter cleanup on unmount, a shared retry epoch for diagram blocks. Docs: HTML Refresh documented, the WebMCP design pointer fixed, marketing pages updated. AI-assisted (Claude) under maintainer direction.	2026-08-27 15:23:28 -07:00
Michael Ramos	7d6dd29c08	perf(ui): load the HTML viewer bridge by URL for hosts, with a protocol version and ready timeout (#1398) ... Opt-in bridgeScriptUrl on HtmlViewer so multi-chunk hosts can serve the 185 KB bridge as a hashed asset instead of an inlined string; the inline bridge stays the default and Plannotator's own builds, the Pi and OpenCode copies, and the live-app proxy are unchanged apart from a protocolVersion field on the bridge's ready message. The parent checks the version (one warning naming both versions; on the URL path a dismissible banner plus onBridgeUnavailable while the old bridge keeps working), arms a ready timeout on the URL path only, and resolves the URL against the parent document before it reaches the frame so a page's own base href cannot redirect the load. A prepack-generated bridge-script.asset.js (byte-for-byte the inline string) and a bridge-script.lite.ts alias target ship in the tarball. CSP and CORP requirements for hosts are documented. AI-assisted (Claude) under maintainer direction.	2026-08-27 08:45:22 -07:00
Michael Ramos	0b167cc478	perf(ui): lazy diagram and math renderers with eager entries for Plannotator (#1394) ... Bundle-weight optimization of @plannotator/ui for multi-chunk hosts, requested by Workspaces: the Mermaid runtime and Graphviz engine load inside the render effect, the username dictionary sits behind a synchronous identity generator slot, and KaTeX sits behind a math renderer slot with a loader seam on configurePlannotatorUI. Plannotator's own apps import eager entries (math, identity, and Mermaid for the plan editor) so their behavior is unchanged: single-file builds within noise of main, math typeset on first paint, identities from the full dictionary, and the share portal keeps Mermaid in its entry chunk so its failure surface matches main. Built-HTML registration markers guard the eager imports. Hosts that omit the eager entries get the lazy paths, a one-shot automatic re-attempt, and a Retry affordance on the diagram error panel; the module-map limitation of in-page retries is documented. AI-assisted (Claude) under maintainer direction.	2026-08-27 07:35:49 -07:00
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
Leonardo Reis	6407ef5d97	feat(annotate): manual refresh of rendered HTML from disk (#1232) ... Local rendered-HTML annotate sessions get a Refresh action beside Hide tools: the document is re-fetched through /api/doc, the sandboxed viewer remounts, annotations are re-anchored and the ones that no longer match are reported while their comments are kept, and stale diff and share state is reset. Maintainer additions on top of the contributor's work: share-link invalidation no longer keys on the resolver's identity, /api/plan and /api/share-html serve a local root HTML file from its current bytes on both runtimes so a reload does not revert the page under the annotations, the Refresh button keeps keyboard focus via aria-disabled, and the tests were hardened. Verified end to end in a real browser. Thanks @leoreisdias. AI-assisted (Claude) under maintainer direction.	2026-08-26 14:53:28 -07:00
Michael Ramos	6903d7a3dd	feat(webmcp): expose plan review and annotate as WebMCP tools for browser agents (#1393) ... Phase 1 of WebMCP support: a zero-dependency, feature-detected engine in packages/ui/webmcp plus a read-and-comment tool catalog for plan review and annotate (read_document, add_comments, update_comment, remove_comments, reveal, nudge_user, list_documents). No decision tools; the human approves. Zero footprint in browsers without document.modelContext (DOM, network, console, timers, and cookies identical to main), idle until called where the API exists, and never registered inside the annotate iframes. Adds an optional inReplyTo field on annotations for threaded replies. Client-only; no server changes. AI-assisted (Claude) under maintainer direction.	2026-08-26 14:39:37 -07:00
Michael Ramos	0ae40e73a4	feat(annotate): restore a restricted thumbs-up on comment-only HTML surfaces ... The v0.27.5 comment-only ruling removed every label affordance from HTML and live-app annotate surfaces, leaving no one-click positive feedback: the only path was opening the composer and typing prose. Restore exactly ONE affordance, the hardcoded 'Looks good' thumbs-up, on both input routes: - selection toolbar: commentOnly + a provided onQuickLabel now renders only the thumbs-up (no Delete, no Zap picker, Alt+digit suppressed); HtmlViewer passes a handler that filters by label id as defense in depth - pinpoint: the composer gains an optional one-click 'Looks good' footer action (disabled once anything is typed, so it can never discard a draft), emitting the same isQuickLabel comment shape with the draft's multi-select targets The trust-boundary clamp is untouched: redline/quickLabel modes stay collapsed to selection, so a hostile page still cannot force a DELETION or an arbitrary label. THUMBS_UP_LABEL moves to utils/quickLabels as the canonical definition.	2026-08-24 09:27:43 -07:00
Michael Ramos	b23ffee9ec	fix(vscode): migrate the legacy auto-seeded dark theme cookie to system (#1362) ... #1357 made the panel defer to the app's stored theme mode and seeded System only when no mode was stored. That helped first-time panels and nobody else: every panel opened before it already stored `dark`, written by ThemeProvider on its first mount rather than chosen by anyone, so the seed never fired and the panel stayed dark in a light IDE. That is issue #1053 exactly, still broken for the users who reported it. There is no provenance in the store to read: it is one flat cookie string in globalState with no timestamps and no per-cookie metadata, and the app writes the same `plannotator-theme=dark` whether the user picked Dark or never opened the theme settings. The migration leans on the three signals that do exist. `light` and `system` are values the auto-seed cannot produce, so they are choices and are never touched. A new `plannotator-vscode-seed` marker, written on every load, makes the re-seed run at most once per store, so a Dark picked afterwards is permanent. And a mode the user actually picked is recorded server-side in ~/.plannotator/config.json by configStore.set, which configStore.init applies over the cookie and writes back, so a real choice outranks the seed and re-asserts itself in the same page load. What remains is a Dark that exists only as a cookie with nothing in config.json behind it. That is indistinguishable from the auto-seed and is reset once: invisible in a dark IDE, and in a light IDE one re-pick makes it stick for good. Also fixes the type error #1357 shipped in applyPanelCookieDefaults and adds the extension's own tsc to CI, which had never run there. Co-authored-by: Michael Ramos <backnotprop@gmail.com>	2026-08-21 08:42:09 -07:00
Michael Ramos	4f80360351	fix(vscode): user-chosen theme wins over IDE theme sync (#1357) ... The theme bridge wrote VS Code's colors as inline custom properties on <html>, the same element ThemeProvider stamps `theme-<palette>` and `light` on, and it forced the `light` class to the IDE's theme kind. An inline property outranks every `.theme-*` rule, so picking Light in a dark IDE produced dark VS Code tokens sitting under a `.light` class, and any palette chosen in Plannotator's settings was painted over. The bridge now reconciles instead of applying once on arrival: VS Code colors are only painted while the user is on the default palette and the app is already rendering the IDE's light/dark side, anything it painted is removed the moment that stops holding, and it no longer writes the `light` class except to map System onto the IDE's theme kind. Panels that have never stored a mode seed System, so a first-time user in a light IDE still gets a light panel now that the bridge does not force the mode. Reported by @it-sha.	2026-08-20 13:02:34 -07:00
Burak Varlı	a0aa448f98	fix(review): clamp annotation toolbar to viewport (#1354)	2026-08-20 12:38:25 -07:00
Michael Ramos	2ca55c8332	feat(annotate): live local app annotation through a loopback reverse proxy (#1352) ... * feat(bridge): additive live-mode gate + LIVE_BRIDGE_BOOTSTRAP Adds the config-gated live branch to BRIDGE_SCRIPT: frame gate, pinned parent origin, token-stamped postToParent, origin+token checks on both inbound handlers, pinpoint-only clamp, vim and resize off, pageUrl on ready, and coalesced page-change reporting for SPA history navigation. With no config present (srcdoc) every branch is inert and behavior is unchanged; the existing html-viewer suites pass unmodified as the regression proof. LIVE_BRIDGE_BOOTSTRAP installs the annotation CSS from the JSON config prelude before the IIFE runs. New package export exposes the string constants without the React barrel. * feat(ui): live-session parent side for proxied app annotation useHtmlAnnotation gains a live option (origin + token validated before parseBridgeMessage; token + concrete targetOrigin on every outbound post) and a validated page-change message with onPageChange. HtmlViewer gains src/liveSession/currentPageUrl/onPageChange: src-mode iframe with no sandbox and no srcdoc, ready pageUrl handling, per-page restore filtering with explicit clear-marks + re-sync on navigation, and one postToBridge choke point for its direct posts. Annotation.pageUrl is additive; exportAnnotations groups by page (with global numbering kept) only when a pageUrl is present, byte-identical otherwise. AnnotationPanel shows the page label; AnnotationToolstrip can hide the input switch. The editor app wires mode annotate-app: full-viewport live surface, forced pinpoint, vim off, diff/share hidden, pageUrl stamping. * feat(server): loopback reverse proxy for live app annotation Whole-origin mirror of a local dev server on a dedicated 127.0.0.1 port: streaming bridge injection (after the head open tag, before a bare </head>, or appended; exactly one per document; 8-byte holdback plus a state machine for tags split across chunks), header hygiene (upstream Host rewrite, X-Forwarded-*, identity Accept-Encoding on document intent only, hop-by-hop strip), CSP drop-and-replace with frame-ancestors listing the editor origins, X-Frame-Options removal, target-origin Location rewrite, byte-identical passthrough for assets and encoded HTML (no injection, once-per-session diagnostic), SSE streaming, and WebSocket passthrough with a bounded pending queue for HMR. Host header validation runs before any upstream contact; the bind is the literal loopback constant and the advertised-URL override is never applied. Tests boot a fake dev server and cover injection, hygiene, fidelity, WS echo, and the security posture. * feat(annotate): annotate-app server mode + CLI live probe with remote hard-off startAnnotateServer gains mode annotate-app and a liveApp option: it throws under PLANNOTATOR_REMOTE, generates the per-session token, composes the proxy-served bridge body (JSON config prelude with both editor origin forms, localhost first, plus bootstrap and bridge supplied by the caller so packages/server never imports @plannotator/ui), starts the loopback proxy after the annotate port is known, serves the live /api/plan payload (no rawHtml, no version fields, sharing off), and stops the proxy with the server. Version history and durable submission records stay excluded via the explicit mode gate. The CLI resolution probes loopback http URLs (3s, accept text/html) and defaults them to live mode when the probe returns HTML; --static forces conversion, --app forces live and fails loudly on non-loopback, https, unreachable, or non-HTML targets; both flags are mutually exclusive transport-shape flags never echoed in the tolerant handoff. A live resolution under PLANNOTATOR_REMOTE is a startup failure suggesting --static. OpenCode and Pi parsers are untouched this phase. * test(live-annotate): protocol, server, and probe suites + smoke script + docs htmlLiveProtocol.test.tsx covers the parent trust boundary (origin and token rejection before parseBridgeMessage, token + targetOrigin on every outbound post, validated page-change and ready pageUrl, per-page restore filtering with full-list numbering) and the bridge live gate, executed as the composed config + bootstrap + bridge body inside a dedicated harness iframe so the srcdoc suites keep running the same script uncontaminated in this process. annotate.test.ts gains annotate-app cases (live payload shape, composed bridge served by the proxy, no-history version endpoints, proxy stopped with the server, remote rejection); annotate-live-resolution.test.ts covers the probe matrix. The two post helpers now drop unmatched-targetOrigin posts silently, matching browser semantics where some DOM environments throw. Adds the manual Vite/Next smoke script and the AGENTS.md live app annotation section (phase gate, security posture, limitations). * test(annotate-cli): cover the CLI layer of the live app remote hard-off Spawns the real CLI entry (async, so the in-process fake app can answer the live probe) with PLANNOTATOR_REMOTE=1 against a loopback HTML server and asserts the startup-failure exit with the --static hint. Completes per-layer coverage of the three-layer hard-off (CLI exit, server throw, unconditional loopback proxy bind). * fix(live-annotate): harden the loopback trust boundary end to end - isLoopbackHostname (now canonical in live-proxy.ts, re-exported by the CLI resolution) requires localhost, ::1, or a LITERAL 127/8 IPv4 address: DNS names like 127.0.0.1.evil.example no longer classify as loopback, so neither the default probe nor --app can start a live proxy against an off-box origin. - The live-eligibility probe judges the FINAL response URL: a target that redirects off its loopback origin falls back to the static pipeline (or fails loudly under --app) instead of opening a live session whose iframe immediately leaves the proxy. - WS upgrades with a browser Origin not naming the proxy itself are refused, so a hostile page's cross-site connect is never laundered into the origin-less shape dev servers trust as a non-browser client (Vite CVE-2025-24010 class). - /__plannotator__/bridge.js refuses cross-site/same-site Sec-Fetch-Site fetches: the per-session token is no longer readable via an off-origin script include on modern browsers. - X-Frame-Options is stripped only on HTML responses (where frame-ancestors replaces it); non-HTML responses keep the app's own framing protection. - Redirect Locations are re-anchored by loopback-host + port equivalence instead of a string prefix: alternate loopback spellings are now caught and lookalike ports (5173 vs 51730) pass through untouched. - --app on a non-URL target fails loudly instead of being silently swallowed. * fix(live-annotate): session correctness for SPA restores, origins, and pathful targets - A live find-and-mark that resolves nothing keeps its record, seeded with unresolved placeholder targets from the durable anchor/text params, so the mutation-driven reconcile re-acquires the pin once a lazy route or data-dependent tree renders (SPA navigation no longer permanently drops pins). Srcdoc restores keep the fail-closed drop. - The bridge posts every outbound message once per listed editor origin; the browser delivers only the one matching the parent document, so an editor opened at 127.0.0.1 instead of localhost no longer silently loses ready and every subsequent message. - The advertised appUrl is the proxy under its localhost spelling with the target URL's own path and query: the framed app stays same-site with the editor, shares the dev app's host-only localhost cookies and storage, and a pathful target opens its page instead of the app root. The proxy still binds the 127.0.0.1 literal. * ci(live-annotate): run the live protocol DOM suite; document the hardened posture htmlLiveProtocol.test.tsx is DOM-gated and was absent from the workflow's DOM_TESTS file list, so none of its trust-boundary assertions ran in CI. Add it, and update the live-app section of the project docs: literal-loopback gate, probe redirect rule, WS Origin check, bridge.js delivery gate, localhost appUrl advertisement, live restore resilience, and the remote-mode behavior change (loopback URL annotate under PLANNOTATOR_REMOTE now exits asking for --static instead of silently converting). * fix(live-annotate): absorb the v0.27 mainline into the live session surface Post-rebase seam work after replaying the branch onto main (v0.27.4 era): - Route the bridge's unanchored-transparency report through postToParent so live sessions deliver it token-stamped to the listed editor origins; the raw '*' post main introduced for srcdoc would be dropped by the live parent's message authentication exactly where restores fail most. New live-harness test pins the contract. - Extend the live remote hard-off to --tailscale sessions (flag postdates the branch): CLI startup failure + startAnnotateServer throw keyed on tailnetPublished, matching how the annotate agent terminal treats tailnet publication. Covered in annotate.test.ts and documented in AGENTS.md. - Keep main's compact-touch input controls and effective mode/input values on the HTML surface while preserving the live pinpoint-only clamps. - Regenerate the pinned guide-viewer manifest (CSS hash moved with the new UI classes; JS unchanged). * feat(live-annotate): Interact/Annotate mode toggle for live app and raw HTML sessions A live app session used to be unusable: the pinpoint capture-phase click handler owned every click, so buttons, checkboxes, inputs, and links never fired. One boolean mode now governs the HTML/live viewer surface: - Interact: the bridge is fully passive. Pinpoint capture, hover outline, drag-selection toolbar, [data-annotate] clicks, and committed-highlight click interception are all gated behind annotateModeActive, so clicks, forms, text selection, and SPA navigation reach the page natively. Committed markers and highlights stay VISIBLE, and marker buttons keep their clicks (a marker click still opens its comment). - Annotate: classic behavior, unchanged. Live sessions annotate exclusively via pinpoint while armed. Control: a single bubble icon button in the editor header (icon never changes; armed = accent + visible border, idle = transparent border of the same width, so the box is pixel-identical in both states), plus a subtle inset accent ring floated over the viewer while armed (pointer-transparent, no layout shift). Keyboard: Mod+Shift+A through the shortcut registry (html-annotate scope; the bridge mirrors the chord inside the iframe and forwards it over the authenticated postToParent path). Esc gains a final ladder rung: draft closes first, then the hover outline clears, then Esc exits Annotate back to Interact (bridge posts annotate-exit; a parent-side listener covers Esc with editor focus). The parent owns the mode and pushes it with the same re-post-on-ready pattern as set-input-method, so it survives live page changes, HMR reloads, and bridge re-injection without ever reloading the iframe. Defaults: live app sessions START in Interact; static/raw HTML sessions START in Annotate (today's behavior preserved, and the srcdoc bridge default keeps behavior byte-identical when no set-annotate-mode ever arrives). Session-only state, no persistence. Vim navigation is available only while Annotate is armed. Covered by new bridge-harness and parent-side DOM tests in htmlLiveProtocol.test.tsx and htmlPinpointProtocol.test.tsx: Interact pass-through, armed capture, the Esc ladder order, mode survival across re-injection, marker clicks in Interact, and both defaults. * feat(live-annotate): pinpoint-armed default, always-on drag comments, comment-only HTML surfaces Simplifies the Interact/Annotate design after live review. The new contract replaces the previous one where they conflict: - BOTH surfaces (raw HTML and live app) now START ARMED with pinpoint; the live-session Interact default is gone. Esc keeps the ladder (close draft, clear hover, then exit to Interact) and the header toggle re-arms. The bridge also paints the pinpoint cursor at init instead of waiting for the parent's first round trip. - The header toggle is a PEN icon: the old bubble sat next to the annotations-panel bubble and the two were indistinguishable. Same box geometry (armed = accent + visible border, idle = transparent border of identical width), aria-pressed, Mod+Shift+A, and the armed ring over the viewer are all unchanged. - Text drag-selection commenting is ALWAYS live on HTML/live surfaces, in BOTH states: the selection pass is ungated from annotateModeActive and from the pinpoint input method. In armed pinpoint, click = pin an element and drag = select text, simultaneously; the >4px drag arming decides which one a gesture was, a completed drag's trailing click never re-pins (one-shot dragEndedClick), and a plain click is never swallowed (the pass only acts on a real selection and never preventDefaults). Esc in Interact still closes an open drag draft before yielding to the page. - HTML/live surfaces are COMMENT-ONLY: useHtmlAnnotation clamps redline/quickLabel (both the host mode and a bridge-posted modeOverride, so a hostile page cannot force a DELETION), the selection toolbar drops Delete and quick labels behind a new commentOnly seam on AnnotationToolbar, and the quick-label picker portal is gone from HtmlViewer. Markdown surfaces keep the full toolbar, and persisted DELETION annotations still restore. - The "Show tools"/"Hide tools" header button is removed. It hid the floating toolstrip (now gone from HTML surfaces entirely: with comment-only plus both input paths live there is nothing left to switch), the collapsed sidebar tab flags, and the viewer's floating action cluster (attachments + global comment + version-diff toggle), all of which are now always visible. htmlChrome persistence keeps only the sidebar/panel state; an old cookie's toolsHidden flag is read tolerantly and ignored, so a stale record cannot strand a user with hidden chrome and no way back. - HTML surfaces pin the viewer input method to pinpoint (the drag/ pinpoint switch is meaningless when both are live); the Alt input switch no-ops there. Vim stays armed-only, as built. No server, proxy, or protocol-security changes; the armed flag stays session-only. Tests: the live-bridge harness is reworked around the armed default (forged-DISARM posture, drag-selection passes in armed and Interact, the trailing-click guard), the pinpoint suite covers the comment-only toolbar and the redline/quickLabel clamp at the trust boundary, a new AnnotationToolbar.commentOnly seam test guards both surfaces' toolbars, App.htmlChrome.test.tsx replaces App.htmlHideTools.test.tsx (no tools button, stale-cookie tolerance, pen armed default), and the htmlChrome tests cover the narrowed persisted shape. * feat(live-annotate): collapsible floating controls cluster The simplification removed the Hide tools toggle, which left the floating comment/attachments cluster permanently over the page. Restore a hide affordance on the cluster itself: a collapse chevron shrinks it to a small expand pill in the same corner, so the page is never obstructed without a way back. Collapsed state persists with the rest of the HTML chrome cookie (sidebar/panel), tolerantly read. Hosts that do not wire the toggle (readOnly viewers, review-editor panels) are unchanged. * feat(live-annotate): header Show/Hide tools replaces the collapse pill The collapse pill was a half measure: it left its own artifact over the page and the sidebar tongue tabs stayed. Revert it and restore the real thing as a header control: an eye toggle immediately left of the pen that removes ALL floating chrome over the page from the DOM (sidebar tongue tabs + the comment/attachments cluster), leaving nothing behind. The toggle lives in the header, so a hidden state always has a way back, which also makes honoring a persisted (or pre-existing) toolsHidden cookie safe again.	2026-08-19 10:44:21 -07:00
Michael Ramos	15f8d4fe4c	feat(review): collapse generated files by default in the all-files view (#1346) ... * feat(review): collapse linguist-generated files by default (#1317) Code review now respects linguist-generated (and linguist-generated=true) from .gitattributes, collapsing those diffs by default the way GitHub does. Server (Bun + Pi mirror): a generatedFiles sidecar rides /api/diff and /api/diff/switch, resolved through git's own attribute machinery — one batched 'git check-attr --stdin -z' over the served patch's paths at the review cwd, so stacked and negated rules land exactly as git resolves them. Plain local git sessions only; PR worktrees, workspace multi-repo, jj, GitButler, and P4 omit the sidecar (degrade to no-collapse). Shared logic in packages/shared/generated-files.ts, vendored to Pi. Client: generated files SEED their CodeView item collapsed (the existing Pierre collapse state — same mechanism as commit-diff folding), render the one-line FileHeader bar with a 'generated' tag next to the +/- counts, and expand per file on click. Expansion is session-local App state so it survives remounts and diff switches. Presentation-only: the diff data, annotations, search, and Edit Mode are untouched; the file tree and single-file tabs list generated files normally (tag, no auto-collapse). Guide viewer manifest pin regenerated (AllFilesCodeView/FileHeader are bundled into the guides.show viewer) from a clean frozen-lockfile install. * feat(review): built-in generated defaults, visible collapsed strip, review-round fixes (#1317) Round 2 on PR #1346, per maintainer review. Built-in generated defaults (industry-standard two-layer detection): packages/shared/generated-files.ts (vendored to Pi) now carries DEFAULT_GENERATED_PATTERNS — lockfiles (package-lock.json, yarn.lock, bun.lock, Cargo.lock, go.sum, ...) plus *.min.js / *.min.css / *.map — matched against the path's last segment only. Explicit .gitattributes wins in BOTH directions: linguist-generated (set/true) marks any file, -linguist-generated / =false un-marks even a built-in name, unspecified falls through to the defaults. In plain local git sessions check-attr refines the defaults; the non-git degrade modes (piped patches, PR worktrees, workspace, jj, GitButler, P4) now emit the sidecar from the name-based defaults alone instead of omitting it. Visible collapsed state: a collapsed generated card no longer renders as a bare header — a GeneratedFileNotice strip ('Generated file collapsed', +N/-N, 'Click to view') styled like the other below-header notices sits in the card, and clicking it expands through the SAME reportFileCollapsed funnel as the chevron. Review findings: - F1: search-match and sidebar-comment navigation expanded items without reporting through the funnel, so those expansions died on diff switch. Both now call syncAllCollapsedMirror + reportFileCollapsed; the funnel invariant comment lists the navigation-driven sites. - F2: the check-attr call gets the same 5000ms timeout as review-core's stdin git callers, and Pi's vcs.ts stdin write gets the one-line EPIPE guard (call-flow.ts shape) a timeout kill makes reachable. - F3: removed the dead prevGeneratedRef + collectSetDelta leg — a changed generated set always remounts via fileSetKey, so the delta path was unreachable. Tests: default-list matching (glob + directory-named-bun.lock), both- direction precedence, non-git sidecar from defaults (dual-runtime), the placeholder strip through the funnel, and search expansion surviving a re-seed round-trip. AGENTS.md payload docs updated. Guide viewer manifest pin regenerated from this clean frozen-lockfile worktree.	2026-08-17 23:32:29 -07:00
Michael Ramos	a899ea657f	fix(review): stop the call-flow Lens closing mid-scroll and opening on drive-by hovers (#1338) ... * fix(review): stop the call-flow Lens closing mid-scroll and opening on drive-by hovers The Lens popup scrolls internally but never contained overscroll, so momentum that hit its edge chained to the page; the page scroll moved the anchor, the popup tracked it out from under a stationary pointer, and the resulting mouseleave closed the Lens mid-scroll (worst under Safari rubber-banding). Scrolling the diff also dragged file headers under the cursor, and the zero-delay hover open popped the Lens for every badge that passed. - overscroll-behavior: contain on .call-flow-popover - 100ms hover-intent delay before opening; a pointer that leaves first never opens it - close grace 140ms -> 250ms - while a scroll is in flight, pending hover-closes are held; once it settles the Lens closes only if the pointer really ended up outside Reported by @acewhocares on X (Safari). * ci(guides-show): publish the built viewer manifest as an artifact The viewer build is deterministic per platform but not across platforms, and the committed pin must match the Linux CI build. When the manifest check fails on a PR, a maintainer on macOS previously had no way to obtain the authoritative hashes. The uploaded manifest.json closes that: download, drop into apps/guides-show/dist/viewer/, run sync:manifest, commit. * fix(guides): regenerate viewer pin for the Lens changes; correct the manifest-artifact comment The cross-platform nondeterminism claim in the previous commit was wrong. A clean install builds identical viewer hashes on macOS and Linux; the divergence observed here came from a drifted local bun store carrying stale duplicate package versions (rm -rf node_modules + frozen install reproduces CI exactly, verified against the deploy run's published names). The artifact upload stays: it is the authoritative recovery path when a local environment is the thing that is wrong. Pin regenerated from a verified-clean build for the Lens bundle changes.	2026-08-17 10:37:05 -07:00
FND	e1ce7dabe1	feat(ui): Totman/Classic P favicon style switcher (#1325) ... Favicon style switcher in Settings > Theme: the Totman mascot or the historical dark-navy P tile (byte-identical to the pre-Totman asset, sha256 pinned). Served server-side from first paint in both runtimes; opt-in for hosts of the published UI package. Contributed by @FNDEVVE	2026-08-16 21:58:46 -07:00
Michael Ramos	d0c32c8863	fix(review): preserve dragged diff ranges on compact touch before commenting (#1333) ... * feat: add mobile touch range selection prototypes * docs: redirect mobile range selection spike * revert: remove command-mediated touch range prototype * docs: align mobile line selection with DiffsHub * feat: preserve mobile diff ranges before commenting * 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. * 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. * 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. * chore(guides-show): regenerate viewer manifest pin for the touch selection changes	2026-08-16 21:47:43 -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
Michael Ramos	d6d727b34f	ci(release): add SBOM and Grype release gate (#1298)	2026-08-13 11:45:48 -07:00
Michael Ramos	58598bbf2b	ci(security): add isolated ZAP DAST monitoring (#1299)	2026-08-13 09:46:41 -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	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	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	d4ce3dcb57	ci: harden releases and add security scanning (#1274) ... * ci: harden release and add security scanning * Harden release and deploy recovery paths * Fix npm artifact pack destinations	2026-08-12 11:40:15 -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	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	e53a933106	fix(ci): bump Bun build pin to 1.3.14 for sandbox env loading (#1249) (#1250) ... Bun <= 1.3.11 loads an empty process.env when a cwd ancestor directory is unreadable, the normal state inside OS sandboxes (Seatbelt/Landlock): every released binary silently ignored all PLANNOTATOR_* env vars there (oven-sh/bun#27802, fixed in 1.3.13). The 1.3.11 pin existed for the Bun 1.3.12 cross-compile signing regression (#541, binaries SIGKILLed on macOS); verified gone on 1.3.14: cross-compiled darwin-arm64 output carries the same linker-signed CodeDirectory as the known-good shipped binaries and executes cleanly on macOS 26.3, and the unreadable-ancestor env repro passes on a 1.3.14 build. Guardrails so neither regression class can ship silently again: the release smoke matrix gains a macOS arm64 leg (executing the binary is the signing assertion), and a new smoke step runs the annotate server from a cwd with an unreadable ancestor, where responding on the fixed PLANNOTATOR_PORT proves env vars were read.	2026-08-09 21:06:26 -07:00
Michael Ramos	4d9147200d	ci: wire 8 orphaned DOM-gated test files into the DOM test step ... The DOM step runs an explicit file list, and eight describe.if(hasDom) suites were never added to it, so they silently reported 0 pass / all skip on every CI run: the three new #1243 suites (htmlPinpointProtocol, htmlChrome, inputMethod) plus five older vim/highlight suites (vimHud, Viewer.vimMode.integration, useVimSelection, codeHighlight, vimNavigation). Found by the pre-release sweep. All 69 tests pass together locally under the exact CI invocation.	2026-08-09 17:33:32 -07:00
Michael Ramos	005c32c7c8	ci: remove the install-script auto-sync job (manual sync only) ... The deploy-install-scripts job added in #1214 assumed the CI deploy role could write to the plannotator-install-scripts bucket. It cannot, by design, and the job's first-ever real execution (the #1239 installer change) failed on AccessDenied. Install-script syncs are manual with local credentials; the workflow now carries a comment pointing at the release runbook's exact commands instead of a job that cannot succeed.	2026-08-09 16:24:47 -07:00
Raúl	183cae2c44	fix(vim): keep j/k cursor clear of HUD bands when scrolling (#1154) ... Document Vim navigation moved the cursor with `Element.scrollIntoView({ block: 'nearest' })`, which parks the target flush against the nearest viewport edge — exactly where the sticky action bar (top) and the key HUD / status pill (bottom) float. Motion to the top or bottom of a document then hid the caret behind an overlay, while a mouse wheel (which the browser lets overshoot) kept the same line nearer centre. Add vimScroll.ts: a pure computeVimScrollDelta returning the signed scrollTop delta needed to clear a HUD band at each edge (0 when the target is already inside the safe band; a target taller than the band aligns to its top edge so reading order wins), resolveVimScrollMargin sizing the fallback band as clamp(20% of viewport height, 24px..160px), and a scrollVimTargetIntoView wrapper. Both bands are measured from live geometry instead of guessed constants: the top band widens past the ratio margin to clear the sticky action bar, and the bottom band derives from the portaled key HUD / mode pill rects ([data-vim-key-hud] / [data-vim-mode-badge]). The opt-in key HUD (fixed bottom: 150, height: 88 — a ~238px band, past the 160px clamp) is actually cleared, while the default pill reserves ~49px instead of over-reserving 160px. An expanded key HUD is a deliberate modal state that can outgrow the viewport, so it keeps the ratio margin. The scrolling element is the native-scroll host fed through ScrollViewportContext, so the wrapper takes that element from the caller (useScrollViewport() in Viewer, the same node the reticle measures against) and falls back to the historical scrollIntoView when it is absent, so behaviour never regresses. Route every cursor/target move in useVimSelection through it, and add vimScroll.test.ts to the DOM allowlist in test.yml so its integration tests run in CI.	2026-08-09 16:16:59 -07:00
Michael Ramos	7ad4d39ed9	feat(comments): reference agent skills with / or $ in plan review and annotate comments (#1229) ... * feat(comments): reference agent skills with / or $ in plan and annotate comments Typing / or $ at the start of a word in the document-UI comment composer opens a picker of the user's global agent skills (~/.claude/skills, ~/.codex/skills, ~/.agents skills roots), served by a new GET /api/skills on the plan and annotate servers in both runtimes (Bun + Pi mirror). Multiple references per comment are supported; references live in the comment text itself and are appended to exported feedback as a 'Skills referenced' block so the acting agent knows which skills to apply. Human-invocation-only skills (disable-model-invocation: true frontmatter) stay listed and selectable but render dimmed with a badge, warn in the menu and composer, and are marked in the export so the agent is never asked to invoke something it cannot. Discovery reuses the review-skill loader (same roots, precedence, and skip-and-log discipline), reads only an 8KB head per SKILL.md, caps the catalog at 500 skills, takes no client input, and is never persisted; any failure degrades to plain typing. * fix(comments): harden skill references per review (trigger, IME, seam, fail-closed frontmatter) Blockers: - B1: a trigger now requires at least one query character. A bare / or $ no longer opens the catalog, so Enter stays a newline and Tab still leaves the field ("This costs $" + Enter, "cd /" + Tab, bullets). - B2: the menu ignores keys mid-IME-composition (nativeEvent.isComposing), matching the 16 existing guards; Enter committing a Pinyin/Telex/Korean candidate can no longer insert a skill. - B3: the catalog request is a host seam (skillCatalogTransport via configurePlannotatorUI), defaulting to the existing GET /api/skills. - B4: resetSkillCatalogCache() invalidates outstanding requests (generation counter), and a late-resolving stale request can no longer overwrite a newer cached value or the export registry. The catalog tests reset in beforeEach, so they hold in any file order. Also: - F1: skillReferences={false} is fully inert — the human-only notice memo and the cache seed are gated on the prop. - F4: frontmatter flag parsing no longer fails open: trailing YAML comments are stripped, on/1 (and TRUE/yes etc.) read as true, the head read is 64KB, and truncated unterminated frontmatter fails CLOSED on disable-model-invocation. - F5: extraction ignores markdown link destinations ([x](/name)), shell redirects (cat /x > out), and /-triggered FHS root names (/run, /tmp); menu insertion switches / to $ for those names so inserted references always survive extraction. - F6: the 500-skill cap slices after sorting, so which skills survive no longer depends on readdir order. - F3: /api/skills wiring guards for the Bun and Pi plan + annotate servers (skills-endpoint.test.ts). - Keyboard state machine tests against the real CommentPopover in happy-dom (bare trigger, insertion, composition, Escape, highlight bounding, opt-out inertness), added to the CI DOM step. - The insertion path dismisses the trigger start so the menu close is ordering-safe against React's select-plugin re-reading a stale caret. * feat(comments): redesign the skill reference menu (bare triggers, no preselection, highlighted tokens) Per maintainer direction, reversing the earlier bare-trigger opt-out deliberately: typing a bare / or $ at the start of a word now opens the full skill catalog immediately, and the safety story moves from the trigger to the menu itself. No preselection (the load-bearing rule): the menu opens with NO row active, and while nothing is active every key behaves exactly as if the menu were closed. "This costs $" + Enter is a newline; "cd /" + Tab leaves the field (the proven regression that must never return). A row activates only via ArrowDown/ArrowUp (Down from none lands on the first row, Up on the last); only then do Enter/Tab insert. Pointer hover never activates a row, because the menu floats exactly where the mouse rests over the composer; a click inserts directly and never arms Enter. Continuing to type re-filters and disarms any active row. Escape clears the active row and dismisses when the user engaged (query typed or row active); an unengaged bare-trigger menu passes Escape through so closing the composer still costs one press. Menu redesign to the reference look: icon, bold name, dimmed inline description with ellipsis, right-aligned source column (Agents / Claude / Codex from the discovery roots), rounded generously padded rows, and a subtle active-row background; human-only rows stay dimmed with their badge and the warning now shows while such a row is ACTIVE. Inserted references render highlighted in the composer via a mirrored aria-hidden overlay behind a transparent-text textarea (identical font, padding and wrapping metrics; scroll synced; tokens change color and background only, drawn from the --primary theme token so every palette works in light and dark). The caret keeps --foreground, selection uses a translucent primary wash, and IME composition temporarily restores native textarea text so composition underlines render normally. skillReferences={false} still renders the plain pre-feature textarea. Also, per review: - extraction: dropped the over-broad shell-redirect exclusion (false negatives on prose like "use /animate <- this one"; the motivating case stays covered by the reserved-path rule) - frontmatter: an unterminated frontmatter block now fails CLOSED on disable-model-invocation even in complete (untruncated) files - the reserved-path / to $ insertion switch stays: extraction still reads /run as a path, and the new token highlight makes the switch self-explanatory (an unhighlighted insert would look broken) The composition guard, transport seam, catalog generation counter, enabled gating, and export rules are unchanged and re-covered by the rewritten DOM test matrix. * fix(comments): give the skill reference menu adaptive, viewport-clamped placement The menu rendered bottom-full with a fixed max-h-64: always upward, up to 256px, with no viewport awareness. With the comment popover near the top of the viewport (annotating near the top of a document), typing a trigger ran the menu off the top of the screen with its upper rows unreachable. Placement now mirrors the popover's own computePosition idiom: measure the space above and below the composer wrapper against window.innerHeight, prefer above (the shipped direction; keeps the action row and human-only notice visible), flip below when the list fits below but not above, and when neither side fits pick the roomier side. The list's max height is clamped to the available space (still capped at the former 256px), so the menu never extends past a viewport edge. Recomputes on every commit (drag moves, popover flips, filtering changing the item count, warning-footer toggles) plus capture-phase scroll and resize listeners, matching the popover's tracking. Visual design of the menu and rows is unchanged. * feat(comments): inject human-only skill instructions into exported feedback A human-only skill (disable-model-invocation: true) referenced in a review comment used to export as a dead name the agent could do nothing with. A human referencing a human-only skill IS the human invocation, so the export now injects the skill's SKILL.md body verbatim (frontmatter stripped) inside clearly delimited BEGIN/END SKILL INSTRUCTIONS markers, with the absolute skill directory and the resolve-relative-paths pointer so references/, scripts/, and assets/ stay actionable. Model-invocable skills keep exporting as names the agent can invoke itself. Transport is lazy: a new GET /api/skills/content?name= endpoint (Bun and Pi) serves one discovered skill's body, capped at 20k chars with an explicit truncation notice pointing at the file; the client fetches contents only for the human-only skills actually referenced, keyed off comment state, and the catalog now carries each skill's absolute dir so every failure path (deleted skill, unreadable file, race with submit) degrades to naming the skill plus its directory. Names are matched against discovery only and never used as paths, so traversal cannot escape the skill roots. A per-export dedupe injects each skill once even when several comments reference it, and GLOBAL_COMMENT annotations run through the same block. The referenced-skills header now says the reviewer is asking for the invocation, and the human-only menu footer and composer notice explain that the skill's instructions will be included with the feedback instead of warning that the reference will not work. * polish(comments): quiet, progressive human-only skill treatment The human-only surfaces shipped with too much emphasis: a dimmed row plus a bordered uppercase badge, an amber warning footer, and a persistent amber notice in the composer after insertion. Human-only is a property of a skill, not an error state, so the treatment is now quiet and progressively disclosed: - Menu rows render at full strength with a small muted 'human-only' pill (bg-muted / muted-foreground tokens; no border, no dimming). - The plain-language explanation (a model cannot invoke it, so its instructions will be included with your feedback) appears as a muted footer only while a human-only row is active (keyboard) or hovered (pointer). Hover disclosure is purely visual state local to the menu; it never touches activeIndex, so the no-preselection invariant and the hover-never-arms-Enter rule are unchanged and re-asserted by a new test. - When not disclosed, the same sentence stays in the DOM sr-only and human-only rows point at it with aria-describedby, so the state reaches assistive tech as text rather than as a purely visual badge (this does not attempt the #1233 combobox semantics, and does not worsen them). - After insertion, the highlighted token itself carries the quiet inline marker (a dotted primary underline; text-decoration cannot move glyphs, so overlay alignment is untouched) and the standing amber notice is replaced by a native <details> disclosure: a single muted 'Includes skill instructions' summary line that expands to the full accurate sentence, operable by pointer, keyboard, and AT alike. No amber remains; every color is a theme token (muted, muted-foreground, border, primary, ring), so the treatment follows every palette in light and dark. Copy is unchanged where it was accurate. Behavior is unchanged: human-only skills stay selectable and injection still happens. * fix(comments): harden human-only skill injection per adversarial review Three findings on the injection path, each with tests that fail pre-fix: 1. Marker forgery: an injected SKILL.md body containing our own `--- BEGIN/END SKILL INSTRUCTIONS ---` markers (or an `[Instructions truncated:` notice) could close the block early — making everything after it read as the reviewer's own words — forge a block for a skill nobody referenced, or forge a truncation notice pointing at an attacker-chosen path. Body lines matching the structural marker forms (leading-whitespace and case variants included) are now visibly neutralized before injection: kept verbatim but prefixed, never silently deleted (neutralizeSkillMarkerLines). 2. Forged human invocation: POST /api/external-annotations is unauthenticated on localhost, so any local process could submit a comment referencing a human-only skill and cause its instructions to be injected "at the reviewer's request". Annotations carrying a `source` now still LIST their skill references but never cause verbatim injection — human-only references fall back to naming the skill plus its directory, with an honest reason. The content-prime effect skips external texts for the same reason. A human referencing a human-only skill IS the human invocation; a tool is not. 3. Unbounded read: readReferenceSkillContent read the whole SKILL.md before slicing to the 20k cap, so an unauthenticated no-cors fetch loop could balloon RSS by file size per request (measured +64.4MB for a 64MB file). It now uses the same bounded readFileHead as the catalog, reading only frontmatter allowance + 4 bytes per capped char + slack; truncation detection is unchanged for any file whose frontmatter fits the catalog bound, and frontmatter that overflows the read falls back to null rather than serving raw YAML. Measured: 12 reads of a 64MB SKILL.md now cost +5.1MB total. Also: the fast-fail guard no longer rejects legitimately discovered names — `name.includes("..")` 404'd a real `v1..2` skill dir forever (and `\` is legal in POSIX names) while defending nothing, since the name is only ever matched against discovery output and never joined into a path. It now rejects exactly the names that can never be a readdir entry: empty, `.`, `..`.	2026-08-07 09:50:42 -07:00
Michael Ramos	548497e6f1	fix(annotate): hide the collapsed sidebar tab flags with "Hide tools" (#1226) ... The header's "Hide tools" toggle dropped the annotation toolstrip and the HtmlViewer action cluster, but the collapsed sidebar tab flags kept protruding from the left edge, so a rendered HTML page never actually got the whole viewport. Fold both overlay guards into one derived `htmlChromeHidden` and apply it to the SidebarTabs render. The flags unmount rather than fade, so nothing focusable stays in the tab order and no click target sits over the page. The toggle only exists on HTML surfaces, and both restore paths stay visible: "Show tools" lives in the header (never hidden), and Mod+B still opens the sidebar directly.	2026-08-06 18:29:07 -07:00
Michael Ramos	75e8b78cc7	fix(review): stop stubbing files whose worktree content the size probe cannot find (#1220) ... The oversized preflight in buildBoundedTrackedDiff mapped every object the cat-file batch could not size to infinity. `missing` is routine: for tree-vs-worktree diffs git hashes the WORKING-TREE content of any path pulled into rename/copy detection and prints that hash in --raw output without ever writing the blob, and partial clones report it for unfetched blobs. Those files were excluded by pathspec and replaced with a contents-free binary stub, so a renamed-and-edited file rendered as a silently empty card and /api/file-content refused it as binary. Missing now means unknown, not oversized, and an unreadable new side is bounded by the working-tree file's stat size instead. Every rendered diff stays bounded git-side by core.bigFileThreshold (#1205), genuinely oversized files still stub via real probe sizes plus the stat door, and a blob git truly cannot read now fails loudly through assertGitSuccess instead of blanking a file. Client side, a chunk with a binary marker and no hunks now renders an explicit placeholder in both the all-files view and the single-file viewer, so an empty card can never again pass for "no changes here". AI-assisted.	2026-08-06 00:47:19 -07:00
Michael Ramos	7ba4e3b3e4	fix(review): mint content-derived diff cache keys so single-file tabs render fully (#1219) ... * fix(review): mint content-derived diff cache keys so single-file tabs render fully Single-file diff tabs have not rendered their full-content diff since v0.26.0: the expansion gap bars show no chevrons and clicking them does nothing, at every file size. @pierre/diffs 1.3.2 (the 1.2.8 -> 1.3.2 bump, upstream "Fix diff rerender in edit mode (#878)") added name-based cacheKey defaulting in FileDiff.render: an unset `fileDiff.cacheKey` becomes the file's name. `areDiffTargetsEqual` — the only identity check its render and highlight caches make — compares nothing but that key. DiffViewer renders each file twice on one surviving FileDiff instance (key={filePath}): first the PARTIAL diff from getSingularPatch, then the AUGMENTED full-content diff from processFile once /api/file-content lands. Neither set a cacheKey, so both defaulted to the filename and Pierre served the stale partial render forever. Only the augmented diff is expandable, hence the dead gap bars. Both diffs now mint content-derived keys (`<path>#<hash>` and `<path>#full#<hash>`), matching how AllFilesCodeView already keys its items — which is why the all-files view was never affected. The hash (not patch.length) matters because Pierre's worker highlight cache is a singleton that outlives remounts. The partial diff needs its own key too: with key={filePath} the instance also survives diff-type and base switches, where a same-named new patch would otherwise hit the same name-keyed stale cache. hashString moves from AllFilesCodeView to utils/hashString.ts so both surfaces mint keys the same way. Covered by a new DOM test that mounts DiffViewer against the real @pierre/diffs renderer, holds the /api/file-content response until the non-expandable partial baseline is asserted, then requires the expansion affordances to reach the pixels. It fails against the unfixed tree. * fix(review): explain why an oversized file's card has no diff Files over the 5 MB review limit are replaced by a contents-free stub (buildOversizedTrackedStub, plus the untracked equivalent), which renders as a header-only card with no counts and no explanation. Users read that as a broken diff. The stub now carries an explicit marker line in its extended header (OVERSIZED_REVIEW_STUB_MARKER). A marker rather than a client heuristic because the only other signal, `Binary files ... differ`, is exactly what a genuine binary file emits, so a heuristic would put a false size-cap explanation on every image in the diff. The marker lives in shared/diff-paths so the browser bundle can detect it without pulling in the node-facing review core; both server runtimes pick it up from review-core, which vendor.sh already copies to Pi. Git ignores unknown extended-header lines and @pierre/diffs parses the stub identically with or without it, so nothing else moves. Which files get stubbed is unchanged. Both review surfaces now render one line under the file header saying the file is over the limit and only a stub is shown. * test(review): make the diff-swap proof machine independent, not stopwatch based CI failed two tests that pass locally. Both were timing races, neither was an app bug. 1. DiffViewer.fullContentSwap: the swap assertion carried a 15s internal wall-clock budget, which a cold, contended CI runner blows and a warm laptop clears. Two changes, both aimed at the clock rather than the symptom: - The waits are now budgeted in SCHEDULER TURNS, not milliseconds. A slower box spends longer inside each turn but needs no more of them, so the budget never has to be retuned for CI hardware. - Pierre's shared Shiki highlighter is preloaded before mounting. It is a module singleton, and building it was the entire multi-second cost the old budget was accidentally measuring; warming it moves that work into an unbounded await OUTSIDE the observed window. Disposed in afterAll, because packages/ui/utils/codeHighlight.test.ts asserts the pre-attachment behaviour of that same singleton. Verified against an artificially stalled clock: forcing 20s of dead time into every wait (41s total, far past the old 15s budget) still passes, and with the cacheKey fix removed it still fails on the assertion (not as an opaque timeout) in ~12s. A 20-turn budget with the preload removed and every core saturated also passed 10/10, so 400 turns is a wide margin rather than a guess. 2. App.archiveReadOnly compared the fenced block's innerHTML before and after a click. Since #1218, applyHighlight writes plain text first and swaps in Shiki markup when the grammar attaches, so that MARKUP changes on its own schedule and the assertion was racing the swap. The test is checking that the click opened no mutation entry point, which textContent plus the absence of an annotation <mark> says exactly, and which no highlight swap can perturb. Latent on main; the branch's run happened to lose the race. Also fixed while confirming the above: codeHighlight.test.ts asserted a GLOBAL precondition ("no grammar attached yet") that any earlier file attaching a typescript fence invalidates, so `DOM_TESTS=1 bun test packages/ui packages/editor` failed by file order alone. It now resets the attachment cache through the existing __resetCodeHighlightCacheForTests seam and asserts the contract instead of the run order. Not currently reachable from CI (that file is not in the DOM list), but one list edit away. * test(review): drop the highlighter preload, harden the swap proof, report why a paint is missing The preload added in the previous commit made CI strictly worse, so it is gone. Before it, CI's partial diff painted and only the swap was missing; with it, CI never painted at all. It was an optimization for a theory the evidence has since killed, and it mutated a process-wide singleton to buy it, so it is not worth keeping while the real failure is unexplained. The afterAll dispose that existed only to undo the preload goes with it. What the CI log actually shows: - The "WorkerPoolManager: operation canceled because the pool terminated" error is inside discardRestoreRender.test.tsx's own group, ~0.3s BEFORE this file's group opens. It is that file's provider unmounting and terminating the pool singleton it created: end-of-file teardown, the same benign noise documented on #1209. It also prints on every local run, where the whole list passes. It is not a mid-test terminator, and nothing in this file uses the worker pool (no WorkerPoolContextProvider is mounted, so useWorkerPool() is undefined and rendering takes the main-thread path). - This file's group prints NOTHING for its whole 10.3s: no console.warn from the stale-content guard, no error. Pierre simply painted nothing. Not reproducible locally: the exact DOM list from test.yml, one bun process, forward and reverse order, 13 runs with every core saturated, all green. So the remaining difference is the environment, which cannot be reasoned out from here. Three changes make the next CI run answer it instead of costing another guess: - renderDiagnostics() dumps what Pierre actually painted (container / separator / chevron / line-number counts plus a markup fragment) when a wait gives up. Prints only on failure, so it is worth keeping. - The precondition is asserted rather than assumed: the REAL getSingularPatch and processFile must produce partial-then-full on these fixtures. Bun's mock.module is process global and an earlier file in this very list mocks '@pierre/diffs', so a leaked mock now fails in milliseconds with a clear message instead of as a render that never arrives. - The first paint is now REPORTED, not asserted. The verdict belongs to the swap; gating on the partial paint let a slow or absent first paint mask the result the test exists for. Removing the cacheKey fix still fails it (verified), because that tree paints no chevrons at any point. Also fixed a real trap in the fixture: the hunk header said @@ -61 while its context lines start at line 59 of both contents. Pierre realigns a misaligned header rather than rejecting it, so it was silently tolerated. * test(review): stop a leaked module mock from silently unrendering the diff tests Root cause, and it was never a timing problem. AllFilesCodeView.lifecycle.test.tsx calls `mock.module('@pierre/diffs', ...)` with a hunk-less `getSingularPatch` and `processFile: () => null`. Bun's module mocks are process global and are not unwound at file boundaries, and that file sits immediately before DiffViewer.fullContentSwap.test.tsx in the DOM step's list. On the Linux runner the stub reached this file; on macOS it did not, which is why 13 local runs of the exact list, both orders, cores saturated, stayed green. It explains both CI symptoms exactly, including the one that looked like a contradiction: `processFile: () => null` means the augmented diff never exists, so no chevrons ever (the failure before the preload); the stub `getSingularPatch` has `hunks: []`, so nothing paints at all (the failure after it). The "WorkerPoolManager: operation canceled because the pool terminated" line was a red herring throughout: it is inside discardRestoreRender's own group, ~0.3s BEFORE this file's group opens, is that file's provider unmounting the pool it created, and prints on every local run too. The precondition assertion added in the previous commit is what proved it, turning a 10.3s mystery into a 0.45ms verdict: 228 | expect(expected?.isPartial).toBe(false); error: expect(received).toBe(expected) Expected: false Received: undefined Fixed at both ends: - Source: the mocking file now captures the real modules before it stubs them and restores both library specifiers in afterAll, so no later file in any run inherits its stubs. This fixes the class for every future DOM test that needs the real renderer, which was the actual leak. - Consumer: the two tests that render against the real @pierre/diffs get their own CI step, the same isolation (and for the same kind of reason) this workflow already gives useFileBrowser.test.tsx. They are removed from the shared list so that step is their single source of truth. The restore above should make this unnecessary; it is not something to bet a green build on from a machine that cannot reproduce the platform behaviour. Verified with a CI-faithful harness: one bun process per step, the exact lists from test.yml, isolated + shared-forward + shared-reverse, 10 iterations with every core saturated, then 6 more after the final split. All green, plus the full suite and typecheck. * docs(test): point the diff-renderer DOM tests at the CI step that actually runs them	2026-08-06 00:31:25 -07:00