skills/backnotprop__plannotator
1
0
Fork 0
mirror of https://github.com/backnotprop/plannotator.git synced 2026-09-14 14:17:26 +08:00
Code Issues Packages Projects Releases Wiki Activity
1,096 Commits 375 Branches 154 Tags
feat/opencode2-native-commands
Commit Graph

122 Commits

Author SHA1 Message Date
Michael Ramos c2950e709f fix: pre-release QA findings for 0.27.9 (#1405) 
Fixes from the 0.27.9 pre-release review. Servers: an unreadable rendered-HTML root falls back to the startup snapshot on both runtimes with a once-per-process warning instead of hanging (Pi) or answering 500 (Bun); the version diff is recomputed against current bytes on reload and carried through the in-app Refresh instead of being dropped, with no history write on a GET. Client: a Refresh action on the compact touch shell; HtmlSurfaceControls renders Refresh independently of the eye; the dead HtmlSurfaceActions removed. Threading: one linear, cycle-safe reply resolution shared by the annotations panel, its sort, and the export (5,000-chain tests), PATCH ingest on both runtimes rejects self-references and cycles, nothing is ever dropped from feedback. WebMCP and viewer hygiene: bounded tombstone and request memories, per-instance minted ids, nudge id caps, waiter cleanup on unmount, a shared retry epoch for diagram blocks. Docs: HTML Refresh documented, the WebMCP design pointer fixed, marketing pages updated.

AI-assisted (Claude) under maintainer direction.
 2026-08-27 15:23:28 -07:00
Leonardo Reis 6407ef5d97 feat(annotate): manual refresh of rendered HTML from disk (#1232) 
Local rendered-HTML annotate sessions get a Refresh action beside Hide tools: the document is re-fetched through /api/doc, the sandboxed viewer remounts, annotations are re-anchored and the ones that no longer match are reported while their comments are kept, and stale diff and share state is reset. Maintainer additions on top of the contributor's work: share-link invalidation no longer keys on the resolver's identity, /api/plan and /api/share-html serve a local root HTML file from its current bytes on both runtimes so a reload does not revert the page under the annotations, the Refresh button keeps keyboard focus via aria-disabled, and the tests were hardened. Verified end to end in a real browser.

Thanks @leoreisdias.

AI-assisted (Claude) under maintainer direction.
 2026-08-26 14:53:28 -07:00
Michael Ramos 89f0b6628e feat(pi): live local app annotation through a shared proxy core and Node transport (#1366) 
Phase 2 of live app annotation: full parity on Pi over one shared
implementation instead of drifting copies.

- Extract every proxy decision into packages/shared/live-proxy-core.ts
  (HTML injector state machine, loopback/Host/Origin predicates,
  CSP/X-Frame-Options policy, redirect rewrite, WS origin gate, bridge
  assembly, liveAppDraftIdentity) and the CLI probe + live-mode messages
  into packages/shared/live-probe.ts. packages/server/live-proxy.ts is
  now a thin Bun transport over the core; its test suite passes
  unmodified.
- Add packages/shared/live-proxy-node.ts, the node:http transport the Pi
  extension runs: streaming request/response piping through the shared
  injector, and WebSocket (HMR) passthrough that replays the client's
  handshake upstream over raw TCP and pipes the sockets byte-for-byte.
  Transport tests run the proxy in a real node child process, because
  Bun's node:http shim drops writes to an upgrade event's socket.
- Wire Pi: /plannotator-annotate probes loopback URLs live-first with
  the shared probe (same 3s timeout, same <500 gate, same messages),
  recognizes --app/--static via parseAnnotateArgs's liveFlags opt-in
  (OpenCode deliberately does not opt in), and serves mode annotate-app
  from serverAnnotate.ts with the shared per-target draft identity,
  live sessions excluded from history/submissions, the remote hard-off
  throw, and guarded live-proxy shutdown.
- Vendor live-proxy-core/live-probe/live-proxy-node plus the
  dependency-free bridge-script constants to generated/.
- Docs: AGENTS.md phase-gate passages, marketing annotate page, Pi
  README.
 2026-08-21 10:29:29 -07:00
Michael Ramos 67f47dbac1 fix(annotate): armed-mode interaction fixes from the v0.27.5 QA gate (#1363) 
* fix(annotate): pre-release QA fixes for the armed-mode interaction seams

Six confirmed QA findings on the HTML/live annotate surface plus missing
pi-extension resync coverage:

1. Armed pinpoint drifted click (>4px, no selection) was swallowed AND
   leaked to the page: the always-on drag work armed the trailing-click
   suppression on drift alone. The mouseup arming site now requires the
   drag to have actually produced a text selection; drifted clicks pin
   normally and never reach the page. Bridge tests for armed drift,
   armed real drag, and Interact drift.
2. Esc ladder: hover-clear is no longer its own rung; clearing the
   pinpoint outline and posting annotate-exit happen on the same press
   when no draft is open. Draft-close keeps its own press.
3. Compact touch layouts no longer apply a restored toolsHidden:true
   chrome cookie (both header toggles are desktop-only, so applying it
   stranded the user); the cookie value is preserved for desktop.
4. The live-app probe now announces the static-conversion downgrade on
   stderr when a loopback probe fails, naming --app to force live mode.
5. Live-app export: page group headers are now '## Page:' with '### N.'
   entries nested below them; exports without pageUrl stay byte-identical.
6. Shift+1-4 mode shortcuts no longer fire while the annotation
   toolbar's type-to-comment listener owns printable keys, so typing
   ! @ # $ into a starting comment cannot silently switch modes.

Also adds the missing tests for the two resyncPhaseFromSession
executing->idle fallbacks that arm idleNoticePending (verified by
mutation: flipping either arm fails its test).

* fix(annotate): compact arm/disarm affordance, guarded shutdown, restored chrome guards

Follow-up scope from the forensics sweep, same surface:

- Compact touch layouts get Options-menu actions for the HTML/live
  surface: 'Annotate page'/'Interact with page' (the desktop pen and
  Mod+Shift+A were unreachable on touch, so every tap annotated with no
  way out) and 'Show tools'/'Hide tools' (the desktop eye). With the
  menu as the way back, the toolsHidden cookie now applies on compact
  again (desktop parity) instead of being ignored.
- The annotate servers' stop() now guards every disposal step
  individually (Bun: runGuardedShutdown, mirrored inline in Pi): a
  throwing agent-terminal teardown (#1314-class) no longer skips
  liveProxy.stop() and the other disposals after it. Unit-tested with a
  throwing disposer.
- Re-added the two regression guards dropped in the htmlHideTools ->
  htmlChrome test rename: the restore commit never writes stale
  pre-restore chrome values to the cookie, and the sidebar stays
  reachable via Mod+B while tools are hidden.

* fix(annotate): scope the Agent TUI display reset to display settings only

The Display popover's 'Reset terminal display settings' button also called
onSideChange('left'), durably overwriting a user's chosen right/hidden
placement in config.json with no disclosure — the label scopes the reset
to font/appearance. Position is a layout preference with its own explicit
segmented control right below, so the reset no longer touches it: the
button now resets exactly the display settings through the panel's one
sanitized update path, and the popover no longer has any code path from
reset to the side.

AgentTerminalDisplayPopover is now exported with a defaultOpen test seam
(the surrounding panel needs a live WebTUI session to render it); tests
assert reset restores the display defaults without firing onSideChange,
and that the Position control remains the explicit way to change
placement.
 2026-08-21 08:55:30 -07:00
Leonardo Reis 81ecd67e75 feat(annotate): configurable Agent TUI placement with durable config and Hidden state (#1050) 
* Allow annotate terminal to dock on either side

* Allow annotate terminal to dock on either side

* fix(annotate): persist Agent TUI preferences through the settings registry

The Position control introduced in #1050 stored its choice in a cookie via
hand-rolled helpers that bypassed the settings registry. Every annotate
session runs on its own random port, so a cookie is scoped to one session:
the placement silently reset on the next annotate. The sibling
`plannotator-annotate-agent-terminal-default` cookie (preferred agent) had
the same gap.

Both now follow the `conventionalComments` precedent exactly:

* `agentTerminalSide` and `agentTerminalDefaultAgent` join `SETTINGS` with
  serverKey/fromServer/toServer, reusing their existing cookie keys so a
  user who already picked a side keeps it across the upgrade.
* `PlannotatorConfig` gains both as flat keys (only diffOptions, theme,
  reviewAnalysis and prompts deep-merge in saveConfig), emitted from
  `getServerConfig()` behind an `isAgentTerminalSide` guard so a
  hand-edited config.json cannot advertise a side that does not exist.
* Both keys are added to the two /api/config allowlists: the Bun annotate
  server and the hand-mirrored Pi one.

The side vocabulary moves to @plannotator/core/agent-terminal (widened to
include the `hidden` state added next) so the registry can reach it without
closing an import cycle through ConfigStore; the ui util keeps its public
API by re-exporting.

Regenerates the pinned guide viewer manifest, which shifts by 0.1 KB gz
because the settings registry now reaches into core/agent-terminal.

AI-assisted (Claude) under maintainer direction.

* feat(annotate): add a Hidden Agent TUI position and extract its layout

Builds on the Left/Right Position control from #1050.

Hidden (third state of the Position control)

  Hidden is a durable preference that the Agent TUI is not part of this
  user's layout: nothing is docked, and choosing Hidden while the terminal
  is open closes it (from either surface that offers the control). It is a
  default, not a lock. The rail toggle, the Shift Shift shortcut and a
  message routed to the agent all still open the panel for the session, and
  none of them rewrites the preference, so explicit intent wins now without
  changing what happens next session. A `hidden` preference owns no dock
  edge, so a session open falls back to the historic left placement.

  Because the Position control lives inside the terminal's own popover, and
  Hidden closes that popover along with the terminal, the same control is
  now also in the Settings dialog (General tab, annotate mode). That is the
  way back from Hidden, and it also answers the review note that Position
  could not be preconfigured before the terminal was ever opened. It is
  gated on the terminal actually being available in the session so a remote
  or runtime-less annotate never offers a dead control. Both surfaces write
  the same `agentTerminalSide` config value and read it through ConfigStore,
  so they cannot drift.

  The existing transient hide affordances (header X, resize handle click and
  drag-snap, rail toggle, Shift Shift) are unchanged and stay session
  scoped. A running agent still stays mounted off-layout when collapsed, so
  hiding the panel never kills the PTY.

Review fixes

* Extract `getAgentTerminalLayout` from App.tsx into
  packages/editor/agentTerminalLayout.ts with a table test over
  {side including hidden} x {open} x {running} x {wideMode} x
  {belowBreakpoint} x {rightPanelOpen}, asserting the invariants that can
  actually regress: never docked on both edges, never visible below `lg` or
  in wide mode, a collapsed running terminal stays mounted zero-width on its
  own edge, and the right panel is suppressed exactly when a VISIBLE
  right-docked terminal holds the slot.
* Fix `aiSurfaceOpen`, which still read `effectivePanelOpen &&
  rightSidebarTab === 'ai'` after its siblings moved to
  `isRightPanelVisible`. A right-docked terminal visually suppresses the
  panel but left the Ask AI model-discovery effect firing for an invisible
  surface, which is exactly the eager provider work that gate exists to
  avoid. The layout computation is hoisted above the consumer so it can use
  the same fact the JSX does.
* Document the right-slot invariant at both coordination sites. The
  asymmetry is deliberate: the panel evicts the terminal (which keeps
  running off-layout, so reopening resumes the same session), while the
  terminal only suppresses the panel visually so dismissing it restores the
  user's place. Symmetry would make every short terminal detour cost the
  reviewer their open surface.
* Name the `useIsMobile(1024)` literal `AGENT_TERMINAL_LG_BREAKPOINT`, tied
  to the panel's own `hidden lg:flex`.
* Restore `hideAgentTerminal()` in the resize hook instead of the raw
  setter, and point the handle at the resolved placement.

AI-assisted (Claude) under maintainer direction.

---------

Co-authored-by: Michael Ramos <backnotprop@gmail.com>
 2026-08-20 17:00:22 -07:00
Michael Ramos 15f8d4fe4c feat(review): collapse generated files by default in the all-files view (#1346) 
* feat(review): collapse linguist-generated files by default (#1317)

Code review now respects linguist-generated (and linguist-generated=true)
from .gitattributes, collapsing those diffs by default the way GitHub does.

Server (Bun + Pi mirror): a generatedFiles sidecar rides /api/diff and
/api/diff/switch, resolved through git's own attribute machinery — one
batched 'git check-attr --stdin -z' over the served patch's paths at the
review cwd, so stacked and negated rules land exactly as git resolves
them. Plain local git sessions only; PR worktrees, workspace multi-repo,
jj, GitButler, and P4 omit the sidecar (degrade to no-collapse). Shared
logic in packages/shared/generated-files.ts, vendored to Pi.

Client: generated files SEED their CodeView item collapsed (the existing
Pierre collapse state — same mechanism as commit-diff folding), render
the one-line FileHeader bar with a 'generated' tag next to the +/- counts,
and expand per file on click. Expansion is session-local App state so it
survives remounts and diff switches. Presentation-only: the diff data,
annotations, search, and Edit Mode are untouched; the file tree and
single-file tabs list generated files normally (tag, no auto-collapse).

Guide viewer manifest pin regenerated (AllFilesCodeView/FileHeader are
bundled into the guides.show viewer) from a clean frozen-lockfile install.

* feat(review): built-in generated defaults, visible collapsed strip, review-round fixes (#1317)

Round 2 on PR #1346, per maintainer review.

Built-in generated defaults (industry-standard two-layer detection):
packages/shared/generated-files.ts (vendored to Pi) now carries
DEFAULT_GENERATED_PATTERNS — lockfiles (package-lock.json, yarn.lock,
bun.lock, Cargo.lock, go.sum, ...) plus *.min.js / *.min.css / *.map —
matched against the path's last segment only. Explicit .gitattributes wins
in BOTH directions: linguist-generated (set/true) marks any file,
-linguist-generated / =false un-marks even a built-in name, unspecified
falls through to the defaults. In plain local git sessions check-attr
refines the defaults; the non-git degrade modes (piped patches, PR
worktrees, workspace, jj, GitButler, P4) now emit the sidecar from the
name-based defaults alone instead of omitting it.

Visible collapsed state: a collapsed generated card no longer renders as a
bare header — a GeneratedFileNotice strip ('Generated file collapsed',
+N/-N, 'Click to view') styled like the other below-header notices sits in
the card, and clicking it expands through the SAME reportFileCollapsed
funnel as the chevron.

Review findings:
- F1: search-match and sidebar-comment navigation expanded items without
  reporting through the funnel, so those expansions died on diff switch.
  Both now call syncAllCollapsedMirror + reportFileCollapsed; the funnel
  invariant comment lists the navigation-driven sites.
- F2: the check-attr call gets the same 5000ms timeout as review-core's
  stdin git callers, and Pi's vcs.ts stdin write gets the one-line EPIPE
  guard (call-flow.ts shape) a timeout kill makes reachable.
- F3: removed the dead prevGeneratedRef + collectSetDelta leg — a changed
  generated set always remounts via fileSetKey, so the delta path was
  unreachable.

Tests: default-list matching (glob + directory-named-bun.lock), both-
direction precedence, non-git sidecar from defaults (dual-runtime), the
placeholder strip through the funnel, and search expansion surviving a
re-seed round-trip. AGENTS.md payload docs updated. Guide viewer manifest
pin regenerated from this clean frozen-lockfile worktree.
 2026-08-17 23:32:29 -07:00
FND e1ce7dabe1 feat(ui): Totman/Classic P favicon style switcher (#1325) 
Favicon style switcher in Settings > Theme: the Totman mascot or the historical dark-navy P tile (byte-identical to the pre-Totman asset, sha256 pinned). Served server-side from first paint in both runtimes; opt-in for hosts of the published UI package. Contributed by @FNDEVVE
 2026-08-16 21:58:46 -07:00
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
 2026-08-16 12:17:13 -07:00
Graeme Folk e3091331a5 feat(review): jj support for Call Flow analysis (#1312) 
Adds Jujutsu (jj) as a Call Flow analysis provider: jj-current/jj-last/jj-line/jj-all snapshot revsets with deterministic first-parent resolution across merge revisions, root-anchored filesets so results are cwd-independent, bounded snapshot materialization (base tree + changed-file delta) with a streamed 64MB output ceiling in both the Bun and Pi runtimes, and real-jj regression tests covering merges and subdirectory invocation.

Contributed by @graemefolk, who also built the original jj integration. Review fixes pushed in-branch: merge-parent resolution, root-glob filesets, bounded materialization and buffering, plus CI gating guards for runners without jj.
 2026-08-15 10:47:09 -07:00
Michael Ramos 192b026073 fix(annotate): stop the folder watcher freezing the server (#1314) 
* fix(annotate): stop the folder watcher freezing the server (#1313)

The file-browser content watcher built a chokidar scan over the whole
workspace synchronously on the request path. Under Bun that scan
monopolizes the event loop (a 780-directory nested tree measured 79
seconds), and because teardown was immediate on the last unsubscribe,
every EventSource reconnect paid the scan again: the reconnect the
freeze itself provoked made the hang self-sustaining.

The watcher engine now lives once in
packages/shared/file-browser-watch-core and both runtimes keep only
their transport:

- construction is deferred off the request path, so the SSE ready event
  and concurrent API requests are served before any scan starts
- teardown gets a 30s reconnect grace; a reload reuses the warm watcher
- on macOS and Windows the content watcher is the platform's native
  recursive fs.watch (measured ~0ms for the same tree); chokidar stays
  the Linux backend and the runtime fallback, with a forced catch-up
  refresh on the swap so no events are lost
- server stop tears every watcher down immediately in both runtimes

The responsiveness regression test reproduces the reported freeze on
the pre-fix implementation (79s, fails) and passes in under a second on
the fix.

* docs: folder annotate sessions do write per-file version history

The PLANNOTATOR_ANNOTATE_HISTORY row claimed URL, folder, and
annotate-last sessions never write to the data dir. The folder /api/doc
path deliberately runs the per-file version-history pipeline (lazily,
memoized per resolved path, gated on the same flag) to power the
per-file version diff, and has since it shipped. The code is the
intended behavior; the sentence was stale. URL and annotate-last
sessions remain fully stateless, and submit records remain single-file
only.

* fix(annotate): review follow-ups for the watcher engine

Applied from the independent review of #1314:

- contentWatchBackend gains a forced 'native' mode and the fallback
  tests use it, so the native-to-chokidar paths (creation failure and
  runtime error) genuinely execute on Linux CI; the runtime-error test
  is no longer macOS-only
- a platform-agnostic responsiveness test pins that SSE ready is served
  before the scan starts on the chokidar backend, via the runtime test
  hooks; the tight full-scan bound stays macOS-only
- watcher construction failures and the native-to-chokidar swap now log
  one console.error each instead of stranding subscribers silently;
  the swap also increments the diagnostics start counter honestly
- closeEntry guards both watcher close() calls; the Bun annotate stop
  chain got the same try/finally shape as the plan server; all four
  stop chains close watchers ahead of throwable disposals so a failing
  dispose cannot strand a watcher keeping embedded hosts alive
- a broadcast that empties the subscriber map by deleting dead
  subscribers now schedules the teardown grace instead of leaving the
  entry live until closeAll
- bun.lock drift reverted: only the chokidar edge and the workspace
  version corrections remain (27 unrelated esbuild resolution entries
  dropped; frozen-lockfile install verified)
- stale never-write comments in both annotate servers corrected to
  match the folder per-file history reality documented in AGENTS.md;
  the engine header now states plainly that chokidar is a correctness
  fallback, not a performance one
 2026-08-13 16:02:41 -07:00
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.
 2026-08-13 09:47:18 -07:00
Michael Ramos 14e5c9ebd1 Fix folder watcher cold-start refs scan (#1306) 2026-08-13 09:45:59 -07:00
Michael Ramos 356b628b6f ci(security): add Semgrep CE and Trivy monitoring (#1294) 
* ci(security): add Semgrep CE and Trivy monitoring

* fix(ci): diagnose Trivy coverage assertions

* fix(ci): accept Trivy repository scan metadata

* fix(ci): harden scanner failure diagnostics
 2026-08-12 20:40:08 -07:00
Michael Ramos 1aaedf9330 fix(review): detach open-in-editor launches and bound the wait (#1289) 
Open-in-app launchers are now spawned in their own process group and the
request waits only a short grace (2s) for instant failures, with stderr
drained concurrently from spawn time. A launcher that is still running at
the deadline is treated as launched and the request resolves ok; instant
failures keep the existing friendly error shape (not-found, exit code plus
stderr). Mirrored in the Pi server (two-runtime law).

Fixes two demonstrated defects: the request (and the UI button) was held
hostage until the launcher CLI exited, and a launcher in the session's
process group could be killed along with the session, taking a cold-started
editor down with it.
 2026-08-12 17:06:22 -07:00
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.
 2026-08-12 12:07:30 -07:00
Michael Ramos fc348687bf fix(review): contain /api/call-flow analysis throws as JSON error responses (#1272) 
* fix(review): contain /api/call-flow analysis throws as JSON error responses

A hard VCS failure during patch materialization escaped the handler in
both runtimes. On Pi the unhandled rejection reached the process-level
handler and killed the user's session; on Bun it surfaced as a non-JSON
500 the client's quiet-failure UX could not parse. Both handlers now
return the standard { status: "error", reason: "analysis-failed" }
envelope.

* fix(review): cut the Call Flow consent copy down to the three facts that matter

Six sentences of disclosure read as noise. The dialog and Settings now
say: what it does, what it installs (languages + size), Node 22+, and
that other languages install as needed. Nothing consent-relevant was
removed.

* test(review): pin consent-copy facts, not prose

The presentation test now asserts the server-derived facts (languages,
size, Node floor); the dialog and Settings tests assert only that the
disclosure prop renders, via a sentinel string. Copy edits no longer
break three test files.

* docs: add Testing Rules to AGENTS.md (no prose-pinning, no round-trip prop tests)

* docs: refine copy-pinning rule — deliberate locks allowed, incidental snapshots banned

* fix(review): use the maintainer's Call flow description in the intro dialog and Settings

* fix(review): Call flow description is the maintainer's exact copy; remove the dynamic disclosure plumbing

The intro dialog and Settings now show only: 'Diffs for function call
stacks across git commits. 22 languages supported (AST-based, built
using Tree-sitter).' The callFlowEnableDescription prop, its App wiring,
and getCallFlowEnableDescription are removed; install size and Node
requirements remain visible in the Call Flow panel itself.

* fix(review): reject empty-path worktree diff types; clean up QA findings

- parseWorktreeDiffType returns null for a worktree diff type with no path.
  An empty path resolved to an empty cwd, and Bun.spawn({ cwd: "" }) runs
  git in the server's own directory instead of the target repo, so a
  malformed 'worktree:' switch returned an unrelated checkout's diff.
  Fail closed to the caller's real cwd. (Pre-existing; surfaced by QA.)
- Remove an orphaned JSDoc comment left by the callFlowEnableDescription
  prop removal in Settings.tsx.
- Add useCallFlowAnalysis.test.tsx to the CI DOM_TESTS list; its two
  tests were silently skipping on every run.
 2026-08-11 22:55:18 -07:00
Michael Ramos 9ee2e83287 feat(review): make the CallDiff runtime a strictly opt-in, in-UI install (#1270) 
* feat(review): make the CallDiff runtime a strictly opt-in, in-UI install

The merged CallDiff integration eagerly installed a ~784MB runtime for
every user at install time, for a feature that is off by default. The
runtime is now strictly opt-in and the opt-in lives in the review UI:
toggle Call flow, click Install in the panel, watch staged progress, and
use the analysis in the same session.

Installers: the default sequence no longer installs the runtime. Opt in
with --with-call-flow (PowerShell: -WithCallFlow),
PLANNOTATOR_INSTALL_CALLDIFF=1, or { "installCallFlow": true } in
config.json (flag > env > config). PLANNOTATOR_SKIP_CALLDIFF_INSTALL is
deleted; --minimal keeps excluding the runtime; the installer prints an
honest note pointing at the in-app install. The headless CLI path
(plannotator install-runtime call-flow) is unchanged.

Server (both runtimes, contract-identical): POST /api/call-flow/install
starts installCallFlowRuntime() in the background via a single-flighted
coordinator (concurrent POSTs join the in-flight install), runs a
Node 22+ preflight before any download (distinct node-unavailable
error), and rejects cross-origin POSTs with 403. GET
/api/call-flow/install-status reports idle/running/done/error with
stage: downloading, verifying, installing-deps, building. Install
completion invalidates the 30s runtime probe cache so the next
capability advert resolves available without a server restart.

Client: the Call flow Dock's runtime-missing state is now the opt-in
funnel with an honest disclosure (about 800 MB on disk, Node 22+,
one-time), staged reduced-motion-safe progress, and error + retry with
a no-node hint. On done the advert is refreshed through
POST /api/review-analysis and the existing available-change refetch
starts the analysis for the current snapshot with no reload. The intro
dialog and Settings toggle note the separate first-use runtime.

Docs: AGENTS.md env table + Review Server API table, marketing
environment-variables / installation / ui-settings / code-review /
api-endpoints pages, and the CallDiff ADR runtime-boundary and server
contract sections.

* test(review): stop leaking PLANNOTATOR_DATA_DIR from the install endpoint tests

The call-flow install endpoint tests overrode PLANNOTATOR_DATA_DIR at
module-eval time and never restored it. bun runs CI's full suite in one
process and evaluates every test file's module before running tests,
while Pi's generated/storage.ts caches its data dir at import time; the
override therefore made storage's cached dir and later files' live
getPlannotatorDataDir() calls disagree, failing the Pi annotate-history
unwritable-dir test and both durable-submit-record tests.

An afterAll restore alone is not enough: it reproduces the same three
failures with the mismatch inverted (storage caches the leaked dir at
module eval, tests then run against the restored one). The env var is
now never touched at module-eval time at all; it changes only inside
tests and is restored to its original value in afterEach, exactly like
the PORT/PATH pattern. The config writes the advert tests persist
through the process's frozen config module are snapshotted at load and
restored in afterAll so a standalone run never flips a real
config.json setting, and the process-global scope of the mock.module
seams is documented.

Regression proof (previously failing in either mismatch direction, now
green in both orderings):

  bun test packages/server/call-flow-install-endpoint.test.ts \
    apps/pi-extension/server/annotate-history.test.ts \
    apps/pi-extension/server/annotate-submission.test.ts

* feat(review): install CallDiff grammars selectively

* fix(review): harden CallDiff worker environment

* fix(review): close CallDiff verification gaps
 2026-08-11 16:28:08 -07:00
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
 2026-08-11 13:18:35 -07:00
Michael Ramos 98113182b5 feat(guide): reviewer-supplied extra instructions for Guided Review (#1267) 
* feat(guide): reviewer-supplied extra instructions for Guided Review (#1265)

Adds a quiet, collapsed-by-default Custom instructions affordance to the
guide launch page. The text is APPENDED to the built-in organizer
methodology as a clearly delimited section (composeGuideMethodology) and
never replaces it; absent or blank instructions produce byte-identical
prompts to before. Persisted in a dedicated cookie
(plannotator-guide-instructions) so a standing team preference survives
sessions without bloating the plannotator.agents blob past the browser's
per-cookie limit.

Server side, the launch body gains an optional guide-only instructions
field (both the Bun and Pi node:http agent-jobs handlers accept and
thread it); prompt composition lives in the shared guide-review.ts that
vendor.sh already vendors to Pi, so both runtimes compose identically.
Text is capped at GUIDE_EXTRA_INSTRUCTIONS_MAX_CHARS (2000) server-side
and mirrored by the textarea maxLength. Repair launches deliberately
ignore instructions: a repair is a mechanical JSON fix, not a rewrite.

Tests pin the regression contract (empty input keeps prior prompt bytes),
appended-not-replacing composition, the length cap, repair isolation, and
the cookie round-trip via the storage backend seam.

* refactor(guide): store standing instructions server-side, not in a cookie

Review findings on the cookie approach (silent write failure past the
encoded 4KB per-cookie limit for multi-byte text) pointed at the real
design problem: the instructions are consumed by the SERVER at launch
time, so they belong in the data dir like review-skills.json, where no
size ceiling or encoding inflation exists and the preference follows
the machine instead of one browser profile.

New GET/PUT /api/agents/guide-instructions in both runtimes backed by
shared guide-instructions-store (vendored to Pi). Guide launches apply
the stored text when the body carries none; the launch page still sends
its live textarea value (explicit wins), so a just-typed preference can
never race the debounced save. The sidebar surface sends nothing and
inherits the stored text server-side. All cookie machinery removed.

Also folds in the review fixes: marker-tag-shaped strings in
instructions are defanged so first-match nonce recovery cannot be
hijacked by pasted examples.
 2026-08-11 10:24:39 -07:00
Michael Ramos e24bd8464f fix(annotate): persist submitted feedback before deleting the draft (#678) (#1237) 
* fix(annotate): persist submitted feedback before deleting the draft (#678)

* fix(annotate): scope durable submit records to single local files

Adversarial verification found the durable record had no mode gate: an
annotate-last or URL session, which was completely stateless before,
would persist submitted feedback quoting the agent's message or the
fetched page under history/, widening the documented annotateHistory
contract without a docs change. The record now shares the exact
eligibility gate the version history uses (mode annotate, non-URL path),
so previously-stateless modes stay stateless.

Also makes persistSubmittedDecision defensive about body types:
/api/feedback does no validation (unlike /api/approve), and a non-string
feedback previously flowed through settle() untouched with a 200; the
new .trim() guard turned that into a thrown 500 after the decision had
already settled. Malformed values now degrade to the exact legacy
behavior (settle, delete draft, 200) instead of throwing.

Both changes mirrored in the Pi server, with regression tests in both
runtimes: stateless modes write no record, and a malformed feedback body
returns 200 with the draft deleted and nothing persisted.
 2026-08-09 16:16:57 -07:00
Michael Ramos 7ad4d39ed9 feat(comments): reference agent skills with / or $ in plan review and annotate comments (#1229) 
* feat(comments): reference agent skills with / or $ in plan and annotate comments

Typing / or $ at the start of a word in the document-UI comment composer
opens a picker of the user's global agent skills (~/.claude/skills,
~/.codex/skills, ~/.agents skills roots), served by a new GET /api/skills
on the plan and annotate servers in both runtimes (Bun + Pi mirror).
Multiple references per comment are supported; references live in the
comment text itself and are appended to exported feedback as a
'Skills referenced' block so the acting agent knows which skills to apply.

Human-invocation-only skills (disable-model-invocation: true frontmatter)
stay listed and selectable but render dimmed with a badge, warn in the
menu and composer, and are marked in the export so the agent is never
asked to invoke something it cannot.

Discovery reuses the review-skill loader (same roots, precedence, and
skip-and-log discipline), reads only an 8KB head per SKILL.md, caps the
catalog at 500 skills, takes no client input, and is never persisted;
any failure degrades to plain typing.

* fix(comments): harden skill references per review (trigger, IME, seam, fail-closed frontmatter)

Blockers:
- B1: a trigger now requires at least one query character. A bare / or $
  no longer opens the catalog, so Enter stays a newline and Tab still
  leaves the field ("This costs $" + Enter, "cd /" + Tab, bullets).
- B2: the menu ignores keys mid-IME-composition (nativeEvent.isComposing),
  matching the 16 existing guards; Enter committing a Pinyin/Telex/Korean
  candidate can no longer insert a skill.
- B3: the catalog request is a host seam (skillCatalogTransport via
  configurePlannotatorUI), defaulting to the existing GET /api/skills.
- B4: resetSkillCatalogCache() invalidates outstanding requests
  (generation counter), and a late-resolving stale request can no longer
  overwrite a newer cached value or the export registry. The catalog
  tests reset in beforeEach, so they hold in any file order.

Also:
- F1: skillReferences={false} is fully inert — the human-only notice memo
  and the cache seed are gated on the prop.
- F4: frontmatter flag parsing no longer fails open: trailing YAML
  comments are stripped, on/1 (and TRUE/yes etc.) read as true, the head
  read is 64KB, and truncated unterminated frontmatter fails CLOSED on
  disable-model-invocation.
- F5: extraction ignores markdown link destinations ([x](/name)), shell
  redirects (cat /x > out), and /-triggered FHS root names (/run, /tmp);
  menu insertion switches / to $ for those names so inserted references
  always survive extraction.
- F6: the 500-skill cap slices after sorting, so which skills survive no
  longer depends on readdir order.
- F3: /api/skills wiring guards for the Bun and Pi plan + annotate
  servers (skills-endpoint.test.ts).
- Keyboard state machine tests against the real CommentPopover in
  happy-dom (bare trigger, insertion, composition, Escape, highlight
  bounding, opt-out inertness), added to the CI DOM step.
- The insertion path dismisses the trigger start so the menu close is
  ordering-safe against React's select-plugin re-reading a stale caret.

* feat(comments): redesign the skill reference menu (bare triggers, no preselection, highlighted tokens)

Per maintainer direction, reversing the earlier bare-trigger opt-out
deliberately: typing a bare / or $ at the start of a word now opens the
full skill catalog immediately, and the safety story moves from the
trigger to the menu itself.

No preselection (the load-bearing rule): the menu opens with NO row
active, and while nothing is active every key behaves exactly as if the
menu were closed. "This costs $" + Enter is a newline; "cd /" + Tab
leaves the field (the proven regression that must never return). A row
activates only via ArrowDown/ArrowUp (Down from none lands on the first
row, Up on the last); only then do Enter/Tab insert. Pointer hover never
activates a row, because the menu floats exactly where the mouse rests
over the composer; a click inserts directly and never arms Enter.
Continuing to type re-filters and disarms any active row. Escape clears
the active row and dismisses when the user engaged (query typed or row
active); an unengaged bare-trigger menu passes Escape through so closing
the composer still costs one press.

Menu redesign to the reference look: icon, bold name, dimmed inline
description with ellipsis, right-aligned source column (Agents / Claude
/ Codex from the discovery roots), rounded generously padded rows, and a
subtle active-row background; human-only rows stay dimmed with their
badge and the warning now shows while such a row is ACTIVE.

Inserted references render highlighted in the composer via a mirrored
aria-hidden overlay behind a transparent-text textarea (identical font,
padding and wrapping metrics; scroll synced; tokens change color and
background only, drawn from the --primary theme token so every palette
works in light and dark). The caret keeps --foreground, selection uses a
translucent primary wash, and IME composition temporarily restores
native textarea text so composition underlines render normally.
skillReferences={false} still renders the plain pre-feature textarea.

Also, per review:
- extraction: dropped the over-broad shell-redirect exclusion (false
  negatives on prose like "use /animate <- this one"; the motivating
  case stays covered by the reserved-path rule)
- frontmatter: an unterminated frontmatter block now fails CLOSED on
  disable-model-invocation even in complete (untruncated) files
- the reserved-path / to $ insertion switch stays: extraction still
  reads /run as a path, and the new token highlight makes the switch
  self-explanatory (an unhighlighted insert would look broken)

The composition guard, transport seam, catalog generation counter,
enabled gating, and export rules are unchanged and re-covered by the
rewritten DOM test matrix.

* fix(comments): give the skill reference menu adaptive, viewport-clamped placement

The menu rendered bottom-full with a fixed max-h-64: always upward, up to
256px, with no viewport awareness. With the comment popover near the top of
the viewport (annotating near the top of a document), typing a trigger ran
the menu off the top of the screen with its upper rows unreachable.

Placement now mirrors the popover's own computePosition idiom: measure the
space above and below the composer wrapper against window.innerHeight,
prefer above (the shipped direction; keeps the action row and human-only
notice visible), flip below when the list fits below but not above, and when
neither side fits pick the roomier side. The list's max height is clamped to
the available space (still capped at the former 256px), so the menu never
extends past a viewport edge. Recomputes on every commit (drag moves,
popover flips, filtering changing the item count, warning-footer toggles)
plus capture-phase scroll and resize listeners, matching the popover's
tracking. Visual design of the menu and rows is unchanged.

* feat(comments): inject human-only skill instructions into exported feedback

A human-only skill (disable-model-invocation: true) referenced in a review
comment used to export as a dead name the agent could do nothing with. A
human referencing a human-only skill IS the human invocation, so the export
now injects the skill's SKILL.md body verbatim (frontmatter stripped) inside
clearly delimited BEGIN/END SKILL INSTRUCTIONS markers, with the absolute
skill directory and the resolve-relative-paths pointer so references/,
scripts/, and assets/ stay actionable. Model-invocable skills keep exporting
as names the agent can invoke itself.

Transport is lazy: a new GET /api/skills/content?name= endpoint (Bun and Pi)
serves one discovered skill's body, capped at 20k chars with an explicit
truncation notice pointing at the file; the client fetches contents only for
the human-only skills actually referenced, keyed off comment state, and the
catalog now carries each skill's absolute dir so every failure path (deleted
skill, unreadable file, race with submit) degrades to naming the skill plus
its directory. Names are matched against discovery only and never used as
paths, so traversal cannot escape the skill roots. A per-export dedupe
injects each skill once even when several comments reference it, and
GLOBAL_COMMENT annotations run through the same block.

The referenced-skills header now says the reviewer is asking for the
invocation, and the human-only menu footer and composer notice explain that
the skill's instructions will be included with the feedback instead of
warning that the reference will not work.

* polish(comments): quiet, progressive human-only skill treatment

The human-only surfaces shipped with too much emphasis: a dimmed row plus
a bordered uppercase badge, an amber warning footer, and a persistent
amber notice in the composer after insertion. Human-only is a property of
a skill, not an error state, so the treatment is now quiet and
progressively disclosed:

- Menu rows render at full strength with a small muted 'human-only' pill
  (bg-muted / muted-foreground tokens; no border, no dimming).
- The plain-language explanation (a model cannot invoke it, so its
  instructions will be included with your feedback) appears as a muted
  footer only while a human-only row is active (keyboard) or hovered
  (pointer). Hover disclosure is purely visual state local to the menu;
  it never touches activeIndex, so the no-preselection invariant and the
  hover-never-arms-Enter rule are unchanged and re-asserted by a new test.
- When not disclosed, the same sentence stays in the DOM sr-only and
  human-only rows point at it with aria-describedby, so the state reaches
  assistive tech as text rather than as a purely visual badge (this does
  not attempt the #1233 combobox semantics, and does not worsen them).
- After insertion, the highlighted token itself carries the quiet inline
  marker (a dotted primary underline; text-decoration cannot move glyphs,
  so overlay alignment is untouched) and the standing amber notice is
  replaced by a native <details> disclosure: a single muted 'Includes
  skill instructions' summary line that expands to the full accurate
  sentence, operable by pointer, keyboard, and AT alike.

No amber remains; every color is a theme token (muted, muted-foreground,
border, primary, ring), so the treatment follows every palette in light
and dark. Copy is unchanged where it was accurate. Behavior is unchanged:
human-only skills stay selectable and injection still happens.

* fix(comments): harden human-only skill injection per adversarial review

Three findings on the injection path, each with tests that fail pre-fix:

1. Marker forgery: an injected SKILL.md body containing our own
   `--- BEGIN/END SKILL INSTRUCTIONS ---` markers (or an
   `[Instructions truncated:` notice) could close the block early — making
   everything after it read as the reviewer's own words — forge a block for
   a skill nobody referenced, or forge a truncation notice pointing at an
   attacker-chosen path. Body lines matching the structural marker forms
   (leading-whitespace and case variants included) are now visibly
   neutralized before injection: kept verbatim but prefixed, never silently
   deleted (neutralizeSkillMarkerLines).

2. Forged human invocation: POST /api/external-annotations is
   unauthenticated on localhost, so any local process could submit a
   comment referencing a human-only skill and cause its instructions to be
   injected "at the reviewer's request". Annotations carrying a `source`
   now still LIST their skill references but never cause verbatim
   injection — human-only references fall back to naming the skill plus
   its directory, with an honest reason. The content-prime effect skips
   external texts for the same reason. A human referencing a human-only
   skill IS the human invocation; a tool is not.

3. Unbounded read: readReferenceSkillContent read the whole SKILL.md
   before slicing to the 20k cap, so an unauthenticated no-cors fetch loop
   could balloon RSS by file size per request (measured +64.4MB for a 64MB
   file). It now uses the same bounded readFileHead as the catalog,
   reading only frontmatter allowance + 4 bytes per capped char + slack;
   truncation detection is unchanged for any file whose frontmatter fits
   the catalog bound, and frontmatter that overflows the read falls back
   to null rather than serving raw YAML. Measured: 12 reads of a 64MB
   SKILL.md now cost +5.1MB total.

Also: the fast-fail guard no longer rejects legitimately discovered names —
`name.includes("..")` 404'd a real `v1..2` skill dir forever (and `\` is
legal in POSIX names) while defending nothing, since the name is only ever
matched against discovery output and never joined into a path. It now
rejects exactly the names that can never be a readdir entry: empty, `.`,
`..`.
 2026-08-07 09:50:42 -07:00
Michael Ramos b69742c3bf feat: add PLANNOTATOR_URL_HOST display-only override for advertised URLs (#1225) 
* feat: add PLANNOTATOR_URL_HOST display-only override for advertised URLs

Remote mode binds 0.0.0.0 but every advertised URL hardcoded
http://localhost:<port>, so a session opened from another device (e.g. a
phone on the same tailnet) got an unopenable link (#657).

- resolveUrlHost() in packages/shared/config.ts: PLANNOTATOR_URL_HOST env
  var over config.json urlHost, validated host-only (bare hostname, IPv4,
  bracketed IPv6); invalid values warn once and fall back to localhost.
- buildAdvertisedUrl(port) in packages/server/remote.ts and its Pi mirror
  in apps/pi-extension/server/network.ts; all 7 construction sites use it.
- Strictly display-only: binding stays governed by PLANNOTATOR_REMOTE, and
  agent-review jobs get a pinned http://127.0.0.1:<port> API URL.
- Remote-ready copy says "open on your device" when the host is
  overridden; local sessions with an override warn it is unreachable.
- Tests for validation, precedence, and URL composition in both runtimes;
  docs in CLAUDE.md and the marketing site.

* fix(review): ignore urlHost in local sessions, harden warning output

Review follow-ups on #1225:
- Local (loopback-bound) sessions no longer honor the advertised-host
  override: honoring it auto-opened http://<host>:<port> against a server
  nothing was listening on, openBrowser still reported success, and the
  agent blocked on waitForDecision. Local sessions now advertise and open
  localhost, warning once that PLANNOTATOR_REMOTE=1 is required.
- The invalid-host warning JSON-encodes the echoed value so an embedded
  newline cannot forge extra stderr lines (hosts surface session-ready
  lines as clickable links); warn-once is now per value.
- Docs: local-session behavior reworded, the empty-env-suppresses-config
  semantic documented, secure-context note generalized.
 2026-08-06 18:29:11 -07:00
Michael Ramos 2d65c65596 feat(ui): pair a light theme and a dark theme, switched by mode (#1217) 
* feat(ui): pair a light theme and a dark theme, switched by mode

ThemeProvider stored one palette plus a mode, so picking a dark-only
palette pinned the mode and greyed out the Light/System buttons. Store a
pair instead: { mode, light, dark }, resolved as pair[preferredMode], so
System flips between the two choices as the OS scheme changes.

The Settings Theme tab now assigns one half at a time. A Light/Dark
switch decides which half the grid is filling, the grid lists only the
palettes that can render that half (from the registry's modeSupport), and
a summary line names both halves with each side clickable. Every mode
button is permanently enabled: a dark-only palette simply never occupies
the light slot, so no mode coercion is left to do.

The pair round-trips through the SETTINGS registry to the `theme` key in
~/.plannotator/config.json the way diffOptions does. A user upgrading
seeds both halves from their stored single palette, and the legacy
plannotator-color-theme key keeps tracking the active palette so a
downgrade never lands on an unstyled first frame.

Addresses part 1 of #1211.

* fix(ui): make the theme pair seed local, and keep the legacy API non-destructive

Review of #1217 found a data-loss path and three published-API regressions.

Seeding: ThemeProvider handed its resolved pair to the config store through
set(), which queues a debounced POST. configStore.init() applies the server
config but never cancelled that queued write, so a single cookie-less visit
(fresh profile, incognito, cleared cookies) flushed a default pair to
~/.plannotator/config.json AFTER the real one had arrived, and the next
session restored those defaults over the user's cookies. The provider now
uses a new configStore.seed(): memory plus cookie, never the server, and
never over a value init() already applied. init() additionally retracts
queued writes for the leaves the server just spoke for, which closes the
same race for every server-synced setting rather than this one key.

Deprecated APIs: isThemeModeAvailable() and normalizeThemeMode() are back as
one-line wrappers with @deprecated notes, since packages/ui exports utils/*.

setColorTheme: assigns exactly one half and nothing else. A both-mode palette
goes to the half on screen instead of clobbering both; a mode-restricted one
goes to its half without yanking a System user to an explicit mode (render
time already resolves that). It persists through configStore.setLocal(), so
it stays cookie-only as it was before the pair, unless a host installed its
own serverSync transport.

storageKey / colorThemeStorageKey are honored on the read path, so a host's
stored pre-pair preference is migrated rather than discarded. The two halves
have no pre-pair equivalent and stay on fixed keys, documented on the props.

Tests: a fresh-mount case that pins zero POSTs (the previous helper pre-seeded
cookies, which is why this was invisible), a case that pins a real choice
still reaching config.json, direct setColorTheme cases for all three
semantics, a host-storage-keys migration case, and configStore seed/retract
unit tests. All of them fail against the code they replace.
 2026-08-05 21:51:55 -07:00
Raúl bc5470b90d fix(review): bound server memory for large tracked-file diffs (#1167) 
* fix(review): bound server memory for large tracked-file diffs

PR #1118 renders large untracked files as binary additions, but staging
one moves it into the tracked `git diff` path, which had no size guard and
buffered the full multi-megabyte patch (~240 MB RSS on a 51 MB text
artifact). Any large tracked text file modified in the working tree hits
the same unguarded path.

Add a per-invocation `git -c core.bigFileThreshold=<MAX_REVIEW_FILE_CONTENT_BYTES>`
prefix to every content-producing git diff, so git renders oversized blobs
as "Binary files ... differ" instead of a text patch. Their bytes never
enter git's diff machinery or the server's buffered stdout, mirroring the
untracked-file guard. The flag is a no-op at or below the threshold, so
smaller files are byte-for-byte unaffected, and the blob hash git emits in
the binary diff still changes with content, so staleness detection holds.

The guard is applied in the shared cores, so the Bun and Pi runtimes
inherit it identically: `review-core.ts` covers the ordinary git provider
(working-tree, staged, commit, and the freshness fingerprint) and
`gitbutler-core.ts` covers the GitButler object diff. The jj provider runs
`jj diff`, which has no `core.bigFileThreshold` equivalent, so it is out of
scope here and stays unbounded as before.

* fix(review): preflight oversized tracked diffs

* fix(review): batch tracked diff preflight

* fix(review): restore browser-safe diff core

* fix(review): preserve gitlinks and textconv

* fix(review): require filesystem runtime seam

Fail compilation when a runtime omits file metadata or symlink support instead of silently disabling bounded reads and expansion.
 2026-08-03 13:25:35 -07:00
Syahrul 0d9128794f fix: release pi review ports on session shutdown (#1160) 
* fix: release pi review ports on session shutdown

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

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

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

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

---------

Co-authored-by: Michael Ramos <mdramos8@gmail.com>
 2026-08-03 13:25:32 -07:00
Michael Ramos d53cbfb373 fix(annotate): enforce archive read-only surfaces (#1171) 
* fix(annotate): enforce archive read-only surfaces

* fix(archive): close remaining read-only leaks
 2026-07-31 17:23:44 -07:00
Michael Ramos 8f84852f97 fix(review): surface partial GitLab comment submissions (#1164) 
Surface partial GitLab submission outcomes and preserve narrowed, duplicate-safe retries across dialog reopen and same-tab refresh. Follow-up for an explicit blocked-recovery escape: #1166.
 2026-07-31 10:59:58 -07:00
Raúl 37acde15d5 fix(ai): defer Codex model discovery until a Codex session starts (#1145) 
* fix(ai): defer Codex model discovery until a Codex session starts

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

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

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

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

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

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

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

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

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

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

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

---------

Co-authored-by: Michael Ramos <mdramos8@gmail.com>
 2026-07-29 23:36:47 -07:00
Raúl c750427ab8 feat(annotate): dismiss abandoned gate sessions (#1143) 
A direct local `plannotator annotate --gate --json` waits for one
authoritative decision. If every review surface disappears without
approving, sending feedback, or exiting, the caller blocks forever: the
server has no notion of whether a client ever connected, whether another
tab is still open, or whether a disconnect is a reload.

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

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

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

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

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

Stopping the server closes live lease streams instead of only releasing their
slots, so a long-lived host process does not retain a heartbeat timer and an
open response for every finished session.
 2026-07-29 23:02:49 -07:00
Raúl 9a450a69e7 feat(annotate): preserve notes on structured approval (#1092) 
* feat(annotate): add strict atomic result output

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

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

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

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

* feat(annotate): preserve notes on structured approval

* test(pi): use exact annotate outcome import

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

* docs(annotate): document the real approvedWithNotes default

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

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

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

---------

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

* test(annotate): cover atomic watcher saves

* fix(watch): survive atomic file replacement

* fix(watch): disable exact-file coalescing

* fix(watch): track exact file signatures

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

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

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

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

---------

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

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

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

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

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

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

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

* test(annotate): cover folder annotate version history

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

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

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

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

No caller passes docKey yet - this is purely additive.

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

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

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

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

Not yet consumed by App.tsx - purely additive.

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

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

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

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

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

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

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

* test(pi): cover folder annotate version history

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

---------

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

* fix: enforce disabled AI review endpoints

* fix: fully disable AI review surfaces

---------

Co-authored-by: Kevin <kcosrdev@gmail.com>
 2026-07-26 09:59:15 -07:00
Kevin 193b07e22c fix(review): bound memory for large untracked files (#1118) 
Co-authored-by: Kevin <kcosrdev@gmail.com>
 2026-07-24 19:22:13 -07:00
Michael Ramos 99d11dca04 Persist Guided Reviews across sessions (#1115) 
* feat(guide): add durable guide store with repo-scoped keys and opt-out

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

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

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

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

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

* docs: document guide persistence endpoints and PLANNOTATOR_GUIDE_HISTORY

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

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

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

Review finding on #1115: a saved:{id} guide has no AgentJobInfo, so
GuideScreen's context match passes trivially (unknown ids are tolerated for
the demo path). Switching PRs or worktrees while a saved guide was open left
it mounted over the new context's diff with a stale moved flag. App.tsx now
clears activeGuideJobId on any prMetadata.url / activeWorktreePath change
when it points at a saved: id; the user reopens it from the Previous guides
list. Live job ids are untouched, GuideScreen's own matching handles those.
 2026-07-23 15:32:43 -07:00
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>
 2026-07-22 16:36:17 -07:00
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
 2026-07-22 08:13:57 -07:00
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
 2026-07-20 15:33:42 -07:00
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
 2026-07-20 08:54:43 -07:00
Michael Ramos fa294b12e4 feat(review): add PR and MR artifact gallery (#1055) 
Adds a first-class artifact gallery and annotation workflow for GitHub pull requests and GitLab merge requests, including images, GIFs, video timestamps, rendered HTML, Markdown, provenance-aware feedback, authenticated provider resources, and reliable CDN/media handling.
 2026-07-17 21:25:23 -07:00
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>
 2026-07-17 07:37:50 -07:00
Michael Ramos 95ac6cd2f8 Use the production Totman favicon (#1066) 2026-07-16 16:04:01 -07:00
Michael Ramos 0d5d1fc628 fix: surface Pi provider errors in reviews (#1061) 2026-07-16 14:15:31 -07:00
Michael Ramos 8e9a359a8f fix(review): make remote discovery noninteractive (#1062) 2026-07-16 14:15:11 -07:00
iury souza 13309e322b feat(server): support bounded port ranges (#1042) 
* feat(server): support bounded port ranges

* fix(server): harden bounded port retries

* fix(server): preserve non-range port behavior

---------

Co-authored-by: Michael Ramos <mdramos8@gmail.com>
 2026-07-16 06:31:52 -07:00
buihongduc132 5b2756b0c7 fix(server): return JSON 404 for unknown /api/* routes instead of HTML (#748) 
* fix(server): return JSON 404 for unknown /api/* routes instead of HTML

All three servers (plan, review, annotate) serve the SPA HTML for any
unmatched route. This means hitting /api/nonexistent returns the full
HTML page with 200 OK — confusing for API clients and wasteful.

Now unknown /api/* paths get a proper JSON 404 response before the SPA
fallback kicks in. Non-API paths (like /some/random/path) still serve
HTML for SPA routing as before.

Tested on all three server types.

* fix(server): guard unknown API routes across runtimes

---------

Co-authored-by: buihongduc132 <buihongduc132@gmail.com>
Co-authored-by: Michael Ramos <mdramos8@gmail.com>
 2026-07-16 06:31:30 -07:00
Michael Ramos 7f0c36028b fix(server): bound startup file discovery (#1036) 
* fix(server): bound startup file discovery (#978)

* test: observe the server's own warm cache key so the ordering tests actually pin the fix

Review finding: both bind-before-warm ordering tests raced against
observeWarmState(projectRoot), but on macOS mkdtempSync returns /var/...
while the chdir'd server warms under the realpath /private/var/... —
a different warmFileListCache key. The tests therefore raced a FRESH
warm (always pending at observation time) and passed on the OLD broken
code too. Observing process.cwd() inside onReady uses the server's real
key. Verified: with old resolve-file/server code checked out, all four
ordering tests now fail; on the fix they pass (17/17 across both files).

Claude-Session: https://claude.ai/code/session_01SFy9fY27SA8g5BtotWPi1G
 2026-07-10 10:18:01 -07:00
Michael Ramos 977f4ce582 fix: final QA sweep fixes — file-browser cap priority, OpenCode project scoping, spotlight Alt-Alt dismiss 
Fixes for the three-agent adversarial sweep findings:

- reference-handlers.ts: seed the user's modified/untracked files BEFORE
  the bulk walk — the 5000-file cap (d3c9de1e) filled in raw readdir order
  and could silently drop the exact files the user just edited from the
  annotate browser (and the truncated latch broke the merge loop on its
  first iteration).
- opencode commands.ts: pass `project` to startAnnotateServer at both
  call sites (annotate + annotate-last) via detectProjectName, matching
  the hook and Pi runtimes — OpenCode annotate history no longer lands in
  the shared "_unknown" bucket.
- App.tsx: the Alt-Alt destination double-tap now dismisses the
  DestinationSpotlight — the coachmark advertised that exact gesture but
  its own keydown handler deliberately ignores modifiers, so performing
  the tip left the dim overlay stranded.
- annotate.ts + pi mirror: degradation notice now reads "warning: annotate
  history unavailable" so OpenCode's logCliWarnings forwarder (which
  filters on \bwarn(ing)?\b) actually surfaces it.
- cli-bridge.ts: a rejected showToast un-marks the URL (the other delivery
  path can retry) and logs the failure instead of being fully silent.
- test.yml: register packages/editor/editableDocumentsHook.test.tsx (7
  draft/conflict tests, DOM-gated since #936, never ran in CI). Repo-wide
  sweep confirms all 19 DOM-gated test files are now registered.

Gates: DOM batch 102/0 across 19 files, review+server 507/0, opencode 70/0,
install harness 85/0, full typecheck incl. strict-consumer, review build OK.

Claude-Session: https://claude.ai/code/session_01SFy9fY27SA8g5BtotWPi1G
 2026-07-10 06:50:40 -07:00
Michael Ramos abceed3f0b chore(server): align annotate-history degradation log with the [plannotator] prefix convention 
Claude-Session: https://claude.ai/code/session_01SFy9fY27SA8g5BtotWPi1G
 2026-07-10 06:19:59 -07:00
Michael Ramos 7d583df980 fix(server): unwritable data dir degrades annotate/sessions instead of failing startup 
QA probe: with PLANNOTATOR_DATA_DIR on a read-only mount, `plannotator
annotate file.md` crashed before the UI opened — twice. (1) #961's
saveToHistory call on annotate open was unguarded, so the new version-diff
enhancement gated startup that was stateless at v0.22.0; now wrapped
(both the Bun server and the Pi mirror) and degrades to no-version-diff
with a stderr notice. (2) registerSession's mkdir/writeFileSync (pre-existing
since #795) threw after the server was already listening; the session
registry only powers `plannotator sessions` discovery, so it is now
best-effort, and unregisterSession/listSessions tolerate an unwritable
sessions dir too. Verified: read-only data dir now starts, serves
/api/plan with no versionInfo, and prints the session URL.

Claude-Session: https://claude.ai/code/session_01SFy9fY27SA8g5BtotWPi1G
 2026-07-10 06:07:45 -07:00