Commit Graph

1173 Commits

Author SHA1 Message Date
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
Syahrul 0d9128794f fix: release pi review ports on session shutdown (#1160)
* fix: release pi review ports on session shutdown

* fix(pi): self-preempt stale sessions on fixed-port contention

A session abandoned without a decision keeps listening on the fixed
remote port in this long-lived process, so the next command's bind
fails (#1159). On a port-in-use or range-exhausted start failure,
stop every tracked browser session and retry once. Also: guard
closeAllConnections for older runtimes, downgrade the superseded
session notification from error to info, and cover the preemption
and stop-drain paths with tests.

* fix(pi): harden session preemption per independent review

Age-gate the preemption sweep so a concurrent command's fresh session
is never stopped, drop the session_shutdown sweep (sessions
deliberately outlive in-process session replacement so cross-session
feedback keeps working; process teardown frees ports anyway), make
server stop() and the sweep exception-safe, register archive sessions
in the registry, type the stopped error instead of matching prose,
handle a stopped plan review without a false startup error, keep the
tests from launching a real browser, and pin the call-site wrappings.

---------

Co-authored-by: Michael Ramos <mdramos8@gmail.com>
2026-08-03 13:25:32 -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 0935cf08ca fix(marketing): avoid sharing article redirect chain (#1180) 2026-08-02 12:09:39 -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
Michael Ramos 7cd023cbc7 docs: correct privacy and network claims (#1163)
* docs: correct privacy and network claims

* docs: address privacy review findings

* docs: clarify GitLab avatar lookup concurrency
2026-07-31 11:22:19 -07:00
Michael Ramos 8f84852f97 fix(review): surface partial GitLab comment submissions (#1164)
Surface partial GitLab submission outcomes and preserve narrowed, duplicate-safe retries across dialog reopen and same-tab refresh. Follow-up for an explicit blocked-recovery escape: #1166.
2026-07-31 10:59:58 -07:00
Michael Ramos 15bca1390e fix(install): stop PowerShell < 7.2 from treating git progress as an error
Under $ErrorActionPreference=Stop with stderr redirected, older
PowerShell turns a native command's first stderr line into a
terminating error, and git prints its normal "Cloning into ..."
progress on stderr, so the skill install failed on the message
announcing the clone had started. Scope the two git calls to a local
Continue preference; success and failure stay detected by Test-Path
and exit codes, never by throws. Reproduced and verified under
pwsh 7.1.3 (throws before, survives after; a real clone failure still
reports). Fixes #1162.
2026-07-31 03:02:12 -07:00
skredik 80065c8462 perf(review): virtualize Guided Review file cards (#1158)
* perf(review): virtualize guided review file cards

* fix(review): preserve guided review card behavior
2026-07-30 20:22:48 -07:00
tbontb 07c89ff145 fix(install): authenticate api.github.com call to avoid 60/hr rate limit (#1157)
* fix(install): authenticate api.github.com call to avoid 60/hr rate limit

All three installers called api.github.com/repos/.../releases/latest
unauthenticated to resolve the latest release tag. GitHub caps
unauthenticated REST API requests at 60/hour per source IP (shared
across NAT/CGNAT/corporate proxy users), causing opaque 'Failed to
fetch latest version' errors.

Each installer now detects a token (precedence: GITHUB_TOKEN env >
GH_TOKEN env > 'gh auth token') and attaches it as an Authorization:
Bearer header to ONLY the version-resolution API call. Falls back to
anonymous (unchanged behavior) when no token is available. Release
downloads and git clone are left unauthenticated - they are not subject
to the 60/hr REST limit.

Token variables are cleared from memory immediately after the API call
(defense in depth). install.ps1 wraps the API call in try/catch to
surface a friendlier error instead of a raw terminating exception under
$ErrorActionPreference=Stop.

Docs updated with a rate-limited-IP install section and a troubleshooting
entry. Parity + negative-assertion tests added.

Closes #1156

* fix(install): retry anonymously when authenticated api call fails

Addresses review feedback on #1157: a stale/revoked token (expired
GITHUB_TOKEN lingering in CI images, dotfiles, direnv) gets a 401 and
would break an install that works fine anonymously today.

Each installer now retries the api.github.com version-resolution call
without the auth header when the authenticated attempt fails:
- install.sh: `|| true` + `[ -z "$latest_tag" ]` gate
- install.ps1: try/catch, retries with no -Headers on catch
- install.cmd: ERRORLEVEL check, retries without !GH_AUTH_HEADER!

A bad token costs one extra request; nobody is worse off than today.
New test asserts the retry path exists in all three installers (89 pass).

Also corrects the AI disclosure: the assistant was opencode, not Claude.

* fix(install): address second-review findings

Errorlevel check adjacency in install.cmd, delayed-expansion token
reads, token charset guard, PATH-resolved gh with --hostname
github.com, 401-only anonymous retry in install.sh and install.ps1,
preserved original errors in install.ps1, honest cleanup comments,
non-vacuous test assertions, and doc corrections (1000/hr Actions
token, zero-scope guidance, -Version for PowerShell).

---------

Co-authored-by: tbontb-iaq <tbontb-iaq@users.noreply.github.com>
Co-authored-by: Michael Ramos <mdramos8@gmail.com>
2026-07-30 20:18:14 -07:00
Michael Ramos b5cf065cba chore: bump version to 0.25.1 v0.25.1 2026-07-30 03:15:51 -07:00
Michael Ramos c353413b55 fix(hook): enable the annotate client lease on OpenCode's last-message bridge
#1143 wired abandoned-gate dismissal into three of the four
startAnnotateServer call sites. The OpenCode annotate-last bridge takes
gate from stdin JSON rather than CLI flags and was missed, so
/plannotator-last --gate under OpenCode still hung on waitForDecision
forever once every review tab was abandoned: exactly the hang that
commit set out to close.

The bridge's inputs map onto the same predicate the other three use:
gate from the stdin payload, json unconditionally true because
emitOpenCodeAnnotateOutcome is the branch's only output path and always
writes a structured record the bridge parses back, hook false because no
flags are parsed here.

The new test scans every startAnnotateServer call site in index.ts
rather than pinning this one line, so the next site added cannot repeat
the omission.

Claude-Session: https://claude.ai/code/session_01H5KQWqXqjrPxyxUNso1QHS
2026-07-30 03:03:13 -07:00
Michael Ramos 4c89418bad fix(pi): contain the pi-todos mirror inside the project root
The mirror resolved <cwd>/.pi/todos with a plain path.join, so a repo
that ships .pi/todos as a symlink pointing elsewhere had its plan
checklist written to the symlink target on the first plan approval. The
mirror is on by default, so checking out a hostile repo and approving a
plan was enough. Write-only and non-overwriting, but not something to
ship knowingly.

The repo-implied path must now realpath inside cwd or the provider reads
as absent, which is the documented no-mirror no-op. An explicit
PI_TODO_PATH stays trusted verbatim: that is the user's own choice at the
same trust level as config.json, and pi-todos honors it the same way.
Containment reuses the shared isWithinDirectory sink, resolved through
the deepest existing ancestor so a not-yet-created todos dir under a
symlinked .pi cannot be created outside either.

Also corrects the detection docstring: pi-todos calls ensureTodosDir on
session_start, so an installed-but-unused provider does not read as
absent.

Claude-Session: https://claude.ai/code/session_01H5KQWqXqjrPxyxUNso1QHS
2026-07-30 03:03:13 -07:00
Raúl 37acde15d5 fix(ai): defer Codex model discovery until a Codex session starts (#1145)
* fix(ai): defer Codex model discovery until a Codex session starts

Opening any plan, annotate, or code review builds the shared AI runtime, and the
runtime called every provider's fetchModels() while constructing itself. For the
Codex provider that starts a throwaway `codex app-server` process, so a review
launched Codex even when the user never opened Ask AI. On macOS with a
quarantined Homebrew Codex payload this surfaces as a Gatekeeper confirmation
dialog in front of the review the user actually asked for.

Codex discovery now runs on explicit activation instead. The provider is still
registered and still advertised through /api/ai/capabilities using its static
fallback model metadata, so nothing about discovery is user-visible until a
session is created for it. createBestEffortOnce() memoizes the discovery call so
it runs at most once per runtime and a failure leaves the static fallback in
place rather than blocking session creation.

/api/ai/session gained a beforeProviderSession hook, invoked for the resolved
provider id before the session is created. /api/ai/capabilities deliberately
does not invoke it: the editor probes capabilities automatically on load, so
activating a provider there would reintroduce the same eager launch through a
different path.

Because discovery can replace the provider's model list, the session handler
compares the requested model against the pre-activation default. A caller that
sent no model, or sent the pre-activation default, gets the post-activation
default; an explicitly chosen model is always honored. Without this a first
Codex session would pin the static fallback model that discovery just replaced.

Both runtimes are changed the same way, and the other providers keep their
existing eager discovery, which beforeCapabilities still awaits.

Tests cover the regression with a fake Codex executable rather than a real one:
runtime construction and a capabilities probe must not invoke discovery, the
first Codex session must, the second must not, and a failing discovery must
still create a session on the fallback metadata.

* fix(ai): refresh provider metadata on explicit activation

Follow-up to the deferred Codex discovery change, addressing the review
findings on #1145 while keeping the deferral intact: constructing the
runtime and probing /api/ai/capabilities still never spawns
`codex app-server`.

- /api/ai/capabilities now accepts ?activate=<providerId>: it runs the
  same createBestEffortOnce initializer the session path uses (no second
  discovery path) and responds with the refreshed capabilities payload.
  A plain capabilities probe still activates nothing. Both runtimes get
  this through the shared endpoint (packages/ai is vendored into the Pi
  server by vendor.sh).
- The apps activate the selected provider on explicit user gestures --
  opening the Ask AI surface or switching the provider picker -- via the
  new useAIProviderActivation hook (single-flight per provider id), then
  merge the refreshed models and reasoning efforts into state so the
  model picker and per-model reasoning-effort selector populate past the
  static fallback. (review finding 1)
- A resolver-derived model is no longer persisted: useAIProviderConfig
  and AISettingsTab write the per-provider model preference only on an
  explicit user pick, so a saved Codex model the pre-activation fallback
  list doesn't include survives instead of being clobbered by the
  fallback id. The session request still falls back; the cookie doesn't.
  (review finding 2)
- The session handler resolves the requested model by membership in the
  post-activation model list instead of comparing against the
  pre-activation default, so sessions after the first can no longer pin
  a stale fallback id that discovery already replaced. (review finding 3)

Tests: activation endpoint behavior (shared endpoints plus both runtimes
against a hermetic fake codex on PATH), effectiveModel membership
resolution, and saved-preference no-clobber (DOM tests for
useAIProviderConfig persistence).

Claude-Session: https://claude.ai/code/session_01H5KQWqXqjrPxyxUNso1QHS

---------

Co-authored-by: Michael Ramos <mdramos8@gmail.com>
2026-07-29 23:36:47 -07:00
Michael Ramos 383ac97a8e fix(pi): survive pi's stale-ctx guard in the post-approval continuation
Pi >= 0.69 invalidates an extension's ctx on session teardown or
replacement, and every guarded member throws thereafter. The plan
continuation timer polled ctx.isIdle() from a setTimeout, so a session
dying mid-poll (print-mode teardown, /new or /reload right after
approval) crashed the whole pi process with an uncaught exception.

Cancel the continuation instead: a sessionAlive latch off the sanctioned
session_shutdown event plus an isCtxAlive() probe, and the timer body
never rethrows. Also .catch() the fire-and-forget browser open, which
could crash pi via unhandled rejection on a failed launcher even
without staleness.

Fixes #1140

Claude-Session: https://claude.ai/code/session_01H5KQWqXqjrPxyxUNso1QHS
2026-07-29 23:09:50 -07:00
Oscar Mederos d06f96decb fix(ui): focus and select existing search text (#1152) 2026-07-29 23:04:25 -07:00
Raúl c750427ab8 feat(annotate): dismiss abandoned gate sessions (#1143)
A direct local `plannotator annotate --gate --json` waits for one
authoritative decision. If every review surface disappears without
approving, sending feedback, or exiting, the caller blocks forever: the
server has no notion of whether a client ever connected, whether another
tab is still open, or whether a disconnect is a reload.

Page lifecycle events cannot answer that. `pagehide` and `beforeunload`
also fire on reload and navigation, so dismissing from them ends reviews
the user expects to resume. Use connection presence instead, which is
exactly what the transport can observe.

Local direct structured gates advertise a client lease in /api/plan and
serve /api/annotate/client-lease as SSE. One open stream is one connected
review surface. The server heartbeats every 5s and, only after at least
one client has connected, starts a 30s reconnect grace when the last one
disconnects. A reconnect inside the grace continues the same review;
expiry resolves the gate through the same path as explicit Close, so it
produces an ordinary `dismissed` decision and inherits the strict-result
contract unchanged. Approve, feedback, explicit exit, and server stop all
cancel a pending expiry.

Presence lives in two runtime-independent pieces so Bun and Pi cannot
drift. createAnnotateClientLeaseTracker owns first-client, active-count,
reconnect, cancellation, and one-shot expiry. createAnnotateClientLease-
StreamSession owns one connected client: acquire the slot, write the
ready comment, heartbeat, release exactly once. Each server passes only
its own write primitive (a ReadableStream controller for Bun, res.write
for Pi). A write that fails closes the session, because a stream that can
no longer be written to is a client that is no longer present; holding
the slot there would make the gate un-dismissable for the rest of the
run, which is reachable only through a half-open connection and so is
covered by unit tests rather than an integration test.

Scope is deliberately narrow. The capability stays off for remote and
shared sessions, where tunnel disconnects would read as abandonment, and
off for hook transport, legacy plaintext, archive, plan, review, and
folder-picker sessions. A session that never receives its first client
never auto-dismisses, so browser-launch failures still need a caller-side
timeout.

Decision settlement is explicit for the same reason: a connected surface and
the lease can both try to settle the session, and the awaited promise ignoring
the second resolve was not enough. The loser still deleted the reviewer's draft
and answered ok, so a tab reported success for a decision the caller never
received. createAnnotateDecisionSettler makes the winner explicit; a loser
changes nothing and answers 409. Expiry deliberately keeps the saved draft,
unlike explicit Close, so an abandoned review stays recoverable.

Stopping the server closes live lease streams instead of only releasing their
slots, so a long-lived host process does not retain a heartbeat timer and an
open response for every finished session.
2026-07-29 23:02:49 -07:00
mararn1618 5d1544fa32 fix(hook): route annotate-last to the live Copilot CLI session (#1150)
* fix(hook): add Copilot session lock detection

Copilot CLI exports no identifying environment variable, so nothing distinguishes a Copilot session from a plain shell. Match ancestor pids against session-state inuse locks to find the live session, and only accept a match when the lock owner still names a copilot process, since locks can outlive their session and pids get reused.

* fix(hook): route annotate-last to the live Copilot session

Under Copilot CLI, annotate-last silently fell back to the default transcript reader and annotated a message from a different tool. Take the Copilot branch when an ancestor process holds a session lock, or when PLANNOTATOR_ORIGIN=copilot-cli is set with the cwd heuristic as fallback, and report origin copilot-cli to the annotate server.

* fix(hook): prefer ancestor lock match in copilot-last

The cwd heuristic can pick a stale session when several exist for one repo. Resolve the session locked by an ancestor copilot process first and keep the heuristic as fallback.

* docs(cli): document copilot-last in help

The subcommand worked but was missing from the top-level usage and the per-subcommand help map.
2026-07-29 22:59:52 -07:00
Souptik Chakraborty 26681abb8c fix(amp,droid): surface Approve-with-Notes feedback instead of dropping it (#1146)
The Approve with Notes flow (#1092) emits decision:"approved" with a
feedback field, but both adapters collapsed every approved decision to a
bare "Approved." -- the reviewer's notes were silently lost.

Amp had two drop sites: formatAnnotationFeedback returned null for
anything not "annotated", and handleAnnotateResult early-returned on
approved before formatting was ever reached. Droid's
emitAnnotateDecision printed only "Approved."

Both adapters now emit the shared approved-with-notes prompt (mirroring
DEFAULT_ANNOTATE_APPROVED_WITH_NOTES_PROMPT in
packages/shared/prompts.ts); note-less approvals keep the old behavior.
The amp config override key approvedWithNotes matches the shared prompt
config shape.

Fixes #1137
2026-07-29 22:59:46 -07:00
BrandonNoad 8d8e643976 fix(hook): read annotate-last from Claude Code's transcript tree, not file order (#1141)
* fix(hook): read annotate-last from the transcript tree, not file order

Claude Code session logs are append-only and tree-shaped: every entry
records the entry it follows in `parentUuid`. `/rewind` writes nothing at
all. The next committed message simply re-parents to an earlier entry,
leaving everything after it orphaned in the file forever.

`extractRecentRenderedMessages` scanned bottom-up in file order, so those
orphans were still offered in the annotate-last message picker even though
they are no longer part of the conversation. On a rewound session in this
repo the picker listed 10 messages where only 7 are live.

Add `resolveActiveBranchIndices`, which walks `parentUuid` from the newest
id-bearing entry back to the root. It returns indices rather than a
filtered array so callers keep reporting real file line numbers, and
returns null on a chain it cannot trust (no ids, dangling parent, cycle) so
callers degrade to the previous file-order read instead of returning
nothing. Note the newest entry is not always the last line: `last-prompt`,
`ai-title`, `mode` and `file-history-snapshot` carry no ids and are often
written last.

Opt in at the Claude Code call site only. Droid's call site, Codex and
Copilot (separate parsers), and Pi/OpenCode/Amp (live APIs, no transcript
reads) are all unaffected. Both new parameters default to off.

Checked against 311 local transcripts: every one walks cleanly to the root
with no dangling parents or cycles, and the default pick is byte-identical
under both readings in all 298 that contain a message. That is expected,
since a committed rewind's new branch is always the newest lines in the
file. The picker is where the difference shows up.

Test fixtures previously assigned random `parentUuid`s, which left every
entry an orphan and made branch resolution untestable, so `buildLog` now
links them into a real chain and `buildRewoundLog` models a fork.

* fix(hook): fail open when the active branch has no assistant messages

A /compact boundary is written with parentUuid: null, so it is a tree
root: the active-branch walk stops there and a freshly-compacted session
yields zero messages. Callers treat an empty result as "wrong log file"
and walk off to an older session, so fail open to the file-order read
instead. Adds tests for the compaction cut, the fallback, and the
post-compaction recovery.

Claude-Session: https://claude.ai/code/session_01H5KQWqXqjrPxyxUNso1QHS

---------

Co-authored-by: Michael Ramos <mdramos8@gmail.com>
2026-07-29 22:59:43 -07:00
Michael Ramos b752080204 feat(ai): add Claude Opus 5 across providers and review agents (#1151)
* feat(ai): add Claude Opus 5 to the model catalogs

Adds `claude-opus-5` to both static Claude model lists:

- `packages/ai/providers/claude-agent-sdk.ts` — the Ask AI picker
- `packages/ui/components/AgentsTab.tsx` — the review-agent picker, which
  `TOUR_CLAUDE_MODELS` spreads, so tour and guide pick it up too

No `claude-opus-5[1m]` variant: the `[1m]` suffix opts the 4.x models into
the larger context window, and the 5-series models are already 1M by default
— which is why the existing `claude-fable-5` and `claude-sonnet-5` entries
carry no variant either. A test pins that invariant.

Guide, tour, and marker review need no change — they pass the picked model
straight through to the CLI with no server-side allowlist.

Tests cover the Bedrock/Vertex family matcher accepting the new bare alias
(a matcher keyed on "opus-4" would have silently dropped it) and pin the
existing Fable fallthrough, which has no ANTHROPIC_DEFAULT_*_MODEL of its own.

Also refreshes the marketing Ask AI model list, which had gone stale at
Sonnet 4.6 / Opus 4.6 / Haiku 4.5.

* feat(ai): default Claude review agents to Opus 5

Bumps the two Claude review-agent defaults from Opus 4.7 to Opus 5:

- `DEFAULT_CLAUDE_MODEL` (packages/ui/hooks/useAgentSettings.ts) — the
  review-agent picker's default for users with no saved cookie preference
- `buildClaudeCommand`'s default parameter (packages/server/claude-review.ts)
  — the server-side fallback when a job arrives with no model

BEHAVIOR CHANGE: users who have never picked a Claude review model move from
Opus 4.7 to Opus 5. Saved picks are untouched — the cookie already holds an
explicit model for anyone who has chosen one.

Kept deliberately separate from the catalog commit so it can be reverted on
its own if the maintainer prefers to ship the new models as options only.

Ask AI's default (`DEFAULT_MODEL` in claude-agent-sdk.ts) stays on Sonnet 5,
and the tour/guide defaults stay on the `sonnet` latest-alias — those are
interactive, latency-sensitive surfaces where the cheaper tier is the
intentional choice.
2026-07-29 08:51:33 -07:00
jms830 a54b46bbee feat(pi): mirror plan checklist to pi-todos (#1139)
* feat(pi): mirror plan checklist into an editable todo provider

Closes the one-way half of #484: on plan approval, feature-detect an
editable todo provider and mirror the approved checklist into it, then
reflect [DONE:n] markers onto todo status as execution proceeds.

pi-todos is the first provider. It exposes no API to other extensions,
so the integration surface is its on-disk format (.pi/todos/<id>.md with
JSON front-matter, <id>.lock taken with O_EXCL). The provider writes
through that contract, reconciles by tag so repeated syncs are
idempotent, closes steps dropped from an edited plan, and skips any todo
another session holds a lock on. Provider state resets on return to
idle, so a second plan re-detects rather than inheriting the first
plan's decision.

The mirror is additive: the existing progress widget is untouched.
pi-todos has no live surface of its own -- its list renders on demand in
/todos -- so suppressing the widget would trade a visible tracker for
files behind a keystroke. Sync is one-way, so provider-side edits can
never desync plan execution.

oh-my-pi's native todo panel is the obvious second provider and slots in
behind the same interface, but is not implementable from an extension
today: the panel repaint is gated to the built-in `todo` tool, and the
session handed to an extension-registered tool is a read-only projection
without the todo accessors. Details in todo-providers/index.ts.

Gated by PLANNOTATOR_TODO_PROVIDER / config.todoProvider, and inert when
no provider is detected.

* fix(pi): harden todo provider sync

---------

Co-authored-by: jms830 <jms830@noreply.github.com>
2026-07-27 10:27:03 -07:00
Michael Ramos 68c1291cd7 chore: bump version to 0.25.0
Claude-Session: https://claude.ai/code/session_01H5KQWqXqjrPxyxUNso1QHS
v0.25.0
2026-07-27 00:39:19 -07:00
Michael Ramos 951175ec1c fix(pi): ship annotate-outcome.ts in the npm package
The #1092 approval-notes work added annotate-outcome.ts as a top-level
import of index.ts but the package.json files array was never updated,
so the published tarball failed to load under Pi's jiti loader for
every npm-installed user. Found by the pre-release QA sweep; verified
the packed tarball now contains the module and no other top-level
import is missing from the files array.

Claude-Session: https://claude.ai/code/session_01H5KQWqXqjrPxyxUNso1QHS
2026-07-27 00:04:37 -07:00
Reveck 26a5314a99 feat(pi): add external plan execution handoff (#1126)
* feat(pi): add external plan execution handoff

* fix(pi): release phase-added tools on external plan handoff

#1124 made every idle-exit path hand back the tools the phase added, but
the external handoff path introduced here restored the saved model and
thinking level without releasing them. Approving a plan in
executionMode "external" therefore leaked the planning tool scope (grep,
find, ls, plannotator_submit_plan) into the idle session.

Extract the shared exit sequence into returnToIdle() and use it from all
three idle transitions (manual exit, execution completion, handoff), so
a new exit path cannot miss a step again. Callers keep their own
messaging, session entries, and events.

Also add a default-mode parity test: with executionMode unset, approval
enters executing, emits no plannotator:plan-approved, appends the
plannotator-execute entry, and still sends the continuation message.

Claude-Session: https://claude.ai/code/session_01H5KQWqXqjrPxyxUNso1QHS

* chore(pi): warn on unknown executionMode and document the handoff payload

An unrecognized executionMode was silently dropped, so a typo like
"handoff" left the session in automatic execution with no explanation.
Warn through the existing config warning channel (the same one malformed
JSON uses) and keep the automatic fallback.

README: note that planFilePath is relative to cwd and should be resolved
against it, and that companions can subscribe with the raw channel
string without importing Plannotator internals, matching how the
plannotator:request channel is already described.

Claude-Session: https://claude.ai/code/session_01H5KQWqXqjrPxyxUNso1QHS

---------

Co-authored-by: Michael Ramos <support@plannotator.ai>
2026-07-26 23:25:00 -07:00
Michael Ramos 10063d2799 fix(ui): Vim announcement dialog defaults to Close; gate HUD toggle under Vim mode
The one-time Vim announcement dialog auto-focused a primary button that
force-enabled BOTH vim mode and the HUD before dismissing. Anyone who hit
Enter to get the popup out of the way was silently opted into Vim mode —
an Enter-trap on a feature that is meant to be off by default. The primary
action is now a pure dismiss ("Close" from the untouched default state, and
"Start with Vim [+ HUD]" once the user has actually toggled something), so
Enter and Escape both leave settings exactly as they were. The redundant
"Not now" secondary button is gone since both buttons now dismiss identically.

The HUD row also silently enabled Vim mode when tapped while Vim was off.
It now mirrors Settings → Vim: disabled (aria-disabled, dimmed, no-op) while
Vim controls are off, with "Turn on Vim controls first." as its description
and the "Recommended" badge withheld until the row is live. Copy reworked to
say plainly that the feature stays off unless you turn it on.

Claude-Session: https://claude.ai/code/session_01H5KQWqXqjrPxyxUNso1QHS
2026-07-26 22:46:35 -07:00
Peete d26289060d fix(pi): preserve extension tool selections (#1124)
* fix(pi): preserve extension tool selections

* fix(pi): release phase tools when the plan completes

The agent_end plan-completion path dropped back to idle via
restoreSavedState but never called releaseAddedPhaseTools(), so tools
Plannotator activated for the executing phase stayed active in idle and
a stale phaseAddedTools list was persisted. Mirror exitToIdle.

Claude-Session: https://claude.ai/code/session_01H5KQWqXqjrPxyxUNso1QHS

* fix(pi): keep grep/find/ls in the default planning tool set

Dropping them from the planning phase profile silently removed native
search from default planning sessions. Restore them as default planning
phase tools so they now flow through the new tracking mechanism: they
are recorded in phaseAddedTools and released on phase exit, and a
discovery tool another extension already activated is neither claimed
nor removed. A user's own planning.activeTools still overrides them.

Claude-Session: https://claude.ai/code/session_01H5KQWqXqjrPxyxUNso1QHS

* fix(pi): always keep the submit tool active during planning

Planning tools now come purely from the resolved profile's activeTools,
which uses replace semantics: a phases.planning.activeTools in
~/.pi/agent/plannotator.json or cwd/.pi/plannotator.json supersedes the
built-in list wholesale. Pi's setActiveToolsByName then replaces the
session's tool list outright, so such a config silently dropped
plannotator_submit_plan while the planning system prompt kept telling the
model to call it - a dead end with no way to submit a plan.

Union PLAN_SUBMIT_TOOL into the configured list for the planning phase
only (deduped if the config already lists it). It flows through
phaseAddedTools like any other addition, so it is released normally on
phase exit. Other tools are left to the user: overriding
planning.activeTools is a deliberate choice of tool set, and only the
submit tool is contractually required for the phase to function.

Also corrects the README's "extra tools ... additive" wording, which
described the old merge behavior.

Claude-Session: https://claude.ai/code/session_01H5KQWqXqjrPxyxUNso1QHS

---------

Co-authored-by: Michael Ramos <support@plannotator.ai>
Co-authored-by: Michael Ramos <mdramos8@gmail.com>
2026-07-26 22:28:11 -07:00
Michael Ramos 47157e7a55 feat(editor): add Vim keyboard annotation controls and live HUD (#1127)
* feat(editor): add Vim keyboard annotation controls

* feat(ui): add optional live Vim HUD

* feat(ui): finish Vim HUD experience

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

* feat(ui): make Vim document focus automatic

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

* fix(ui): harden Vim selection UX
2026-07-26 22:08:35 -07:00
Brad Beebe ac3a84ba5a fix(opencode): Fix hardcoding default build OpenCode agent when sending responses (#1131)
* fix(opencode): default agent switching to disabled

* fix(opencode): keep plan-approval build handoff; default no-switch for review feedback only

The agent switch cookie is shared by plan approval and code review, so
flipping the stored default to `disabled` also removed OpenCode's
plan-approval hand-off for every user who never configured the setting.

Make the unset default surface-aware instead: `getAgentSwitchSettings('plan')`
keeps the historical build hand-off, `getAgentSwitchSettings('review')`
stays on the current agent. An explicit user choice still applies to both
surfaces. Settings and the agent warning resolve the default from the mode
they render in, and the OpenCode "agent not available" warning now names
plan approval on the plan path.

Claude-Session: https://claude.ai/code/session_01H5KQWqXqjrPxyxUNso1QHS

---------

Co-authored-by: Michael Ramos <mdramos8@gmail.com>
2026-07-26 22:08:04 -07:00
Raúl 9a450a69e7 feat(annotate): preserve notes on structured approval (#1092)
* feat(annotate): add strict atomic result output

* feat(annotate): exit 2 for strict-gate usage and publication errors

Adopt the grep convention for the strict annotate gate's exit codes:
0 = approved, 1 = negative human outcome (annotated/dismissed under
--require-approval), 2 = the gate itself was misconfigured or could not
start/deliver a decision. Previously all usage/startup/validation
failures shared exit 1 with "reviewer did not approve", so callers could
not tell a denied review from a broken gate.

- parseStrictAnnotateOptions failures (bad flag combos, strict flags
  outside annotate --gate --json) now exit 2
- --result-file preflight failures (missing parent, pre-existing or
  dangling-symlink destination) now exit 2
- post-decision publication failures (destination raced into existence,
  hard links unavailable, stdout write failure) now exit 2: they deliver
  no decision record at all, so the code's own fail-closed handling
  presents them as environment errors, never as a reviewer outcome --
  and never approval, since only 0 means approved
- decision outcomes keep 0/1 exactly as before; signal deaths keep 128+n
- document the contract in AGENTS.md and the annotate-gates guide

Claude-Session: https://claude.ai/code/session_01YXkgsNucxDwAL4GdR4XYRk

* feat(annotate): preserve notes on structured approval

* test(pi): use exact annotate outcome import

* fix(annotate): exit 2 for strict-gate startup failures

The six startup-failure sites in the annotate path (missing path, unreachable
URL, empty folder, ambiguous name, missing/unsupported file, oversized file)
run after flag parsing and exited 1. Under --require-approval / --result-file,
1 is the "reviewer requested changes" signal, so a typo'd path made automation
misclassify a configuration error as a legitimate rejection.

Route those sites through exitAnnotateStartupFailure(), which picks its code
from the already-parsed strict options via the new pure helper
annotateStartupFailureExitCode(). Non-strict invocations still exit 1 with
byte-identical stderr; strict invocations exit STRICT_GATE_ERROR_EXIT_CODE (2).

Claude-Session: https://claude.ai/code/session_01H5KQWqXqjrPxyxUNso1QHS

* fix(annotate): emit the strict decision on stdout before publishing it

writeResultFile ran before the decision JSON reached stdout. On a filesystem
without hard links (exFAT, FAT32, most SMB/NFS, some container bind mounts)
publication fails deterministically, the catch exited 2 with nothing written
anywhere — and the reviewer's autosaved draft had already been deleted by the
feedback flow, so their completed decision was lost.

Emit the stdout record first, then publish the result file. Exit semantics are
unchanged: a publication failure still exits 2, but the decision has reached
stdout by then. Only a stdout write failure now leaves no record at all.

Correct the docs and comments that claimed exit 2 delivers no decision record:
it means the result *file* was not published. Also document the two publication
caveats: the 0600 mode is a no-op on Windows, and the atomic link/rename is not
followed by a parent-directory fsync, so publication is atomic but not
crash-durable.

Claude-Session: https://claude.ai/code/session_01H5KQWqXqjrPxyxUNso1QHS

* fix(annotate): parse linked docs with the render-side frontmatter rule on export

buildCompleteAnnotateFeedback re-parsed each linked document with
parseMarkdownToBlocks(entry.markdown) — no options, so frontmatter
stripping defaulted on. The render side parses with
{ frontmatter: shouldStripFrontmatter(path) }.

For plain-text linked docs (.yaml/.json/.toml/…) a leading `---` is real
content, not frontmatter: a multi-document YAML opens with it. Stripping
it on the export side shifted every block id, so ordinary Send Feedback
and deny emitted wrong `(line N)` labels — or dropped them entirely when
the annotation's block no longer existed.

Pass the same shouldStripFrontmatter(filepath) option at the export call
site so both sides agree.

Claude-Session: https://claude.ai/code/session_01H5KQWqXqjrPxyxUNso1QHS

* fix(annotate): carry the message scope through approve-with-notes

/api/feedback forwards selectedMessageId and feedbackScope; /api/approve
dropped them. Pi resolves the anchor message from those fields, so notes
delivered on the approve path anchored to the last message instead of the
one the reviewer picked in a multi-message annotate-last session — while
Send Feedback in the same session anchored correctly.

Forward both fields on the approve path in the Bun and Pi servers, and
have the client build the approval body with the same scope resolution
Send Feedback uses (extracted as getFeedbackMessageScope so the two can
no longer drift).

Claude-Session: https://claude.ai/code/session_01H5KQWqXqjrPxyxUNso1QHS

* docs(annotate): tell agents an approval may carry notes

The skill and slash-command files still described `"decision": "approved"`
as "acknowledge and stop", with no mention of the feedback field the gate
can now attach — so an agent reading them would silently drop the
reviewer's approval notes.

Update the Claude core/claude skills, the Copilot commands, the Gemini
annotate command, and the annotate command reference so the approved
branch names the optional feedback field and says what to do with it:
carry it into subsequent work, do not treat it as a change request.

Claude-Session: https://claude.ai/code/session_01H5KQWqXqjrPxyxUNso1QHS

* docs(annotate): document the real approvedWithNotes default

The default annotate.approvedWithNotes template is
`{{contextBlock}}{{feedback}}`, not `{{context}}` on its own line, and
{{contextBlock}} was missing from the variable table entirely.

Show the actual default, add {{contextBlock}} to the variable table, and
explain why the default prefers it: it collapses to nothing for message
annotations instead of leaving a stray blank line.

Claude-Session: https://claude.ai/code/session_01H5KQWqXqjrPxyxUNso1QHS

---------

Co-authored-by: Michael Ramos <mdramos8@gmail.com>
2026-07-26 21:09:28 -07:00
Raúl 5aaa420080 feat(annotate): add strict atomic result output (#1091)
* feat(annotate): add strict atomic result output

* feat(annotate): exit 2 for strict-gate usage and publication errors

Adopt the grep convention for the strict annotate gate's exit codes:
0 = approved, 1 = negative human outcome (annotated/dismissed under
--require-approval), 2 = the gate itself was misconfigured or could not
start/deliver a decision. Previously all usage/startup/validation
failures shared exit 1 with "reviewer did not approve", so callers could
not tell a denied review from a broken gate.

- parseStrictAnnotateOptions failures (bad flag combos, strict flags
  outside annotate --gate --json) now exit 2
- --result-file preflight failures (missing parent, pre-existing or
  dangling-symlink destination) now exit 2
- post-decision publication failures (destination raced into existence,
  hard links unavailable, stdout write failure) now exit 2: they deliver
  no decision record at all, so the code's own fail-closed handling
  presents them as environment errors, never as a reviewer outcome --
  and never approval, since only 0 means approved
- decision outcomes keep 0/1 exactly as before; signal deaths keep 128+n
- document the contract in AGENTS.md and the annotate-gates guide

Claude-Session: https://claude.ai/code/session_01YXkgsNucxDwAL4GdR4XYRk

* fix(annotate): exit 2 for strict-gate startup failures

The six startup-failure sites in the annotate path (missing path, unreachable
URL, empty folder, ambiguous name, missing/unsupported file, oversized file)
run after flag parsing and exited 1. Under --require-approval / --result-file,
1 is the "reviewer requested changes" signal, so a typo'd path made automation
misclassify a configuration error as a legitimate rejection.

Route those sites through exitAnnotateStartupFailure(), which picks its code
from the already-parsed strict options via the new pure helper
annotateStartupFailureExitCode(). Non-strict invocations still exit 1 with
byte-identical stderr; strict invocations exit STRICT_GATE_ERROR_EXIT_CODE (2).

Claude-Session: https://claude.ai/code/session_01H5KQWqXqjrPxyxUNso1QHS

* fix(annotate): emit the strict decision on stdout before publishing it

writeResultFile ran before the decision JSON reached stdout. On a filesystem
without hard links (exFAT, FAT32, most SMB/NFS, some container bind mounts)
publication fails deterministically, the catch exited 2 with nothing written
anywhere — and the reviewer's autosaved draft had already been deleted by the
feedback flow, so their completed decision was lost.

Emit the stdout record first, then publish the result file. Exit semantics are
unchanged: a publication failure still exits 2, but the decision has reached
stdout by then. Only a stdout write failure now leaves no record at all.

Correct the docs and comments that claimed exit 2 delivers no decision record:
it means the result *file* was not published. Also document the two publication
caveats: the 0600 mode is a no-op on Windows, and the atomic link/rename is not
followed by a parent-directory fsync, so publication is atomic but not
crash-durable.

Claude-Session: https://claude.ai/code/session_01H5KQWqXqjrPxyxUNso1QHS

---------

Co-authored-by: Michael Ramos <mdramos8@gmail.com>
2026-07-26 21:05:57 -07:00
Raúl 53650f3f6b fix(annotate): watch open source files exactly (#1089)
* fix(annotate): watch open source files exactly

* test(annotate): cover atomic watcher saves

* fix(watch): survive atomic file replacement

* fix(watch): disable exact-file coalescing

* fix(watch): track exact file signatures

* fix(annotate): tolerate undefined watcher filenames and harden watch callbacks

The exact-file watcher only treated a `null` filename as "name unavailable".
On Linux, Bun's fs.watch delivers `filename === undefined` for events on the
watched directory itself (chmod/utimes/rename of the parent, as produced by
`tar -x`, `rsync -a`, `cp -a`), so `filename.toString()` threw an uncaught
TypeError and killed the annotate server for every Linux user with a watched
file open.

Widen the guard to `filename == null` (null and undefined) and move the
listener into `createExactFileWatchListener`, whose body is wrapped in
try/catch so no watcher event can ever take the server down. Mirrored in the
Pi runtime, with regression tests in both.

Claude-Session: https://claude.ai/code/session_01H5KQWqXqjrPxyxUNso1QHS

---------

Co-authored-by: Michael Ramos <mdramos8@gmail.com>
2026-07-26 20:28:45 -07:00
Ben Newman 6b542da8b9 feat(annotate): extend per-file version diff to folder sessions (#1105)
* refactor(annotate): extract per-file version history into a shared helper

Move the single-file annotate-history pipeline (slug derivation,
saveToHistory, previous-version lookup, degrade-on-error) out of the
Bun-specific annotate server and into packages/shared/annotate-history.ts,
built on node:fs/node:path/node:crypto only so other runtimes can vendor it
unmodified.

annotate.ts now calls computeAnnotateHistory() instead of inlining the
pipeline; behavior for single-file sessions is unchanged.

* feat(annotate): extend per-file version history to folder annotate sessions

Eligible folder files served through /api/doc now get snapshotted into the
same version history the single-file flow uses, and their doc responses
carry the same previousPlan/versionInfo/diffCurrent fields /api/plan already
returns for single-file sessions. The pipeline runs lazily on first open and
is memoized per resolved absolute path for the life of the server, so
reopening a file never re-snapshots it.

Eligibility mirrors the single-file source-save gates: a local file under
the session's folder root, markdown-branch documents only (.md/.txt, not
HTML, not a Turndown-converted doc), under the existing 2MB annotatable-file
cap, and gated by the same annotateHistory config toggle. Storage failures
degrade to a plain render (never a gate on the request) via the same
try/catch computeAnnotateHistory already wraps.

/api/plan/version and /api/plan/versions gain an optional path (+ base)
query param so folder sessions can ask for a specific file's history; the
slug is always derived server-side from the resolved, containment-checked
path — never accepted from the client, since it gets joined unsanitized
into a filesystem path. Omitting path keeps today's single-session-binding
behavior unchanged.

* test(annotate): cover folder annotate version history

Adds a new describe block exercising the folder-mode history pipeline added
in the previous commit: first-open snapshot + same-session memoization,
storage-level dedupe, cross-mode slug continuity with the single-file flow,
first-ever-open field shape, the config toggle, an ineligible (HTML) file
type, degrade-on-unwritable-history-dir, and the path-parameterized version
endpoints (including containment rejection and the no-path fallback).

* feat(ui): add a docKey seam to usePlanDiff for per-document resets

usePlanDiff's diff-base state (diffBasePlan, diffBaseVersion, versions, ...)
was seeded once from its constructor args and only ever synced later via a
"still falsy" guard - fine for a single root document, but switching to a
different document (a different previousPlan/versionInfo) would silently
keep the previous document's diff base around instead of adopting the new
one's.

Add an optional docKey param identifying which document the current
previousPlan/versionInfo belong to. When it changes between renders, reset
diffBasePlan/diffBaseVersion/versions (and in-flight loading/selecting
flags) to the newly-provided values. Omitting docKey (or keeping it stable)
preserves exactly today's one-time-hydration behavior, so the root
document's call site is unaffected until it opts in.

No caller passes docKey yet - this is purely additive.

* feat(ui): carry a per-document version-diff baseline through useLinkedDoc

/api/doc now returns previousPlan/versionInfo/diffCurrent for eligible
folder files (same shape /api/plan already returns for single-file
sessions). Extend LinkedDocLoadData with those fields and carry them
through the same activate/cache/back lifecycle annotations and markdown
already use, so a document's diff baseline:

- is captured once when the document is first opened
- persists in the per-filepath cache across back()/re-open, instead of
  being lost or needing a re-fetch
- resolves cache-first via the new resolveDiffBaseline helper, gated on
  whether a baseline was ever captured (versionInfo presence) rather than
  truthiness of previousPlan - a document at its first-ever version
  legitimately caches previousPlan: null, which is a resolved fact, not a
  cache miss

The hook exposes the active document's baseline as diffPreviousPlan/
diffVersionInfo, both null when no document is active or the active one has
no eligible history (every non-folder linked doc, since /api/doc never
populates these fields for those).

Not yet consumed by App.tsx - purely additive.

* feat(editor): render folder-doc version diffs via the active document

Folder annotate's version-diff UI (inline PlanDiffViewer blocks, the +N/-M
badge, and the Version Browser) was root-document-coupled: usePlanDiff was
fed only the root's previousPlan/versionInfo, and every render site keyed
off linkedDocHook.isActive to blank out the badge/version tab whenever any
linked or folder document was open.

Wire the two new per-document seams together instead:
- Feed usePlanDiff the active document's own previousPlan/versionInfo/
  filepath (falling back to the root document's when none is active), using
  the document's filepath as usePlanDiff's new docKey so switching documents
  resets the diff base instead of inheriting the previous one's.
- Add per-document fetchers (fetchVersion/fetchVersions with
  &path=<filepath>) so selecting a base version or listing versions targets
  the active document's own history, not the session-bound bare endpoints.
- Replace the root-only versionInfo/showVersionsTab reads with the active
  document's, so the Version Browser now reflects whichever document is on
  screen (previously it kept showing the root document's versions while a
  linked doc was open).
- Drop the blanket "linkedDocHook.isActive ? null/false : ..." suppression
  at the Viewer callsite and in DocBadges - planDiffStats/hasPreviousVersion
  already resolve to the active document's own (possibly absent) diff data,
  so the badge now shows for folder docs with history and stays hidden for
  every other document exactly as it did before.

Root-document behavior (single-file, plan, review, HTML surfaces) is
unaffected: none of those ever set a docKey or have an eligible document
history, so they fall through to the same defaults as before.

* feat(pi): extend per-file version history to folder annotate sessions

Mirrors the Bun runtime's folder annotate history support
(packages/server/annotate.ts + reference-handlers.ts) in the Pi Node server:

- Vendor the shared annotate-history helper (deriveAnnotateHistorySlug,
  computeAnnotateHistory) from packages/shared into generated/ via
  vendor.sh, and delegate the single-file version-history pipeline in
  serverAnnotate.ts to it instead of the hand-duplicated inline block.
  Behavior for single-file sessions is unchanged.
- Eligible folder files served through /api/doc now get snapshotted into
  the same version history the single-file flow uses, and their doc
  responses carry the same previousPlan/versionInfo/diffCurrent fields
  /api/plan already returns. The pipeline runs lazily on first open and
  is memoized per resolved absolute path for the life of the server, so
  reopening a file never re-snapshots it.
- /api/plan/version and /api/plan/versions gain an optional path
  (+ base) query param so folder sessions can ask for a specific file's
  history; the slug is always derived server-side from the resolved,
  containment-checked path (resolveAllowedDocPath in reference.ts) —
  never accepted from the client.

* test(pi): cover folder annotate version history

Adds apps/pi-extension/server/annotate-history.test.ts, the Node mirror of
packages/server/annotate.test.ts's folder-history describe block: first-open
snapshot + same-session memoization, cross-mode slug continuity with the
single-file flow, the config toggle, an ineligible (HTML) file type,
degrade-on-unwritable-history-dir, and the path-parameterized version
endpoints (including containment rejection and the no-path fallback).

History writes land in the real ~/.plannotator data dir rather than a
per-test PLANNOTATOR_DATA_DIR override: generated/storage.js caches its
data directory in a module-level constant at first import, so a per-test
env var override taken after that point silently no-ops. Each test uses
its own unique project namespace instead, same approach as the Bun-side
suite.

* ci: run docKey/linked-doc DOM tests in CI

usePlanDiff.test.tsx and useLinkedDoc.test.tsx use the test.skipIf(!hasDom)
pattern but were never added to the DOM_TESTS step, so they silently
skipped under CI's plain `bun test` and never actually ran.

* refactor(annotate): drop diffCurrent from the folder /api/doc path

diffCurrent equals the document's own markdown and the client never reads
it off /api/doc — it only exists on /api/plan for legacy single-file
shape parity, which is untouched. Stop merging it into folder /api/doc
responses and stop retaining it in the per-launch folder history memo
(Bun and Pi), and drop the now-unused field from LinkedDocLoadData.

- packages/server/reference-handlers.ts: new FolderAnnotateHistory type
  (AnnotateHistoryResult minus diffCurrent); applyDocOptions no longer
  copies diffCurrent onto the response
- packages/server/annotate.ts: the folder memo now stores/returns only
  slug/previousPlan/versionInfo
- apps/pi-extension/server/reference.ts + serverAnnotate.ts: mirrored
  changes for the Pi runtime
- packages/ui/hooks/useLinkedDoc.ts: removed the unused diffCurrent field
  from LinkedDocLoadData

* test(annotate): stop leaking history dirs; update diffCurrent expectations

The folder annotate history tests (Bun and Pi) minted a fresh project
namespace per test but never cleaned up, leaving hundreds of directories
under the real ~/.plannotator/history over repeated runs. Track every
minted project and remove its history directory in afterAll — this also
covers the stray non-directory artifact the "unwritable data dir" test
deliberately plants inside its own project's history dir, since removing
the project dir recursively takes it with it.

Also update the two assertions that expected diffCurrent on the folder
/api/doc response: that field is no longer propagated on the folder path
(see the preceding diffCurrent-removal commit), so both now assert its
absence instead.

* fix(ui): remember per-document diff-base selection across navigation

usePlanDiff reset diffBasePlan/diffBaseVersion to the newly-provided
document's defaults on every docKey change. That discarded a manually
selected base version when navigating away from a document and back
(e.g. root -> linked doc -> root), regressing behavior upstream relied
on keeping (nothing reset the selection before this seam existed).

Track each docKey's selection in a ref-held Map (keyed by docKey,
including null for the root document) and restore it on return instead
of re-seeding defaults; a key visited for the first time still seeds
from its own initialPreviousPlan/versionInfo exactly as before, and
selections never leak between distinct keys.

Adds two DOM-gated tests: restoring a manual selection after a detour to
another document, and confirming distinct docKeys don't leak into each
other.

* fix(annotate): match folder history eligibility to the single-file plain-text set

The folder /api/doc history gate was a hardcoded /\.(md|txt)$/i in both
runtimes, so any other annotatable plain-text file (.mdx, .yaml, .json,
.toml, ...) opened via a folder session silently skipped snapshotting —
breaking the cross-mode continuity this feature advertises (a .yaml with an
existing single-file version thread showed no diff when opened via its
folder).

Reuse the canonical predicate instead: isAnnotatableTextPath
(ANNOTATABLE_TEXT_REGEX in @plannotator/core/annotatable), the exact set the
single-file pipeline snapshots. HTML stays deferred and .env stays excluded,
both by that same definition. Tests extended in both runtimes: .mdx mints on
first open, .yaml single-file history serves as the folder baseline, .env
mints nothing, .html unchanged.

* feat(ui): label the folder diff badge with its baseline

The in-file version-diff badge in annotate/folder sessions shows +N/-M
against the file's last-reviewed snapshot, while the git badges in the file
tree count uncommitted-vs-HEAD — same numbers, different baselines. Give the
badge an optional baseline suffix and tooltip override (PlanDiffBadge
baselineLabel/baselineTooltip, threaded through DocBadges, Viewer, and
StickyHeaderLane) and have annotate mode pass 'since last review' /
'Changes since you last reviewed this file'. Plan review passes nothing and
renders byte-identically to before. DOM tests cover both the labeled and the
unchanged default rendering.

* fix(editor): exit diff view when the active document loses its baseline

Follow-up to the per-document diff baselines: with diff view active on file
A, opening a history-less file B left isPlanDiffActive latched on — the diff
viewer could not render for B, but the stale flag hid the annotation
toolstrip and sticky header until the user pressed Escape. Auto-exit the
diff view whenever the active (non-HTML) document has no baseline. The
--render-html surface is explicitly gated out: its diff view is driven by
htmlDiffHtml with usePlanDiff fed nulls, so hasPreviousVersion is always
false there and auto-exiting would kill the HTML diff toggle. Plan review is
unaffected — the root document's baseline never goes false mid-session. DOM
tests cover the exit, the keep-active document switch, the HTML gate, and
the no-baseline activation snap-back.

---------

Co-authored-by: Michael Ramos <mdramos8@gmail.com>
2026-07-26 15:33:18 -07:00
Michael Ramos 5d716e2271 feat(core): add Zed Preview as an Open In App target (#1130)
Zed Preview installs as a separate app bundle on macOS (Zed Preview.app),
so the stable 'zed' entry never detects or launches it there. On Linux and
Windows the preview channel installs its CLI as plain 'zed' (verified in
Zed's install.sh and bundle-windows.ps1), so the existing entry already
covers preview on those platforms and the new entry is mac-only.

Closes #1119

Claude-Session: https://claude.ai/code/session_01H5KQWqXqjrPxyxUNso1QHS
2026-07-26 13:26:21 -07:00
Bala Sivagnanam 78dc46b8e2 fix(ui): move Save to the first tab stop in the comment popover footer (#1122)
* fix(ui): move Save to the first tab stop in the comment popover footer

Reorders the comment popover footer so Save is the first focusable
control after the textarea, ahead of Ask AI and the image-upload
button. Tab order previously matched DOM order left-to-right (image
icon, Ask AI, Save), so fast keyboard-driven flows (select text →
comment → Tab, Tab, Enter) could land on the image-upload button
instead of Save. Pure reorder, no behavior change.

Fixes #1121

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

* fix(ui): keep Save visually right; use row-reverse so DOM puts Save first

Keeps the tab-order fix from the previous commit (Save is the first
focusable control after the textarea) while restoring the original
visual layout: image upload far left; Ask AI, submit hint, Save on
the right with Save rightmost. The footer container and actions
group use flex-row-reverse so DOM order (Save first) sets tab order
without changing rendering.

Claude-Session: https://claude.ai/code/session_01H5KQWqXqjrPxyxUNso1QHS

---------

Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
Co-authored-by: Michael Ramos <mdramos8@gmail.com>
2026-07-26 12:19:15 -07:00
Kevin 0eda139cbe Add an option to disable AI features (#1129)
* feat: allow disabling Plannotator AI

* fix: enforce disabled AI review endpoints

* fix: fully disable AI review surfaces

---------

Co-authored-by: Kevin <kcosrdev@gmail.com>
2026-07-26 09:59:15 -07:00
Kevin 193b07e22c fix(review): bound memory for large untracked files (#1118)
Co-authored-by: Kevin <kcosrdev@gmail.com>
2026-07-24 19:22:13 -07:00
Ben Newman 3455d8285f fix(review): treat cross-drive relative() results as repo escapes (#1117)
- path.relative returns the target's absolute path when base and target
  are on different Windows drives; normalizeAgentPath's escape guard only
  checked ".." and "/" prefixes, so such results slipped through and were
  glued onto the repo label (e.g. "api/L:/checkout/src/file.ts")
- extract the guard into isRepoRelative and reject drive-letter-prefixed
  results at all three call sites
- fixes the annotation-path assertions in review-workspace.test.ts, which
  failed on Windows whenever the checkout and %TEMP% live on different
  drives; add a platform-independent unit test for the new guard
2026-07-24 19:22:10 -07:00
Michael Ramos 99d11dca04 Persist Guided Reviews across sessions (#1115)
* feat(guide): add durable guide store with repo-scoped keys and opt-out

Runtime-agnostic guide persistence for #1112: packages/shared/guide-store.ts
writes validated guides to ${PLANNOTATOR_DATA_DIR}/guides/{repo-key}/{id}.json
with atomic tmp+rename writes and graceful corrupt-file handling. The repo key
is a sanitized host__owner__repo from the origin remote (or the PR url), with
a dir-name+hash8 fallback when no remote parses, so PR and branch sessions of
one repository share a shelf and same-named branches in different repos never
collide. Includes the session glue (repo-key/headSha/label resolution plus the
jobId-to-savedId map) shared by both server runtimes, the guideHistory config
key with resolveGuideHistory (PLANNOTATOR_GUIDE_HISTORY, coerced booleans),
and the browser-safe SavedGuideListEntry/CodeGuideData extensions.

* feat(guide): autosave guides and serve saved: ids in both server runtimes

Both packages/server/review.ts and the Pi mirror serverReview.ts now: autosave
a guide the moment it passes the existing validateGuideOutput gate (including
manual-repair submits); write reviewed-state changes on a live job id through
to that job's saved file; serve persisted guides through the existing guide
endpoints as saved:{id} pseudo job ids (GET guide + PUT reviewed); and expose
GET /api/guides (repo-scoped list with progress and a moved flag comparing the
stored head sha to the current head) and DELETE /api/guides/:id. guide-store
joins vendor.sh's flat copy list; cross-runtime endpoint wiring is covered by
packages/server/guide-persistence.test.ts against both servers, including
reviewed-state persistence across a server restart and traversal-id rejection.

* feat(guide): previous-guides list, Saved chip, and outdated Regenerate hint

GuideEmptyState grows a Previous guides section under the Generate controls:
rows show the target label chip, title, age, reviewed progress, a quiet diff
changed flag when the stored head no longer matches, and a per-row delete;
clicking a row loads the guide via its saved:{id} pseudo job id (the existing
useGuideData/GuideScreen id plumbing already treats ids as opaque). GuideView
shows a small Saved chip once the active guide is persisted and, for an
outdated saved guide, one muted hint line whose Regenerate action launches a
fresh guide with the persisted defaults. The engine/model resolution and
launch-param shapes move into the shared useGuideLaunch hook so the empty
state and the hint stay in lockstep. DOM tests cover the new GuideView states.

* docs: document guide persistence endpoints and PLANNOTATOR_GUIDE_HISTORY

* fix(guide): label saved envelopes with launch-time context, not completion-time state

Review finding on #1115: saveForJob read the live session getters when the
job COMPLETED, but guide jobs run for minutes while the session supports
mid-generation PR switching (/api/pr-switch) and diff switches. Launch on PR
A, switch to B, complete: the envelope permanently carried A's content
labeled with B's PR label/url/headSha (and could even land on B's repo
shelf). The review-target context (pr url/label/head, branch label, head
sha) is now snapshotted at job LAUNCH via guideStore.captureLaunchContext()
in the guide buildCommand branch of both runtimes and carried on the job
itself as AgentJobInfo.guideContext, the same discipline as
changedFilesSnapshot, so it is garbage-collected with the job and needs no
separate cleanup. saveForJob prefers the snapshot (falling back to the live
getters only for jobs launched without one), derives the shelf from the
launch-time PR url, and records the shelf alongside the saved id so reviewed
write-through follows the file wherever it landed. Repair jobs reuse the
FAILED job's own snapshot. Covered by new session tests that mutate the
injected getters between launch capture and completion.

* fix(guide): close a saved guide when the review context switches

Review finding on #1115: a saved:{id} guide has no AgentJobInfo, so
GuideScreen's context match passes trivially (unknown ids are tolerated for
the demo path). Switching PRs or worktrees while a saved guide was open left
it mounted over the new context's diff with a stale moved flag. App.tsx now
clears activeGuideJobId on any prMetadata.url / activeWorktreePath change
when it points at a saved: id; the user reopens it from the Previous guides
list. Live job ids are untouched, GuideScreen's own matching handles those.
2026-07-23 15:32:43 -07:00