694 Commits

Author	SHA1	Message	Date
Michael Ramos	f8951cd3c6	feat(annotate): shift-click multi-element selection for raw-HTML pinpoint (#1254) ... * feat(annotate): shift-click multi-element selection for raw-HTML pinpoint drafts One comment covering multiple targets: shift-clicking elements while a pinpoint draft composer is open toggles them in/out of the SAME draft. - bridge: pendingMultiTargets registry with per-target pinned outline boxes, DOM-identity + anchor-equality toggle dedup, primary promotion on removal, draft cancel on last removal, shift-hover preview, rAF pointer relay for the composer yield, remove-target/flash-target parent messages, capped at 16 additional targets at the source - pins: registerPin now allows several elements per annotation id (deduped by (id, element)/(id, anchor)); badges number by first-seen id so all targets of one annotation share one number; find-and-mark restores additionalAnchors as same-numbered pins (anchor-only, fail-closed) - parent hook: multi-target-added/removed/pointer messages validated and capped at the trust boundary (key<=64, label<=64 truncated, text via the 10k surrogate-safe cap, anchors via parseHtmlElementAnchor, array cap 16); draftTargets state, chip removal with deterministic promotion mirrored on both sides, composerFocusToken for focus return - types: additive Annotation.htmlAdditionalTargets (label/text/anchor per extra target; anchor optional so fail-closed targets still export) - CommentPopover (all seams optional, default off): horizontally scrollable target chips with remove buttons and hover-to-flash, refocusToken, captureStrayKeys first-keystroke guard, yieldState fade/click-through with 180ms transition and prefers-reduced-motion fallback - HtmlViewer: composer-yield state machine (composerYield.ts) fed by parent mousemoves plus bridge-relayed pointer positions, with 48px/96px hysteresis - export: multi-target comments append an 'Also applies to N more elements' block (label + excerpt per target); single-target output byte-identical - share URLs: additional targets are dropped exactly like htmlAnchor (the compact tuple format never carried anchors); drafts carry them verbatim * test(annotate): cover shift-click multi-select across bridge, DTO, composer, export, drafts, sharing - srcdoc.test.ts (bridge DOM): shift-click add + toggle-off with echoed removals and per-target pinned boxes; create-mark commits all targets under one id with one badge number (second annotation numbers 2); primary promotion and last-removal cancel; parent remove-target mirrors without echo; flash-target; 16-target cap at the source; find-and-mark restores additionalAnchors as same-numbered pins with stale anchors failing closed - htmlPinpointProtocol.test.tsx: multi-target-added/removed/pointer DTO validation (key/label/text/anchor caps, hostile payloads), selection targetKey/targetLabel validation; mounted-composer flows — primary chip, shift-adds into ONE submitted comment carrying htmlAnchor + 2 additional targets, single-target submit shape unchanged, promotion onto the comment, last-removal closes the composer, chip removal, 16-cap at the trust boundary, drag selections never arm multi-select - CommentPopover.multiTarget.test.tsx: chips render primary-first with remove/hover handlers, refocusToken focus return, captureStrayKeys stray keydown routing (and non-interference when focused), yieldState classes + reduced-motion-aware 180ms style; default composer renders none of it - composerYield.test.ts: distance + hysteresis state machine (48px over-exit, 80/96px near enter/exit) - parser.test.ts: multi-target export block (labels, excerpt clipping, fail-closed targets) and byte-identical single-target output - useAnnotationDraft.seam.test.tsx: multi-target annotations round-trip the draft transport verbatim (save body + restoreDraft) - sharing.multiTarget.test.ts: share tuples drop anchors AND additional targets while the comment itself still shares * fix(annotate): keep pinpoint drafts alive when the pinned element scrolls out of view Found by the real-browser signoff harness: reaching a second element to shift-click often scrolls the pinned primary out of the viewport BEFORE any additional target exists, and the scroll-out teardown then cleared the bridge's pin state mid-compose — the shift-click landed on a dead draft and started a new one instead of adding to it (and even a single-target pinpoint draft silently lost its visual pin on commit after scrolling). Pinpoint drafts (pendingPinViaPinpoint) now survive scroll-out; drag selections keep the existing close-on-scroll-out behavior unchanged. * fix(annotate): address adversarial review of multi-select (arm handshake, label sanitization, iframe shift relay, removal resync) D1 (blocker): the bridge accepted shift-toggles for ANY pinpoint draft while the parent only mirrors targets when the comment composer owns it — in quickLabel mode the user could pin elements the saved annotation would never carry. Multi-select is now ARMED EXPLICITLY: the parent posts arm-multi-select (keyed to the primary, so a stale arm can never arm a new draft) only from the composer flow, and the bridge refuses the toggle — shift-click behaves as a plain click — until armed. D2: target labels derive from page-controlled attributes (aria-label), so newlines could smuggle real markdown structure (fake headings) into agent-read feedback. parseTargetLabel now collapses all whitespace at the trust boundary, and the exporter collapses again (defense in depth for persisted pre-fix drafts). D3: the composer yield armed Shift only from parent-window keydown/mousemove, but window blur (focus entering the iframe) cleared it and modifier keydowns don't reach the parent from the sandbox — from the second shift-click on, the composer never yielded. The bridge pointer relay now carries the observed shiftKey (validated strict boolean) and arms/disarms yield directly. D4: a forged multi-target-removed desynced parent (promotes) from bridge (keeps original). applyTargetRemoval now ALWAYS echoes remove-target — idempotent for legit bridge-side removals, forcing convergence after forgery. D5: the 'Also applies to N more elements' block gains a leading blank line so markdown lazy continuation cannot fold it into the preceding blockquote. D6: the stray-key guard registers in capture phase (a bubbling global shortcut can no longer both fire and have its character appended), inserts at the textarea's remembered caret instead of end-of-text, and refocusToken now preserves the caret rather than jumping to the end. D7: the expanded dialog no longer carries the dead yield class/style — its data-comment-popover wrapper spans the viewport, making proximity meaningless; the dialog deliberately does not yield. Also reverts the incidental bun.lock version-catch-up churn. Tests: unarmed/stale-arm refusal, quickLabel non-arming and non-mirroring, newline-label collapse at both layers, bridge-shift-driven yield, forged removal echo + bridge-side idempotent resync, caret-preserving stray keys, blockquote separation. Signoff harness re-run green (14/14) on rules-ui-signoff.html including the arm handshake. * fix(annotate): reset multi-select arm on every new pinpoint draft (D1-R) The re-review caught that multiSelectArmed was never cleared when annotateElement started a fresh draft — only clearPendingPin reset it. So a comment-mode draft (armed) followed by a mode switch the parent doesn't mirror (quick label posts no arm) and a new pinpoint click left the stale arm live: the bridge accepted shift-clicks and pinned elements the saved annotation would never carry. Reset the flag at the top of annotateElement alongside clearMultiTargets. Regression test reproduces the exact sequence (armed draft -> new unarmed draft -> shift-click must not add a target); mutation-verified that removing only this reset fails it. * docs(annotate): correct the first-keystroke guard comment reasoning The comment claimed capture phase prevents a global shortcut from also firing; preventDefault does not stop the dispatcher (it ignores defaultPrevented by design). Restate the actual invariant: the guard is safe only because no bare printable single-key binding exists on this surface, and flag that as a constraint for future bindings. Comment only.	2026-08-10 10:19:21 -07:00
Mehdi	696bf1f24e	fix(vim): preserve plan position on refocus (#1252)	2026-08-10 10:09:55 -07:00
Michael Ramos	62c1eab119	chore: bump version to 0.26.7	2026-08-09 23:02:38 -07:00
Michael Ramos	608a8003aa	feat(annotate): hit-test pinpoint targeting for raw-HTML sessions (#1251) ... * feat(annotate): hit-test pinpoint hover instead of the semantic whitelist Pinpoint hover in raw-HTML sessions now resolves the real element under the cursor via deep elementFromPoint (piercing open shadow roots) instead of the SEMANTIC_* tag-whitelist graph, so styled div/span prototypes (chips, icon buttons, cards) are individually targetable. Scope control stays mouse-only and geometric, matching the markdown surface: the deepest element under the pointer wins; pointing at a container's padding or any area not covered by a child selects the container. - Exclusions are identity-based: html/body, script/style, and an identity Set of viewer overlay nodes (pin badges, pinpoint box/label, vim UI) so page markup cannot spoof its way out of targeting. Annotation marks are transparent (resolve to their parent). - Tiny-element promotion: a leaf under 16px on both axes climbs to the nearest ancestor at least 16px on one axis (MIN_CAPTURE_SIZE floor, not a whitelist). SVG shape primitives still promote to their <g>. - Hover no longer builds the semantic target graph per pointer frame (graphForPointerFrame removed); it is per-event hit-testing with a 2px/16ms last-position cache that the scroll reconcile invalidates. The graph survives untouched as the vim-navigation vocabulary. - Hover labels for generic containers use a cascade: aria-label -> role -> 1-2 meaningful class tokens (hash-ish tokens stripped) -> own text when <= 40 chars -> 'container'. Known semantic tags keep their names. - Click annotates exactly what the hover box shows; text-less elements post '[element: <label>]' instead of being untargetable. - Anchors stay fail-closed: data-test-id/data-cy/data-qa join the author-controlled identity attrs, and text-less elements store a '[[pn-shape]]' signature (tag + sorted classes + child count + 16px size buckets) verified whole-string on restore. * test(annotate): cover the pinpoint hit-testing contract New DOM regression tests modeled on the signoff prototype pages: chip/button targeting on styled div/span markup, container selection via uncovered-area pointing, tiny-element promotion (mocked 8x8 dot promotes to its 200x100 card), the generic-container label cascade, the text-less shape-signature anchor round trip (verify + fail-closed rejection on shape drift), data-cy identity-attr trust, and an instrumented assertion that hover hit-testing performs zero document-wide querySelectorAll calls across 30 mousemoves. Also reattaches the cached hover label element when the body was replaced (parity with the hover box), and accepts the shape-prefixed snapshot through the parent-side DTO test. * fix(annotate): drop shape-signature anchors; fail closed on text-less weak selectors Review findings on the pinpoint hit-testing branch: - D1 (blocker): structure-derived shape signatures are identical across identical siblings by construction, so a positional selector whose sibling was removed resolved the WRONG element while the signature still verified — the exact wrong-binding failure the anchor system exists to prevent. The mechanism is removed entirely: text-less elements now anchor ONLY when the element itself carries a stable-identity rung (#id or the data-* identity attrs); otherwise no anchor ships and the pin simply does not restore (same as before the branch, fail closed). A wrong-binding anchor is worse than no anchor. D2-D4 (in-band prefix discriminator, uncapped class-list component, viewport-dependent size bucket) die with it. - D5: the pinpoint click handler gated pin-badge ownership on a [data-plannotator-pin-badge] selector match, contradicting the identity-only overlay rule. It now gates on the overlay identity set: a page element spoofing the attribute hovers and annotates like any other element, while real badges keep owning their clicks. - D6: class-token hover labels now route through the 40-char label cap (two long tokens could reach 73 chars), and the '[element: ...]' posted text goes through capSelectionText for consistency. - D7: the pointer-hit cache invalidation moved to the top of the rAF reconcile, ahead of its guards, so every reconcile pass (scroll, resize, and body ResizeObserver — i.e. layout-changing mutations) clears it; the 2px/16ms TTL bounds any remaining staleness to one frame. Tests updated to the new contract, with regressions for D1 (identical text-less siblings ship no anchor; crafted positional anchors with empty snapshots resolve nothing after sibling removal), D5 (spoofed badge is annotatable, real badge posts mark-click), and the D6 label cap.	2026-08-09 23:01:31 -07:00
Michael Ramos	d579ff8db2	chore: bump version to 0.26.6	2026-08-09 21:07:30 -07:00
Michael Ramos	9c40ffadcc	chore: bump version to 0.26.5	2026-08-09 17:49:14 -07:00
Michael Ramos	2d61b76c44	fix: address the three pre-release sweep findings (#1246) ... Three real, new-in-range bugs from the free-hunt regression sweep: - The pinpoint anchor builder ran a document-wide uniqueness query per ancestor against a growing selector with no depth cap, so one click on a deeply wrapped document froze the tab synchronously (measured 58s at depth 800). The walk now abandons the anchor past 40 ancestors (fail closed: text-search restoration takes over), bounding the work to interactive time. Regression test uses two identical depth-60 chains so uniqueness cannot short-circuit before the cap. - The 10k selection-text cap sliced UTF-16 code units and could split a surrogate pair at the boundary, silently corrupting the annotation tail into U+FFFD downstream. Both sides of the bridge now back the cut off one unit when it would land mid-pair. - The old-git sparse fallback matched git's "unknown option" error literally, which localized git builds translate, so non-English users on old git hit the exact hard failure #1239 was written to fix. All three installers now pin LC_ALL=C around the probe clone (saved and restored; kept single-line in install.ps1 for the scoped-call scanner, whose expectation is updated to the new prefix).	2026-08-09 17:48:05 -07:00
Michael Ramos	9b14b19a7b	chore: fold in pre-release QA findings (print overlays, doc alignment) (#1245) ... * chore: fold in pre-release QA findings (print overlays, doc alignment) From the 25-item QA sweep over the v0.26.4..main range (all items passed; these were the three real minors worth folding into the release): - The raw-HTML iframe's injected CSS had no print rules, so pin badges (and, in a narrow window, the pinpoint outline box) printed into hard copies of annotated HTML pages. The overlay elements now carry an explicit @media print hide inside the iframe document, where the outer print.css cannot reach. Inline annotation marks stay printable on purpose, matching markdown documents. - AGENTS.md/CLAUDE.md now state that PLANNOTATOR_ANNOTATE_HISTORY also gates the durable submitted-feedback records from #1237, and the Annotation interface listing includes the htmlAnchor field from #1243. - apps/codex/README.md aligns with the #1241 top-level wording (Windows Codex hooks are experimental with printed manual steps, not disabled). - Marketing docs: annotate page documents the pinpoint-first default and minimal-first chrome for raw-HTML sessions; installation page notes the old-git plain-clone fallback from #1239. * chore: drop deprecated marketing-docs edits (canonical docs are Mintlify)	2026-08-09 17:19:50 -07:00
Michael Ramos	9ab5392bad	feat(annotate): pinpoint-first raw-HTML sessions with element anchors and a minimal-first render (#1243) ... * feat(annotate): rebuild HTML pinpoint mode with element anchors, pin badges, and a minimal-first render Raw-HTML annotate sessions now open as just the page: first-ever run hides all annotation chrome (toolstrip, tongue tabs, action cluster, sidebar) and the user's last chrome state is restored on every later session via the plannotator-html-chrome cookie. Pinpoint becomes the default input method on the HTML surface, persisted separately from the markdown preference. Pinpoint mechanics are rebuilt in the sandbox bridge, modeled on app-notes-extension and agentation: a fixed-position outline box + label replaces class writes on author elements, hover is identity-gated and re-hit-tested on scroll/resize via a rAF-coalesced reconcile pass, clicks pin the element (crosshair cursor, pinned outline while composing) and go straight to the comment popover. Each pin serializes a verified-unique CSS selector anchor (id > identity attrs > meaningful classes > nth-of-type path) with a text-snapshot fingerprint; restoration is anchor-first with fail-closed validation and falls back to document-wide text search. Element pins that cannot take an inline mark (SVG etc.) get numbered pin badges that track their element and re-acquire it after re-renders. All DOM inspection stays inside the bridge; only validated, size-capped DTOs cross postMessage. Annotation model is extended additively (htmlAnchor), so exported feedback, drafts, and share links keep working. * fix(annotate): harden HTML element anchors per adversarial review Four fixes from the pre-merge adversarial review of the pinpoint round: - Stable identity is now only the element's own #id or data-* rung, re-derived from the resolved element and compared whole instead of parsed out of the selector string. Behavioral attributes (role, href, aria-label, name, alt) no longer exempt an anchor from the text check, so a regenerated page can no longer bind an annotation to the wrong element. A weak anchor with a missing or empty snapshot is rejected, not exempted. - Restoration order: the document-wide text search now runs before the pin-badge fallback, so text that moved elsewhere in a regenerated page is followed rather than badging the stale container. Resolved SVG anchors still pin directly. - Selection text is capped at 10k chars on both sides of the bridge (truncated, not rejected); one pinpoint click on a huge pre/table no longer ships an unbounded page-controlled string into drafts, feedback, or share URLs. - buildAnchorSelector returns null instead of a terminal fallback path already proven non-unique; no anchor beats a known-ambiguous one. Also moves the HTML chrome restore out of the one-shot mount effect into a surface-transition effect, so a linked .html doc opened from a markdown session gets the minimal-first chrome and persists its state, and a markdown surface's sidebar use never leaks into the HTML cookie. * fix(annotate): skip the chrome save in the restore commit itself The reviewer's residual finding: htmlChromeRestoredRef flips synchronously inside the restore effect, but the restored state lands a commit later, so the save effect's run in the restore commit wrote pre-restore values over the remembered state (self-corrected next flush, but a page ending between the two writes would freeze the inverted value). The restore now arms a skip-one flag the writer consumes, so the stale write never happens. A follow-up save fires from the changed deps when the restore changed anything; when it changed nothing the cookie already holds those values. Regression test instruments every chrome cookie write on a returning-user mount and asserts none ever differs from the remembered state (mutation tested: removing the skip fails it).	2026-08-09 16:17:06 -07:00
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	e24bd8464f	fix(annotate): persist submitted feedback before deleting the draft (#678) (#1237) ... * fix(annotate): persist submitted feedback before deleting the draft (#678) * fix(annotate): scope durable submit records to single local files Adversarial verification found the durable record had no mode gate: an annotate-last or URL session, which was completely stateless before, would persist submitted feedback quoting the agent's message or the fetched page under history/, widening the documented annotateHistory contract without a docs change. The record now shares the exact eligibility gate the version history uses (mode annotate, non-URL path), so previously-stateless modes stay stateless. Also makes persistSubmittedDecision defensive about body types: /api/feedback does no validation (unlike /api/approve), and a non-string feedback previously flowed through settle() untouched with a 200; the new .trim() guard turned that into a thrown 500 after the decision had already settled. Malformed values now degrade to the exact legacy behavior (settle, delete draft, 200) instead of throwing. Both changes mirrored in the Pi server, with regression tests in both runtimes: stateless modes write no record, and a malformed feedback body returns 200 with the draft deleted and nothing persisted.	2026-08-09 16:16:57 -07:00
Michael Ramos	d5ae439f7a	chore: bump version to 0.26.4	2026-08-07 15:13:27 -07:00
Michael Ramos	3dd80f37f0	fix(skills): remove the human-only menu treatment and its hover jitter (#1236)	2026-08-07 15:12:35 -07:00
Michael Ramos	c760fc522b	chore: bump version to 0.26.3	2026-08-07 14:32:16 -07:00
Michael Ramos	ffd49080ee	fix(skills): harden skill references before first release (#1235)	2026-08-07 14:22:58 -07:00
Michael Ramos	2edad89dfe	fix(editor): stop the skill-content priming effect from re-render looping (#1234)	2026-08-07 14:15:35 -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	57b0358265	test(review): actually restore @pierre/diffs after the lifecycle mocks (#1230) ... The afterAll restore in AllFilesCodeView.lifecycle.test.tsx captured the module namespace object with `await import(...)`. A namespace is a live view of the module record and bun's `mock.module` rewrites that record in place, so the captured handle became the stub the moment the mocks were installed and handing it back re-installed the stubs as themselves. The stubs therefore stayed live for the rest of the process. Whenever bun's file walk put this file before discardRestoreRender.test.tsx, that test's `processFile(...)` returned null and it failed on its first assertion. Spread-snapshot the exports at capture time, the idiom configure.test.ts already documents, and add an in-file guard so a future revert to the bare import fails here instead of somewhere downstream. Also correct the claim that the relative-path sibling mocks cannot escape this file: `mock.module` resolves them to absolute module paths and they leak like any other.	2026-08-06 20:22:17 -07:00
Ivan Giuliani	b075677033	fix(review): show the full branch name when hovering the worktree switcher (#1223) ... On a long branch name you can't tell which branch you're on from the worktree switcher button because it truncates its label to keep the toolbar compact. The trigger already had a native title, but it never showed the branch. On the current checkout it just said "Main repository", while on a worktree it showed the path rather than the branch, and the dropdown rows had no title at all. The full name is now always one hover away because we put the full branch label into the button's title, add the worktree path when one is active, and give each dropdown row a title for both its name and its path.	2026-08-06 18:39:06 -07:00
Michael Ramos	b69742c3bf	feat: add PLANNOTATOR_URL_HOST display-only override for advertised URLs (#1225) ... * feat: add PLANNOTATOR_URL_HOST display-only override for advertised URLs Remote mode binds 0.0.0.0 but every advertised URL hardcoded http://localhost:<port>, so a session opened from another device (e.g. a phone on the same tailnet) got an unopenable link (#657). - resolveUrlHost() in packages/shared/config.ts: PLANNOTATOR_URL_HOST env var over config.json urlHost, validated host-only (bare hostname, IPv4, bracketed IPv6); invalid values warn once and fall back to localhost. - buildAdvertisedUrl(port) in packages/server/remote.ts and its Pi mirror in apps/pi-extension/server/network.ts; all 7 construction sites use it. - Strictly display-only: binding stays governed by PLANNOTATOR_REMOTE, and agent-review jobs get a pinned http://127.0.0.1:<port> API URL. - Remote-ready copy says "open on your device" when the host is overridden; local sessions with an override warn it is unreachable. - Tests for validation, precedence, and URL composition in both runtimes; docs in CLAUDE.md and the marketing site. * fix(review): ignore urlHost in local sessions, harden warning output Review follow-ups on #1225: - Local (loopback-bound) sessions no longer honor the advertised-host override: honoring it auto-opened http://<host>:<port> against a server nothing was listening on, openBrowser still reported success, and the agent blocked on waitForDecision. Local sessions now advertise and open localhost, warning once that PLANNOTATOR_REMOTE=1 is required. - The invalid-host warning JSON-encodes the echoed value so an embedded newline cannot forge extra stderr lines (hosts surface session-ready lines as clickable links); warn-once is now per value. - Docs: local-session behavior reworded, the empty-env-suppresses-config semantic documented, secure-context note generalized.	2026-08-06 18:29:11 -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	bbae458e5a	chore: bump version to 0.26.2	2026-08-06 00:50:56 -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
Michael Ramos	c08b188812	perf(ui): single Shiki highlighter, palette-matched code blocks, drop highlight.js (#1218) ... * perf(build): stub out the dead Oniguruma WASM in every bundle @pierre/diffs picks its Shiki engine with a runtime ternary: engine: preferredHighlighter === "shiki-wasm" ? createOnigurumaEngine(import("shiki/wasm")) : createJavaScriptRegexEngine() Plannotator pins `preferredHighlighter: 'shiki-js'` (and Pierre's own default is 'shiki-js'), so the Oniguruma branch never executes. Because the choice is a runtime ternary, bundlers keep the `import("shiki/wasm")` edge anyway and inline `@shikijs/engine-oniguruma/wasm-inlined`, a ~622 KB base64 blob, into the single-file HTML builds. The review app paid for it twice: once on the main thread (via `highlighter/shared_highlighter.js`) and once inside the `?worker&inline` Pierre worker. Alias `shiki/wasm` to a stub that throws if it is ever reached. Wired via `resolve.alias` rather than a plugin because `resolve.alias` is shared with Vite's worker build and `plugins` are not. Highlighting output is unchanged: the JS regex engine was already the one doing the work. Opting back into 'shiki-wasm' now fails loudly instead of silently costing every user a megabyte of dead bytes. apps/review/dist/index.html 19,424,646 -> 18,180,545 (-1,244,101 raw / -463,348 gzip) apps/hook/dist/index.html 23,032,467 -> 22,410,416 (-622,051 raw / -233,485 gzip) * perf(ui): consolidate code highlighting onto Shiki, drop highlight.js The app shipped two highlighters. Shiki already tokenised the code-review diff pane (via @pierre/diffs, JavaScript regex engine); highlight.js separately coloured markdown fences and review suggestion snippets at ~982 KB minified for a full build of ~190 grammars. That second highlighter is now gone. Every call site moves onto `packages/ui/utils/codeHighlight.ts`, a thin wrapper over Pierre's SHARED Shiki instance: CodeBlock, Viewer, PlanCleanDiffView markdown fences InlineMarkdown code-file hover preview HighlightedCode review suggestion snippets Reusing Pierre's instance rather than standing up a second fine-grained one is deliberate. Pierre imports Shiki's full bundle, so every grammar and theme is ALREADY inlined in the single-file builds: a separate highlighter with a curated language list would have duplicated a subset of bytes that are already there. Sharing costs nothing, gives every language Shiki bundles instead of a shortlist, and — the point of the change — guarantees fences resolve the exact same theme the diff pane resolves. Theming. `SHIKI_THEME_MAP` / `resolveSyntaxTheme` move from `packages/review-editor/hooks/usePierreTheme.ts` to `packages/ui/utils/syntaxTheme.ts`; usePierreTheme re-exports them, so the review editor's imports are unchanged. `useFenceTheme()` feeds the components and re-highlights on palette or mode change. Code blocks now follow the active palette across all ~52 themes in both light and dark, instead of always rendering github-dark and relying on hand-written `.hljs-*` override stacks to stay legible. Those stacks are deleted: `packages/editor/index.css`'s light-mode token palette, and `colorblind.css`'s hand-tuned tokens which existed to APPROXIMATE @pierre/theme's protanopia-deuteranopia themes that are now simply used. Behaviour held fixed: - Language-less fences stay plain text (#1212). No auto-detection anywhere, including the hover preview, which previously called `hljs.highlightAuto`. `HighlightedCode` derives its language from the caller's file path; an unknown extension renders plain. - `applyHighlight(el, ...)` keeps the imperative `hljs.highlightElement` DOM contract the annotation layer reaches into, and writes plain text at final size first so async highlighting causes no layout shift. Already-attached grammars highlight synchronously — no flicker on cached highlights. - It also verifies the rendered text is byte-identical to the source and falls back to plain otherwise, because annotations address code blocks by text offset. - `@plannotator/ui`'s public API is unchanged: the highlighter is a module-level default like the package's other seams, no new props. The `hljs` class on fenced `<code>` becomes `pn-code` (it is a structural hook for blockTargeting, vim navigation and print.css, and it named a library we no longer ship). `language-*` stays. apps/review/dist/index.html 18,180,545 -> 17,270,889 (-909,656 raw / -291,921 gzip) apps/hook/dist/index.html 22,410,416 -> 21,704,434 (-705,982 raw / -238,096 gzip) Verified the diff pane is untouched: the rendered Pierre shadow-DOM markup is byte-for-byte identical between an origin/main build and this one (SHA-256 aa1ee88a…). * fix(ui): strip stray NUL bytes from the code-highlight source Two U+0000 bytes slipped into comments in the previous commit, which made git treat the file as binary. Replaced with spaces; no behaviour change. * fix(ui): keep code-block annotation marks across highlight swaps Fenced code is annotated by hand: one `<mark data-bind-id>` inside the `<code>` element, which `applyHighlight` also owns. Every highlight swap (palette change, dark/light toggle, or the first async grammar attach after load) replaces that element's children, so the mark was silently wiped and nothing put it back. Annotation state, the sidebar panel and exports were unaffected; the loss was purely visual, and deterministic. `applyHighlight` now publishes every write through `onCodeHighlightSwap`, synchronously, immediately after it. `Viewer` subscribes and re-paints the fence's mark, so a swapped block ends up with BOTH the new theme's tokens and its annotation. The shared painter (`paintCodeBlockMark`) moves the token spans into the mark instead of flattening them to text, so creating an annotation no longer costs a block its colours either. Being driven by the swap also fixes the cousin race by ordering rather than timing: share/draft restore runs on a timer after load, and on a slow machine the first async swap could land after it and wipe the restored marks per block. A restore that painted before the swap is now re-established in the same task the swap ran in, and one that runs after finds the mark already there. Removal tombstones the id before re-highlighting, because the host drops the annotation from state a tick later — without it the swap listener would paint the just-removed annotation back in, and a fence carrying a second annotation would end up bare. Also closes the named gap in the WASM coverage: entry-assets only grepped source, so a future @pierre/diffs bump could reintroduce the inlined blob through a different import specifier unnoticed. It now greps the built `apps/{review,hook}/dist/index.html` for the base64 WASM magic, skipping on an unbuilt checkout and running for real in the CI job that builds the bundles.	2026-08-05 21:54:40 -07:00
Michael Ramos	2d65c65596	feat(ui): pair a light theme and a dark theme, switched by mode (#1217) ... * feat(ui): pair a light theme and a dark theme, switched by mode ThemeProvider stored one palette plus a mode, so picking a dark-only palette pinned the mode and greyed out the Light/System buttons. Store a pair instead: { mode, light, dark }, resolved as pair[preferredMode], so System flips between the two choices as the OS scheme changes. The Settings Theme tab now assigns one half at a time. A Light/Dark switch decides which half the grid is filling, the grid lists only the palettes that can render that half (from the registry's modeSupport), and a summary line names both halves with each side clickable. Every mode button is permanently enabled: a dark-only palette simply never occupies the light slot, so no mode coercion is left to do. The pair round-trips through the SETTINGS registry to the `theme` key in ~/.plannotator/config.json the way diffOptions does. A user upgrading seeds both halves from their stored single palette, and the legacy plannotator-color-theme key keeps tracking the active palette so a downgrade never lands on an unstyled first frame. Addresses part 1 of #1211. * fix(ui): make the theme pair seed local, and keep the legacy API non-destructive Review of #1217 found a data-loss path and three published-API regressions. Seeding: ThemeProvider handed its resolved pair to the config store through set(), which queues a debounced POST. configStore.init() applies the server config but never cancelled that queued write, so a single cookie-less visit (fresh profile, incognito, cleared cookies) flushed a default pair to ~/.plannotator/config.json AFTER the real one had arrived, and the next session restored those defaults over the user's cookies. The provider now uses a new configStore.seed(): memory plus cookie, never the server, and never over a value init() already applied. init() additionally retracts queued writes for the leaves the server just spoke for, which closes the same race for every server-synced setting rather than this one key. Deprecated APIs: isThemeModeAvailable() and normalizeThemeMode() are back as one-line wrappers with @deprecated notes, since packages/ui exports utils/*. setColorTheme: assigns exactly one half and nothing else. A both-mode palette goes to the half on screen instead of clobbering both; a mode-restricted one goes to its half without yanking a System user to an explicit mode (render time already resolves that). It persists through configStore.setLocal(), so it stays cookie-only as it was before the pair, unless a host installed its own serverSync transport. storageKey / colorThemeStorageKey are honored on the read path, so a host's stored pre-pair preference is migrated rather than discarded. The two halves have no pre-pair equivalent and stay on fixed keys, documented on the props. Tests: a fresh-mount case that pins zero POSTs (the previous helper pre-seeded cookies, which is why this was invisible), a case that pins a real choice still reaching config.json, direct setColorTheme cases for all three semantics, a host-storage-keys migration case, and configStore seed/retract unit tests. All of them fail against the code they replace.	2026-08-05 21:51:55 -07:00
Zeke Sikelianos	c072a16ca6	fix(ui): render language-less code blocks as plain text (#1212) ... Bare fenced code blocks (no language tag) were passed to hljs.highlightElement() unconditionally, so highlight.js auto-detected a language and colored ordinary words. Skip highlighting when the block has no language so it renders as plain monospaced text. Fixes #1210	2026-08-05 19:45:10 -07:00
Michael Ramos	50a54c872b	chore: bump version to 0.26.1	2026-08-05 11:39:02 -07:00
Michael Ramos	b1745683fd	fix(review): fall back across GitButler JSON flag syntaxes (but 0.22.0) (#1216) ... GitButler 0.22.0 removed the global --format flag in favor of --json (gitbutlerapp/gitbutler#15026), so `but --format json status` now dies with clap's unexpected-argument error and GitButler review sessions fail to start. 0.21.x accepts only --format json (gitbutlerapp/gitbutler#14061), so neither spelling works everywhere. Keep --format json as the primary invocation and, only when it fails with clap's narrow unexpected-argument rejection for the exact flag we passed, retry once with the other spelling. Real status failures never retry and keep failing loudly per the module's contract-error philosophy. The accepted spelling is remembered per runtime so 0.22.0 installs pay the failed probe once. Error strings now name the syntax actually used. Closes #1215	2026-08-05 11:37:07 -07:00
Michael Ramos	f555168deb	chore: bump version to 0.26.0	2026-08-05 09:19:51 -07:00
Michael Ramos	5fbff4bff3	fix(review): make the Edit Mode opt-in an explicit switch, not a primary CTA (#1213) ... The announcement footer's primary Turn it on button read as a generic continue and invited blind clicks: opting into an experimental feature was one reflexive Enter away. The decision is now an explicit Enable Edit Mode switch (defaulting off, with the shared TextShimmer label treatment) beside a single neutral Done button that applies whatever the switch says. Pressing Done untouched is a plain dismissal, and the focused default action is asserted consent-neutral in tests. Announcement cookie version bumped so the revision re-shows (pre-release, no users have the old cookie).	2026-08-05 09:07:23 -07:00
Michael Ramos	a4816e7208	feat(review): announce Edit Mode with an opt-in welcome dialog (#1207) ... * feat(review): announce Edit Mode with an opt-in welcome dialog One-time cookie-gated announcement for the experimental Edit Code to Suggest feature, slotted last in the first-run dialog chain (guide intro, look-and-feel, review setup, edit mode) so the chain dialogs never stack. Turn it on persists the real editSuggestions setting; Keep it off just marks the announcement seen. The demo panel carries a video slot that the single-file build inlines as a data URI; until the recording lands it renders a static placeholder behind one constant. * feat(review): bundle the Edit Mode demo recording into the announcement Wires the quality-verified screen capture (VP9 webm, ~563KB, 1100x700, 18.8s) plus its poster still through the editModeDemoMedia swap point. The single-file build inlines both as data URIs (verified in the built bundle). The video slot autoplays muted, loops, plays inline, and keeps the recording's 11:7 aspect so nothing is cropped. The static placeholder stays as a test seam and safety net, with its coming-soon caption removed.	2026-08-05 08:50:56 -07:00
Michael Ramos	7aff70281f	fix(review): repaint pristine diff immediately on edit-mode Discard (#1209) ... writeRestore's combined item write leaves upstream's highlighted render cache serving the stale edited content; clear the live instance's render cache and rerender so the pristine diff paints immediately (plaintext first, then the normal async highlight). Covers Discard, Suggest, finishIfEditing, and the deferred external-teardown restore. Adds a DOM-gated integration test driving the real @pierre/diffs CodeView + EditProvider + worker pool through startEdit -> real Editor.applyEdits -> cancelEdit, asserting the pristine content is back in the shadow DOM within a bounded settle.	2026-08-05 08:50:53 -07:00
Michael Ramos	3d435184dc	fix(review): keep large-diff memory bound when the object-size probe fails (#1205) ... A failed `cat-file --batch-check` used to map every changed object to infinity, replacing the ENTIRE review diff with `Binary files ... differ` stubs and no visible error, and silently degrading the staleness fingerprint. The memory bound is now probe-independent: every rendered diff carries `core.bigFileThreshold=<MAX_REVIEW_FILE_CONTENT_BYTES>` injected through `GIT_CONFIG_*` environment variables (never `-c` argv flags, so argv stays byte-identical), making git itself stub oversized blobs. On probe failure blob sizes read as unknown-but-bounded and files render normally; the stat-based exclusion door for oversized working-tree files (which git's threshold does not cover) never depended on the probe and keeps working. Per-object doors (missing / unparseable size) stay conservative when the probe ran. The probe itself gains timeoutMs + interaction:"forbid" so a hung git cannot stall the review server. Three existing mocks that returned non-batch-check output (and passed only because of the all-infinity bug) now return well-formed batch-check lines.	2026-08-04 22:21:56 -07:00
Michael Ramos	6b8723cc57	test(review): repair guide virtualization mock and run its tests in CI (#1200) ... The AllFilesCodeView lifecycle test stubs '@pierre/diffs/react' with mock.module, which replaces the whole specifier. #1193 added an EditProvider import to AllFilesCodeView, but the stub was never extended, so the file throws at import when run on its own. The three DOM tests #1158 added for Guided Review virtualization were also never registered in the DOM_TESTS=1 CI step, so nothing enforced the 8-viewer mount cap and nothing surfaced the broken stub.	2026-08-04 20:44:28 -07:00
Michael Ramos	7682628db7	feat(install): Codex opt-out and credential-free attestation verification (#1197) ... * feat(install): Codex opt-out and credential-free attestation verification Implements both asks from #1178 (reported and designed by @astradevkin): - Per-agent installer opt-outs: --skip-codex / --skip-gemini / --skip-kiro flags, PLANNOTATOR_SKIP_{CODEX,GEMINI,KIRO}_INSTALL env vars, and config.json skipInstall.{codex,gemini,kiro} keys, with flag > env > config precedence mirroring verifyAttestation. Detected-but-skipped is reported as its own honest state, never conflated with not-detected, and a skipped agent's home is neither written nor cleaned up. - Credential-free provenance verification: when --verify-attestation is active, the Sigstore bundle is fetched from GitHub's public attestations endpoint (single unauthenticated attempt, no retry) and verified via gh attestation verify --bundle with the same --repo/--source-ref/--signer-workflow constraints; gh's authenticated fetch remains the fallback. TUF trust-root failures are reported as connectivity, distinct from real provenance failures; every path stays fail-closed. Zero behavior change for users who do not opt in: the default install path is unchanged (verified by sandbox-HOME parity runs against main). * fix(install): address #1197 review round (H1 retry, M2-M7, lows) - H1: a failed gh --bundle invocation now retries once through the exact authenticated path before any classification, so an older gh (unknown flag) or a corrupt bundle never reports as a provenance failure. Pinned by a functional stub-gh test; a real failure still fails again on the retry and aborts. - M2: the sh config layer extracts the skipInstall object (awk, character indexed) before matching per-agent keys and honors explicit false as a veto; cmd now parses the real JSON via PowerShell like ps1. Functional tests cover the foreign-key collision and explicit-true cases. - M3: sh names the real cause when the bundle path cannot run (no JSON extractor vs fetch vs extraction failure) and gains python3 and jq fallback extractors; docs state the dependency. - M4: ps1/cmd gate the existing-integration note on plannotator content in hooks.json and word the skip state as what those platforms actually do (manual instructions suppressed). - M5: sh bundle lives inside a private mktemp -d, one rm -rf on every exit, and a mktemp failure degrades to the fallback instead of aborting. - M6: README, verifying-your-install, environment-variables, and installation docs updated for the credential-free path and skip flags. - M7: ps1/cmd extraction replaced with a byte-exact string scanner (no ConvertFrom/ConvertTo round trip, immune to DateTime coercion), with PowerShell-driven unit tests over the captured real attestations response plus synthetic DateTime and brace-in-string controls. - Lows: fail-closed abort pinned by a functional test; TUF beats auth in cmd classification (matches sh/ps1); skipped-state output mentions the shared ~/.agents/skills; Gemini summary is skip-aware and gains an honest not-detected state; --skip-opencode do-not-write switch added (flag, env var, config key) across all three scripts. * fix(install): review round three (EncodedCommand fetcher, mutation-proof fail-closed test, lows) - R1: install.cmd's attestation fetcher no longer touches disk. The %RANDOM%-named %TEMP% .ps1 (predictable-path code execution, the M5 class escalated) is replaced by powershell -NoProfile -EncodedCommand with a base64(UTF-16LE) payload defined next to its full REM PS: plaintext; a test decodes the blob and asserts byte equality with the documented lines plus the security-relevant shape (env-var inputs, ordinal scan, no JSON round trip, distinct exit codes). Inputs still travel via env vars. Verified end to end under pwsh: the decoded blob fetched the real attestations response, wrote 2 bundles, gh verified the real v0.25.1 binary credential-free (exit 0) and rejected a wrong binary (exit 1). - R2: the fail-closed test now asserts the output ENDS with 'Refusing to install.' - mutation-verified: with the verify-failure exit 1 deleted the mutant still exits 1 via an incidental mv failure, but the trailing mv error breaks the endsWith and the test fails; restored, it passes. - Lows: CI guard test fails loudly when process.env.CI is set and no pwsh/powershell is on PATH (scanner coverage cannot silently vanish); scanner IndexOf calls are ordinal in ps1 and the encoded cmd variant; the awk skipInstall extraction requires optional-whitespace-then-colon- then-brace after the key (string values can no longer anchor it, with non-token occurrences skipped, unit-checked against escaped-embedded payloads); install.cmd comments warn that the fallback-reason literals inside parenthesized blocks must stay parenthesis-free.	2026-08-04 13:17:27 -07:00
Rian Dolphin	506dabe03a	feat(review): add Cmd/Ctrl+Shift+Y shortcut to copy feedback (#1155) ... * feat(review): add Cmd/Ctrl+Shift+Y shortcut to copy feedback * fix(review): address review on the copy-feedback shortcut - Add the shortcut to the in-app Settings keyboard panel, which renders a hard-coded list rather than the registry. - Toast on copy success/failure so agent-backed sessions get confirmation; the inline affordance only renders in the standalone header. - Exclude alt so Mod+Alt+Shift+Y and AltGr layouts don't fire it. - Use the shared isTypingTarget guard, which also covers contentEditable.	2026-08-04 11:32:31 -07:00
Michael Ramos	156761a3fa	feat(review): author suggestions by editing code in place (experimental, flag-gated) (#1193) ... * feat(ui): add experimental editSuggestions setting (cookie-only, default off) Registers the flag in the settings registry and surfaces it under a new Experimental section of the review Display tab. Not synced to server config while experimental; when off, no edit UI renders anywhere. * feat(review): edit-to-suggestion core (adapter wall, derivation, session controller) pierreEditAdapter.ts is the single module allowed to reference @pierre/diffs/edit: the editor chunk loads via dynamic import on first use, types are derived structurally, and the EditProvider factory declines attaches until the module is ready (CodeView retries). deriveSuggestions.ts diffs the session's final contents against the pre-edit new-side content with the diff package and emits one minimal hunk per contiguous changed region, expanding pure inserts/deletes to include an unchanged anchor line so originalCode and suggestedCode stay non-empty. CRLF input is normalized. cloneDiff.ts deep-clones FileDiffMetadata before a session starts; Pierre's editor mutates it in place (additionLines, hunks, editSessionDirty), and the clone is what gets republished when the session ends. useEditSession.ts owns the lifecycle: one file at a time, full-content hydration before entry (partial diffs throw in applyDocumentChange), suggestion creation on completion, and session end as ONE combined item write (edit off plus restored pristine fileDiff with a fresh cacheKey), which is upstream's documented commit pattern and avoids racing CodeView's teardown re-render. persistState is deliberately unused (upstream bug, open PR #1048). Existing annotations are projected to editor severity markers best-effort via onAttach. * feat(review): wire edit-to-suggestion into the plain all-files view (flag-gated) The plain all-files dock panel is the only surface that opts in; Guided Review deliberately stays off because its viewport manager evicts CodeViews beyond ~8 mounted, which would destroy an active editor session. When the flag is off the surface renders byte-identical to before: no EditProvider, no edit props, no header buttons. FileHeader gains the Edit entry button (disabled with a tooltip when content cannot be fetched) and the in-session Editing badge with Suggest/Discard controls. Slot portals republish before React commits state, so header rendering reads the session refs, not state. AllFilesCodeView routes every collapse path through finishIfEditing so a collapse ends the session on our one code path, blocks the annotation toolbar and augmentation applies for the file being edited, and drops session state on fileSetKey remounts (diff switches are user-initiated; in-progress edits are discarded, documented v1 behavior). App.tsx converts derived hunks into ordinary suggestion annotations (type comment with suggestedCode/originalCode, side new, anchored to new-side line numbers), the same shape SuggestionModal produces, so rendering, sidebar, drafts, and feedback export work unchanged. The browser never writes files; the agent applies suggestions. * test(review): cover edit-to-suggestion derivation, clone invariant, adapter wall, and flag-off UI deriveSuggestions: no-op edit, single and multi region edits, whole-line add and delete with anchor expansion, insertion at file start, file emptied, CRLF vs LF, trailing newline. cloneDiff: pristine clone stays byte-identical after the live object is mutated the way Pierre's edit session mutates it. adapterWall: only pierreEditAdapter.ts may reference @pierre/diffs/edit, and only via import type or dynamic import. FileHeader DOM tests (DOM_TESTS=1): zero edit UI without the feature, entry button, disabled tooltip, and the Suggest/Discard session controls; registered in test.yml's DOM step. * fix(review): resolve anchor collisions in suggestion derivation deriveSuggestionHunks could emit overlapping hunks when adjacent regions claimed the same anchor line (a file-start region expanding forward into a line the next region's backward expansion also claimed). An agent applying the suggestions against original line numbers would silently drop one or duplicate code. Regions are now emitted left to right with collision-aware anchoring: backward expansion is preferred but only when the preceding line is not already claimed by the previous hunk; otherwise the region anchors forward (always an unchanged line, and later regions see it as claimed); when both directions are unavailable (previous hunk adjacent and region runs to end of file) the region merges into the previous hunk as one spanning suggestion. A defensive fold enforces the non-overlap invariant (lineStart > previous lineEnd) as a safety net. Tests: unit cases for both reproduced collision shapes plus adjacent regions at file start and end, and a 5000-case seeded fuzz asserting zero overlaps, exact originalCode anchors, and full reconstruction of the edited text under a sequential offset-tracking applier. A 20,000-case run of the same generator reports zero overlaps, zero anchor mismatches, and zero round-trip failures. * feat(review): export originalCode as a Replaces block in feedback Suggestions carried originalCode internally but the export never emitted it, so the applying agent had no way to validate an anchor before applying. Each suggestion now exports a fenced Replaces block (the exact current lines being swapped out) ahead of the existing Suggested code block, for both line and file scoped annotations. SuggestionModal-authored and edit-session-derived suggestions flow through the same single format. A deletion-only suggestion (no suggestedCode) still emits its Replaces block so the anchor stays verifiable. Tests assert the block pairing per annotation, the deletion-only case, and that annotations without originalCode are unchanged. * fix(review): never silently discard a dirty edit session on file-set change fileSetKey changes on sort-order and collapse-default flips as well as diff switches and refreshes, and the remount tears the editor down without a completion callback. A dirty session was dropped silently there, destroying in-progress work, while the file-switch path prompted. The session now keeps the FileContents delivered with each change event (its contents getter is lazy and stays readable after editor cleanup), and handleFileSetChange recovers the last-known document text, derives the suggestions, and prompts to keep them, matching the dirty file-switch pattern. The call site is a post-commit effect, so the synchronous confirm is safe. Declining discards; a clean session or a no-op edit still drops without a prompt. New DOM-gated test (registered in test.yml's DOM step) covers the confirm, decline, clean, and no-op paths through the real hook, plus non-DOM units for the pure recovery helper. * fix(review): give editor markers real range width so they render Single-line annotation markers were projected as collapsed ranges (start and end both at character 0 of the same line); upstream's overlay renderer skips zero-width blocks, so markers never painted. Marker ranges now end at character 0 of the line after the last annotated line (exclusive-end LSP form), covering every annotated line in full; the editor's TextDocument clamps a past-the-end position back to end-of-document. Verified via CDP against a live review server: the severity squiggle renders across the annotated line during an active edit session and the hover popover shows the annotation source and message. Also corrects overstating comments: the flag-off claim that the editor module is never imported (the single-file build inlines dynamic imports, so the namespace exists at page load but stays inert) and the createEditor decline-and-retry framing (the React wrapper throws on an undefined return; the controller awaits the chunk before any item enters edit mode, and onAttach delivers once per editor rather than re-firing across virtualization re-attaches). * polish: drop suggestion card left accent, move Edit to far right of file header Suggestion cards keep the colored SUGGESTION header as their sole identity marker; the green left border on the card body is removed for all suggestion cards (inline and sidebar, including modal-authored ones). The Edit entry button moves to the far right of the file header action row, after the Sem badge and adjacent to the file actions dropdown, so the experimental affordance stays out of the everyday Viewed/Add/Comment cluster. Responsive isVeryTight behavior is unchanged (icon-only at narrow widths). * prototype: edit-session HUD strip below the file header While an edit session is active, a slim strip renders directly below the file header (inside the file card, above the content) carrying the session controls and state: Suggest, Discard, a debounced net change count (for example 3 changes), and the experimental label. The header no longer renders in-session controls; the Edit entry button still lives there when no session is active. The HUD rides Pierre's memoized custom-header slot portal, so the change count is delivered through a small external store (useSyncExternalStore) that useEditSession updates on a 250ms debounce per change event; slot height re-measures via the version-bumped updateItem that session start and end already perform. Prototype for design review; revert this commit to drop the HUD. * fix(review): pierce shadow DOM in file-nav keyboard guards Window-level keydown handlers in SectionsPanel and FileTree guarded only against e.target being an input or textarea. Events from inside the Pierre editor's shadow DOM retarget to the host element, so Home/End/arrow keys typed during an edit session fell through to file navigation and switched the visible file mid-edit. Use composedPath()[0] and isContentEditable, matching the guard AllFilesCodeView already uses. * feat(review): Make annotation selection action in edit sessions Selecting text in a Pierre edit session now shows the editor's Selection Action popover with a single Make annotation button (plain DOM, inline styles, colors via inherited theme tokens so light/dark and theme switches work through the shadow boundary). Clicking it snapshots the selection and opens the app's existing CommentPopover anchored at the button rect; the submitted comment becomes a normal line-scoped CodeAnnotation. Anchoring: the selection lives in the edited buffer, but annotations anchor to the rendered diff's new side, which is the session's pre-edit content. mapEditedRangeToPristine (edit/selectionAnchor.ts) maps the selected line range through diffLines(preEdit, edited) at click time. Unedited regions map exactly; ranges overlapping session edits anchor to the pristine lines those edits replace and are flagged approximate; pure insertions anchor to the adjacent pristine line. Pre-edit coordinates are session-invariant, so the anchor stays correct whether the session completes or is discarded. The captured selection text is stored on the annotation (selectedText, plus selectedTextFromEdits when approximate) and exported in feedback as a Highlighted text block with an honesty note for approximate anchors. New comments are re-projected into editor markers mid-session, and the editor selection collapses after submit so the popover does not reopen over the annotated lines. Entry UX note: the comment entry deliberately lives outside the editor's popover. Focusing an input inside it would blur the editor, collapse the selection, and tear the popover down mid-typing, so the popover only snapshots and hands off to the app-side CommentPopover. Tests: anchor-mapping unit tests (exact mapping through shifts, edited and inserted regions, deletions inside a selection, clamping, CRLF), export coverage for the Highlighted text block, and a DOM-gated popover builder test registered in the CI DOM step. Adapter wall unchanged and passing. * refactor(review): rename Display settings tab to Editor and move edit toggle to top * refactor(review): do not project annotations as editor markers Wavy underlines read as errors, which misrepresents comments. Annotations render in their normal slots below the code instead. --------- Co-authored-by: Michael Ramos <ramos@plannotator.ai>	2026-08-04 11:03:30 -07:00
Michael Ramos	c4acec988c	feat(ui): add colorblind theme (#1192) ... * feat(ui): add colorblind and tritanopia themes Two new built-in themes tuned for color vision deficiency: - Colorblind: for red-green CVD (protanopia and deuteranopia). Diff additions are blue (#4f83d1 dark / #216cab light) and deletions are orange (#c96e12 dark / #a55c1e light), a pair on the blue-yellow axis that survives red-green dichromacy. - Colorblind Tritanopia: for blue-yellow CVD. Additions are teal (#2b939e dark / #1b7981 light) and deletions are vermilion (#db5f38 dark / #c14f2b light), a pair on the surviving red-green axis. Both pairs were verified computationally with Machado et al. (2009) severity-1.0 CVD simulation matrices: the simulated add/del line backgrounds keep CIEDE2000 >= 8.9 (red-green theme) and >= 18.6 (tritanopia theme) at every diffLineBgIntensity in both dark and light modes, and normal-vision foreground contrast stays >= 4.7:1 over the strongest tint. The +/- gutter signs and indicator bars reuse the same base colors and keep >= 4.2:1 contrast on the page background. The review editor's Pierre diff pipeline picks up the pair via the library's own override seam: the theme files set --diffs-addition-color-override / --diffs-deletion-color-override, which inherit into the diff shadow DOM, so no app code changes were needed for the line backgrounds. Syntax colors in diffs map to the @pierre/theme colorblind shiki themes (already in the dependency tree), and plan-editor highlight.js code blocks get theme-scoped token overrides mirroring the same palettes. * feat(ui): drop the tritanopia theme after adversarial review Independent re-simulation showed the shipped default diff pair (the pierre-dark/pierre-light gitDecoration colors, green #07c480 / red #ff2e3f in dark and #18a46c / #d52c36 in light) beats the teal/vermilion pair under tritanopia simulation, and a matched-pair grid search over green/yellow-green vs red/vermilion candidates confirms why: tritanopia preserves the red-green axis, so the stock green/red palette is already near-optimal for tritanopes. No candidate pair that also keeps 4.5:1 normal-vision text contrast over the strongest line tint can beat the default in all six mode/intensity configurations under both Machado severity-1.0 and Vienot 1999 (the best candidates converge on the default's own vivid green/red and still fall 3 to 6 dE00 short in the worst configuration). Shipping a theme named for tritanopia that at best ties the default would invite false confidence, so it is removed rather than renamed. The Colorblind theme (red-green CVD, the common ~95% of cases) is unaffected and also holds up under tritanopia simulation (Vienot dE00 >= 32.8 in every configuration).	2026-08-04 11:03:27 -07:00
Michael Ramos	84846d9b9d	chore(deps): bump @pierre/diffs to 1.3.2 (#1191)	2026-08-03 21:58:55 -07:00
Michael Ramos	8abc685460	chore(deps): bump @pierre/diffs to 1.3.1 with @pierre/theme 2.0.0 and @pierre/theming 1.0.0 (#1190) ... Retune buildLineBgOverrides for the 1.3.x hover pipeline: per-selector hover mix rules are gone; hover is now one central rule mixing the active-line bg 97% (light) / 91% (dark) toward --diffs-hover-mix-target. Emitted hover --mix-* values are divided by those factors so the final rendered hover bg shares match 1.2.12 exactly at normal and strong, and subtle pins the 1.2.x hover finals (deletion 80/75, addition 80/70). Also add @pierre/theme and @pierre/theming to the bunfig minimumReleaseAge excludes (review follow-up from #1188).	2026-08-03 19:37:06 -07:00
Michael Ramos	767be3bfef	chore(deps): bump @pierre/diffs to 1.2.12 (stage 1 of 2) (#1188) ... Bump the exact pin from 1.2.8 to 1.2.12 in all six package.json files (root, packages/ui, packages/server, packages/review-editor, apps/review, apps/pi-extension) and resolve the lockfile. 1.2.12 pulls in @pierre/theme 1.1.0 (minor, transitive-only; we have no direct theme dependency) and a new transitive @pierre/theming 0.0.2. Stage 2 (1.3.x, next week once aged) carries the theme major and the hover pipeline rework. Verified: typecheck, full bun test, CI DOM set, all four builds, SSR parity at 1.2.8 vs 1.2.12 (structure identical except the intentional tabindex removal from the 1.2.12 focus fix), and a fresh registry install of the pi-extension tarball resolving a working 1.2.12 (guarding against the 1.2.9-era broken-tarball failure mode, #880).	2026-08-03 18:44:15 -07:00
Michael Ramos	46f1e8d5b2	fix(annotate): recognize wrapped URLs in token probe and port #1185 coverage (#1187) ... Ports five small items from the closed parallel PR #1185 into the tolerant annotate argument resolution that landed in #1183 (#1182): - Bug fix: the token probe tested the raw token against the URL regex, but the pipeline strips the @ reference marker and wrapping quotes first, so a multi-token 'annotate @https://example.com/page and summarize it' probed to nothing and emitted the handoff instead of opening the URL. The probe now unwraps with stripAtPrefix before the regex and returns the unwrapped form (the pipeline re-strips harmlessly). Tests cover @-prefixed and quote-wrapped URLs as multi-token candidates. - Test ports: absolute-path candidate, the wider plain-text set (.txt, .yaml) guarding ANNOTATABLE_DOC_REGEX breadth, the scoped-package literal-@ fallback against a real @scope/ directory, and the whole-un-split-string preference over its own tokens ('Meeting Notes.md' wins over a resolving 'Notes.md' token) covering annotateInputNamesExistingTarget. - Defensive scan: the strict-mode source-scan test now asserts the annotate startup block gates tolerance on !strictAnnotate via isStrictAnnotateInvocation, since an inverted gate cannot be spawn-tested without starting a server. - DRY: the strict predicate was defined twice (strict-annotate-result exit-code helper and the index.ts tolerance bypass). Extracted isStrictAnnotateInvocation with a StrictAnnotateFlags type; both sites use it so the exit-code path and the tolerance bypass can never drift. Behavior byte-identical; existing subprocess tests unchanged. - Docs: the tolerant-resolution section now cites #872 (commit aac5aacb) for why the bang prefix is deliberate and states that argument-shape issues belong in the CLI's resolution, not the skill templates. Refs #1185, #1182 Co-authored-by: Josh Nichols <josh.nichols+agent@gusto.com>	2026-08-03 13:25:41 -07:00
Raúl	9389a8c543	feat(ui): render markdown reference links (#1168) ... * feat(ui): render markdown reference links The simplified markdown parser only understood inline links `[text](url)`, so CommonMark reference links rendered as raw text: `[text][id]` and the `[id]: url` definition both showed literally (#923). Add `resolveReferenceLinks`, a pure pass run at the top of `parseMarkdownToBlocks` that rewrites full (`[text][id]`), collapsed (`[text][]`), and shortcut (`[text]`) references, plus their image forms, into inline `[text](url)` links, so the existing inline renderer draws them. Link reference definitions are collected first (first definition wins, labels matched case-insensitively with collapsed whitespace, `<url>` and quoted-title forms supported) and then blanked in place, so a definition never renders and every block keeps its original source line number. Resolution is code-aware: references and definitions inside fenced code blocks and inline code spans are left verbatim, a shortcut is skipped when an inline `(...)` destination follows it or when it is a task-list checkbox marker at the start of a list item, and an unknown reference stays literal so bracketed prose like `[TODO]` or `[0]` never becomes a false link. A definition-shaped line is only collected when it can start a block (after a blank line, a code fence, another definition, or the document start), so a `[word]: token` line that continues a paragraph is left as text rather than deleted (CommonMark: a definition cannot interrupt a paragraph). A document with no definitions is returned unchanged. * fix(ui): protect code/HTML/footnotes and quadratic risk in reference-link resolution Owner review round for reference-style link resolution (#923): - Fence detection now mirrors the block parser's own naive rule exactly (full .trim() + startsWith('```'), any indentation, backtick-only — no ~~~ support) instead of a looser 0-3-space approximation, so indented and list-nested fences the block parser treats as code can never be rewritten. Aligns tilde-fence behavior the same way: since the block parser has no ~~~ support, the resolver no longer protects ~~~ blocks either. - Raw HTML blocks (<details>, <pre>, etc.) are now protected using the same HTML_BLOCK_TAGS/HTML_BLOCK_OPEN_RE/VOID_HTML_TAGS the block parser itself uses, with the same three termination rules (blank-line, void single-line, balanced-depth). - GFM footnote definitions ([^label]: ...) are excluded from collection entirely, so they and their [^label] references are never rewritten into inline links. - A definition-shaped line is now only blanked when its label was actually consumed by a resolved reference outside a protected region. Unused definitions, and definitions referenced only from inside code/HTML, stay visible. This also fixes a plan-diff bug: a URL-only edit to a definition line used to blank to nothing on both sides of the diff (a real change rendering as empty); now the isolated diff chunk keeps the definition visible and diffs normally. - CRLF lines are now recognized (definition regex tolerates a trailing \r) and preserved (a blanked line keeps its own \r). - Bound the label/text capture groups (999 chars, CommonMark's own label limit) and the code-span alternative (5000 chars) so a long run of unmatched brackets/backticks can no longer cause quadratic backtracking within the 2MB annotate cap; added a defense-in-depth cap on the number of definitions tracked per document. - Added coverage for nested brackets, backslash-escaped brackets, parenthesized destinations, idempotence, and confirmed dangerous destinations still flow through the existing sanitizeLinkUrl path unchanged. Added a migration-caveat note to the existing annotation-anchor section of packages/ui/HANDOFF.md: documents using reference-style links render differently now, which can shift position-based anchors captured before a host upgrades past this change. * fix(ui): bound HTML-block extent scan to kill quadratic unclosed-opener case markProtectedLines and parseMarkdownToBlocks each independently scanned line-by-line from a multi-line HTML opener until its balanced open/close depth returned to zero, giving up only at end-of-document. That scan never advanced the outer index on failure, so a document with many consecutive unclosed openers (e.g. thousands of bare <div> lines with no </div> anywhere) made every one of them re-run the same O(N) tail scan — O(N^2) total, a real hazard well within the 2MB annotate cap. Extract the scan into one shared helper, findHtmlBlockEnd, used by both call sites so they can't drift apart: - closeExistsFromLine lazily builds (once per tag name, cached per document) a suffix array answering whether a closing tag exists at or after a given line, so an opener that can never close is rejected in O(1) instead of scanning to EOF. - MAX_HTML_BLOCK_SCAN_LINES bounds the residual case (a closing tag exists far away but depth never actually reaches zero before it) to a constant amount of work per start position — a documented, safe degradation: a block whose true close sits beyond the cap is treated as unclosed, identically to today's 'no close ever found' case. Added a failing-before-fix perf test (many unclosed <div> lines took ~2.3-3.4s and blew a 800ms bound; now ~12-15ms) for both parseMarkdownToBlocks and resolveReferenceLinks, plus a parity test proving a real <details>...</details> block stays intact and identically protected/parsed among thousands of decoy unclosed <div> lines. * fix(ui): remove HTML-block scan cap that truncated valid long blocks MAX_HTML_BLOCK_SCAN_LINES (2000) fixed the O(N^2) unclosed-opener case but as a side effect also truncated genuinely valid, longer HTML blocks: a <details> or raw <table> block whose closing tag sits beyond 2000 lines got cut off mid-block, with its remaining content and the real closing tag falling through as separate, incorrect blocks. closeExistsFromLine already rejects an opener that can never close in O(1) (no closing tag anywhere in the document) without scanning a single line — that already eliminates the pathological 'many failing scans' case on its own. A cap on top of that only ever hurt the opposite case: a scan that DOES succeed, which happens once per document and costs O(L) for an L-line block exactly like reading any other block's content once. So the cap bought nothing further and could silently corrupt valid parsing for any block longer than it, however generous its value. Removed it; the scan now runs unbounded to its real end once closeExistsFromLine confirms a close exists at all. findHtmlBlockEnd is still the single shared implementation used by both markProtectedLines and parseMarkdownToBlocks, so both stay in parity. Added regression tests: a >2000-line <details> and a >2000-line raw <table> block each stay one whole html block (previously truncated); a link definition inside a >2000-line <details> block stays protected and never wins over a real definition outside it; a valid long <details> block survives even preceded by thousands of unclosed <div> decoys. Re-verified the 40k-unclosed-opener perf/parity case (already handled by closeExistsFromLine alone) stays fast and unaffected. * fix(ui): replace HTML-block close scan with a linear prefix-sum index Removing the fixed line-count cap fixed truncation of valid long HTML blocks, but reopened a closely related O(N^2) case: N unclosed <div> openers followed by a single trailing </div> all still pass the 'does a close exist anywhere' pre-check, so every one of them independently scanned forward (mostly to end-of-document) before giving up. Measured before this fix: 5000 openers ~1.0s, 10000 ~4.1s, 40000 timed out past 69s. Replaced the scan entirely with a per-tag-name prefix-sum index (buildTagCloseIndex): the running open-minus-close count for a tag name, plus a classic 'next element at or below this one' index over that prefix sum (an O(N) monotonic-stack construction, each position pushed/popped at most once). Finding where (if anywhere) a block starting at a given line closes is exactly that classic query, so it is now an O(1) lookup with zero scanning per opener, whether the block never closes, closes after 3 lines, or closes 3000 lines away. Both markProtectedLines and parseMarkdownToBlocks still share the single findHtmlBlockEnd implementation, so they stay in parity. 40000 unclosed <div> openers + one trailing </div> now resolve in ~30-45ms (parser and resolver both), with block-boundary parity between them. Re-verified: no truncating cap reintroduced (>2000-line <details>/<table> blocks still stay whole), nested same-tag blocks still balance on true depth (not just tag presence), and independent tag types (e.g. a <table> nested inside a <details>) don't cross-talk between their separate per-tag indices.	2026-08-03 13:25:38 -07:00
Raúl	bc5470b90d	fix(review): bound server memory for large tracked-file diffs (#1167) ... * fix(review): bound server memory for large tracked-file diffs PR #1118 renders large untracked files as binary additions, but staging one moves it into the tracked `git diff` path, which had no size guard and buffered the full multi-megabyte patch (~240 MB RSS on a 51 MB text artifact). Any large tracked text file modified in the working tree hits the same unguarded path. Add a per-invocation `git -c core.bigFileThreshold=<MAX_REVIEW_FILE_CONTENT_BYTES>` prefix to every content-producing git diff, so git renders oversized blobs as "Binary files ... differ" instead of a text patch. Their bytes never enter git's diff machinery or the server's buffered stdout, mirroring the untracked-file guard. The flag is a no-op at or below the threshold, so smaller files are byte-for-byte unaffected, and the blob hash git emits in the binary diff still changes with content, so staleness detection holds. The guard is applied in the shared cores, so the Bun and Pi runtimes inherit it identically: `review-core.ts` covers the ordinary git provider (working-tree, staged, commit, and the freshness fingerprint) and `gitbutler-core.ts` covers the GitButler object diff. The jj provider runs `jj diff`, which has no `core.bigFileThreshold` equivalent, so it is out of scope here and stays unbounded as before. * fix(review): preflight oversized tracked diffs * fix(review): batch tracked diff preflight * fix(review): restore browser-safe diff core * fix(review): preserve gitlinks and textconv * fix(review): require filesystem runtime seam Fail compilation when a runtime omits file metadata or symlink support instead of silently disabling bounded reads and expansion.	2026-08-03 13:25:35 -07:00
Michael Ramos	747b5ea7e6	fix(annotate): resolve natural-language arguments or hand off to the agent (#1183) ... * fix(annotate): resolve natural-language arguments or hand off to the agent Claude Code skills run the CLI through a bash-substitution prefix that executes before the model sees anything, so any trailing natural language in /plannotator-annotate died with 'File not found: the'. Worse, a non-zero exit from that prefix aborts the whole prompt before the model runs (verified empirically), so the error was never even visible to the agent. Three-tier resolution in the binary's annotate argument handling, shared by every host via packages/shared/annotate-target.ts: 1. Fast path: probe each whitespace-delimited token; exactly one naming an existing file, URL, or folder proceeds with it directly. 2. Ambiguity: two or more tokens resolve; error naming every candidate, never guess. 3. Handoff: nothing resolves; emit an agent-addressed message echoing the words tried and asking the reading agent to interpret the request and re-run with a concrete target, preserving flags. In plain mode it lands on stdout with exit 0, the only combination that reaches the model through the bang prefix; in --json/--hook mode it goes to stderr with exit 1 so machine stdout stays clean. Single-token invocations run the unchanged pipeline first, so bare correct invocations are byte-identical. Strict gates (--require-approval or --result-file) bypass the tolerance entirely: a typo'd path stays a startup failure with exit 2 and no agent-facing prose. The CLI resolution pipeline moves to apps/hook/server/annotate-resolution.ts (returns typed outcomes instead of exiting) so the token fallback can run it once with a selected candidate; OpenCode and Pi wire the same shared selection into their own not-found paths. Skill bodies gain one line telling the agent to re-run with a concrete target when the command reports unresolvable arguments. Closes #1182 Reported-by: @technicalpickles * fix(annotate): harden tolerant resolution per review Review fixes for the three-tier annotate argument handling: - A single unresolvable token now falls through to the legacy pipeline verbatim: 'annotate nope.md' is exit 1 with 'File not found: nope.md' again in every non-strict mode, instead of an exit-0 handoff that fail-opened scripts gating on the exit code. The handoff fires only when two or more words resolve to nothing. - Unrecognized dash-prefixed tokens disable tolerance instead of being skipped, so a typo'd flag ('--no-jna') errors the way it did on base rather than silently fetching via Jina. Known flags are stripped before selection as before. - Token selection now receives the original argv tokens, so a quoted missing path ('my notes.md') is probed as one token and can never be re-split into a silently resolving 'notes.md'. - Bare directory names only count as fast-path candidates when they are the sole argument; a stray word matching a directory (or '.') hands off instead of opening folder mode. Explicit paths like 'src/' keep resolving, and the bare-existence probe fallback is file-only. - The handoff re-run suggestion echoes content flags only (--markdown, --no-jina, --render-html), never transport flags (--gate, --json, --hook). - New subprocess suite (annotate-cli.test.ts) spawns the real CLI entry and pins the contract: single-token typo exit 1, strict invocations (--require-approval and --result-file) exit 2 with empty stdout and no handoff prose, unknown-flag error, quoted-token preservation, and the directory-hijack case. Placeholder dist files are created when a build is absent so the suite runs in CI. - The copilot and gemini annotate command bodies gain the same handoff instruction as the Claude, core, and kiro skills. - AGENTS.md documents the three tiers under Annotate Flow and corrects the strict-section sentences that claimed non-strict behavior was fully unchanged; the marketing annotate doc mentions the tolerant arguments. Refs #1182	2026-08-03 10:14:37 -07:00
Michael Ramos	463f6ed57c	fix(ui): fall back to legacy copy in insecure browser contexts (#1174) ... * fix(ui): fall back to legacy copy in insecure browser contexts navigator.clipboard only exists in secure contexts. Remote mode serves plain HTTP on a non-localhost host, so every bare navigator.clipboard.writeText call threw TypeError and copy buttons silently broke. Add copyTextToClipboard(text): Promise<boolean> to packages/ui/utils/clipboard.ts: it tries the async Clipboard API (guarded against synchronous throws), falls back to the existing copy-event plus execCommand path, reports success as a boolean, and never throws. copyTextWithFallback now returns whether the copy happened and accepts an optional focusOwner; copyTextPreservingFocus keeps its exported signature and behavior unchanged. Route all bare call sites through the helper, preserving each site's UX: Copied states only flip on success, error toasts and console errors remain for the failure case, fire-and-forget sites stay fire-and-forget. GoalSetupSurface gains the fallback and keeps its error surface for the all-strategies-failed case. Add DOM-gated unit tests for the helper and register them in CI. Closes #1173 * fix(ui): address clipboard fallback review findings Review follow-ups for the insecure-context clipboard fallback: 1. Tag the fallback textarea with data-clipboard-fallback and whitelist it in PopoutDialog's ANNOTATION_SELECTORS so the transient focus shift during a fallback copy no longer closes popout dialogs (TablePopout copy buttons, CodeFilePopout copy contents). 2. Widen AnnotationPanel's onQuickCopy prop to Promise<void | boolean>. A false resolution now suppresses the Copied flash; void resolution stays success so existing hosts keep today's behavior. The editor quick-copy site returns the helper's boolean. 3. In copyTextWithFallback, only flag the copy-event path as success when clipboardData was present and setData actually ran, and only when execCommand also reported success. A null clipboardData no longer calls preventDefault, so the textarea retry still runs. 4. Add tests pinning that the fallback runs synchronously when navigator.clipboard is absent (execCommand fires before the call returns, keeping it inside the user-gesture window), that a copy event without clipboardData is not treated as success, and that the fallback textarea carries the PopoutDialog focus-out marker and is removed after the copy resolves. 5. GoalSetupSurface surfaces the real writeText rejection message when the Clipboard API exists but fails and the fallback also fails; the generic unavailable message is reserved for the API-absent case.	2026-08-03 10:14:34 -07:00
Michael Ramos	2ab377f77d	fix(uninstall): require host cleanup (#1177)	2026-08-03 10:11:29 -07:00
Michael Ramos	93b66e0ab2	feat(cli): add safe uninstall lifecycle (#1170) ... * feat(cli): add safe uninstall lifecycle * fix(uninstall): harden cleanup and add Windows QA * fix(uninstall): detach Windows self-delete worker * fix(uninstall): preserve PowerShell worker syntax * fix(uninstall): harden purge and host recovery * fix(uninstall): revalidate purge boundary * fix(uninstall): unlink managed link entries safely	2026-08-01 10:26:42 -07:00
Michael Ramos	d53cbfb373	fix(annotate): enforce archive read-only surfaces (#1171) ... * fix(annotate): enforce archive read-only surfaces * fix(archive): close remaining read-only leaks	2026-07-31 17:23:44 -07:00