mirror of
https://github.com/backnotprop/plannotator.git
synced 2026-09-14 14:17:26 +08:00
v0.26.1
939 Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
 Michael Ramos
|
50a54c872b | chore: bump version to 0.26.1 v0.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
|
8396eeb787 |
ci: auto-sync install scripts to their dedicated S3 bucket (#1214)
plannotator.ai serves /install.sh, /install.ps1, and /install.cmd from the plannotator-install-scripts bucket, not the marketing bucket the deploy workflow syncs. Nothing in CI wrote to that bucket, so the served scripts went stale between hand syncs: v0.26.0 shipped while the site still served the July 31 installer, missing --skip-skills, the checkout guard, and the PowerShell < 7.2 fix. This adds a deploy-install-scripts job triggered by changes to the three scripts (plus a workflow_dispatch target), uploading them with content types and invalidating exactly those paths. |
||
|
Michael Ramos
|
f555168deb | chore: bump version to 0.26.0 v0.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
|
9c693ae348 |
fix(opencode): drop the bun peerDependency that made npm download a 50MB runtime (#1204)
npm >= 7 auto-installs peer dependencies, and the npm registry package named bun ships the full Bun binary, so every install of @plannotator/opencode pulled a useless ~50MB second copy of Bun. Express the runtime requirement as an informational engines field instead, which npm never installs. Also update the stale fixture comment that referenced the peer dependency's install weight. |
||
|
Michael Ramos
|
e97b703479 |
ci: glob the OpenCode smoke tarball instead of pinning the version (#1203)
The installed-package smoke passed a hardcoded plannotator-opencode-0.25.1.tgz path to the fixture, so every version bump would silently break the job on the bump commit. npm pack writes exactly one plannotator-opencode-*.tgz into RUNNER_TEMP, so a glob resolves to the same file and survives bumps. |
||
|
Michael Ramos
|
10a5104888 |
fix(install): repair the skills checkout guard, add --skip-skills (#1201)
* fix(install): make a failed skills checkout stop reporting success The skills/commands checkout runs in a subshell written as `( set -e; ... ) || checkout_failed=1`. POSIX ignores `set -e` for every command of an AND-OR list except the last, and bash 3.2.57 (what `curl | bash` gets on macOS), bash 5.3, dash, zsh and ksh all carry that suppression into the subshell. The `set -e` was inert. A failed clone therefore ran the whole block anyway, the subshell exited with the status of its trailing `if` (0), `checkout_failed` stayed 0, and the installer printed "YOU'RE ALL SET!" with no skills installed. It also blamed the wrong thing, printing "Tag vX.Y.Z predates the per-agent skill layout" when the real cause was a failed clone. Drop the inert `set -e` and guard the four fetch steps with explicit `|| exit 1`. Everything after the checkout stays best-effort, matching install.cmd, which only checks git clone and lets every xcopy run unchecked. A local cp/mkdir/rm failure must not surface as the "network or git error" message. Verified on bash 3.2.57 in a sandboxed HOME: the failure case now exits 1 with the fetch error and no success banner, and original vs patched success runs produce byte-identical trees (74 paths, 35 files) and identical logs. * feat(install): add --skip-skills opt-out The skills and slash commands come from a sparse `git clone` of the release tag. There was no way to decline that fetch short of --minimal, which also drops the sem sidecar, the agent-terminal runtime, the hooks, and every per-agent config. Anything that installs a tag github.com cannot serve had no option at all. Add --skip-skills to all three installers, following the existing --skip-codex / --skip-gemini / --skip-kiro / --skip-opencode family: CLI flag (-SkipSkills in PowerShell), PLANNOTATOR_SKIP_SKILLS_INSTALL env var, skipInstall.skills config key, resolved flag > env > config. It is not a per-agent switch; it covers every scope the checkout writes (Claude, ~/.agents, OpenCode, Gemini, Kiro), the extras, and the skill-scope cleanup sweeps. Skip means do-not-write: nothing already installed is replaced or removed, and git stops being a hard requirement. The run reports "Skills: skipped (<source>)" and the closing banner no longer claims the /plannotator-* commands are ready, which is the same false-success the checkout guard exists to prevent. Use it in the install-script-smoke job. That job installs a synthetic v9.9.9: the fake curl serves the freshly built binary for any URL, but the skills clone goes to real github.com, where the tag does not and cannot exist. That clone has always failed; it only went unnoticed while the broken guard let the installer exit 0 anyway. The job asserts Codex hook config, not skills, so it opts out rather than ignoring a real error. Both run_installer call sites go through the one function definition. Verified in an env -i sandbox on bash 3.2.57 (what `curl | bash` gets on macOS) with a fake curl and a local stand-in remote. Flag, env var, and config each skip and name their own source; flag beats env=0; env=0 beats config true; an explicit "skills": false stays a veto. Without the flag, pre-change and post-change runs produce byte-identical trees (39 entries) and identical logs. A bad clone URL without the flag still exits 1 with the fetch error and no success banner. A --skip-skills re-run over an existing install leaves all 12 skill and command files byte-identical. The CI step was reproduced locally: both run_installer calls exit 0 and every Codex assertion still passes. * test(install): cover --skip-skills and repoint the pinned source strings scripts/install.test.ts asserts against exact install-script source text, so six assertions broke when --skip-skills landed. Each is repointed at the new string with its intent preserved, not weakened: - The "hook/config writing happens before the git hard-fail" ordering test keeps proving the ordering; it just matches the gate's new conditional form. git being a hard requirement is now a narrower invariant (it applies only when the checkout actually runs), so that is asserted separately rather than dropped. - The skipInstall walk assertions follow codex/gemini/kiro/opencode gaining a skills entry, in install.sh's `for _agent` loop and install.cmd's PowerShell key list. - The three "never remove" sweep assertions follow the Codex stale-skill cleanup gaining its skills-opt-out arm, in all three installers. Add nine tests covering --skip-skills itself in the same style as the per-agent family: flag/switch parsing, PLANNOTATOR_SKIP_SKILLS_INSTALL, skipInstall.skills, and flag > env > config precedence by textual layering, for each installer. Each installer also gets a test that the opt-out bails before the clone and leaves the checkout guard intact (#1201's fix must keep failing a real fetch error), and one that the run reports honestly, never prints the "commands are ready" banner over an empty skills dir, and suspends the extras, the model-invocation rewrite, and the stale-stub sweeps rather than applying them partially. bun test scripts/: 116 pass, 6 skip, 0 fail. Full bun test: 2884 pass, 234 skip, 0 fail. bun run typecheck clean. |
||
|
Michael Ramos
|
d7be3406f6 |
fix(ci): repair the OpenCode 2 installed-package smoke (#1202)
The smoke's wait budgets were sized on a warm macOS dev box (5s to a healthy server, 20s to plugin activation). On a cold Linux runner OpenCode 2 needs longer to boot and has to install the packed plugin plus its whole dependency closure through the fixture's throwaway registry first, so the job has failed on every run since it was introduced. Measured, same opencode2 build and the same packed tarball: macOS, warm caches: healthy 0.8s, plugin activated 6.3s linux/amd64 container: healthy 8.2s, plugin activated 45.3s Raise the budgets to 120s and 300s (overridable via PLANNOTATOR_SMOKE_HEALTH_TIMEOUT_MS / PLANNOTATOR_SMOKE_PLUGIN_TIMEOUT_MS) and give the job a 25 minute backstop. The assertion is untouched: the smoke still requires the plugin registry to report the plannotator plugin. Also make a failure legible and prompt. Each poll gets a per-request timeout so one wedged request cannot swallow the budget, waits report progress, failures carry the elapsed time and the last HTTP status/body, and teardown escalates to SIGKILL and force-closes the registry. The CI failure previously burned five minutes in teardown before printing anything. |
||
|
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
|
308e4ba8a5 |
fix(opencode): drop runtime dependency on prerelease plugin nightly (#1199)
* fix(opencode): drop runtime dependency on prerelease plugin nightly
`@opencode-ai/plugin` was a runtime dependency pinned to the exact nightly
0.0.0-next-16775, so every `npm install @plannotator/opencode` resolved a
prerelease snapshot sitting inside npm's 72h unpublish window and pulled
95MB across 101 packages (effect@4.0.0-beta.101 alone is 47MB). None of it
is executed by OpenCode 1 users.
The only runtime use was `Plugin.define`, which is an identity function
(`export function define(plugin) { return plugin; }`, verified identical
across 0.0.0-next-16775, next-16600, next-16797 and stable 1.18.13). The
import is now type-only and the plugin is a plain object literal checked
with `satisfies Plugin.Plugin`. OpenCode 2's loader validation is purely
structural (`Schema.Struct({ id: String, setup: function })` in its
supervisor), so an object literal satisfies it.
The package moves to devDependencies. Built `dist/index.js` and
`dist/embedded.js` are byte-identical to the pre-change build;
`dist/server.js` differs only by the dropped import and the two
`Plugin.define(...)` wrapper lines.
* docs(opencode): explain why the V2 logReady callback is empty
The old one-liner read as an unfinished TODO. The empty function is
correct: OpenCode 2's server-plugin Context exposes no `log` or `tui`
domain, `@opencode-ai/client` has no `tui` namespace and zero `/tui/*`
routes (checked on 0.0.0-next-16775 and next-16797), and `createV2Client`'s
`app.log` bottoms out in `console.error`, the same stderr stream
`handleServerReady` already writes to. Wiring it would print the session
URL twice in remote mode and add a stray line locally. V1 targets
`client.app.log` and `client.tui.showToast`, which are HTTP surfaces
distinct from stderr, so V1 never repeats itself. `tui.toast.show` exists
in V2 only as a subscribe-only event and on the separate
`@opencode-ai/plugin/tui` context, a different plugin kind in a different
process, so a real toast needs an upstream OpenCode API.
Comment only, no behavior change.
|
||
|
Sergiy Dybskiy
|
050dfcda9a |
feat(opencode): add OpenCode 2 plan review adapter (#1194)
* feat(opencode): add OpenCode 2 plan review adapter * fix(opencode): harden V2 review lifecycle * fix(opencode): address V2 review feedback * fix(opencode): update V2 target and isolate tests * test(opencode): assert prompt composition invariants |
||
|
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. |
||
|
Syahrul
|
0d9128794f |
fix: release pi review ports on session shutdown (#1160)
* fix: release pi review ports on session shutdown * fix(pi): self-preempt stale sessions on fixed-port contention A session abandoned without a decision keeps listening on the fixed remote port in this long-lived process, so the next command's bind fails (#1159). On a port-in-use or range-exhausted start failure, stop every tracked browser session and retry once. Also: guard closeAllConnections for older runtimes, downgrade the superseded session notification from error to info, and cover the preemption and stop-drain paths with tests. * fix(pi): harden session preemption per independent review Age-gate the preemption sweep so a concurrent command's fresh session is never stopped, drop the session_shutdown sweep (sessions deliberately outlive in-process session replacement so cross-session feedback keeps working; process teardown frees ports anyway), make server stop() and the sweep exception-safe, register archive sessions in the registry, type the stopped error instead of matching prose, handle a stopped plan review without a false startup error, keep the tests from launching a real browser, and pin the call-site wrappings. --------- Co-authored-by: Michael Ramos <mdramos8@gmail.com> |
||
|
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
|
0935cf08ca | fix(marketing): avoid sharing article redirect chain (#1180) | ||
|
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. |
||
|
Michael Ramos
|
15bca1390e |
fix(install): stop PowerShell < 7.2 from treating git progress as an error
Under $ErrorActionPreference=Stop with stderr redirected, older PowerShell turns a native command's first stderr line into a terminating error, and git prints its normal "Cloning into ..." progress on stderr, so the skill install failed on the message announcing the clone had started. Scope the two git calls to a local Continue preference; success and failure stay detected by Test-Path and exit codes, never by throws. Reproduced and verified under pwsh 7.1.3 (throws before, survives after; a real clone failure still reports). Fixes #1162. |
||
|
skredik
|
80065c8462 |
perf(review): virtualize Guided Review file cards (#1158)
* perf(review): virtualize guided review file cards * fix(review): preserve guided review card behavior |
||
|
tbontb
|
07c89ff145 |
fix(install): authenticate api.github.com call to avoid 60/hr rate limit (#1157)
* fix(install): authenticate api.github.com call to avoid 60/hr rate limit All three installers called api.github.com/repos/.../releases/latest unauthenticated to resolve the latest release tag. GitHub caps unauthenticated REST API requests at 60/hour per source IP (shared across NAT/CGNAT/corporate proxy users), causing opaque 'Failed to fetch latest version' errors. Each installer now detects a token (precedence: GITHUB_TOKEN env > GH_TOKEN env > 'gh auth token') and attaches it as an Authorization: Bearer header to ONLY the version-resolution API call. Falls back to anonymous (unchanged behavior) when no token is available. Release downloads and git clone are left unauthenticated - they are not subject to the 60/hr REST limit. Token variables are cleared from memory immediately after the API call (defense in depth). install.ps1 wraps the API call in try/catch to surface a friendlier error instead of a raw terminating exception under $ErrorActionPreference=Stop. Docs updated with a rate-limited-IP install section and a troubleshooting entry. Parity + negative-assertion tests added. Closes #1156 * fix(install): retry anonymously when authenticated api call fails Addresses review feedback on #1157: a stale/revoked token (expired GITHUB_TOKEN lingering in CI images, dotfiles, direnv) gets a 401 and would break an install that works fine anonymously today. Each installer now retries the api.github.com version-resolution call without the auth header when the authenticated attempt fails: - install.sh: `|| true` + `[ -z "$latest_tag" ]` gate - install.ps1: try/catch, retries with no -Headers on catch - install.cmd: ERRORLEVEL check, retries without !GH_AUTH_HEADER! A bad token costs one extra request; nobody is worse off than today. New test asserts the retry path exists in all three installers (89 pass). Also corrects the AI disclosure: the assistant was opencode, not Claude. * fix(install): address second-review findings Errorlevel check adjacency in install.cmd, delayed-expansion token reads, token charset guard, PATH-resolved gh with --hostname github.com, 401-only anonymous retry in install.sh and install.ps1, preserved original errors in install.ps1, honest cleanup comments, non-vacuous test assertions, and doc corrections (1000/hr Actions token, zero-scope guidance, -Version for PowerShell). --------- Co-authored-by: tbontb-iaq <tbontb-iaq@users.noreply.github.com> Co-authored-by: Michael Ramos <mdramos8@gmail.com> |
||
|
Michael Ramos
|
b5cf065cba | chore: bump version to 0.25.1 v0.25.1 | ||
|
Michael Ramos
|
c353413b55 |
fix(hook): enable the annotate client lease on OpenCode's last-message bridge
#1143 wired abandoned-gate dismissal into three of the four startAnnotateServer call sites. The OpenCode annotate-last bridge takes gate from stdin JSON rather than CLI flags and was missed, so /plannotator-last --gate under OpenCode still hung on waitForDecision forever once every review tab was abandoned: exactly the hang that commit set out to close. The bridge's inputs map onto the same predicate the other three use: gate from the stdin payload, json unconditionally true because emitOpenCodeAnnotateOutcome is the branch's only output path and always writes a structured record the bridge parses back, hook false because no flags are parsed here. The new test scans every startAnnotateServer call site in index.ts rather than pinning this one line, so the next site added cannot repeat the omission. Claude-Session: https://claude.ai/code/session_01H5KQWqXqjrPxyxUNso1QHS |
||
|
Michael Ramos
|
4c89418bad |
fix(pi): contain the pi-todos mirror inside the project root
The mirror resolved <cwd>/.pi/todos with a plain path.join, so a repo that ships .pi/todos as a symlink pointing elsewhere had its plan checklist written to the symlink target on the first plan approval. The mirror is on by default, so checking out a hostile repo and approving a plan was enough. Write-only and non-overwriting, but not something to ship knowingly. The repo-implied path must now realpath inside cwd or the provider reads as absent, which is the documented no-mirror no-op. An explicit PI_TODO_PATH stays trusted verbatim: that is the user's own choice at the same trust level as config.json, and pi-todos honors it the same way. Containment reuses the shared isWithinDirectory sink, resolved through the deepest existing ancestor so a not-yet-created todos dir under a symlinked .pi cannot be created outside either. Also corrects the detection docstring: pi-todos calls ensureTodosDir on session_start, so an installed-but-unused provider does not read as absent. Claude-Session: https://claude.ai/code/session_01H5KQWqXqjrPxyxUNso1QHS |
||
|
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> |
||
|
Michael Ramos
|
383ac97a8e |
fix(pi): survive pi's stale-ctx guard in the post-approval continuation
Pi >= 0.69 invalidates an extension's ctx on session teardown or replacement, and every guarded member throws thereafter. The plan continuation timer polled ctx.isIdle() from a setTimeout, so a session dying mid-poll (print-mode teardown, /new or /reload right after approval) crashed the whole pi process with an uncaught exception. Cancel the continuation instead: a sessionAlive latch off the sanctioned session_shutdown event plus an isCtxAlive() probe, and the timer body never rethrows. Also .catch() the fire-and-forget browser open, which could crash pi via unhandled rejection on a failed launcher even without staleness. Fixes #1140 Claude-Session: https://claude.ai/code/session_01H5KQWqXqjrPxyxUNso1QHS |
||
|
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. |
||
|
mararn1618
|
5d1544fa32 |
fix(hook): route annotate-last to the live Copilot CLI session (#1150)
* fix(hook): add Copilot session lock detection Copilot CLI exports no identifying environment variable, so nothing distinguishes a Copilot session from a plain shell. Match ancestor pids against session-state inuse locks to find the live session, and only accept a match when the lock owner still names a copilot process, since locks can outlive their session and pids get reused. * fix(hook): route annotate-last to the live Copilot session Under Copilot CLI, annotate-last silently fell back to the default transcript reader and annotated a message from a different tool. Take the Copilot branch when an ancestor process holds a session lock, or when PLANNOTATOR_ORIGIN=copilot-cli is set with the cwd heuristic as fallback, and report origin copilot-cli to the annotate server. * fix(hook): prefer ancestor lock match in copilot-last The cwd heuristic can pick a stale session when several exist for one repo. Resolve the session locked by an ancestor copilot process first and keep the heuristic as fallback. * docs(cli): document copilot-last in help The subcommand worked but was missing from the top-level usage and the per-subcommand help map. |
||
|
Souptik Chakraborty
|
26681abb8c |
fix(amp,droid): surface Approve-with-Notes feedback instead of dropping it (#1146)
The Approve with Notes flow (#1092) emits decision:"approved" with a feedback field, but both adapters collapsed every approved decision to a bare "Approved." -- the reviewer's notes were silently lost. Amp had two drop sites: formatAnnotationFeedback returned null for anything not "annotated", and handleAnnotateResult early-returned on approved before formatting was ever reached. Droid's emitAnnotateDecision printed only "Approved." Both adapters now emit the shared approved-with-notes prompt (mirroring DEFAULT_ANNOTATE_APPROVED_WITH_NOTES_PROMPT in packages/shared/prompts.ts); note-less approvals keep the old behavior. The amp config override key approvedWithNotes matches the shared prompt config shape. Fixes #1137 |
||
|
BrandonNoad
|
8d8e643976 |
fix(hook): read annotate-last from Claude Code's transcript tree, not file order (#1141)
* fix(hook): read annotate-last from the transcript tree, not file order Claude Code session logs are append-only and tree-shaped: every entry records the entry it follows in `parentUuid`. `/rewind` writes nothing at all. The next committed message simply re-parents to an earlier entry, leaving everything after it orphaned in the file forever. `extractRecentRenderedMessages` scanned bottom-up in file order, so those orphans were still offered in the annotate-last message picker even though they are no longer part of the conversation. On a rewound session in this repo the picker listed 10 messages where only 7 are live. Add `resolveActiveBranchIndices`, which walks `parentUuid` from the newest id-bearing entry back to the root. It returns indices rather than a filtered array so callers keep reporting real file line numbers, and returns null on a chain it cannot trust (no ids, dangling parent, cycle) so callers degrade to the previous file-order read instead of returning nothing. Note the newest entry is not always the last line: `last-prompt`, `ai-title`, `mode` and `file-history-snapshot` carry no ids and are often written last. Opt in at the Claude Code call site only. Droid's call site, Codex and Copilot (separate parsers), and Pi/OpenCode/Amp (live APIs, no transcript reads) are all unaffected. Both new parameters default to off. Checked against 311 local transcripts: every one walks cleanly to the root with no dangling parents or cycles, and the default pick is byte-identical under both readings in all 298 that contain a message. That is expected, since a committed rewind's new branch is always the newest lines in the file. The picker is where the difference shows up. Test fixtures previously assigned random `parentUuid`s, which left every entry an orphan and made branch resolution untestable, so `buildLog` now links them into a real chain and `buildRewoundLog` models a fork. * fix(hook): fail open when the active branch has no assistant messages A /compact boundary is written with parentUuid: null, so it is a tree root: the active-branch walk stops there and a freshly-compacted session yields zero messages. Callers treat an empty result as "wrong log file" and walk off to an older session, so fail open to the file-order read instead. Adds tests for the compaction cut, the fallback, and the post-compaction recovery. Claude-Session: https://claude.ai/code/session_01H5KQWqXqjrPxyxUNso1QHS --------- Co-authored-by: Michael Ramos <mdramos8@gmail.com> |
||
|
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_01H5KQWqXqjrPxyxUNso1QHSv0.25.0 |