mirror of
https://github.com/backnotprop/plannotator.git
synced 2026-09-14 14:17:26 +08:00
codex-mobile-touch-selection
105 Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
 Michael Ramos
|
64062af9a1 |
feat: Portable Guided Reviews — export, share links, agent-authored guides, guides.show (#1324)
A Guided Review can now leave Plannotator: as a single self-contained HTML file that renders exactly like the in-app guide, as an encrypted-by-default share link on guides.show, or authored by any agent through the new guide CLI. Highlights: packages/guide-viewer extracted from review-editor at the injection seam (read-only host, no third renderer); guides.show Worker with R2-backed share storage, per-IP rate limiting on creation, delete tokens hashed at rest, and 128-bit ids; portable exports pin the viewer by SRI hash with budget and manifest gates in PR CI and at deploy; two-runtime parity across Bun and Pi verified; v0.27.x saved guides load unchanged. Retention is indefinite by explicit decision, to revisit with the lean sharing refactor. Decision record: adr/decisions/007-portable-guided-reviews-20260815.md |
||
|
Michael Ramos
|
d2d2dba7fa |
feat(annotate): configurable extra markdown extensions (#1309)
* feat(annotate): configurable extra markdown extensions (#1307) Adds a config-only `markdownExtensions` key to ~/.plannotator/config.json, e.g. { "markdownExtensions": [".livemd"] } for Livebook notebooks. A listed extension is accepted everywhere .md is on the annotate path: CLI target resolution, folder discovery and the file browser, /api/doc plus relative and wiki-link navigation between sibling docs, the 2MB size cap, and per-file version history. Listed extensions render as markdown with frontmatter stripped, never as raw HTML, and they only widen the accepted set. Design: - packages/core/annotatable.ts stays browser-safe and zero-dep. Its regexes and predicates now take an optional, defaulted-empty list of extra extensions, plus a normalizer and regex builders. - packages/shared/markdown-extensions.ts is the node-side seam: it reads config.json once per process through the existing loadConfig() and threads the normalized list into those pure functions. resolve-file re-exports the config-aware predicates so both runtimes pick them up; the Bun server, the Pi mirror, the OpenCode plugin and the CLI all go through them. - The annotate /api/plan payload ships the resolved list so the renderer can linkify links to sibling documents (module-level UI registry, empty by default, so nothing changes without config). Validation: entries must be dot-led, lowercase-normalized, and free of path separators, globs and whitespace. Invalid entries are dropped silently, built-ins are deduplicated, and `.env` is denylisted so config can never register it (annotate copies file contents into the data dir). Deliberately unchanged: the Pi plan-write allowlist (ALLOWED_PLAN_EXTENSIONS in tool-scope.ts) and Edit Mode source save (SOURCE_SAVE_FILE_REGEX), which keep their own narrower allowlists. * fix(annotate): deny the dotenv family and sandbox config-aware tests Review follow-ups on #1309: - deny the whole dotenv family (.prod.env, .env.local, ...) in normalizeMarkdownExtensions, not just the exact .env name - resolve config.json path per call instead of at module scope so PLANNOTATOR_DATA_DIR sandboxing works in single-process test runs - stop resolve-file.test.ts reading the real user config: pure predicate imports plus pinned empty extras on every resolve call - add the config.json -> memo -> predicate integration test using resetMarkdownExtensionsCache under a temp data dir * test(call-flow): make the stale-read advert test self-sufficient The read-only GET only probes the node runtime while Call flow is enabled. The stale-read test relied on earlier tests' settings POSTs leaking callFlow=true through the process-frozen config path; with lazy config resolution each sandbox is genuinely isolated, so the test now enables Call flow in its own data dir. Locally the dependency was masked by an fnm-shimmed sem sidecar spawning node coincidentally. |
||
|
Michael Ramos
|
ed6f44bf2e |
fix(release): tailscale gate exit codes and lease gating, conditional SIGHUP, informative guide validation error (#1286)
- annotate --tailscale publish failures now exit through annotateStartupFailureExitCode: exit 2 under a strict gate (--require-approval / --result-file), where exit 1 is reserved for "the reviewer did not approve, decision record published". Non-strict annotate and review keep the documented exit 1. - the annotate client lease (auto-dismiss on abandonment) is forced off while tailnetPublished is set: --tailscale reads as local to the CLI predicate, but clients connect through the serve proxy, and a proxy disconnect longer than the grace would dismiss a live review. Same rationale as remote/shared sessions; decided at the single point both the /api/plan advert and the SSE endpoint read. - the SIGHUP-to-process.exit route moved from an unconditional CLI-entry listener into enableTailscaleServe's success path, installed only once a serve mapping exists. Any SIGHUP listener overrides the ignored disposition nohup depends on, so plain sessions now keep zero listeners and "nohup plannotator review &" survives terminal close again; --tailscale sessions still tear their mapping down on HUP (exit 129). - validateGuideOutput explains a fully-invalidated guide whose refs named files outside the changeset (count plus up to 3 example paths, with a pointer to the Commits panel) instead of the bare generic message; the generic message stays for genuinely structural emptiness. The informative error now flows through onJobComplete to the job failure card; Pi picks the change up via the vendored guide-review copy. |
||
|
Michael Ramos
|
5f33b72b2f |
feat(remote): tailnet auto-advertise, ready QR code, and a first-class --tailscale mode (#1280)
* feat(remote): resolve urlHost auto from Tailscale for advertised URLs PLANNOTATOR_URL_HOST=auto (or config urlHost: "auto") detects this machine's tailnet host at first use in a remote session: MagicDNS name from tailscale status --json, falling back to the single tailscale ip -4 CGNAT address. Detection is cached per process, never spawns in local sessions, warns once and falls back to localhost on failure, and stays strictly display-only: binding remains governed by PLANNOTATOR_REMOTE. Pure parsers live in the new @plannotator/shared/tailscale module, vendored to the Pi extension; both runtimes mirror the resolution. * feat(remote): render a terminal QR code for remote-ready session URLs Remote sessions print their advertised URL as the lifeline; the usual next step is opening it on another device (iPad, phone, laptop off the VPS). handleServerReady now also renders a compact unicode QR of that URL via the zero-dependency uqr package, TTY-gated so piped stderr and hook transcripts keep only the plain URL line. Pi keeps URL-only parity: its ready surface is an in-chat notification, not a TTY stream, so a QR block would not render there. * feat(cli): first-class --tailscale mode for review and annotate sessions plannotator review --tailscale (also annotate and annotate-last/last) publishes the session over the user's tailnet: the server stays loopback-bound and the CLI orchestrates tailscale serve --bg --https=<port> http://127.0.0.1:<port>, then advertises the HTTPS tailnet URL with a terminal QR code. Nothing listens beyond localhost and nothing is ever public (serve, never funnel). Guarantees: preconditions fail with actionable errors (CLI missing, daemon down or logged out); a pre-existing serve mapping on the chosen port aborts instead of being stolen and other ports are never touched; every mapping the process creates is torn down on normal completion, SIGINT/SIGTERM, and errors via the exit-routed cleanup handler. When combined with PLANNOTATOR_REMOTE or SSH detection, --tailscale wins and forces local mode with a stderr notice, which also restores the random local port so simultaneous sessions get distinct serve mappings. * fix(remote): await tailscale-ready failures, harden serve teardown and conflict detection Review fixes for #1280 (external review plus internal security review). Startup failures no longer hang the session: startReviewServer and startAnnotateServer now await async ready handlers and stop the server on rejection, and the CLI's --tailscale ready path resolves publishing failures itself with an actionable stderr message and exit 1. Under the bang-prefix skill a hanging loopback server blocked the whole Claude Code prompt. Serve teardown is checked, not assumed: a failed off retries once, then warns with the exact manual command, and a port is only forgotten after a successful off. SIGHUP (terminal close) is now routed through process.exit like SIGINT/SIGTERM so exit-time cleanup runs. Docs no longer claim guaranteed cleanup: --bg mappings survive SIGKILL and reboots, and the manual removal command is documented. Conflict detection sees foreground serve sessions (Foreground.*.TCP), which Tailscale prefers over background mappings, and fails CLOSED on unrecognizable serve status output instead of assuming the port is free. The extracted serve URL must match the requested port, so a version-dependent output shape cannot advertise another mapping's URL. The annotate agent terminal is gated off by default under --tailscale behind the existing PLANNOTATOR_AGENT_TERMINAL_REMOTE opt-in: the PTY token is not an auth boundary against network peers, and tailnet reachability implies terminal reachability. Also: --tailscale is rejected with a clear error on unsupported subcommands and documented in review/annotate/annotate-last and top-level help; the remote-ready QR renders only for URLs actually reachable off-machine (never localhost); urlHost is suppressed for --tailscale runs so the local-session warning cannot mislead; the duplicated auto-host resolution moved into the shared vendored module; tailscale-serve tests restore module and process state via a reset seam. |
||
|
Michael Ramos
|
3245310aa8 |
feat(review): add optional CallDiff call-flow analysis (#1268)
* feat(review): add optional CallDiff call-flow analysis * fix(review): harden CallDiff integration |
||
|
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
|
||
|
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
|
2ab377f77d | fix(uninstall): require host cleanup (#1177) | ||
|
Michael Ramos
|
93b66e0ab2 |
feat(cli): add safe uninstall lifecycle (#1170)
* feat(cli): add safe uninstall lifecycle * fix(uninstall): harden cleanup and add Windows QA * fix(uninstall): detach Windows self-delete worker * fix(uninstall): preserve PowerShell worker syntax * fix(uninstall): harden purge and host recovery * fix(uninstall): revalidate purge boundary * fix(uninstall): unlink managed link entries safely |
||
|
Michael Ramos
|
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 |
||
|
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. |
||
|
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> |
||
|
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> |
||
|
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 |
||
|
Michael Ramos
|
56df64c751 |
Add modern GitButler review support (#1067)
Adds current-architecture GitButler workspace, stack, and branch review support across Bun and Pi while preserving the existing Git, JJ, and P4 paths. Co-authored-by: Dan Susman <56033661+dansusman@users.noreply.github.com> |
||
|
Michael Ramos
|
d0665571c7 | Fix OpenCode plan review cancellation cleanup (#1064) | ||
|
Michael Ramos
|
60b5e8d31a | Narrow review feedback validation to submitted findings (#1065) | ||
|
김영준E
|
3b2d899e49 |
fix(hook): echo tool_input as updatedInput so plan approval survives Claude Code 2.1.199+ (#1008)
Since Claude Code 2.1.199, a PermissionRequest "allow" decision for ExitPlanMode is silently discarded unless it echoes updatedInput, because ExitPlanMode requires user interaction and is not an MCP tool. The CLI then falls back to its built-in approval dialog, so clicking Approve in the Plannotator UI never returned control to the agent session (deny was unaffected). This matches the reported behavior on 2.1.199 through 2.1.202. Echo the original tool_input (in scope as event.tool_input) as updatedInput in the Claude Code allow decision. Backward compatible: older Claude Code versions treat the echoed input as unchanged (verified no-op on 2.1.198). Fixes #995 |
||
|
egouilliard-leyton
|
73efacfa0c |
feat(annotate): per-file version diff for .md and .html (rendered HTML highlights) (#961)
* feat(annotate): version diff for annotated files Annotate mode never tracked version history, so the existing Plan Diff (highlighted diff vs a previous version) only worked in plan mode. Wire per-file version history into the annotate server so the same diff UI — badge, Version Browser, block-level comments — works when annotating a standalone .md/.txt/.html file. - key history by file path (stable across edits) rather than the plan flow's heading+date slug - save the markdown (or raw HTML source) to history on each open, expose previousPlan + versionInfo + diffCurrent on /api/plan - add /api/plan/version and /api/plan/versions to the annotate server Markdown lights up end to end; HTML needs frontend follow-ups (feed the HTML source as the diff content, surface the badge on the html surface, default to source diff mode). * feat(annotate): rendered HTML version diff with inline highlights For --render-html files, render the version diff as the real page with inline <ins>/<del> highlights instead of a markdown/source diff: - add packages/shared/html-diff.ts: a tag-aware htmlDiff() that wraps changed text in <ins>/<del> while keeping tags balanced (script/style opaque). 9 unit tests. - annotate server computes diffHtml = rewriteHtml(htmlDiff(prev, current)) and exposes it on /api/plan - HtmlViewer: inject ins/del highlight CSS, add a 'Show/Hide changes' toggle in its action bar - App: store diffHtml, swap the iframe to the diff page when toggled, and suppress the markdown block-diff path on the HTML surface Commenting still works because the diff page renders through the same HtmlViewer iframe bridge. * docs(annotate): document the annotate version diff + endpoints * feat(annotate): mirror version diff into the Pi server Parity for the Pi (node:http) runtime: per-file version history, previousPlan/versionInfo/diffCurrent + diffHtml on /api/plan, the /api/plan/version[s] endpoints, and project wiring from the Pi CLI. Vendors @plannotator/shared/html-diff into pi-extension/generated. * review fixes: pi diff dependency, attr-aware tokenizer, history opt-out, hide dead version picker on HTML - apps/pi-extension/package.json: declare the 'diff' dependency — generated/html-diff.js imports it at module load, so a standalone Pi install failed to resolve it and broke every annotate session (the monorepo masked this via root hoisting) - packages/shared/html-diff.ts: tag tokenizer now consumes quoted attribute values whole, so a '>' inside title="a > b" no longer splits the tag and corrupts the diff output; 3 regression tests - annotate history is now gated by PLANNOTATOR_ANNOTATE_HISTORY / config.annotateHistory (default on) and disclosed in AGENTS.md — it writes copies of annotated files into the data dir, which users should be able to see coming and turn off - packages/editor/App.tsx: hide the sidebar Versions tab on the HTML surface — the base-version picker has nothing to drive there (the HTML diff is fixed to current-vs-previous); the viewer's Show changes toggle is unaffected * fix(ui): document content clears the badge cluster dynamically The repo/diff badge cluster is absolutely positioned in the card's top padding, sized by guesswork (py-5..py-12). One chip row fit; the diff badge's second row overflowed into the H1, and mobile wrapping made the badge sit on top of the heading. Measure the cluster (ResizeObserver) and insert exactly the clearance it needs — zero when it fits, so existing single-row layouts don't shift. Pre-existing plan-mode bug surfaced by the annotate version diff. --------- Co-authored-by: Edouard Gouilliard <edouard.gouilliard13@gmail.com> Co-authored-by: Michael Ramos <mdramos8@gmail.com> |
||
|
Michael Ramos
|
82acc4b0aa |
fix(cli): print per-subcommand help instead of launching the UI (#974)
* fix(cli): print per-subcommand help instead of launching the UI `plannotator review --help` (and other subcommands) fell through to their command branch because only top-level `--help` was handled. For `review`, `--help` was parsed as a non-URL positional, dropping into local review mode and opening a browser tab. When Claude Code probes the CLI with `--help`, that stray tab's close injects a bogus "no feedback → proceed" signal into the session. Handle `--help`/`-h` for every user-facing subcommand (review, annotate, annotate-last/last, setup-goal, archive, sessions) before any subcommand branch can run: print command-specific usage on stdout and exit 0. Also accept `-h` at the top level and advertise per-command help there. Fixes #964 * fix(cli): handle `improve-context --help` too The top-level help advertises `plannotator <command> --help`, but `improve-context` (the only internal hook command listed there) had no help entry, so `improve-context --help` fell through to the hook branch and emitted additionalContext JSON instead of usage. Add a help entry so every advertised command responds to --help. |
||
|
Michael Ramos
|
740d6fb2eb |
Add WebTUI agent panel to annotate mode (#941)
* feat(annotate): add WebTUI agent terminal * feat(annotate): wire WebTUI agent into annotate UI * docs: recap annotate agent terminal work * fix(annotate): harden agent terminal runtime * docs: add annotate agent terminal runtime ADRs * fix(annotate): polish agent terminal integration * fix(ui): preserve comment draft on Ask AI failure * fix(annotate): address terminal review findings * fix(annotate): harden agent terminal runtime fallback |
||
|
Yoni Hornstein
|
201ca11ec4 |
feat(config): support share toggle via config.json (#921)
Allow disabling URL sharing through ~/.plannotator/config.json
({ "share": "disabled" }) in addition to the PLANNOTATOR_SHARE
env var. Adds a resolveSharingEnabled() helper (env var > config >
default enabled) and routes all sharing checks through it across the
hook server, OpenCode plugin, and Pi extension. Docs updated.
|
||
|
Michael Ramos
|
9ed3ba8937 |
feat(editor): markdown edit mode — direct document editing with diff-to-agent feedback
Adds direct markdown editing, source-backed annotate saves, folder edit buffers, and review-hardening fixes. |
||
|
Michael Ramos
|
be2d06a7c2 |
Make HTML annotations render HTML by default
* feat(annotate): render html files by default * fix(annotate): support raw html assets and sharing * fix(annotate): address html first review followups * fix(editor): avoid raw html sidebar init crash * fix(annotate): support portable html shares * fix(annotate): harden html share support * fix(share): clear attachments when loading shared payloads * fix(share): warn on remote share link failures * perf(annotate): lazy-build html share payloads * test(annotate): guard lazy html share generation * test(annotate): drop flaky html share server test |
||
|
Michael Ramos
|
6c64b96dd7 |
fix(review): signal-safe cleanup + triage suffix for PR feedback (#914)
Two QA follow-ups found while validating the release: - Route SIGINT/SIGTERM through process.exit() so the existing "exit" handlers actually run on Ctrl-C / termination. A signal death previously skipped them, leaking background PR-checkout warmup children and stale `git worktree` registrations. A second signal still force-quits if cleanup hangs. - Append the review-denied triage suffix for PR-mode feedback, not just local diffs. The old `!isPRMode` gate suppressed it for every PR review. Gate on whether the reviewer actually sent annotations instead: genuine feedback always carries annotations, while platform PR actions (approve/comment posted to the host) return an empty annotation set + status message and correctly get no suffix. Applied consistently across hook, OpenCode, and Pi. |
||
|
Michael Ramos
|
6ec1a66c9b |
feat(review): large-PR pipeline, instant-open checkout, scroll perf, and worker-pool highlighting (#893)
* feat(review): large GitHub PR fallback + non-blocking PR checkout
Two PR-mode improvements:
1. Large GitHub PRs no longer fail to load. When `gh pr diff` is refused
(HTTP 406 for oversized diffs), fetchGhPR pages through the pulls files
API and stitches the per-file patches into a unified diff — mirroring
the existing GitLab raw_diffs fallback. Path quoting matches git's
exact rules (bare spaces unquoted) so downstream parsers round-trip;
truncation at the API's 3000-file cap is surfaced, never silent.
2. The --local worktree/clone no longer blocks startup. The review server
opens as soon as the platform diff arrives; the checkout warms in the
background as a seeded not-ready pool entry. Consumers that need real
files (agent jobs, full-stack diff, code-nav, semantic diff, AI
sessions) await pool.ensure(), with creations serialized so concurrent
fetches can't clobber the shared FETCH_HEAD. Cross-repo clone steps
converted from spawnSync to async spawns; warmup children are killed
on exit (plus `git worktree prune`) so aborted sessions can't leak
stale registrations; failed checkouts degrade honestly (no agent runs
in the wrong directory claiming local access) with a 30s retry
cooldown.
* fix(review): survive long PR checkout warmups + classify reconstructed renames
Stress-testing against oven-sh/bun#30412 (2,188 files) surfaced three bugs:
- Bun.serve's default 10s idleTimeout killed /api/semantic-diff while it
parked on the background checkout warmup (a clone that can take minutes).
Disable the idle timeout on all servers — AI SSE streams can also stall
>10s between bytes while a permission prompt waits.
- The file-badge hook memoized that failed fetch in a module-level cache
keyed by patch, pinning every badge to empty until a hard refresh. Never
cache failures; retry with backoff (5s/15s/30s).
- reconstructGhPatch/reconstructPatch omitted the `similarity index` line,
which Pierre's parser keys rename classification off — pure renames
rendered as blank plain changes with no old path. Emit 100% for
patch-less renames/copies (exactly accurate) and a synthetic 99% for
patched ones (consumers only branch on 100% vs not).
* feat(review): local full-diff upgrade for PRs whose API diff is truncated
On oversized PRs the platform APIs withhold per-file patch content entirely
(bun#30412: 1,066 of 2,188 files came back with status added/modified, zeroed
counts, and no patch). Those files rendered as empty stubs with no diff.
- fetchGhPR/fetchGlMR flag the result `patchIncomplete` when patch-less
non-rename entries exist or the 3000-file cap truncates the listing.
- New runPRLayerLocalDiff (pr-stack.ts) recomputes the exact layer diff in
the local checkout: platform merge-base + head SHA two-dot diff (three-dot
vs baseSha fallback), fetch-by-SHA for objects missing from shallow clones,
-l0 so rename detection doesn't silently degrade on huge PRs.
- The review UI shows a "Partial diff · Load full diff" notice in layer
scope; clicking re-requests the layer scope and the server swaps in the
recomputed full diff (waiting out the background clone if needed).
- PR scope/switch state writes are epoch-guarded: a request parked on the
checkout warmup can no longer overwrite a newer scope select or pr-switch.
- draftKey follows the upgraded patch so annotation drafts survive pr-switch
round-trips; recompute failures surface in the response error field.
- Pi server mirrors all of it, including an agentCwd fallback so the upgrade
works for PRs switched-to under a cross-repo clone pool.
* fix(review): use GitLab's too_large/collapsed flags for withheld-diff detection
External review caught a false negative: a too-large ADDED file comes back
new_file:true with an empty diff — indistinguishable from a legitimately
empty new file under the old heuristic, so the partial-diff upgrade was
never offered for exactly the files that matter most on big MRs.
The REST /diffs endpoint marks withheld content explicitly per entry
(verified against gitlab.com): too_large/collapsed are now authoritative in
both directions — withheld adds/deletes are flagged, binaries and empty
files are never misflagged. Older GitLab without the fields keeps the
empty-diff-on-modification heuristic.
* feat(prompts): unify review-denied suffix — triage first, no coding off raw feedback
The per-runtime defaults map (#627) gave OpenCode and Pi a different
review-denied suffix than every other runtime; updating one meant the
others silently kept "you must address all of them" — an instruction to
start coding immediately. Claude Code, Amp, Droid, Codex, Copilot, Gemini,
and Kiro were all still on it.
One default for every runtime now: triage the feedback, verify it against
the code, discuss before changing anything. Per-runtime customization
remains available via config (prompts.review.runtimes.<rt>.denied), which
resolves above the built-in default as before.
* fix(prompts): generalize review-denied suffix — 'from review', not 'external AI reviewers'
Review feedback isn't always from AI reviewers or agent jobs; often it's
the human reviewer's own annotations. Neutral wording covers both.
* fix(review): non-blocking 'Load full diff' + flag-handling hardenings
Self-review findings:
- The partial-diff upgrade reused the scope-switch handler, so clicking
"Load full diff" raised the full-screen PRSwitchOverlay — blocking the
entire UI, potentially for minutes behind a cold clone, with no text and
no cancel. The upgrade now has its own loading state: the notice shows a
spinner ("Loading full diff…") and the reviewer keeps working with the
partial diff while the request parks. Server-side epoch guards already
handle scope/PR changes made during the wait.
- GitLab too_large/collapsed: treat explicit null like absent (flags
inconclusive → legacy heuristic decides) instead of silently exonerating.
- Rename-limit lift uses -l100000 instead of -l0 ("0 = unlimited" only
holds on git >= 2.29; on older git it could disable detection outright).
* fix(review): stop scroll-driven sem stampede when semantic diff is failing
The badge retry change (
|
||
|
Devin
|
4b76db9396 |
fix(annotate): improve error message for unsupported file types (#870)
- When a file exists but has an unsupported type (.cs, .ts, .py, etc.), show 'File type not supported' instead of misleading 'File not found' - Display supported file types (.md, .mdx, .html, .htm) - Suggest using 'plannotator review' for code files - Maintains backward compatibility for actual missing files Fixes #757 Co-authored-by: ishowman <ishowman@users.noreply.github.com> |
||
|
Oscar Silva
|
f08764063d |
feat(review): support multi-repo workspace reviews (#543)
* feat(review): support multi-repo workspace reviews (#527) * fix(workspace): address critical issues from deep review - Fix race condition in label generation by pre-computing labels sequentially - Fix rewritePatchLine to support quoted paths and rename/copy headers - Add separator between aggregated patches to avoid invalid diffs - Normalize input paths in resolveWorkspaceFilePath - Add timeout to PR discovery (15s) to prevent server hangs - Fix PATCH /api/workspace/repo to rollback state on failure via applyRepoMutation - Validate body.source runtime (must be 'local' or 'pr') - Snapshot active repo in agent jobs at launch to prevent race in onJobComplete - Prevent double-prefixing of agent findings when paths are already prefixed - Fix frontend findWorkspaceRepoForPath to use longest-prefix matching - Fix shared types: diffType uses DiffType, platformUser is string | null * fix: remove duplicate gitRuntime export in vcs.ts * fix: resolve remaining merge conflict in local review mode, remove stale detectManagedVcs import * Add local multi-repo workspace review support * Fix workspace review edge cases * Remove session query from browser launch * Add switchable workspace review modes * fix(review): cover opencode workspace bridge * fix(review): clean workspace review plumbing * fix(review): clarify workspace agent finding paths * fix(review): preserve diff paths with spaces --------- Co-authored-by: Michael Ramos <mdramos8@gmail.com> |
||
|
Michael Ramos
|
26ca4e0275 |
Single-source skills (core/extra), replace Claude Code commands with skills, de-hardcode installers (#850)
* feat: single-source skills into core/extra, replace Claude commands with skills, de-hardcode installers
- apps/skills/core/{review,annotate,last,archive}: single authoritative
source for the always-installed skills (archive is new); all carry
disable-model-invocation + agents/openai.yaml sidecars
- apps/skills/extra/{compound,setup-goal,visual-explainer}: no longer
default-installed (except Kiro); installers print an
`npx skills add backnotprop/plannotator/apps/skills/extra` suggestion
- Claude Code: apps/hook/commands/ deleted, command heredocs removed;
core skills in ~/.claude/skills are the slash commands now
- Installers: OpenCode/Gemini command files copied from an extended
sparse checkout instead of heredocs; install.cmd gains the previously
missing OpenCode command install; aggressive cleanup of legacy
~/.claude/commands and ~/.codex/skills artifacts
- Codex: core skills install to ~/.agents/skills (official path);
~/.codex/skills install removed
- Pi: extension no longer bundles skills; #670 settings filter removed
* fix: review findings — old-tag soft guards, cmd replace-not-merge, plugin-update hint, frontmatter test
- install.sh: a --version tag predating apps/skills/core no longer aborts
the whole copy subshell (which also skipped OpenCode/Gemini commands);
core skills now soft-skip with an accurate message, matching ps1/cmd
- install.sh: subshell failure message no longer claims "git required"
when git was present (clone/network errors get their own wording)
- install.cmd: pre-remove skill dirs before xcopy so upgrades replace
rather than merge (stale files from renamed/deleted skill files no
longer linger; parity with sh/ps1)
- all installers + docs: tell upgraders to run /plugin marketplace update
so the plugin's old namespaced plannotator:* commands disappear (#817)
- install.test.ts: assert every core SKILL.md sets
disable-model-invocation: true — the load-bearing line that keeps core
skills out of Pi's system prompt (#842 regression guard)
* test: pin old-tag soft-guard behavior, dedupe core-skill list in tests
* fix: interrogation review findings — cross-installer diagnostic parity
- install.ps1/install.cmd: emit the "predates the core/extra skill
layout" diagnostic on old pinned tags instead of silently skipping
core skills (parity with install.sh)
- install.ps1: clone/network failure no longer claims "git required"
(git was already verified present); the outer catch now reports the
actual exception
- install.sh: "Installed OpenCode/Gemini commands" echoes are guarded
on the copy actually having a source, so old pinned tags don't print
false success (ps1/cmd already gated this way)
- AGENTS.md: opencode-plugin commands/ comment now reflects all four
command stubs
- install.test.ts: shared test asserts the soft-skip diagnostic exists
in all three installers and pins ps1's honest failure wording
* fix: respect CODEX_HOME for Codex home directory (#852)
Codex stores config and state under $CODEX_HOME when set, falling back
to ~/.codex (developers.openai.com/codex/config-advanced). Plannotator
hardcoded ~/.codex in two places:
- runtime: codex-session.ts scanned ~/.codex/sessions for rollout
files, so `plannotator last` failed with "No rendered assistant
message found" when CODEX_HOME pointed elsewhere. Now resolved the
same way copilot-session.ts handles COPILOT_HOME and session-log.ts
handles CLAUDE_CONFIG_DIR.
- installers: detection, config.toml/hooks.json paths, manual-setup
instructions, and the stale-skills cleanup now derive from
CODEX_HOME in all three scripts.
Tests: codex-session.test.ts covers rollout discovery under a
CODEX_HOME temp dir; install.test.ts asserts all three installers
respect the variable and that the fallback is the only hardcoded
~/.codex path left in install.sh.
* fix: hard-fail skill install, guard command cleanup, one-time extras migration
External review triage on PR #850 surfaced two real installer issues:
P1 — commands deleted before replacement: the Claude command cleanup
ran before the git-gated skill install, so a missing git, a failed
clone, or an old pinned tag deleted the user's slash commands and
installed nothing (a regression — the old installer needed no git).
Now:
- missing git is a hard failure before anything is touched ("install
git, then run this installer again")
- a failed fetch is a hard failure ("something went wrong — run the
installer again") instead of a silent skip
- the legacy command cleanup runs AFTER the install and only removes a
command file when its same-name replacement skill exists on disk
- old pinned tags keep the soft-skip (no deletion happens, commands
survive, CI e2e against old tags stays green)
P2 — recurring extras deletion: the extras cleanup ran on every
invocation, deleting copies users reinstalled via the suggested
`npx skills add` (the copies are byte-identical, so only provenance
can tell them apart). The cleanup is now a one-time migration recorded
in a migrations ledger under the Plannotator data dir
(<PLANNOTATOR_DATA_DIR|~/.plannotator>/migrations/), the same
record-what-you-did pattern package managers use.
All three installers (sh/ps1/cmd) updated in parity; tests pin the
guard condition, the ledger gating, and the hard-fail messages.
* test: tripwire — install.cmd must never contain /dev/null redirects
* fix: every skill sets disable-model-invocation — no exceptions
Maintainer rule: all Plannotator skills are user-invoked, never
model-auto-invoked. setup-goal (missing since #665) and the three Kiro
skills now carry the flag. The frontmatter test scans every SKILL.md in
apps/skills/core, apps/skills/extra, and apps/kiro-cli/skills
dynamically — with a floor of 10 — so a future skill cannot ship
without it.
* docs: git is a hard installer requirement; clarify post-gate sections complete on re-run
* docs: align ps1/cmd comments with hard-fail semantics
* feat: guided install — extras opt-in via skills CLI, model-invocation picker
Interactive terminals get a two-question wizard on first run:
1. Install the extra skills? Yes delegates to `npx skills add
backnotprop/plannotator/apps/skills/extra` (its UI picks the agents),
wired to /dev/tty so piped curl|bash installs still work. Skipped
when extras already exist on disk.
2. Make any skills callable by the model? Yes opens a space-toggle
checkbox (sh/ps1) or numbered toggles (cmd), listing all skills if
extras were chosen, core-only otherwise. Chosen skills get
disable-model-invocation stripped from their INSTALLED copies and the
Codex sidecar's allow_implicit_invocation flipped — re-applied every
run since installs replace skill folders. Repo sources stay locked.
Answers persist to <data dir>/install-prefs (shared format across all
three installers) and re-runs reuse them silently; --reconfigure
re-opens the wizard. Automation is untouched: no terminal means no
prompts and today's defaults; --extras/--no-extras/--model-invocable/
--non-interactive give scripts explicit control.
* fix: self-review of guided install — cmd pipe expansion bug, flag/wizard interplay
- install.cmd: the checkbox preselection used `echo !var! | findstr` —
each side of a cmd pipe runs in a child WITHOUT delayed expansion, so
the saved choices passed through as literal !var! text and
preselection never matched. Replaced with a substring-replace
containment test (no pipe).
- all three: a wizard question whose answer was already provided by a
CLI flag (--extras/--no-extras/--model-invocable) is no longer asked
and then silently overridden — the flag pre-answers it.
- install.cmd: unknown-option usage line now lists the wizard flags.
* feat: guided install question 3 — install Glimpse (native window)
glimpseui (third-party npm package, PR #840) gives Plannotator a native
WebView window instead of a browser tab; the runtime already
auto-detects it on PATH, so a global install is all that's needed.
- Wizard asks "Install Glimpse?" (default yes) after the skills
questions; skipped when glimpseui is already on PATH
- Yes runs `npm install -g glimpseui` (bun fallback on sh/ps1; printed
instruction when neither exists) — wizard or explicit flag only,
silent re-runs never install software
- --glimpse / --no-glimpse flags for automation; choice persisted to
install-prefs like the others
- docs + tests updated (glimpse detection, install command, flags, and
persist-condition assertions across all three installers)
* fix: self-review of Glimpse question — cmd bun fallback, stale usage text
* fix: merge-window hardening — guard Codex cleanup, remove old-installer junk dirs
plannotator.ai serves install.sh live from main (public/ symlink,
deployed on push), while the script fetches repo files at the LATEST
RELEASE TAG. Between merging the core/extra restructure and cutting the
release that ships it, the live script runs against the old-layout tag.
Two hazards in that window:
1. The Codex stale-skill cleanup removed working ~/.codex/skills with
no successor installed (core skills soft-skip on old tags). Now the
cleanup runs AFTER the install and removes a core skill only once
its replacement exists in ~/.agents/skills — same guard the Claude
command cleanup uses. The compound/setup-goal stale copies stay
unconditional (never Codex's to begin with).
2. The reverse combo (cached OLD script + NEW release tag) wholesale-
copies apps/skills/* and leaves junk core/ and extra/ directory
copies in ~/.claude/skills. Never valid skill names — all three
installers now remove them on every run.
* fix: glimpseui is a devDependency — consumers never use it from node_modules
PR #840 added glimpseui to dependencies in @plannotator/server and
@plannotator/pi-extension, but nothing imports it: both runtimes detect
the CLI on PATH (Bun.which / a manual PATH walk) and spawn it. The dep
only ever mattered in repo development, where `bun run` prepends
node_modules/.bin to PATH. For consumers it was inert download weight —
OpenCode plugin installs and `pi install` pulled a third-party package
that could never be detected (Pi's loader does not expose
node_modules/.bin; verified). Moved to devDependencies in both: dev
flows keep working, published packages stop shipping it. The sanctioned
end-user path is the guided installer's global `npm install -g
glimpseui`.
* fix: clean stale plugin command files from the installed plugin checkout (#817)
The installer already manages hooks.json inside
~/.claude/plugins/marketplaces/plannotator/apps/hook/, so the earlier
"don't reach into plugin storage" rationale for leaving the old
namespaced plannotator:* command files there was inconsistent. All
three installers now remove them — same replacement-skill guard as the
bare ~/.claude/commands cleanup — making the #817 duplicate menu
entries die on a single installer run + restart instead of waiting for
/plugin marketplace update. Hints/docs reworded accordingly.
* Revert "fix: clean stale plugin command files from the installed plugin checkout (#817)"
This reverts commit
|
||
|
Michael Ramos
|
3de555f5e5 |
Fix OpenCode plugin runtime compatibility (#849)
* fix(opencode): add host-compatible runtime bridge * fix(opencode): preserve parity in cli bridge * test(opencode): add isolated sandbox launcher * test(opencode): keep reusable sandbox launchers * test(opencode): export local plugin default * test(opencode): install OpenChamber deps when needed * test(opencode): avoid OpenChamber default port collision * fix(opencode): harden cli bridge fallback * test(opencode): clean isolated sandbox helpers |
||
|
Graham Lipsman
|
be2c81fa3b |
annotate-last: pick which message to annotate (fixes #800) (#809)
* feat: message picker for annotate-last (#800) When running /plannotator-last after /rewind, the newest transcript entry is no longer the message the user intended to annotate, and there was no affordance to pick a different one. Adds a picker UI that surfaces the recent assistant messages so the user can choose which one to annotate: - A "Message N of M" button in the Viewer's sticky-top action bar (alongside Copy / Global comment / Attachments), so it stays accessible while scrolling. - A "Messages" tab in the left sidebar with the full list (newest-first, preview + timestamp, default ★), mirroring the existing Files / Versions / Archive tab pattern. Wired for Claude Code, Codex, and Droid (all share apps/hook/server). OpenCode, Pi, and Copilot still get the original single-message behavior — they don't emit recentMessages, so the picker affordances hide cleanly. Default selection (index 0) matches today's "last message" behavior, so users who don't interact with the picker see no change. * feat: extend annotate-last picker to Copilot and OpenCode The picker UI from #800 was wired for Claude / Codex / Droid only. Pull Copilot and OpenCode onto the same shape so users on those harnesses also get the recent-messages picker when annotating the last assistant message. - Copilot: replace getLastCopilotMessage with getRecentCopilotMessages, walking events.jsonl newest-first up to 25 assistant.message events. - OpenCode: rewrite the session walk to collect up to 25 messages (newest first) instead of bailing on the first hit; normalize the SDK time.created (ms epoch) to ISO to match the shared picker contract. - Both pass recentMessages to startAnnotateServer only when length > 1, matching the existing Claude/Codex/Droid behavior. Also trims a leftover narrating comment in MessagesBrowser and refreshes the stale Copilot session-parser header. Pi parity follows in the next commit (needs round-trip of the picker selection through /api/feedback so its post-submit anchoring quotes the right message). * feat(pi): wire annotate-last picker with feedback round-trip Extends the picker UI (#800) to Pi and fixes a Pi-specific anchoring bug the picker would otherwise introduce. Picker plumbing - assistant-message: getRecentAssistantMessages walks the active branch newest-first, returning { messageId, text, timestamp? } in the same shape the other harnesses produce. - Plumbed through plannotator-browser / plannotator-events so the Bun server's recentMessages option is populated when the branch has more than one assistant message. Anchoring fix - Pi quotes the targeted assistant message back to the agent because its UX is async — the conversation may have moved on by feedback time. With the picker, that target is no longer guaranteed to be the snapshot taken when the UI opened. The editor now sends the user's selectedMessageId with /api/feedback; Pi looks it up in the current branch via findAssistantMessageByEntryId and quotes that message instead. Falls back to the original snapshot if the entry is gone. - The round-trip field is optional and only meaningful in annotate-last mode; other harnesses (and other modes) ignore it. Timestamp safety - Pi's SDK currently types SessionEntryBase.timestamp as string, but the picker contract everywhere else is ISO. Treat the value as unknown and normalize string/number(ms)/Date to ISO; drop anything else, rather than blind-casting and risking silent drift if the SDK changes. * chore: strip issue-number references from comments Comments shouldn't rely on external references — issue numbers age out of context, link rot is a thing, and a reader shouldn't need to open GitHub to understand why a line exists. Strip the `(#800)` and `(#570)` parentheticals from comments and doc strings across the picker and review-gate code; the surrounding "why" content is preserved. * fix: prevent removeChild crash when switching annotate-last messages Switching the picked message remounted nothing, so React reconciled new content against DOM that web-highlighter had mutated with <mark> nodes, throwing removeChild. Drive the Viewer key (and StickyHeaderLane's remount token) off a shared viewerContentKey so a message switch fully remounts the Viewer and re-anchors the sticky-header observer. Also cap MessagesBrowser row previews via previewText() and drop the redundant 'block' class that was overriding line-clamp-2. * feat: persist annotate-last feedback across messages --------- Co-authored-by: Michael Ramos <mdramos8@gmail.com> |
||
|
Michael Ramos
|
8c947c5419 |
Add Amp plugin integration (#803)
* Add Amp plugin integration * Use official Amp logo on landing page * Tighten landing agent selector * Default landing selector to Claude Code * Stabilize server ready handoff test * Create ready handoff directory before writing * Stabilize server ready handoff tests * Fix Amp command cancellation and cwd * Preserve Plannotator browser handling for Amp * Fix Amp review edge cases * Add PowerShell installer smoke coverage |
||
|
Michael Ramos
|
4de62e83e1 |
Add Droid slash-command integration (#787)
* feat: add Droid slash-command integration * fix: restore Droid command launcher behavior * chore: remove archive command from homepage * fix(droid): tighten last-message session resolution |
||
|
Michael Ramos
|
5438f66456 |
fix: honor CLAUDE_CONFIG_DIR in session log discovery (#786)
Resolve DEFAULT_SESSIONS_DIR and DEFAULT_PROJECTS_DIR from CLAUDE_CONFIG_DIR when set, falling back to ~/.claude. Add projectsDirOverride param to findSessionLogsByAncestorWalk so all four resolution tiers respect custom config paths. Closes #783 |
||
|
Michael Ramos
|
82636e1286 |
Add interactive goal setup UI (#731)
* Add interactive goal setup UI * Refine goal interview skip and question flow * Fix review findings: recommendation combo, option-only recs, single deselect * Persist goal setup working JSON files * Refine goal setup copy and facts controls * Remove generated goal package from PR * Address goal setup review issues * Remove goal setup slash command adapters * Disable fact comment attachments * Fix goal setup fact comment state * Address goal setup review cleanup * Fix goal setup fact submission edge cases |
||
|
Muhammad Hashmi
|
9873cdb3f6 | Fix Codex annotate-last message selection (#740) | ||
|
Michael Ramos
|
3f91cd7c0e |
feat: add --version / -v flag to CLI (#725)
Injects the version from package.json at compile time via Bun's --define so compiled binaries report the correct version (e.g. `plannotator 0.19.16`). Uncompiled dev runs fall back to `plannotator dev`. |
||
|
Michael Ramos
|
1eb561551c |
feat: standalone skills package + HTML render-annotate mode (#687)
Add --render-html flag to plannotator annotate that renders HTML files as-is in an iframe instead of converting to markdown. Includes annotation support via postMessage bridge, sharing via paste service, and theme inheritance from Plannotator's 30+ themes. New skill: plannotator-visual-explainer — wraps nicobailon/visual-explainer with Plannotator theme tokens, extended patterns (timelines, SVG diagrams, code blocks, risk tables, Pierre diffs via CDN), and plan/PR-specific guidance. All three servers (Bun, Pi, OpenCode) support the new flag. |
||
|
Michael Ramos
|
347663a4de |
feat(pfm): code line range references, hover preview, sketch Graphviz (#692)
Code file line range references with hover preview + Graphviz improvements. Line ranges: `file.ts:42` and `file.ts:10-20` are fully supported with syntax-highlighted hover preview popover (150ms delay, GitHub-style persistence). New parseCodePath() utility, server-side line suffix stripping on both Bun and Pi servers, ambiguous picker preserves line suffix. useCodeFilePopout moved to hooks/pfm/. Graphviz: responsive container height from SVG aspect ratio, white background polygon removed, default colors (black, lightgrey) replaced with theme tokens via SVG post-processing. User-specified colors preserved. |
||
|
Michael Ramos
|
13c667c044 |
feat(hook): PFM reminder & improvement hook support across all runtimes (#689)
PFM reminder & improvement hook support across Claude Code, OpenCode, and Pi. - Add opt-in PFM reminder (pfmReminder config flag) injected on EnterPlanMode - Wire composeImproveContext() into all three runtimes - Fix OpenCode system.transform array reference bug (pushes were going to dead array) - Fix install scripts silently stripping PreToolUse/EnterPlanMode hook entry - Isolated Pi sandbox testing (--no-extensions -e) |
||
|
Graeme Folk
|
69ef11bdfb |
feat(review): add jj review workflows (#675)
* feat(review): add jj support for local diffs * feat(review): add jj review workflows * fix(review): tighten jj diff defaults * test(review): add jj manual sandbox * fix(review): share jj agent diff prompts * fix(review): quote jj agent revsets * feat(review): share jj vcs handling with pi * fix(review): tighten jj bookmark and pi pr handling * fix(review): tighten jj defaults and detection * fix(review): harden jj diff and vcs detection --------- Co-authored-by: Michael Ramos <mdramos8@gmail.com> |
||
|
Michael Ramos
|
49c55e6e25 |
Revert "Expose bypass clear reminder permission mode (#668)"
This reverts commit
|
||
|
Nehr
|
3b88415aeb |
Expose bypass clear reminder permission mode (#668)
* Preserve truthful approval semantics for Claude plan bypass Thread a clear-context reminder flag through approval decisions, expose a Claude Code-only approval entry that requests bypass mode, and keep the hook response honest by emitting a reminder instead of claiming context was cleared. Constraint: Claude Code PermissionRequest hooks have no documented clearContext response field and bypassPermissions is only a request when the mode is available. Rejected: adding a permission-mode enum or undocumented clearContext field | those would misrepresent hook capabilities and broaden the contract. Confidence: high Scope-risk: moderate Directive: Do not claim Plannotator clears context until Claude Code documents a hook field for that behavior; keep reminder copy truthful. Tested: bun test packages/server apps/hook; bun test packages/editor/wideMode.test.ts packages/ui/hooks/useAgentSettings.test.ts; bun test; bun run build:review; bun run build:hook; bun run typecheck; git diff --check --cached. Not-tested: Browser warning-dialog replay and interactive Claude Code smoke test. Co-authored-by: OmX <omx@oh-my-codex.dev> * Keep approval payloads truthful across Claude Code and Pi Constraint: Claude Code hooks cannot clear context directly; the new action remains a truthful reminder plus bypass-mode approval. Rejected: Sending OpenCode agent-switch state for Claude Code | it leaked build (?) UI state and misleading payload fields. Confidence: high Scope-risk: narrow Directive: Keep OpenCode agent switching gated to opencode-origin approval payloads. Tested: bun run typecheck; bun test; bun run build:review; bun run build:hook Not-tested: Manual browser click-through of the Claude Code dropdown. * Expose the clear-context reminder permission default Constraint: Claude Code hooks can only emit a systemMessage nudge, not clear context directly. Rejected: Server protocol changes | existing permissionMode plus clearContextNudge wire fields already support the behavior. Confidence: high Scope-risk: narrow Directive: Keep bypassPermissionsClearReminder as a UI/storage-only synthetic mode that decomposes before /api/approve. Tested: bun test packages/editor/approvalBody.test.ts; bun run --cwd apps/review build && bun run build:hook; bun run --cwd packages/ui typecheck; git diff --check Not-tested: Root bun run typecheck could not run because tsc was not on PATH for the root script. * Reject invalid persisted permission modes Constraint: Stored browser values can be stale, corrupt, or from a future Plannotator build. Rejected: Trusting the storage read with a type assertion | invalid values could flow into UI state and approval request construction. Confidence: high Scope-risk: narrow Directive: Keep PermissionMode storage reads validated against PERMISSION_MODE_OPTIONS when adding or renaming modes. Tested: bun test packages/editor/approvalBody.test.ts; bun run --cwd packages/ui typecheck; git diff --check Not-tested: Full repo typecheck/build, outside this review-fix scope. * Ensure approvals use the live permission setting Settings persisted permission mode changes, but App kept the original mode in React state and used that stale value when building approval payloads. Push the Settings change back into App so the selected clear-reminder mode reaches the hook decision. Constraint: Permission mode storage is cookie-backed while approval payload construction reads App state.\nRejected: Read permission cookies during approval | would couple approval construction to browser storage and duplicate Settings state.\nConfidence: high\nScope-risk: narrow\nDirective: Keep permission-mode writes synchronized with approval state when adding or changing modes.\nTested: bun test packages/editor/approvalBody.test.ts packages/ui/components/ApproveDropdown.test.tsx; bun x tsc --noEmit -p packages/ui/tsconfig.json; bun run build:hook; direct Playwright hook smoke verified bypassPermissionsClearReminder UI and hook payload.\nNot-tested: Live interactive Claude CLI session; direct hook/server simulation covered the wire output. --------- Co-authored-by: OmX <omx@oh-my-codex.dev> |
||
|
Michael Ramos
|
e0fb690039 |
fix(session-log): detect ghost sessions from /clear to resolve correct log (#661)
* fix(session-log): detect ghost sessions from /clear to resolve correct log After /clear, Claude Code creates a new session (new .jsonl file) but never updates ~/.claude/sessions/<pid>.json — the metadata retains the old sessionId. The ancestor-PID resolver would confidently return the stale log, preventing fallthrough to mtime-based tiers. Fix: after tier-1 matches a log that isn't the newest by mtime, check whether the newer file's sessionId is registered in any metadata file. If not, it's a "ghost" session from /clear — prefer it. If it IS registered, it belongs to a concurrent session — keep the PID result. Fixes #643 * test: fix flaky mtime ordering in ghost detection tests Explicitly backdate the "older" file by 5 seconds instead of relying on write order, which is non-deterministic when both files are created within the same millisecond. |
||
|
Michael Ramos
|
86f95e13e3 |
fix(codex): use getPlanDeniedPrompt for Stop-hook deny path
planDenyFeedback was called but never imported — the import was removed by #627 (configurable feedback pipeline) while #577 was in flight. Also corrects the tool name from hardcoded "Stop" to getPlanToolName("codex"). For provenance purposes, this commit was AI assisted. |
||
|
Andrei Ivanov
|
a22a744749 |
Add Codex Stop-hook plan review (#577)
* feat: add codex stop hook plan review * Install Codex plan review hooks * Remove Codex manual test screenshots * Update Codex plan mode docs * Tighten Codex release readiness * Preserve custom Codex hook wrappers * ci: smoke test release artifacts * ci: reduce release smoke flake risk * fix: keep Codex last-message extraction to output text * ci: poll release smoke servers on loopback * ci: skip macOS release smoke jobs --------- Co-authored-by: Michael Ramos <mdramos8@gmail.com> |
||
|
Michael Ramos
|
9b7c39d2a4 |
fix(review): move hide-whitespace to server-side git diff -w (#635 follow-up) (#638)
The client-side approach from PR #635 normalized file contents before diffing, which destroyed all indentation. Move whitespace handling to the server by threading a `-w` flag through the git diff pipeline. - Add `GitDiffOptions` to review-core.ts, inject `-w` in all 7 diff type paths + untracked file diffs - Thread options through git.ts → vcs.ts → review.ts / Pi server - Read `hideWhitespace` from ~/.plannotator/config.json on startup so the initial diff already respects the persisted preference - Accept `hideWhitespace` in `/api/diff/switch`, echo in responses - Client toggles trigger a lightweight server refetch that preserves the active file (no panel reset) - Handle edge case where current file disappears when `-w` removes whitespace-only diffs - Remove broken client-side parseDiffFromFile/normalize hack - Update API docs in AGENTS.md Closes the indentation bug reported by @zeroZshadow on PR #631. For provenance purposes, this commit was AI assisted. |
||
|
Michael Ramos
|
64c845fd2e |
feat(feedback): configurable plan, annotation, and review feedback (#627)
Unified feedback pipeline for all plan approvals, plan denials, annotation
feedback, and review suffixes. Users customize messages via config.json with
{{variable}} template interpolation and per-runtime overrides.
Closes #624
Co-authored-by: Aviad Shiber <aviadshiber@users.noreply.github.com>
For provenance purposes, this commit was AI assisted.
|