Commit Graph

582 Commits

Author SHA1 Message Date
Michael Ramos eb6a59e2dc seo: index live root blog pages (#1332) 2026-08-16 15:53:03 -07:00
Michael Ramos 94f8d45daa guide-viewer: readable on phones and tablets, desktop untouched (#1329)
* fix(guide-viewer): readable on phones and tablets, desktop untouched

Every change is behind a breakpoint; 1440px and 1024px renders of the same
guide are byte-identical before and after (screenshot MD5s match).

- Split diffs below lg (1024px) are forced unified in the portable viewer's
  diff renderer (matchMedia; the setting is untouched, so a wider window
  gets split back). A phone has ~350px of pane and a portrait tablet ~430px,
  so two columns were under 220px each.
- Padding scales: page px-3/sm:px-6/lg:px-10, chapter column px-4/md:px-6,
  diff column px-1.5/md:px-4. Code pane on a 390px phone: 276px → 352px.
- Tablets: the chapter column is proportional (minmax(260px,36%)) from md
  and the fixed 440px only from lg. Pane at 768px: 214px → 426px.
- Header actions (Download, theme) sit in a right-aligned row above the
  title below md instead of floating into it.

Viewer rebuilt and published (viewer.dWt7KCum.js), manifest synced.

* fix(guide-viewer): touch targets, labels, and no tap delay on coarse pointers

Only under `pointer: coarse` (Tailwind's `pointer-coarse:` variant), so mouse
layouts are unchanged:
- Reviewed checkbox and the collapse chevron get an invisible ::before hit
  area (visual 15–17px, hit ≥ 44px); the "Reviewed" text button and file
  chips get taller padding; the theme toggle and hosted Download button grow
  to a 44px hit box.
- `touch-action: manipulation` on controls in the portable viewer and the
  landing page (no double-tap-to-zoom delay; the page still pinch-zooms).
- `aria-label` on the two icon-only buttons (theme toggle, collapse chevron).
- Landing page: the GitHub link and the Copy button are 44px tall on touch.

Tailwind v4 already gates `hover:` behind `@media (hover: hover)`, so no
false hover states on tap. Viewer rebuilt and published, manifest synced.

* guide-viewer: manifest for the combined build (labels + mobile), viewer.sFtOnb1i.js published
2026-08-16 13:30:58 -07:00
Michael Ramos d2278fb853 guides-show: GitHub link in the landing page header (#1327) 2026-08-16 13:15:07 -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
Michael Ramos 3f12a6cf97 blog: repo link, image alt text, and larger blog type (#1323)
* blog: link Plannotator to the repo, SEO alt text on grill-me images

* blog: bump blog prose scale about 10 percent
2026-08-15 13:21:47 -07:00
Michael Ramos a41e83d788 blog: grill-me post additions (#1322) 2026-08-15 13:00:26 -07:00
Michael Ramos 20e61c0e35 blog: an interactive UI for the grill-me skill (#1321)
* blog: the best interface for grill-me sessions

* blog: cut prose, lead with /plannotator-last and the screenshot

* blog: rename to an interactive UI for the grill-me skill

* blog: click-to-zoom lightbox for post images via native dialog

* blog: center the lightbox against the global margin reset

* blog: cut the middle to the workflow itself

* blog: humble Codex nod
2026-08-15 12:50:25 -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 aa0bf860d8 chore: bump version to 0.27.3 2026-08-13 16:06:13 -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 8b9dfe7e5f chore: bump version to 0.27.2 2026-08-13 11:36:16 -07:00
Michael Ramos 8e88dcec8c fix: v0.27.2 pre-release QA batch (mobile TOC, dialog bounds, seed guard) (#1311)
* fix(plan): make the compact TOC scroll the document again

The compact navigator overlay rendered outside App's ScrollViewportProvider,
so the TableOfContents it hosts resolved a null viewport and every "jump to
heading" tap was a silent no-op on phones. The provider is context-only, so
hoisting it above the overlay fixes the lookup without touching desktop DOM
structure or order.

* fix(plan): scope the permission-mode chooser to plan review and bound its card

The one-time chooser fired in every non-goal-setup Claude Code session, so
annotate, annotate-last, annotate-folder and archive reviewers got a blocking
dialog about what happens after plan approval. Gate it on plan review, which
is the absence of a mode field in the /api/plan payload.

The card itself was hand-rolled with no height cap and no internal scroll, so
on a short landscape phone it overflowed both edges of a modal that has no
dismiss control. Give it the same bounded shell the sibling one-time dialogs
use: safe-area padding, a visible-viewport max height, and the option list as
the only scrolling region. Content and cookie behavior are unchanged.

* fix(review): never seed Tree over a persisted panel view

The first-run initializer gated only on the setup-seen cookie, but sessions
that never reach it (non-git, workspace, PR, no since-base) still let Settings
persist a panel view. A reviewer could hold an explicit Git status choice with
"seen" unset, and the next plain git session seeded Tree over it. Treat a
persisted view as the decision: consume the one-time setup and write nothing.

* fix(comments): give the geometry-forced composer a working Escape

When the anchor has no room the position tracker forces dialog mode. On a
fine-pointer viewport Escape took the collapse branch, the tracker instantly
re-forced the dialog, and the keystroke was eaten; the Collapse button bounced
the same way. Track forced expansion separately from the preferred kind: in
that state Escape closes (draft-preserving) and Collapse is hidden, because
collapsing is geometrically impossible.

* fix(review): stop the compact Editor tab editing the desktop diff style

The dock's Split/Unified control returns null under the compact touch layout,
but the Settings copy of it kept rendering while the phone showed the
session-only unified diff. It looked dead and silently rewrote the persisted
desktop preference. Hide it on compact and state what the session is doing;
the prop defaults to false, so the plan editor and desktop are untouched.

* fix(portal): give the share portal the mobile app shell

The portal mounts the same plan editor App as the hook but kept the pre-mobile
entry document: no viewport-fit=cover (so every safe-area token was inert) and
a min-h-screen body without the shell's scroll ownership. Mirror the hook's
body class, root class, and viewport meta, and extend the entry-asset pin to
cover the portal alongside them.

* fix(plan): keep compact overlays out of the printed document

The compact plan stage and the compact navigator are full-viewport transient
surfaces with no print-hide marker, so printing on a touch device with
Annotations, Ask AI, Versions or Archive open clipped the document behind
them. Mark both with data-print-hide, which print.css already hides. The
desktop rail is untouched.

* fix(plan): give the selection toolbar real touch targets

Copy / Delete / Comment / quick label / looks-good / Cancel measured 28x28
with 2px gaps on a phone because the toolbar never got the touch-target
markers the rest of the stack uses. Stamp them on its buttons and add a
compact-scoped gap so adjacent destructive and comment actions are not a
mis-tap apart. Both are inert outside the compact scope, so desktop geometry
is unchanged.

* docs: keep the new QA-batch comments free of em dashes
2026-08-13 11:35:00 -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
Leonardo Reis 1d4e490b8e fix(review): update Codex automatic approval flag (#1231) 2026-08-13 09:42:32 -07:00
Michael Ramos e181b824cc Mobile-safe plan and code comment composition (#1297)
* feat: harden mobile comment composition

* fix(ui): keep mobile app inside Safari viewport

* docs: record physical mobile triage

* fix(ui): extend plan canvas behind Safari controls

* fix(ui): let mobile plans drive Safari chrome

* fix(ui): release Safari top edge on mobile plans

* docs: triage mobile feedback and close phase 1b

* fix(ui): harden compact touch behavior
2026-08-13 08:58:55 -07:00
Michael Ramos d3633c9c52 Mobile foundation and quieter first run (#1295)
* feat: establish mobile foundation and simplify onboarding

* fix(ui): finish mobile foundation cleanup
2026-08-13 08:45:00 -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 7ea3e01102 security(marketing): migrate static site to Astro 7.1+ (#1293)
* security(marketing): migrate static site to Astro 7

* test(marketing): guard static security invariants

* test(marketing): tolerate colored Astro build output

* revert(marketing): preserve existing docs ordering metadata
2026-08-12 20:36:41 -07:00
Michael Ramos 561e4e6423 security(pi): require Pi 0.79+ and document project trust (#1291)
* security(pi): require Pi 0.79+ and honor project trust

* fix(pi): explain unsupported project trust hosts
2026-08-12 20:28:08 -07:00
Michael Ramos ef49c701c2 chore: bump version to 0.27.1 2026-08-12 17:07:36 -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 d0d971a3bf chore: bump version to 0.27.0 2026-08-12 14:14:35 -07:00
Michael Ramos ed6f44bf2e fix(release): tailscale gate exit codes and lease gating, conditional SIGHUP, informative guide validation error (#1286)
- annotate --tailscale publish failures now exit through
  annotateStartupFailureExitCode: exit 2 under a strict gate
  (--require-approval / --result-file), where exit 1 is reserved for "the
  reviewer did not approve, decision record published". Non-strict annotate
  and review keep the documented exit 1.
- the annotate client lease (auto-dismiss on abandonment) is forced off
  while tailnetPublished is set: --tailscale reads as local to the CLI
  predicate, but clients connect through the serve proxy, and a proxy
  disconnect longer than the grace would dismiss a live review. Same
  rationale as remote/shared sessions; decided at the single point both
  the /api/plan advert and the SSE endpoint read.
- the SIGHUP-to-process.exit route moved from an unconditional CLI-entry
  listener into enableTailscaleServe's success path, installed only once a
  serve mapping exists. Any SIGHUP listener overrides the ignored
  disposition nohup depends on, so plain sessions now keep zero listeners
  and "nohup plannotator review &" survives terminal close again;
  --tailscale sessions still tear their mapping down on HUP (exit 129).
- validateGuideOutput explains a fully-invalidated guide whose refs named
  files outside the changeset (count plus up to 3 example paths, with a
  pointer to the Commits panel) instead of the bare generic message; the
  generic message stays for genuinely structural emptiness. The informative
  error now flows through onJobComplete to the job failure card; Pi picks
  the change up via the vendored guide-review copy.
2026-08-12 14:04:59 -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 caf7ce1ccd feat(review): install Call Flow automatically in the background on opt-in (#1271) 2026-08-11 17:48: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 a5f9937f9c fix(pi): never touch Pi's system prompt; phase framing as conversation messages (#922) (#1269)
* fix(pi): never touch Pi's system prompt; deliver phase framing as messages (#922)

The extension replaced Pi's system prompt with its phase framing during
planning and execution, dropping AGENTS.md context, the skills catalog,
tools guidance, and user append text, and re-templating the todo list
into the system prompt busted the provider's prompt-cache prefix on
every checklist update.

Plannotator now never returns or modifies systemPrompt (approach
suggested by Karrq on the PR). Phase framing is delivered exactly once
per phase entry as a hidden plannotator-framing conversation message,
execution progress rides in small per-turn plannotator-context todo
messages, and a phase-aware context filter keeps only the newest framing
for the current phase (idle still clears everything). Cache-busting
reduces to conversation-suffix appends plus one history adjustment per
phase transition.

BREAKING CHANGE: the Pi plan-mode toggle command is renamed from
/plannotator to /plannotator-plan-mode with no alias, and the
phases.*.systemPrompt config key is retired in favor of
phases.*.instructions (a phase-entry message template); old systemPrompt
keys are ignored with a session-start warning.

* docs: root README uses the renamed plannotator-plan-mode command

* fix(pi): make the framing latch survive compaction and tree navigation (#922)

Review follow-ups on the message-based framing design:

- session_compact reopens the framing latch: compaction can summarize
  away the delivered framing message, so the next prompt re-delivers it
  (the context filter keeps only the newest copy if the old one survived
  in the kept tail).
- session_tree re-derives phase, latch, and checklist state from the new
  active path via the restore logic session_start uses (now shared as
  resyncPhaseFromSession and reading getBranch(), the active path,
  instead of the whole append-only entry file). A path with no
  plannotator state means idle.
- The per-turn todo message restates the [DONE:n] convention in one line
  so the protocol survives between a compaction and re-delivery.
- Warn at session start when the bundled plannotator.json is missing, so
  a packaging regression cannot silently produce a rule-less planning
  phase.
- Tests pin the persistState payload on both sides of the latch, cover
  compaction re-delivery (exactly once) and tree-switch resync, and the
  harnesses expose getBranch.
2026-08-11 13:15:39 -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 2fff8756d9 chore: bump version to 0.26.8 2026-08-10 17:00:33 -07:00
Michael Ramos 121082430e fix: QA-gate hardening for the v0.26.8 feature set (overlay perf, numbering, OpenCode 2 parity) (#1258)
* fix(opencode): consolidate V2 system parts into one composed prompt (#1114)

The OpenCode 2 adapter still shipped the pre-#1114 multi-part system
injection: replacePlanningSystemParts kept one part per source and the
generic reminder pushed a separate part, so Qwen3.x Jinja template
corruption persisted for OpenCode 2 users. Mirror the V1 entry exactly:
compose the stripped existing text plus additions into a single system
part via composeSystemPrompt, and compose the generic reminder into the
existing text instead of appending a second part.

Also adds the regression tests for the bug class flagged in #1114's
review: both helpers must read/compose the existing system text BEFORE
truncating the array (a reorder to 'system.length = 0' first drops the
host prompt and goes red here).

* perf(annotate): harden the raw-HTML overlay reconcile (dead-target backoff, cull, batching)

Bridge-script hardening for mutation-heavy pages and large annotation
sets, plus the lost click-to-select hover affordance:

- A: dead-target re-search now carries a wall-clock backoff (300ms
  doubling to a 5s cap, reset on success) ON TOP of the generation gate,
  plus a 2-searches-per-reconcile-pass budget with a scheduled follow-up
  pass for budget-skipped eligible targets. A page that mutates every
  frame advances domGeneration every frame, so the generation gate alone
  re-ran the whole-document TreeWalker sweep (and anchor re-resolution)
  per frame forever for permanently unresolvable targets.
- B1: early viewport cull (64px margin) for element and range targets:
  wholly offscreen targets skip targetStyleHidden / getComputedStyle /
  clipBoundsFor / client-rect collection entirely and just omit their
  markers, which is what the visible pipeline produced anyway.
- B2: read/write batching in renderAnnotationOverlay: highlight rects are
  queued during the read phase and flushed as one write phase, so the
  pass no longer forces a synchronous layout per record.
- B3: restoreAnnotation defers its render through the existing
  rAF-coalesced reconcile scheduler; restoring N annotations now renders
  once instead of N full passes (searches stay synchronous for the
  mark-applied reply). DOM tests flush the frame via the suite's
  standard macrotask flush.
- B4: zero-work observer gate: page mutations with no records, no
  pending draft, and pinpoint inactive still bump domGeneration but no
  longer schedule a reconcile frame.
- D: hover affordance for click-to-select: the rAF-throttled mousemove
  hit-tests the pointer against the CACHED rendered committed rects and
  toggles a brightness class on that annotation's rect divs inside the
  shadow root. No page-DOM writes, rects stay pointer-transparent, and
  shadow-root writes are unobserved so there is no reconcile loop.
- G: while a text drag is in progress in drag mode, placed markers yield
  pointer input (data-pn-hittest) so the 25px bubble cannot capture a
  selection drag; armed only by a >4px primary-button move from a
  non-overlay mousedown, so marker clicks and click-to-select paths are
  untouched. withMarkersYielded now restores (not clears) the attribute.

New regression tests for A, B1, B3, B4, D; A/B1/B3 mutation-verified
(fix reverted, test observed failing, fix restored).

* fix(annotate): make on-page marker numbers match exportAnnotations numbering

The HtmlViewer sync excluded GLOBAL_COMMENT annotations before numbering
while exportAnnotations numbers '## N.' sections across the FULL list
including globals — so an on-page 'Comment 2' could be '## 3.' in the
feedback the agent reads. The sync now derives each marker's number from
its position in the full createdA-sorted list (globals occupy a number
but ship no entry, leaving the correct gaps on-page). Export format is
unchanged.

New buildSyncNumbering helper + tests asserting a mixed list yields
identical numbers between the sync payload and exportAnnotations output
(mutation-verified against the pre-fix ordering).

* chore: sync stale workspace versions in bun.lock (0.26.1 -> 0.26.7)

* docs: document raw-HTML overlay model, multi-target types, and known limitations

- Data Types: add htmlAdditionalTargets to the Annotation listing plus
  the HtmlElementAnchor (including the optional normalized point used by
  placed markers) and HtmlAnnotationTarget shapes.
- Annotation System: describe the post-#1257 raw-HTML surface (placed
  comment markers + overlay-projected highlights, no inline mark
  mutation; durable anchors persisted, disposable markers projected) and
  the print-parity limitation.
- URL Sharing: note that share links intentionally drop HTML element
  anchors and additional targets (restore is text-search based, per
  sharing.multiTarget.test.ts).

* test: fix Range.getClientRects stub typing in the B1 cull test

* fix(annotate): hover-race teardown and unbounded one-shot dead-search passes

Polish round on the overlay hardening:

- Hover race (1): switching into pinpoint mode (or opening a draft) now
  tears hover down fully via clearHoverHighlight() — cancels the pending
  rAF hit test and clears the tracked position and id — and the rAF
  callback itself refuses to paint outside drag mode / with an open
  draft. Previously the pending callback re-applied the class after the
  mode switch and every flushQueuedHighlights re-painted it from the
  stale hoverHighlightId, leaving a permanent phantom hover.
- One-shot budgets (3): beginDeadSearchPass takes a per-pass budget.
  Reconcile passes keep 2 (they repeat, skipped targets get follow-up
  frames); print and scroll-to are user-initiated one-shots with no
  follow-up and now run unbounded (backoff and generation gates still
  apply), so printing with 3+ dead-but-recoverable targets no longer
  silently prints fewer highlights.

Both changes carry new regression tests, mutation-verified (fix
reverted, test observed failing, fix restored).

* fix(annotate): number markers by array position and cap entries after dropping globals

The createdA sort made the export-match invariant false with external
annotations: exportAnnotations' sort keys tie for every raw-HTML
annotation (blockId '', startOffset 0), so its stable sort numbers the
combined [...local, ...external] list in ARRAY order — and external
annotations arrive appended with server-stamped createdA values that can
interleave with local timestamps. buildSyncNumbering now numbers by
array position of the input (verified to be the same combined list both
consumers receive from packages/editor/App.tsx allAnnotations; the
viewerAnnotations diffContext filter is order-preserving and vacuous on
the raw-HTML surface).

Also reorders the cap: number the full list, drop globals, THEN slice
512 entries — globals no longer waste sync capacity and a non-global the
export numbers past position 512 still syncs while slots remain. Numbers
may now exceed 512 (array positions); the bridge's own bound (100000)
accepts them and its 512-entry cap still agrees with the sender.

Tests updated: interleaved-external agreement with exportAnnotations
(mutation-verified against the createdA sort) and slice-after-filter
capacity.

* docs(opencode): note the accepted cache-hint flattening trade-off in V2 consolidation
2026-08-10 15:27:04 -07:00
Michael Ramos 3fd6d33906 fix(marketing): redirect self-hosting guide to canonical docs 2026-08-10 11:58:42 -07:00
Andrew bb6a65ac76 Fix OpenCode plugin Jinja template corruption with Qwen3.6 (#1114)
* fix(plugin): consolidate system prompt injections into single array element

The plugin previously pushes planning prompts and improvement contexts as
separate elements in the output.system array. This change appends them to
output.system[0] with newline separators instead. This keeps all system
instructions within a single message block to prevent potential parsing or
formatting issues when the agent processes the context.

* refactor(opencode-plugin): extract composeSystemPrompt helper to centralize system prompt assembly and add unit tests

* style(opencode-plugin): remove extra newline before plan submission reminder heading

* fix(opencode-plugin): store composed prompt result before clearing system array to prevent data loss

Previously, `output.system` was cleared with `length = 0` before being passed into `composeSystemPrompt`, causing the function to compose from an empty array instead of the original system content. The fix stores the composition result in a variable first, then pushes it after clearing. Additionally, add `.trim()` in `stripConflictingPlanModeRules` to normalize whitespace before filtering empty entries, and include a test case for empty string collapse behavior.

* refactor(plan-mode.ts): move string trimming from stripConflictingPlanModeRules to composeSystemPrompt for centralized whitespace handling

* test(plan-mode): add test case for trimming trailing newlines in composeSystemPrompt
2026-08-10 10:09:58 -07:00
Michael Ramos 62c1eab119 chore: bump version to 0.26.7 2026-08-09 23:02:38 -07:00
Michael Ramos d579ff8db2 chore: bump version to 0.26.6 2026-08-09 21:07:30 -07:00
Michael Ramos 9c40ffadcc chore: bump version to 0.26.5 2026-08-09 17:49:14 -07:00
Michael Ramos 9b14b19a7b chore: fold in pre-release QA findings (print overlays, doc alignment) (#1245)
* chore: fold in pre-release QA findings (print overlays, doc alignment)

From the 25-item QA sweep over the v0.26.4..main range (all items passed;
these were the three real minors worth folding into the release):

- The raw-HTML iframe's injected CSS had no print rules, so pin badges
  (and, in a narrow window, the pinpoint outline box) printed into
  hard copies of annotated HTML pages. The overlay elements now carry an
  explicit @media print hide inside the iframe document, where the outer
  print.css cannot reach. Inline annotation marks stay printable on
  purpose, matching markdown documents.
- AGENTS.md/CLAUDE.md now state that PLANNOTATOR_ANNOTATE_HISTORY also
  gates the durable submitted-feedback records from #1237, and the
  Annotation interface listing includes the htmlAnchor field from #1243.
- apps/codex/README.md aligns with the #1241 top-level wording (Windows
  Codex hooks are experimental with printed manual steps, not disabled).
- Marketing docs: annotate page documents the pinpoint-first default and
  minimal-first chrome for raw-HTML sessions; installation page notes
  the old-git plain-clone fallback from #1239.

* chore: drop deprecated marketing-docs edits (canonical docs are Mintlify)
2026-08-09 17:19:50 -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
Will Hampson 033368ede4 Fix feedback delivery after Pi reload (#1240) 2026-08-09 16:16:48 -07:00
Michael Ramos d5ae439f7a chore: bump version to 0.26.4 2026-08-07 15:13:27 -07:00
Michael Ramos c760fc522b chore: bump version to 0.26.3 2026-08-07 14:32:16 -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 bbae458e5a chore: bump version to 0.26.2 2026-08-06 00:50:56 -07:00
Michael Ramos 2dd5b7b25d docs: self-hosting page no longer claims a bundled Highlight.js (#1221)
highlight.js was removed in #1218; the portal bundles Shiki via the diff
renderer. Caught by the v0.26.2 dependency audit.
2026-08-05 23:42:10 -07:00
Michael Ramos c08b188812 perf(ui): single Shiki highlighter, palette-matched code blocks, drop highlight.js (#1218)
* perf(build): stub out the dead Oniguruma WASM in every bundle

@pierre/diffs picks its Shiki engine with a runtime ternary:

    engine: preferredHighlighter === "shiki-wasm"
      ? createOnigurumaEngine(import("shiki/wasm"))
      : createJavaScriptRegexEngine()

Plannotator pins `preferredHighlighter: 'shiki-js'` (and Pierre's own
default is 'shiki-js'), so the Oniguruma branch never executes. Because
the choice is a runtime ternary, bundlers keep the `import("shiki/wasm")`
edge anyway and inline `@shikijs/engine-oniguruma/wasm-inlined`, a
~622 KB base64 blob, into the single-file HTML builds. The review app
paid for it twice: once on the main thread (via
`highlighter/shared_highlighter.js`) and once inside the `?worker&inline`
Pierre worker.

Alias `shiki/wasm` to a stub that throws if it is ever reached. Wired via
`resolve.alias` rather than a plugin because `resolve.alias` is shared
with Vite's worker build and `plugins` are not.

Highlighting output is unchanged: the JS regex engine was already the one
doing the work. Opting back into 'shiki-wasm' now fails loudly instead of
silently costing every user a megabyte of dead bytes.

    apps/review/dist/index.html  19,424,646 -> 18,180,545  (-1,244,101 raw / -463,348 gzip)
    apps/hook/dist/index.html    23,032,467 -> 22,410,416    (-622,051 raw / -233,485 gzip)

* perf(ui): consolidate code highlighting onto Shiki, drop highlight.js

The app shipped two highlighters. Shiki already tokenised the code-review
diff pane (via @pierre/diffs, JavaScript regex engine); highlight.js
separately coloured markdown fences and review suggestion snippets at
~982 KB minified for a full build of ~190 grammars. That second
highlighter is now gone.

Every call site moves onto `packages/ui/utils/codeHighlight.ts`, a thin
wrapper over Pierre's SHARED Shiki instance:

  CodeBlock, Viewer, PlanCleanDiffView   markdown fences
  InlineMarkdown                          code-file hover preview
  HighlightedCode                         review suggestion snippets

Reusing Pierre's instance rather than standing up a second fine-grained
one is deliberate. Pierre imports Shiki's full bundle, so every grammar
and theme is ALREADY inlined in the single-file builds: a separate
highlighter with a curated language list would have duplicated a subset
of bytes that are already there. Sharing costs nothing, gives every
language Shiki bundles instead of a shortlist, and — the point of the
change — guarantees fences resolve the exact same theme the diff pane
resolves.

Theming. `SHIKI_THEME_MAP` / `resolveSyntaxTheme` move from
`packages/review-editor/hooks/usePierreTheme.ts` to
`packages/ui/utils/syntaxTheme.ts`; usePierreTheme re-exports them, so
the review editor's imports are unchanged. `useFenceTheme()` feeds the
components and re-highlights on palette or mode change. Code blocks now
follow the active palette across all ~52 themes in both light and dark,
instead of always rendering github-dark and relying on hand-written
`.hljs-*` override stacks to stay legible. Those stacks are deleted:
`packages/editor/index.css`'s light-mode token palette, and
`colorblind.css`'s hand-tuned tokens which existed to APPROXIMATE
@pierre/theme's protanopia-deuteranopia themes that are now simply used.

Behaviour held fixed:

  - Language-less fences stay plain text (#1212). No auto-detection
    anywhere, including the hover preview, which previously called
    `hljs.highlightAuto`. `HighlightedCode` derives its language from
    the caller's file path; an unknown extension renders plain.
  - `applyHighlight(el, ...)` keeps the imperative `hljs.highlightElement`
    DOM contract the annotation layer reaches into, and writes plain text
    at final size first so async highlighting causes no layout shift.
    Already-attached grammars highlight synchronously — no flicker on
    cached highlights.
  - It also verifies the rendered text is byte-identical to the source
    and falls back to plain otherwise, because annotations address code
    blocks by text offset.
  - `@plannotator/ui`'s public API is unchanged: the highlighter is a
    module-level default like the package's other seams, no new props.

The `hljs` class on fenced `<code>` becomes `pn-code` (it is a
structural hook for blockTargeting, vim navigation and print.css, and it
named a library we no longer ship). `language-*` stays.

    apps/review/dist/index.html  18,180,545 -> 17,270,889  (-909,656 raw / -291,921 gzip)
    apps/hook/dist/index.html    22,410,416 -> 21,704,434  (-705,982 raw / -238,096 gzip)

Verified the diff pane is untouched: the rendered Pierre shadow-DOM
markup is byte-for-byte identical between an origin/main build and this
one (SHA-256 aa1ee88a…).

* fix(ui): strip stray NUL bytes from the code-highlight source

Two U+0000 bytes slipped into comments in the previous commit, which made
git treat the file as binary. Replaced with spaces; no behaviour change.

* fix(ui): keep code-block annotation marks across highlight swaps

Fenced code is annotated by hand: one `<mark data-bind-id>` inside the
`<code>` element, which `applyHighlight` also owns. Every highlight swap
(palette change, dark/light toggle, or the first async grammar attach
after load) replaces that element's children, so the mark was silently
wiped and nothing put it back. Annotation state, the sidebar panel and
exports were unaffected; the loss was purely visual, and deterministic.

`applyHighlight` now publishes every write through `onCodeHighlightSwap`,
synchronously, immediately after it. `Viewer` subscribes and re-paints the
fence's mark, so a swapped block ends up with BOTH the new theme's tokens
and its annotation. The shared painter (`paintCodeBlockMark`) moves the
token spans into the mark instead of flattening them to text, so creating
an annotation no longer costs a block its colours either.

Being driven by the swap also fixes the cousin race by ordering rather
than timing: share/draft restore runs on a timer after load, and on a slow
machine the first async swap could land after it and wipe the restored
marks per block. A restore that painted before the swap is now
re-established in the same task the swap ran in, and one that runs after
finds the mark already there.

Removal tombstones the id before re-highlighting, because the host drops
the annotation from state a tick later — without it the swap listener
would paint the just-removed annotation back in, and a fence carrying a
second annotation would end up bare.

Also closes the named gap in the WASM coverage: entry-assets only grepped
source, so a future @pierre/diffs bump could reintroduce the inlined blob
through a different import specifier unnoticed. It now greps the built
`apps/{review,hook}/dist/index.html` for the base64 WASM magic, skipping
on an unbuilt checkout and running for real in the CI job that builds the
bundles.
2026-08-05 21:54:40 -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
Michael Ramos 50a54c872b chore: bump version to 0.26.1 2026-08-05 11:39:02 -07:00