mirror of
https://github.com/backnotprop/plannotator.git
synced 2026-09-14 14:17:26 +08:00
v0.26.1
617 Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
 Michael Ramos
|
50a54c872b | chore: bump version to 0.26.1 | ||
|
Michael Ramos
|
b1745683fd |
fix(review): fall back across GitButler JSON flag syntaxes (but 0.22.0) (#1216)
GitButler 0.22.0 removed the global --format flag in favor of --json (gitbutlerapp/gitbutler#15026), so `but --format json status` now dies with clap's unexpected-argument error and GitButler review sessions fail to start. 0.21.x accepts only --format json (gitbutlerapp/gitbutler#14061), so neither spelling works everywhere. Keep --format json as the primary invocation and, only when it fails with clap's narrow unexpected-argument rejection for the exact flag we passed, retry once with the other spelling. Real status failures never retry and keep failing loudly per the module's contract-error philosophy. The accepted spelling is remembered per runtime so 0.22.0 installs pay the failed probe once. Error strings now name the syntax actually used. Closes #1215 |
||
|
Michael Ramos
|
f555168deb | chore: bump version to 0.26.0 | ||
|
Michael Ramos
|
5fbff4bff3 |
fix(review): make the Edit Mode opt-in an explicit switch, not a primary CTA (#1213)
The announcement footer's primary Turn it on button read as a generic continue and invited blind clicks: opting into an experimental feature was one reflexive Enter away. The decision is now an explicit Enable Edit Mode switch (defaulting off, with the shared TextShimmer label treatment) beside a single neutral Done button that applies whatever the switch says. Pressing Done untouched is a plain dismissal, and the focused default action is asserted consent-neutral in tests. Announcement cookie version bumped so the revision re-shows (pre-release, no users have the old cookie). |
||
|
Michael Ramos
|
a4816e7208 |
feat(review): announce Edit Mode with an opt-in welcome dialog (#1207)
* feat(review): announce Edit Mode with an opt-in welcome dialog One-time cookie-gated announcement for the experimental Edit Code to Suggest feature, slotted last in the first-run dialog chain (guide intro, look-and-feel, review setup, edit mode) so the chain dialogs never stack. Turn it on persists the real editSuggestions setting; Keep it off just marks the announcement seen. The demo panel carries a video slot that the single-file build inlines as a data URI; until the recording lands it renders a static placeholder behind one constant. * feat(review): bundle the Edit Mode demo recording into the announcement Wires the quality-verified screen capture (VP9 webm, ~563KB, 1100x700, 18.8s) plus its poster still through the editModeDemoMedia swap point. The single-file build inlines both as data URIs (verified in the built bundle). The video slot autoplays muted, loops, plays inline, and keeps the recording's 11:7 aspect so nothing is cropped. The static placeholder stays as a test seam and safety net, with its coming-soon caption removed. |
||
|
Michael Ramos
|
7aff70281f |
fix(review): repaint pristine diff immediately on edit-mode Discard (#1209)
writeRestore's combined item write leaves upstream's highlighted render cache serving the stale edited content; clear the live instance's render cache and rerender so the pristine diff paints immediately (plaintext first, then the normal async highlight). Covers Discard, Suggest, finishIfEditing, and the deferred external-teardown restore. Adds a DOM-gated integration test driving the real @pierre/diffs CodeView + EditProvider + worker pool through startEdit -> real Editor.applyEdits -> cancelEdit, asserting the pristine content is back in the shadow DOM within a bounded settle. |
||
|
Michael Ramos
|
3d435184dc |
fix(review): keep large-diff memory bound when the object-size probe fails (#1205)
A failed `cat-file --batch-check` used to map every changed object to infinity, replacing the ENTIRE review diff with `Binary files ... differ` stubs and no visible error, and silently degrading the staleness fingerprint. The memory bound is now probe-independent: every rendered diff carries `core.bigFileThreshold=<MAX_REVIEW_FILE_CONTENT_BYTES>` injected through `GIT_CONFIG_*` environment variables (never `-c` argv flags, so argv stays byte-identical), making git itself stub oversized blobs. On probe failure blob sizes read as unknown-but-bounded and files render normally; the stat-based exclusion door for oversized working-tree files (which git's threshold does not cover) never depended on the probe and keeps working. Per-object doors (missing / unparseable size) stay conservative when the probe ran. The probe itself gains timeoutMs + interaction:"forbid" so a hung git cannot stall the review server. Three existing mocks that returned non-batch-check output (and passed only because of the all-infinity bug) now return well-formed batch-check lines. |
||
|
Michael Ramos
|
6b8723cc57 |
test(review): repair guide virtualization mock and run its tests in CI (#1200)
The AllFilesCodeView lifecycle test stubs '@pierre/diffs/react' with mock.module, which replaces the whole specifier. #1193 added an EditProvider import to AllFilesCodeView, but the stub was never extended, so the file throws at import when run on its own. The three DOM tests #1158 added for Guided Review virtualization were also never registered in the DOM_TESTS=1 CI step, so nothing enforced the 8-viewer mount cap and nothing surfaced the broken stub. |
||
|
Michael Ramos
|
7682628db7 |
feat(install): Codex opt-out and credential-free attestation verification (#1197)
* feat(install): Codex opt-out and credential-free attestation verification Implements both asks from #1178 (reported and designed by @astradevkin): - Per-agent installer opt-outs: --skip-codex / --skip-gemini / --skip-kiro flags, PLANNOTATOR_SKIP_{CODEX,GEMINI,KIRO}_INSTALL env vars, and config.json skipInstall.{codex,gemini,kiro} keys, with flag > env > config precedence mirroring verifyAttestation. Detected-but-skipped is reported as its own honest state, never conflated with not-detected, and a skipped agent's home is neither written nor cleaned up. - Credential-free provenance verification: when --verify-attestation is active, the Sigstore bundle is fetched from GitHub's public attestations endpoint (single unauthenticated attempt, no retry) and verified via gh attestation verify --bundle with the same --repo/--source-ref/--signer-workflow constraints; gh's authenticated fetch remains the fallback. TUF trust-root failures are reported as connectivity, distinct from real provenance failures; every path stays fail-closed. Zero behavior change for users who do not opt in: the default install path is unchanged (verified by sandbox-HOME parity runs against main). * fix(install): address #1197 review round (H1 retry, M2-M7, lows) - H1: a failed gh --bundle invocation now retries once through the exact authenticated path before any classification, so an older gh (unknown flag) or a corrupt bundle never reports as a provenance failure. Pinned by a functional stub-gh test; a real failure still fails again on the retry and aborts. - M2: the sh config layer extracts the skipInstall object (awk, character indexed) before matching per-agent keys and honors explicit false as a veto; cmd now parses the real JSON via PowerShell like ps1. Functional tests cover the foreign-key collision and explicit-true cases. - M3: sh names the real cause when the bundle path cannot run (no JSON extractor vs fetch vs extraction failure) and gains python3 and jq fallback extractors; docs state the dependency. - M4: ps1/cmd gate the existing-integration note on plannotator content in hooks.json and word the skip state as what those platforms actually do (manual instructions suppressed). - M5: sh bundle lives inside a private mktemp -d, one rm -rf on every exit, and a mktemp failure degrades to the fallback instead of aborting. - M6: README, verifying-your-install, environment-variables, and installation docs updated for the credential-free path and skip flags. - M7: ps1/cmd extraction replaced with a byte-exact string scanner (no ConvertFrom/ConvertTo round trip, immune to DateTime coercion), with PowerShell-driven unit tests over the captured real attestations response plus synthetic DateTime and brace-in-string controls. - Lows: fail-closed abort pinned by a functional test; TUF beats auth in cmd classification (matches sh/ps1); skipped-state output mentions the shared ~/.agents/skills; Gemini summary is skip-aware and gains an honest not-detected state; --skip-opencode do-not-write switch added (flag, env var, config key) across all three scripts. * fix(install): review round three (EncodedCommand fetcher, mutation-proof fail-closed test, lows) - R1: install.cmd's attestation fetcher no longer touches disk. The %RANDOM%-named %TEMP% .ps1 (predictable-path code execution, the M5 class escalated) is replaced by powershell -NoProfile -EncodedCommand with a base64(UTF-16LE) payload defined next to its full REM PS: plaintext; a test decodes the blob and asserts byte equality with the documented lines plus the security-relevant shape (env-var inputs, ordinal scan, no JSON round trip, distinct exit codes). Inputs still travel via env vars. Verified end to end under pwsh: the decoded blob fetched the real attestations response, wrote 2 bundles, gh verified the real v0.25.1 binary credential-free (exit 0) and rejected a wrong binary (exit 1). - R2: the fail-closed test now asserts the output ENDS with 'Refusing to install.' - mutation-verified: with the verify-failure exit 1 deleted the mutant still exits 1 via an incidental mv failure, but the trailing mv error breaks the endsWith and the test fails; restored, it passes. - Lows: CI guard test fails loudly when process.env.CI is set and no pwsh/powershell is on PATH (scanner coverage cannot silently vanish); scanner IndexOf calls are ordinal in ps1 and the encoded cmd variant; the awk skipInstall extraction requires optional-whitespace-then-colon- then-brace after the key (string values can no longer anchor it, with non-token occurrences skipped, unit-checked against escaped-embedded payloads); install.cmd comments warn that the fallback-reason literals inside parenthesized blocks must stay parenthesis-free. |
||
|
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. |
||
|
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> |
||
|
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). |
||
|
Michael Ramos
|
84846d9b9d | chore(deps): bump @pierre/diffs to 1.3.2 (#1191) | ||
|
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). |
||
|
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). |
||
|
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
|
||
|
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. |
||
|
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. |
||
|
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 |
||
|
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. |
||
|
Michael Ramos
|
2ab377f77d | fix(uninstall): require host cleanup (#1177) | ||
|
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 |
||
|
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 |
||
|
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 |
||
|
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. |
||
|
skredik
|
80065c8462 |
perf(review): virtualize Guided Review file cards (#1158)
* perf(review): virtualize guided review file cards * fix(review): preserve guided review card behavior |
||
|
Michael Ramos
|
b5cf065cba | chore: bump version to 0.25.1 | ||
|
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> |
||
|
Oscar Mederos
|
d06f96decb | fix(ui): focus and select existing search text (#1152) | ||
|
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. |
||
|
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. |
||
|
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> |
||
|
Michael Ramos
|
68c1291cd7 |
chore: bump version to 0.25.0
Claude-Session: https://claude.ai/code/session_01H5KQWqXqjrPxyxUNso1QHS |
||
|
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
|
||
|
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 |
||
|
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>
|
||
|
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> |
||
|
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> |
||
|
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> |
||
|
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 |
||
|
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> |
||
|
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> |
||
|
Kevin
|
193b07e22c |
fix(review): bound memory for large untracked files (#1118)
Co-authored-by: Kevin <kcosrdev@gmail.com> |
||
|
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 |
||
|
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. |
||
|
Michael Ramos
|
3f3b3514a0 |
fix(annotate): use annotate feedback template for clipboard copy, not plan-deny (#1109)
Clipboard Copy paths (ExportModal Copy and the annotation panel quick copy) unconditionally wrapped exported annotations with the plan-deny template, so annotate sessions copied text starting with "YOUR PLAN WAS NOT APPROVED." while Send Feedback used the annotate template (including custom prompts.annotate.* config). The annotate servers (Bun and Pi) now ship the resolved, unsubstituted copy-wrapper templates in the /api/plan payload (feedbackTemplates), and the plan editor wraps copied annotations mode-aware: server template when present, browser-safe default annotate template otherwise, plan-deny only in actual plan review. Send Feedback behavior is unchanged. Closes #1107 |
||
|
Ruaridh Williamson
|
6297358994 |
fix(ui): parse YAML block scalars in frontmatter (#1101)
* fix(ui): parse YAML block scalars in frontmatter Folded (>) and literal (|) values were stored as the bare indicator (e.g. ">-") with their body dropped, so multi-line skill descriptions rendered empty in the plan/annotate viewer. * fix: strip trailing CR from block scalar body lines (CRLF sources) --------- Co-authored-by: Michael Ramos <mdramos8@gmail.com> |
||
|
Michael Ramos
|
9bf46e11f3 | chore: bump version to 0.24.2 | ||
|
Michael Ramos
|
8e27a25d31 |
fix(review): expand collapsed guide files when jumping via section file chips (#1103)
Guide section file chips scrolled directly to the target diff card, so a file collapsed by marking it viewed landed on its bare header with no code. Route the chip click through the existing guideRevealFile channel (the same path used by sidebar annotation clicks and AI line citations) via a new onGuideRevealFile setter on ReviewState, so the target expands, focuses, and scrolls in the same order as the other two jump paths. |
||
|
Michael Ramos
|
5ef89a0a2e |
fix(config): coerce quoted boolean strings in config.json resolvers (#1102)
~/.plannotator/config.json is hand-edited, so boolean settings often
arrive as quoted strings ("cursorSandbox": "false"). The resolvers
passed the raw JSON value through, so a quoted "false" reached strict
=== false checks downstream as a string and silently kept the default
behavior. Concretely, the Cursor sandbox opt-out added for NixOS users
in #1095 did nothing when set via config.json with quotes, even though
the equivalent env var accepts the string form.
Add a coerceConfigBoolean helper and apply it to the config-file branch
of resolveUseGlimpse, resolveAnnotateHistory, resolveUseJina, and
resolveCursorSandbox. Real booleans pass through, "true"/"false"/
"1"/"0" strings (any case) coerce, and anything else falls back to
the default. Env-var branches and their precedence are unchanged.
resolveSharingEnabled is left alone: it is keyed on the string sentinel
"disabled" and already fails safe.
Pi picks the fix up automatically via apps/pi-extension/vendor.sh.
|