* fix(review): stop the call-flow Lens closing mid-scroll and opening on drive-by hovers
The Lens popup scrolls internally but never contained overscroll, so momentum
that hit its edge chained to the page; the page scroll moved the anchor, the
popup tracked it out from under a stationary pointer, and the resulting
mouseleave closed the Lens mid-scroll (worst under Safari rubber-banding).
Scrolling the diff also dragged file headers under the cursor, and the
zero-delay hover open popped the Lens for every badge that passed.
- overscroll-behavior: contain on .call-flow-popover
- 100ms hover-intent delay before opening; a pointer that leaves first never opens it
- close grace 140ms -> 250ms
- while a scroll is in flight, pending hover-closes are held; once it settles
the Lens closes only if the pointer really ended up outside
Reported by @acewhocares on X (Safari).
* ci(guides-show): publish the built viewer manifest as an artifact
The viewer build is deterministic per platform but not across platforms, and
the committed pin must match the Linux CI build. When the manifest check fails
on a PR, a maintainer on macOS previously had no way to obtain the
authoritative hashes. The uploaded manifest.json closes that: download, drop
into apps/guides-show/dist/viewer/, run sync:manifest, commit.
* fix(guides): regenerate viewer pin for the Lens changes; correct the manifest-artifact comment
The cross-platform nondeterminism claim in the previous commit was wrong. A
clean install builds identical viewer hashes on macOS and Linux; the divergence
observed here came from a drifted local bun store carrying stale duplicate
package versions (rm -rf node_modules + frozen install reproduces CI exactly,
verified against the deploy run's published names). The artifact upload stays:
it is the authoritative recovery path when a local environment is the thing
that is wrong. Pin regenerated from a verified-clean build for the Lens
bundle changes.
* fix(pr-artifacts): read GitLab uploads through the token-readable API
GitLab serves `/uploads/<secret>/<file>` from a Rails web route that only
honors session cookies. A `PRIVATE-TOKEN` request is redirected to the
sign-in page, so every GitLab upload attachment referenced by an MR was
unreadable — and because the sign-in page is HTML served with HTTP 200,
it was rendered as the artifact instead of the file.
Route upload links through `GET /projects/:id/uploads/:secret/:filename`,
which serves the same bytes for a personal access token. Both the bare
`/uploads/...` and project-scoped `/<path>/uploads/...` forms are rewritten.
Also stop following redirects into a provider sign-in path: fail with 401
and the matching `gh`/`glab auth login` hint so an auth gap is legible
instead of surfacing as a corrupt artifact.
The uploads API answers every file with `application/octet-stream`, and the
content route serves provider types with `nosniff`, so refine an opaque type
from the file extension to keep images and video rendering.
Verified end-to-end against a self-hosted GitLab 18.8.0-ee instance.
* fix(pr-artifacts): rewrite only real GitLab upload paths
The upload rewrite matched `/uploads/<anything>/<anything>`, so a link in an
MR body could name a path GitLab never minted and still be rewritten into
`/api/v4/projects/:id/uploads/...` with the PRIVATE-TOKEN attached. Because
the remainder allowed slashes, a crafted link could append an attacker-chosen
path to that credentialed GET, leaving the safety of the request to GitLab's
router rather than to this allowlist.
Pin the shape GitLab actually mints: a 32 lowercase hex secret and a single
filename segment. A non-conforming path is no longer rewritten, so it is
fetched verbatim as the ordinary web route exactly as before this feature.
Test fixtures move to a real 32-hex secret, and a new case pins that a short
secret, an uppercase secret, and a multi-segment remainder are all left alone.
* fix(pr-artifacts): diagnose a direct provider refusal as a missing login
The sign-in guard only fired on a redirect, but GitLab's `/api/v4` routes
refuse a bad or absent token directly with a 401/403 JSON body. Those fell
through to `Artifact host returned HTTP 401` at status 502, which reads as a
broken artifact rather than the thing the reader can fix.
Hoist the sign-in messaging into `providerAuthRequiredError` and reuse it for a
direct refusal, so both paths produce the same actionable error at status 401.
GitHub's 403 stays a transport status on purpose: it also covers rate limiting
and SSO enforcement, where a `gh auth login` hint points at the wrong problem.
A GitHub 401 is unambiguous and is diagnosed.
The message text moves to a colon instead of a dash to match house style.
* fix(pr-artifacts): fall back to the upload web route when the API route is absent
`GET /projects/:id/uploads/:secret/:filename` landed in GitLab 17.4. On an
older self-hosted instance the rewrite turns an upload that used to load, on a
public project where the web route reads anonymously, into a 404 surfaced as a
502. That is a regression the rewrite introduced.
When the rewritten uploads API URL answers 404, retry the original web route
once before failing. The retry is deliberately narrow: only for the upload
rewrite, only on 404, and only when the 404 came from the rewritten URL itself,
with no version probing anywhere. It reuses the loop's redirect budget, so the
total number of requests stays bounded, and it is same-origin, so credentials
are attached on exactly the existing `shouldSendProviderAuth` terms.
A sign-in redirect from that retry still produces the actionable 401, so a
private project on an old instance reports a missing login rather than a 404.
* fix(pr-artifacts): drop the active content types from the extension map
`html` and `text/javascript` bought nothing. An HTML artifact is read through
`/api/pr-artifact-document`, which always answers `text/plain; charset=utf-8`,
so nothing on that path ever consulted this map. Keeping them meant the media
route could label an upload as active content, leaving safety resting on the
`Content-Security-Policy: sandbox` header staying in place forever.
`css` stays because `shouldRewriteCss` keys on `text/css` to rewrite provider
references, and the image and video types stay because `nosniff` means an
unrefined octet-stream simply does not render.
`svg` stays too, on the evidence of how svg uploads actually reach the screen.
`.svg` is not in the review editor's IMAGE_EXTENSIONS, so an svg upload arrives
either as an authored markdown image, rendered through `<img src>`, which is a
non-scripting context by spec, or as a resource referenced from an HTML
artifact, which renders inside an `<iframe sandbox="">`. Both are served by the
media route with `Content-Security-Policy: sandbox` and `X-Content-Type-Options:
nosniff`, so even a direct navigation to the proxy URL lands in a sandboxed,
opaque-origin document that cannot run script. Dropping it would give up real
rendering for no reduction in reachable capability.
Adds a regression test pinning that an .html or .js upload stays opaque.
* test(pr-artifacts): pin that the upload rewrite does not widen token reach
The rewrite sends PRIVATE-TOKEN to a different path on the provider origin, so
the invariant worth guarding is that it did not also change where that token can
travel. GitLab object storage answers an upload with a 302 to a signed URL on an
unrelated host, which is exactly the hop a credential must not follow.
Verified the test bites: forcing shouldSendProviderAuth to return true for
gitlab fails it on the second request's header.
The module-global auth cache has a 5 minute TTL and no reset seam, so the test
resolves the same `test-token` every other gitlab case in this file resolves,
which makes it correct whether the cache is cold or warm rather than dependent
on test order.
---------
Co-authored-by: Sun Neoh <yuensun.neoh@stashaway.com>
Co-authored-by: Michael Ramos <mdramos8@gmail.com>
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
* feat: add mobile touch range selection prototypes
* docs: redirect mobile range selection spike
* revert: remove command-mediated touch range prototype
* docs: align mobile line selection with DiffsHub
* feat: preserve mobile diff ranges before commenting
* fix(review): repaint a preserved mobile range on a second drag
A preserved range leaves pendingSelection non-null, so DiffViewer hands
Pierre a defined selectedLines and Pierre switches to controlled
selection: updateSelection then only records a proposed range and leaves
painting to the host. With no change handler wired, a second drag never
repainted, so the old highlight stayed put and the finger was untracked
until release. Wire onLineSelectionChange back into app state, only on
compact touch, so desktop keeps an options object with no such key.
Also route a null range to the toolbar host instead of swallowing it in
the preserve branch, mirroring AllFilesCodeView's early return so an open
composer (Ask AI included) closes with the selection it was written for.
* fix(review): 44px hit area for Pierre's gutter comment button on touch
With a dragged range now preserved instead of opening the composer, that
button is the only way to start writing about it, and it is roughly 20px
square: below the data-pn-touch-target standard the rest of the compact
shell holds. Grow its invisible ::before hit area to the 44px token,
leaving the glyph alone.
The rule ships through the same unsafeCSS both diff surfaces already
inject, which lands in Pierre's shadow root inside @layer unsafe (last in
the library's layer order, so no !important). It is injected only when
the shell is compact rather than gated in CSS: html:has() matches nothing
from inside a shadow root, and @media (pointer: coarse) would wrongly
claim a desktop with a touchscreen.
* test(review): cover the compact-touch preserved range on both diff surfaces
DiffViewer.compactTouchSelection.test.tsx drives the FileDiff options the
component hands Pierre: a drag preserves the range instead of opening the
composer and paints it through selectedLines, a second drag repaints
through onLineSelectionChange, the gutter action opens the composer, a
cleared range still reaches the toolbar host, and desktop keeps routing
completed drags straight to the composer with no change handler at all.
The last three assertions fail against the pre-fix component.
The AllFilesCodeView compact test only checked what was published upward,
which a range nothing paints would also satisfy; it now asserts the range
reaches the CodeView props. That needs the App loop, so the mount feeds
published selections back down as pendingSelection: without it the
reconcile effect clears the highlight the preserve branch just painted.
* chore(guides-show): regenerate viewer manifest pin for the touch selection changes
* guide: voice rules in the organizer prompt (short sentences, plain words, no dashes, no verdicts)
Measured on two agent-written guides: overviews averaged 120 to 135 words,
two thirds of the sentences ran past 25 words (longest 78), and one guide
used ' -- ' as a dash nine times. The prompt's Voice section now gives the
same concrete rules the plannotator-guide skill gives, so guides made
in-app and guides made by an agent read the same way.
* guide: note that GUIDE_REVIEW_PROMPT is mirrored into the plannotator-guide skill
* 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
An authored guide's generator.engine is the harness the agent ran in
(claude-code, codex-cli, …), which the in-app engine map does not know, so
the header printed the raw id. Map the common ones; unknown ids still render
as written. Viewer rebuilt and published, manifest synced.
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
* 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
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.
Adds the ARIA autocomplete contract to the comment composer skill picker: aria-autocomplete=list + aria-haspopup=listbox on the textarea, aria-controls/aria-owns while the menu is open, role=option rows with aria-activedescendant tracking. Preserves the no-preselection keyboard state machine and the untouched plain composer when skill references are off.
Closes#1233
Co-authored-by: ashish921998
* 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
* 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
* 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.
* 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
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.
- 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.
* 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.
* feat(editor): focus-mode shortcut to toggle both sidebars (#1276)
Keyboard-first reviewers had to reach for the mouse to collapse the
Contents sidebar and the right-hand panel every time they wanted to read
a document. Bind that to one key.
Mod+. now toggles the existing `focus` view mode from the keyboard, on
both the plan review and annotate surfaces. The first press remembers
whatever was open and closes both panels; the next press restores that
exact arrangement, so a session with only one panel open comes back the
same way. Any manual open (a sidebar tab, the annotation panel, the
agent terminal) already exits the view mode and clears the memory, so
the shortcut cannot leave the layout stuck.
The binding was picked after auditing every plan-review and annotate
binding: Mod+B / Mod+Shift+B are the sidebar toggles, Mod+S, Mod+P,
Mod+Enter, Mod+C and Mod+Z are taken, and code review already uses
Mod+. for its own "collapse the chrome" toggle. It is unshifted on every
keyboard layout and claimed by no browser or OS default.
The dispatcher gate is shared with the annotate sidebar shortcuts, so
the key is inert while a dialog, an overlay, a submission, or a text
field owns the keystroke. HTML surfaces are excluded because they own
their own persisted chrome and never render the Focus control.
* fix(editor): keep focus-mode exit reachable on HTML surfaces; note the HTML caveat in help
* fix(review): restore the prior diff when leaving the Commits view
Clicking a commit in the Commits panel (or its HEAD auto-select) switches
the single session-global diff to commit:<sha>, and switching the panel
back to Tree never switched it back, leaving the review stuck on a
historical commit (shipped as an accepted v1 edge in #994). A page reload
did not escape either, since GET /api/diff serves the current session
diff.
Client-only fix:
- preCommitDiffRef captures the pre-commit diff (full diff type + base)
on the first non-commit to commit switch in handleSelectCommit.
- Leaving Commits for Tree restores it via fetchDiffSwitch; without a
memo (post-reload) it falls back to the session default with the same
resolution handleWorktreeSwitch uses. Exiting while the commit switch
is still in flight also restores, superseding it server-side.
- Any applied non-commit switch clears the memo in fetchDiffSwitch.
- Reload un-trap: a load that is served a commit diff while the opening
panel view is not Commits snaps back to the session default once.
Git status keeps its existing since-base reset; the server keeps its
single-active-diff model unchanged.
* test(review): cover the offered-but-not-first preferred default; document the Commits detour contract
* fix(review): remember the last-used panel view across sessions
- Add cookie-only reviewPanelViewLastUsed config (sections | tree, never
commits); the header toggle records it and new sessions resolve
session state, then last-used, then the persisted reviewPanelView.
Explicit Settings/setup-dialog writes through the reviewView setters
sync last-used so a stale memo cannot shadow them.
- Wire the FileTree fallback PanelViewToggle to the real panel-view
selection and route Tree clicks through the shared select funnel
instead of rendering a hardcoded, no-op tree segment.
- Mark the review-setup-seen cookie when the first-run reset runs, not
only on the Got it click, so closing the tab without dismissing no
longer re-stomps preferences every session.
* fix(review): full-width panel toggle, relocated tree controls, click-to-copy diff count
- Give the Git status | Tree | Commits toggle the entire top row in all
three left panels; segments stretch to fill the row width.
- Move the controls that shared that row (staged count, search,
collapse-all, hide-viewed, viewed counter) into a shared
PanelControlsRow rendered below the All files entry, directly above
the file list; the Commits panel's loaded-commit count moves to its
own sub-row.
- Replace the footer Copy button with the diff count itself as the copy
trigger (shared CopyDiffFooter): a keyboard-reachable button with the
"click to copy the entire diff set" tooltip and transient
Copied/Failed feedback in the old button's slot.
- Add a tooltip to the file-row stage button using the same Tooltip
mechanism as the viewed control ("Stage file (git add)").
* fix(review): put Tree first in the panel view toggle
* fix(review): resolve effective panel view for the toggle; keep self-heal off the last-used memo
- Add resolvePanelView() and pass the EFFECTIVE view to FileTree so a
latent sections/commits selection the session cannot offer highlights
Tree instead of a hidden or wrong segment; allFilesOrder reuses it.
Unit test covers the no-segment and wrong-segment cases.
- setReviewPanelView() gains { recordLastUsed?: boolean } (default
true); the App self-heal passes false so repairing a conflicted
persisted pair no longer overwrites the user's last-used memo.
- Update the AGENTS.md since-base section to the new toggle semantics
(last-used memo, resolution order, setter sync, one-time first-run).
- Hardening: fold the diff counts into the copy button's aria-label,
skip the tooltip while the copy button is disabled (hover never fires
on a disabled native button in several browsers), let toggle segments
shrink (min-w-0 + truncate), and rework the config test onto a fresh
ConfigStoreForTest instead of resolving the singleton against a
throwaway backend.
* 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.
Michael Ramos