mirror of
https://github.com/backnotprop/plannotator.git
synced 2026-09-14 14:17:26 +08:00
b5cf065cbab1a5b0b5edaee90e140e73806504bc
902 Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
 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 |
||
|
Michael Ramos
|
951175ec1c |
fix(pi): ship annotate-outcome.ts in the npm package
The #1092 approval-notes work added annotate-outcome.ts as a top-level import of index.ts but the package.json files array was never updated, so the published tarball failed to load under Pi's jiti loader for every npm-installed user. Found by the pre-release QA sweep; verified the packed tarball now contains the module and no other top-level import is missing from the files array. Claude-Session: https://claude.ai/code/session_01H5KQWqXqjrPxyxUNso1QHS |
||
|
Reveck
|
26a5314a99 |
feat(pi): add external plan execution handoff (#1126)
* feat(pi): add external plan execution handoff * fix(pi): release phase-added tools on external plan handoff #1124 made every idle-exit path hand back the tools the phase added, but the external handoff path introduced here restored the saved model and thinking level without releasing them. Approving a plan in executionMode "external" therefore leaked the planning tool scope (grep, find, ls, plannotator_submit_plan) into the idle session. Extract the shared exit sequence into returnToIdle() and use it from all three idle transitions (manual exit, execution completion, handoff), so a new exit path cannot miss a step again. Callers keep their own messaging, session entries, and events. Also add a default-mode parity test: with executionMode unset, approval enters executing, emits no plannotator:plan-approved, appends the plannotator-execute entry, and still sends the continuation message. Claude-Session: https://claude.ai/code/session_01H5KQWqXqjrPxyxUNso1QHS * chore(pi): warn on unknown executionMode and document the handoff payload An unrecognized executionMode was silently dropped, so a typo like "handoff" left the session in automatic execution with no explanation. Warn through the existing config warning channel (the same one malformed JSON uses) and keep the automatic fallback. README: note that planFilePath is relative to cwd and should be resolved against it, and that companions can subscribe with the raw channel string without importing Plannotator internals, matching how the plannotator:request channel is already described. Claude-Session: https://claude.ai/code/session_01H5KQWqXqjrPxyxUNso1QHS --------- Co-authored-by: Michael Ramos <support@plannotator.ai> |
||
|
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
|
||
|
Peete
|
d26289060d |
fix(pi): preserve extension tool selections (#1124)
* fix(pi): preserve extension tool selections * fix(pi): release phase tools when the plan completes The agent_end plan-completion path dropped back to idle via restoreSavedState but never called releaseAddedPhaseTools(), so tools Plannotator activated for the executing phase stayed active in idle and a stale phaseAddedTools list was persisted. Mirror exitToIdle. Claude-Session: https://claude.ai/code/session_01H5KQWqXqjrPxyxUNso1QHS * fix(pi): keep grep/find/ls in the default planning tool set Dropping them from the planning phase profile silently removed native search from default planning sessions. Restore them as default planning phase tools so they now flow through the new tracking mechanism: they are recorded in phaseAddedTools and released on phase exit, and a discovery tool another extension already activated is neither claimed nor removed. A user's own planning.activeTools still overrides them. Claude-Session: https://claude.ai/code/session_01H5KQWqXqjrPxyxUNso1QHS * fix(pi): always keep the submit tool active during planning Planning tools now come purely from the resolved profile's activeTools, which uses replace semantics: a phases.planning.activeTools in ~/.pi/agent/plannotator.json or cwd/.pi/plannotator.json supersedes the built-in list wholesale. Pi's setActiveToolsByName then replaces the session's tool list outright, so such a config silently dropped plannotator_submit_plan while the planning system prompt kept telling the model to call it - a dead end with no way to submit a plan. Union PLAN_SUBMIT_TOOL into the configured list for the planning phase only (deduped if the config already lists it). It flows through phaseAddedTools like any other addition, so it is released normally on phase exit. Other tools are left to the user: overriding planning.activeTools is a deliberate choice of tool set, and only the submit tool is contractually required for the phase to function. Also corrects the README's "extra tools ... additive" wording, which described the old merge behavior. Claude-Session: https://claude.ai/code/session_01H5KQWqXqjrPxyxUNso1QHS --------- Co-authored-by: Michael Ramos <support@plannotator.ai> Co-authored-by: Michael Ramos <mdramos8@gmail.com> |
||
|
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
|
5aaa420080 |
feat(annotate): add strict atomic result output (#1091)
* feat(annotate): add strict atomic result output * feat(annotate): exit 2 for strict-gate usage and publication errors Adopt the grep convention for the strict annotate gate's exit codes: 0 = approved, 1 = negative human outcome (annotated/dismissed under --require-approval), 2 = the gate itself was misconfigured or could not start/deliver a decision. Previously all usage/startup/validation failures shared exit 1 with "reviewer did not approve", so callers could not tell a denied review from a broken gate. - parseStrictAnnotateOptions failures (bad flag combos, strict flags outside annotate --gate --json) now exit 2 - --result-file preflight failures (missing parent, pre-existing or dangling-symlink destination) now exit 2 - post-decision publication failures (destination raced into existence, hard links unavailable, stdout write failure) now exit 2: they deliver no decision record at all, so the code's own fail-closed handling presents them as environment errors, never as a reviewer outcome -- and never approval, since only 0 means approved - decision outcomes keep 0/1 exactly as before; signal deaths keep 128+n - document the contract in AGENTS.md and the annotate-gates guide Claude-Session: https://claude.ai/code/session_01YXkgsNucxDwAL4GdR4XYRk * fix(annotate): exit 2 for strict-gate startup failures The six startup-failure sites in the annotate path (missing path, unreachable URL, empty folder, ambiguous name, missing/unsupported file, oversized file) run after flag parsing and exited 1. Under --require-approval / --result-file, 1 is the "reviewer requested changes" signal, so a typo'd path made automation misclassify a configuration error as a legitimate rejection. Route those sites through exitAnnotateStartupFailure(), which picks its code from the already-parsed strict options via the new pure helper annotateStartupFailureExitCode(). Non-strict invocations still exit 1 with byte-identical stderr; strict invocations exit STRICT_GATE_ERROR_EXIT_CODE (2). Claude-Session: https://claude.ai/code/session_01H5KQWqXqjrPxyxUNso1QHS * fix(annotate): emit the strict decision on stdout before publishing it writeResultFile ran before the decision JSON reached stdout. On a filesystem without hard links (exFAT, FAT32, most SMB/NFS, some container bind mounts) publication fails deterministically, the catch exited 2 with nothing written anywhere — and the reviewer's autosaved draft had already been deleted by the feedback flow, so their completed decision was lost. Emit the stdout record first, then publish the result file. Exit semantics are unchanged: a publication failure still exits 2, but the decision has reached stdout by then. Only a stdout write failure now leaves no record at all. Correct the docs and comments that claimed exit 2 delivers no decision record: it means the result *file* was not published. Also document the two publication caveats: the 0600 mode is a no-op on Windows, and the atomic link/rename is not followed by a parent-directory fsync, so publication is atomic but not crash-durable. Claude-Session: https://claude.ai/code/session_01H5KQWqXqjrPxyxUNso1QHS --------- Co-authored-by: Michael Ramos <mdramos8@gmail.com> |
||
|
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
|
fc25ddff11 |
Merge pull request #1111 from backnotprop/docs/gitbutler-readme
docs: add GitButler review workflow |
||
|
Devinda Senanayaka
|
929e8f9f59 |
perf(pi-extension): use exact .ts relative specifiers (#1106)
The Pi extension is distributed and executed as raw TypeScript through Pi's jiti loader. Phantom .js paths force jiti through its last-resort fallback, while extensionless paths still require probing. Exact .ts paths avoid both and match the files that actually ship. - rewrite relative .js and extensionless specifiers in hand-written sources (the existing noEmit bundler tsconfig permits .ts imports) - make vendor.sh emit .ts in explicit rewrite rules and normalize both source styles across verbatim-copied generated modules - guard hand-written and generated sources against either inexact form Extension module import: 43.5ms -> 15ms median (PI_TIMING, n=12 interleaved). bun test: 73/73 (pi-extension), 456/456 (server); all seven TypeScript project checks and the packed-artifact audit pass. Co-authored-by: dca123 <dca123@users.noreply.github.com> Co-authored-by: Michael Ramos <mdramos8@gmail.com> |
||
|
Michael Ramos
|
37e8ea2f21 | docs: add GitButler review workflow | ||
|
Michael Ramos
|
7621cd61fd | chore(vscode): bump extension to 0.16.8 (XDG-aware data dir, Open VSX prep #1110) | ||
|
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 v0.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.
|
||
|
Michael Ramos
|
d9099fe089 |
docs: document the 2MB annotatable-file size cap
Claude-Session: https://claude.ai/code/session_01YXkgsNucxDwAL4GdR4XYRk |
||
|
skredik
|
98c761150a | fix(pi): keep PR review progress out of composer (#1098) | ||
|
Michael Ramos
|
3b3c3c3819 |
feat(github): minimal bug-report issue form (#1100)
One required textarea plus four one-click fields: version (plannotator --version), OS, agent, surface. Feature requests stay blank issues. Claude-Session: https://claude.ai/code/session_01YXkgsNucxDwAL4GdR4XYRk |
||
|
Michael Ramos
|
f9a6c1e39d |
feat: annotate accepts YAML, JSON, TOML and other plain-text files (#1099)
* feat(annotate): accept common plain-text config formats (.yaml, .json, .toml, …) Annotate previously rejected every file that wasn't .md/.mdx/.txt (or .html/.htm), even though the pipeline reads files as UTF-8 text and renders anything. Widen the accepted set to unambiguously plain-text config/data formats: .yaml .yml .json .jsonc .json5 .toml .ini .cfg .conf .properties .csv .tsv .log .xml .env.example. They render exactly the way .txt renders today. - New single source of truth: packages/core/annotatable.ts (ANNOTATABLE_TEXT_REGEX / ANNOTATABLE_DOC_REGEX + predicates), re-exported through @plannotator/shared/resolve-file and vendored into the Pi extension. - .env stays excluded (commonly holds secrets; annotate history copies file contents into the data dir). Source-code extensions stay with code review. - Single-file accept + bare-filename fuzzy search widen in resolveMarkdownFile; folder discovery and the file-browser listing widen in all three runtimes (hook CLI, OpenCode, Pi). - /api/doc gains a `doc=1` param set by the file browser so extensions that overlap CODE_FILE_REGEX (.yaml/.json/.toml/.ini/.xml) render as annotatable documents there while code-file links inside documents keep the syntax-highlighted popout. - Error messages now list the wider set; docs updated (AGENTS.md, marketing annotate page). Closes #1029 Claude-Session: https://claude.ai/code/session_01YXkgsNucxDwAL4GdR4XYRk * fix(annotate): frontmatter, size caps, edit-guard, and skill docs from review Review fixes for #1099: - Frontmatter: `--- … ---` stripping is a markdown convention; for non-markdown annotatable sources (multi-document YAML, .txt starting with ---) the delimiters are real content. parseMarkdownToBlocks gains a { frontmatter } option and the editor keys it off the active document's path via shouldStripFrontmatter() (strip for .md/.mdx and pathless/converted sources; keep raw for other annotatable text). - Size caps: new shared MAX_ANNOTATABLE_FILE_BYTES (2MB — same limit the code-file popout always had) now guards the annotate CLI single-file read in all three runtimes and the /api/doc document branches in both servers. Also applies to .md/.txt (behavior change for pathological inputs; previously unbounded). - Editing guard: mid-edit file opens gate on isSourceSaveFilePath (.md/.mdx/.txt) instead of the wider annotatable set — config files are view-only, so switching to one mid-edit no longer silently downgrades "Done editing" to feedback-only edits. - Skill docs: plannotator-annotate SKILL.md (core + Kiro) now mention the plain-text config formats. Claude-Session: https://claude.ai/code/session_01YXkgsNucxDwAL4GdR4XYRk |
||
|
Raúl
|
8042ad07d4 |
fix(install): pass --global to npx skills add so extras do not leak into cwd (#1078)
Apply global scope consistently across the shell, PowerShell, and cmd installers and their public install-later documentation. Add parity coverage so every supported command remains explicitly global. |
||
|
Michael Ramos
|
6fbf0ef560 |
fix: align Codex model catalog and reasoning efforts with the current Codex CLI (#1096)
* fix(agents): align Codex model catalog and reasoning efforts with Codex CLI 0.144 - Remove gpt-5.3-codex from the Codex model picker: saved picks of it are already force-migrated to the fallback (ChatGPT-account Codex rejects it), so offering it was incoherent. The 5.2/5.1 family stays — those models remain available to API-key Codex users. - Per-model reasoning-effort gating: the catalog (now in packages/ui/utils/codexModels.ts, re-exported from AgentsTab) carries each model's supported efforts and default effort from the CLI catalog. The reasoning pickers only offer the selected model's efforts, including the new max/ultra tiers on the GPT-5.6 family, and useAgentSettings clamps an unsupported saved effort to the model's catalog default before it reaches the pickers or a launch payload. - Drop `minimal` everywhere: no current Codex model supports it. Saved perModel reasoning entries migrate minimal -> low, and the guide repair launch no longer hardcodes it. - Ask AI: bump the codex-app-server static fallback model and DEFAULT_MODEL from gpt-5.4 to gpt-5.6-sol (runtime model/list discovery still replaces the fallback and already carries per-model efforts; added Max/Ultra labels). Claude-Session: https://claude.ai/code/session_01YXkgsNucxDwAL4GdR4XYRk * fix(agents): prune API-shut-down Codex models per OpenAI deprecations OpenAI's API deprecations page schedules an API-level shutdown on 2026-07-23 for gpt-5.2-codex (-> gpt-5.5), gpt-5.1-codex-max (-> gpt-5.5), and gpt-5.1-codex-mini (-> gpt-5.4-mini) — dead for ALL auth modes, not just ChatGPT accounts — so they leave the picker and saved picks migrate: the first two to the surface fallback (already gpt-5.5), codex-mini to gpt-5.4-mini with its perModel preference carried under the new key (same pattern as the gpt-5.6 -> gpt-5.6-sol rename). gpt-5.2 stays selectable with no migration: it was retired from the ChatGPT product (steered to 5.5) but the API still serves it, so API-key Codex users retain it. Claude-Session: https://claude.ai/code/session_01YXkgsNucxDwAL4GdR4XYRk |
||
|
Michael Ramos
|
461ab64f92 | feat(ui): soften pinpoint hover feedback (#1097) | ||
|
Jos Dirksen
|
9ee7cb48ac |
fix(review): Collapse viewed files in guided review (#1068)
* fix(review): collapse viewed guided files * fix(review): expand a collapsed guide file when a sidebar jump targets it The per-file viewed-collapse unmounts the diff body, so an annotation click or AI line citation targeting a collapsed file would scroll to the section but silently fail to reveal the line. Mirror GuideSectionCard's token-keyed reveal effect at the file level. Claude-Session: https://claude.ai/code/session_01YXkgsNucxDwAL4GdR4XYRk --------- Co-authored-by: Michael Ramos <mdramos8@gmail.com> |
||
|
Raúl
|
a5c701db01 |
fix(review): use canonical GPT-5.6 Sol model ID (#1076)
Migrate saved review, tour, and guide Codex preferences from gpt-5.6 to gpt-5.6-sol so existing selections continue to launch. |
||
|
Michael Ramos
|
5ff7e542dc |
docs: note the XDG_DATA_HOME fallback requires an absolute path in the README table
Claude-Session: https://claude.ai/code/session_01YXkgsNucxDwAL4GdR4XYRk |
||
|
Michael Ramos
|
9ddb87abb4 |
feat(data-dir): fall back to $XDG_DATA_HOME/plannotator when ~/.plannotator does not exist (#1093)
New resolution order for the Plannotator data directory:
1. PLANNOTATOR_DATA_DIR (unchanged, top priority, ~ expansion)
2. ~/.plannotator when it already exists (legacy default — existing
installs never move)
3. $XDG_DATA_HOME/plannotator when XDG_DATA_HOME is set to a
non-empty absolute path
4. ~/.plannotator (default for everyone else)
This is git's legacy-first pattern: the XDG branch only fires for fresh
installs whose user has explicitly set XDG_DATA_HOME. The XDG spec's
implicit ~/.local/share default is deliberately NOT applied, and the
directory stays monolithic (no config/data/cache split).
Mirrored in every private copy of the resolver: the Amp plugin,
scripts/install.sh, scripts/install.ps1, and scripts/install.cmd. The
Pi runtime picks the change up automatically via vendor.sh. Docs updated
in README.md, AGENTS.md/CLAUDE.md, and the marketing env-var reference.
Claude-Session: https://claude.ai/code/session_01YXkgsNucxDwAL4GdR4XYRk
|
||
|
Michael Ramos
|
c645999761 |
fix(review): allow disabling Cursor sandbox via PLANNOTATOR_CURSOR_SANDBOX for systems where it cannot start (#1095)
The Cursor review engine hardcodes `--sandbox enabled` when launching the
`agent` CLI. On systems where Cursor's sandbox cannot start (NixOS,
AppArmor-restricted Linux) that hard-fails every job with "Sandbox mode is
enabled but not available on this system", and the flag overrides the
user's own `agent sandbox disable` configuration.
Default is unchanged: review jobs still pass `--sandbox enabled` as part
of their read-only posture. Setting PLANNOTATOR_CURSOR_SANDBOX=0 (or
`{ "cursorSandbox": false }` in ~/.plannotator/config.json; the env var
wins) omits the flag pair entirely — never `--sandbox disabled` — so the
user's own Cursor Agent sandbox configuration governs. Resolution follows
the established env-plus-config pattern via resolveCursorSandbox() in
packages/shared/config.ts, applied at the buildMarkerCommand call sites in
both the Bun server (review.ts, guide-review.ts) and the Pi server
(serverReview.ts; guide-review is vendored, with the new ../config import
rewritten in vendor.sh).
Closes #1094
Claude-Session: https://claude.ai/code/session_01YXkgsNucxDwAL4GdR4XYRk
|