npm 12 blocks dependency lifecycle scripts unless the installing project
names the package in allowScripts. node-pty ships prebuilds for macOS and
Windows only, so on Linux its install script is what compiles
build/Release/pty.node. The generated managed-runtime package.json declared
no allowScripts, so npm installed the tree, exited 0, and the Agent tab
later failed with "Agent terminal runtime could not load WebTUI".
Generate the runtime manifest with a name-only node-pty approval, and verify
after install that pty.node actually exists (build/Release, build/Debug, or
prebuilds/<platform>-<arch>, which is node-pty's own resolution order). A
missing binary now triggers one targeted npm rebuild and, if that does not
repair it, fails provisioning with the blocked-scripts diagnostic and the
manual remedy instead of leaving a runtime that looks installed.
Closes#1409
Fixes from the 0.27.9 pre-release review. Servers: an unreadable rendered-HTML root falls back to the startup snapshot on both runtimes with a once-per-process warning instead of hanging (Pi) or answering 500 (Bun); the version diff is recomputed against current bytes on reload and carried through the in-app Refresh instead of being dropped, with no history write on a GET. Client: a Refresh action on the compact touch shell; HtmlSurfaceControls renders Refresh independently of the eye; the dead HtmlSurfaceActions removed. Threading: one linear, cycle-safe reply resolution shared by the annotations panel, its sort, and the export (5,000-chain tests), PATCH ingest on both runtimes rejects self-references and cycles, nothing is ever dropped from feedback. WebMCP and viewer hygiene: bounded tombstone and request memories, per-instance minted ids, nudge id caps, waiter cleanup on unmount, a shared retry epoch for diagram blocks. Docs: HTML Refresh documented, the WebMCP design pointer fixed, marketing pages updated.
AI-assisted (Claude) under maintainer direction.
The Windows uninstaller removed its PATH entry through .NET's SetEnvironmentVariable, whose synchronous settings-change broadcast can stall behind a hung window past the 15 second command timeout and make the uninstaller refuse to proceed (seen three times on one CI runner). The edit now goes through the registry directly (reading unexpanded, preserving the value kind), echoes the original value as proof of the write, and broadcasts the change best-effort with an abort-if-hung timeout that never affects the exit code; the restore path gets the same treatment with a sentinel. A completed write is trusted regardless of how PowerShell ended, while an unproven write still fails closed and preserves the CLI. Tests include a real PowerShell parse check of both scripts.
AI-assisted (Claude) under maintainer direction.
Follow-ups from the Workspaces adoption of 0.33.0: a math-slot module hosts can redirect Mermaid's own katex import to (importer-scoped resolveId recipe in HANDOFF) so a math document fetches one KaTeX chunk owned by the host; HtmlViewer bridgeErrorDisplay ('banner' default, 'none' lets a host own the failure banner while onBridgeUnavailable still fires); the documented bridge alias narrowed to relative sibling imports; resetMathRenderer keeps a registered loader and discards stale in-flight loads via an epoch, with setMathRendererLoader(null) and getMathRendererLoader added. Plannotator's own bundles unchanged (markers and sizes within noise of main). Bumps @plannotator/ui to 0.34.0; core stays 0.25.0.
AI-assisted (Claude) under maintainer direction.
Version bump, lockfile refresh, and docs pass for the ui 0.33.0 release carrying the bridge-as-asset seam (#1398) and the 0.32.0 adoption feedback fixes (#1399). Core stays 0.25.0.
AI-assisted (Claude) under maintainer direction.
Follow-ups from the Workspaces adoption of 0.32.0: the default KaTeX loader moves to its own module so a host that registers a mathRendererLoader can alias it away and drop the unused KaTeX chunk (a registered loader is never backfilled by the default); docs state when onUnanchoredChange first delivers, what projectHostThreads loses on markdown surfaces (export ordering, line labels, repeated-text disambiguation, no-flash restore) while still re-anchoring by text search, and that cap-dropped handling is a backstop once maxAdditionalTargets is enforced upstream; HANDOFF.md now ships in the tarball so the README references resolve. Plannotator's own behavior is unchanged (the eager entry fills the slot before first render; built-bundle markers and sizes match main).
AI-assisted (Claude) under maintainer direction.
Opt-in bridgeScriptUrl on HtmlViewer so multi-chunk hosts can serve the 185 KB bridge as a hashed asset instead of an inlined string; the inline bridge stays the default and Plannotator's own builds, the Pi and OpenCode copies, and the live-app proxy are unchanged apart from a protocolVersion field on the bridge's ready message. The parent checks the version (one warning naming both versions; on the URL path a dismissible banner plus onBridgeUnavailable while the old bridge keeps working), arms a ready timeout on the URL path only, and resolves the URL against the parent document before it reaches the frame so a page's own base href cannot redirect the load. A prepack-generated bridge-script.asset.js (byte-for-byte the inline string) and a bridge-script.lite.ts alias target ship in the tarball. CSP and CORP requirements for hosts are documented.
AI-assisted (Claude) under maintainer direction.
Version bumps, lockfile refresh, and docs for the lockstep npm release carrying the HTML annotation seams (#1395), the lazy renderers with eager entries (#1394), and WebMCP phase 1 (#1393). Core publishes first because ui now imports @plannotator/core/html-anchor.
AI-assisted (Claude) under maintainer direction.
Bundle-weight optimization of @plannotator/ui for multi-chunk hosts, requested by Workspaces: the Mermaid runtime and Graphviz engine load inside the render effect, the username dictionary sits behind a synchronous identity generator slot, and KaTeX sits behind a math renderer slot with a loader seam on configurePlannotatorUI. Plannotator's own apps import eager entries (math, identity, and Mermaid for the plan editor) so their behavior is unchanged: single-file builds within noise of main, math typeset on first paint, identities from the full dictionary, and the share portal keeps Mermaid in its entry chunk so its failure surface matches main. Built-HTML registration markers guard the eager imports. Hosts that omit the eager entries get the lazy paths, a one-shot automatic re-attempt, and a Retry affordance on the diagram error panel; the module-map limitation of in-page retries is documented.
AI-assisted (Claude) under maintainer direction.
Parity seams for hosts of @plannotator/ui, requested by Workspaces after the HTML annotation handoff: projectHostThreads and buildPersistedHtmlAnchor in @plannotator/core; HtmlViewer onUnanchoredChange completed over the annotations prop with a restore-keyed report so hosts can drop their mark-applied listeners; published useHtmlRefresh with a fetchSnapshot adapter; published HtmlSurfaceControls (eye, refresh, pen) with label overrides; AnnotationPanel unanchoredIds chip (wired for Plannotator too); HtmlViewer maxAdditionalTargets and scrollBehavior carried on the bridge; shortcuts and utils/inputMethod blessed as consumer exports. Plannotator's behavior is unchanged apart from the new Unanchored chip, verified by a real-browser A/B including the orphan and re-anchor cycle and by a combined cross-surface verification with #1394.
AI-assisted (Claude) under maintainer direction.
Local rendered-HTML annotate sessions get a Refresh action beside Hide tools: the document is re-fetched through /api/doc, the sandboxed viewer remounts, annotations are re-anchored and the ones that no longer match are reported while their comments are kept, and stale diff and share state is reset. Maintainer additions on top of the contributor's work: share-link invalidation no longer keys on the resolver's identity, /api/plan and /api/share-html serve a local root HTML file from its current bytes on both runtimes so a reload does not revert the page under the annotations, the Refresh button keeps keyboard focus via aria-disabled, and the tests were hardened. Verified end to end in a real browser.
Thanks @leoreisdias.
AI-assisted (Claude) under maintainer direction.
Phase 1 of WebMCP support: a zero-dependency, feature-detected engine in packages/ui/webmcp plus a read-and-comment tool catalog for plan review and annotate (read_document, add_comments, update_comment, remove_comments, reveal, nudge_user, list_documents). No decision tools; the human approves. Zero footprint in browsers without document.modelContext (DOM, network, console, timers, and cookies identical to main), idle until called where the API exists, and never registered inside the annotate iframes. Adds an optional inReplyTo field on annotations for threaded replies. Client-only; no server changes.
AI-assisted (Claude) under maintainer direction.
The v0.27.5 comment-only ruling removed every label affordance from HTML
and live-app annotate surfaces, leaving no one-click positive feedback:
the only path was opening the composer and typing prose. Restore exactly
ONE affordance, the hardcoded 'Looks good' thumbs-up, on both input
routes:
- selection toolbar: commentOnly + a provided onQuickLabel now renders
only the thumbs-up (no Delete, no Zap picker, Alt+digit suppressed);
HtmlViewer passes a handler that filters by label id as defense in
depth
- pinpoint: the composer gains an optional one-click 'Looks good'
footer action (disabled once anything is typed, so it can never
discard a draft), emitting the same isQuickLabel comment shape with
the draft's multi-select targets
The trust-boundary clamp is untouched: redline/quickLabel modes stay
collapsed to selection, so a hostile page still cannot force a DELETION
or an arbitrary label. THUMBS_UP_LABEL moves to utils/quickLabels as
the canonical definition.
MAX_TREES=100 rejected CallDiff results at 41 changed files in
Swift-style languages (many small per-entry trees), making Call Flow
unusable on normal branch reviews. MAX_NODES was never the binding
constraint (3,138 nodes at 470 trees).
- Raise MAX_TREES to 2,000 so realistic reviews parse untouched.
- Truncate deterministically at the cap instead of throwing, and add a
warning diagnostic ("Showing the first 2,000 of N call trees") so the
omission is never silent.
- Keep hard failure for the unbounded-output guards (MAX_NODES,
MAX_TREE_DEPTH, raw length) that protect against pathological worker
output.
Fixes#1351
Opening a plan review from Pi on Windows could exit the entire Pi host with
an uncaught `write EPIPE` raised inside `PiProcessNode.send()`. The provider
checked `stdin.destroyed` and then wrote, which cannot close the race: the
nested `pi --mode rpc` child can close the pipe between the check and the
write. Node then reports EPIPE either as a synchronous throw or as an `error`
event on the stream, and because no stream had an `error` listener that
became an `uncaughtException` and terminated the host agent process.
Add a shared guard (`packages/ai/providers/child-io.ts`) and apply it to both
JSONL/JSON-RPC providers:
- `guardChildStreams` attaches `error` listeners to the child and every pipe
immediately after spawn, so a stream error can never escalate. The previous
one-shot spawn listener was removed on success, leaving the child with no
`error` listener for the rest of its life.
- `writeChildLine` reports a synchronous failure through its return value and
an asynchronous one through the write callback, so both paths converge.
- A failure now resolves as a provider failure: in-flight requests reject, the
process end is broadcast to listeners so a streaming query terminates, the
child is reaped, and `alive` flips false so the next query re-spawns.
Previously a failed write also left `sendAndWait` pending forever, because
`send()` was fire-and-forget and the Pi provider has no RPC timeout.
Also guards the Bun variant's FileSink write/flush symmetrically, and switches
Pi's Node stderr from an un-drained "pipe" to "ignore", matching the
deadlock reasoning already documented in codex-app-server.ts.
Regression test runs the provider in a real `node` child against a fake Pi
that closes its own stdin; the child installs no `uncaughtException` handler,
so surviving to print its results is the proof. Against the unfixed provider
that child dies with `Error: write EPIPE`, exit 1.
Reported by @Kaelenx.
* feat(skills): add the plannotator knowledge-layer skill with a CLI freshness guard
A new model-invocable core skill (apps/skills/core/plannotator) that teaches
an agent the whole CLI surface: decision guide, per-command reference with
flags and exit codes, env vars, the external-annotations API, and a do-not
list. The existing plannotator-* core skills stay lightweight action stubs.
A freshness test (apps/hook/server/plannotator-skill-reference.test.ts)
parses the skill's documented subcommands and flags and diffs them against
cli.ts usage text plus the CLI arg-parsing sources, in both directions, so
the reference cannot drift from the real CLI without failing the suite.
Installers copy the single-sourced core body into ~/.claude/skills and
~/.agents/skills on all three platforms; uninstall removes it from both
scopes. The skill ships model-invocable as a documented exception to the
locked-by-default rule, asserted both ways in install.test.ts.
* feat(marketing): serve the plannotator knowledge skill as /llms.txt
Single-sourced at build time from apps/skills/core/plannotator/SKILL.md
per the llmstxt.org spec (H1, blockquote, detail sections, Docs link
list), so the CLI freshness guard transitively keeps llms.txt current.
* fix(skills): reach every install path with the plannotator knowledge skill
The knowledge skill reached Claude Code and ~/.agents but was missing from
three install paths. Six fixes from the install-reach review of #1377.
Kiro: the installer's Kiro leg copied only the two action skills, so Kiro
users got launchers and no CLI reference. One copy line per installer, and
"plannotator" joins uninstall.ts's KIRO_SKILLS.
OpenCode npm: @plannotator/opencode's postinstall copied only commands/*.md.
The package now ships the skill (copied at build time like the HTML assets,
gitignored so the shipped copy cannot drift) and postinstall places it under
${XDG_CONFIG_HOME:-$HOME/.config}/opencode/skills/plannotator/, which is a
path OpenCode really scans ({skill,skills}/**/SKILL.md under xdgConfig/
opencode). Uninstall sweeps it, skills only, so a user's own
opencode/commands/plannotator.md stays out of scope.
Pi npm: vendor.sh copies the skill to apps/pi-extension/skills/plannotator/
and package.json declares it under pi.skills, which Pi resolves relative to
the package root. Neither vendored copy carries the // @generated header the
.ts files use: a SKILL.md must open with its frontmatter on line 1.
llms.txt: the endpoint resolved the skill through process.cwd(), which breaks
under any invocation but --cwd apps/marketing. new URL(import.meta.url) does
not fix it either, because Vite rewrites import.meta.url to the emitted SSR
chunk's location. Inlined with Vite's ?raw, resolved by the bundler relative
to the source file. Also drops the summary paragraph the required blockquote
already carries; SKILL.md itself is unchanged.
Uninstall: KNOWLEDGE_SKILLS is a separate list from CORE_SKILLS precisely so
the bare name "plannotator" cannot leak into LEGACY_COMMAND_NAMES or
STALE_CODEX_SKILLS and delete a user's own files. Nothing tested that; now a
test proves the five installed scopes are removed and commands/plannotator.md
(Claude and OpenCode) plus ~/.codex/skills/plannotator survive. Also
cleanupStaleSkillLayout now knows KNOWLEDGE_SKILLS.
Origins: oh-my-pi (#1373) was missing from SKILL.md's PLANNOTATOR_ORIGIN row.
The guard now imports AGENT_CONFIG and asserts the row names every key and
invents none, and its header comment is narrowed to what it actually proves:
bidirectional for subcommands and origins, one-directional for flags.
AI-assisted (Claude) under maintainer direction.
* fix(review): detect JJ mutable line-of-work base
Use JJ's mutable-stack revset to find the line boundary directly instead of inferring a parent from bookmark ordering, which is ambiguous because JJ has no current bookmark.
* fix(review): harden the JJ line-of-work base inference
Maintainer follow-up on the line-of-work base detection.
Skip the bookmarks `jj git push --change` generates. They name one change,
not a line of work, and they do reach the fork point: a colleague's pushed
change bookmark arrives as an untracked remote bookmark, which makes its
commit immutable and therefore a candidate base, so the reviewer was told
they were comparing against `push-vmopwunwxopv@origin`. The commit id is
used instead.
Pass a full commit id through `jjCompareTargetRevset` as a revision. It has
no separators, so the commit-id fallback was being wrapped as
`bookmarks(exact:"<sha>")`, which resolves to no revisions and made the
whole Line of work diff fail.
Fall back to `trunk()` instead of throwing. The only live caller is
`getJjContext` on the review startup path, which has no handler above it,
so a throw aborted `plannotator review` with a stack trace before the
server was built rather than reporting anything. That also covers a `jj`
too old for `fork_point`/`reachable`.
Make the query explicitly single-record with `latest(..., 1)`. The parser
reads one record, and bookmark preference (remote before local) is only
meaningful within one commit, so the tie-break belongs in the revset rather
than in a silent "first row wins" slice.
Isolate the real-jj test behind its own JJ_CONFIG. It was reading the
developer's real config, where `[signing] behavior = "own"` alone makes it
fail with a GPG error.
Live fixtures cover the generated-push-bookmark stack, the untracked remote
push bookmark, and a stacked local bookmark.
---------
Co-authored-by: Michael Ramos <mdramos8@gmail.com>
* feat: detect the oh-my-pi harness as its own agent origin
- omp injects OMPCODE=1 (+ a CLAUDECODE=1 compat shim) into every Bash-tool child; the env chain now maps it to a dedicated oh-my-pi origin labeled "Oh My Pi".
- Distinct origin rather than aliasing claude-code, because the claude-code-only gates (permission-mode setup, permissionMode in approve) presuppose a PermissionRequest hook that a bash-invoked plannotator never has; omp has no approve support and no planning integration yet.
- Fallback deliberately left "claude-code"; wording unchanged.
- No dedicated Ask AI provider for oh-my-pi.
* chore: sync lockfile workspace versions
* fix: check OMPCODE last so runtimes inside an OMP session keep their label
OMP exports OMPCODE into every shell it spawns. With the check at the top of the chain, opencode/codex/... launched from an OMP session inherited OMPCODE and were mislabeled "Oh My Pi". Moving it just above the terminal fallback lets specific runtime env vars win; OMPCODE still beats the claude-code fallback for bare shells.
Reported by backnotprop in #1373.
Phase 2 of live app annotation: full parity on Pi over one shared
implementation instead of drifting copies.
- Extract every proxy decision into packages/shared/live-proxy-core.ts
(HTML injector state machine, loopback/Host/Origin predicates,
CSP/X-Frame-Options policy, redirect rewrite, WS origin gate, bridge
assembly, liveAppDraftIdentity) and the CLI probe + live-mode messages
into packages/shared/live-probe.ts. packages/server/live-proxy.ts is
now a thin Bun transport over the core; its test suite passes
unmodified.
- Add packages/shared/live-proxy-node.ts, the node:http transport the Pi
extension runs: streaming request/response piping through the shared
injector, and WebSocket (HMR) passthrough that replays the client's
handshake upstream over raw TCP and pipes the sockets byte-for-byte.
Transport tests run the proxy in a real node child process, because
Bun's node:http shim drops writes to an upgrade event's socket.
- Wire Pi: /plannotator-annotate probes loopback URLs live-first with
the shared probe (same 3s timeout, same <500 gate, same messages),
recognizes --app/--static via parseAnnotateArgs's liveFlags opt-in
(OpenCode deliberately does not opt in), and serves mode annotate-app
from serverAnnotate.ts with the shared per-target draft identity,
live sessions excluded from history/submissions, the remote hard-off
throw, and guarded live-proxy shutdown.
- Vendor live-proxy-core/live-probe/live-proxy-node plus the
dependency-free bridge-script constants to generated/.
- Docs: AGENTS.md phase-gate passages, marketing annotate page, Pi
README.
* fix(annotate): pre-release QA fixes for the armed-mode interaction seams
Six confirmed QA findings on the HTML/live annotate surface plus missing
pi-extension resync coverage:
1. Armed pinpoint drifted click (>4px, no selection) was swallowed AND
leaked to the page: the always-on drag work armed the trailing-click
suppression on drift alone. The mouseup arming site now requires the
drag to have actually produced a text selection; drifted clicks pin
normally and never reach the page. Bridge tests for armed drift,
armed real drag, and Interact drift.
2. Esc ladder: hover-clear is no longer its own rung; clearing the
pinpoint outline and posting annotate-exit happen on the same press
when no draft is open. Draft-close keeps its own press.
3. Compact touch layouts no longer apply a restored toolsHidden:true
chrome cookie (both header toggles are desktop-only, so applying it
stranded the user); the cookie value is preserved for desktop.
4. The live-app probe now announces the static-conversion downgrade on
stderr when a loopback probe fails, naming --app to force live mode.
5. Live-app export: page group headers are now '## Page:' with '### N.'
entries nested below them; exports without pageUrl stay byte-identical.
6. Shift+1-4 mode shortcuts no longer fire while the annotation
toolbar's type-to-comment listener owns printable keys, so typing
! @ # $ into a starting comment cannot silently switch modes.
Also adds the missing tests for the two resyncPhaseFromSession
executing->idle fallbacks that arm idleNoticePending (verified by
mutation: flipping either arm fails its test).
* fix(annotate): compact arm/disarm affordance, guarded shutdown, restored chrome guards
Follow-up scope from the forensics sweep, same surface:
- Compact touch layouts get Options-menu actions for the HTML/live
surface: 'Annotate page'/'Interact with page' (the desktop pen and
Mod+Shift+A were unreachable on touch, so every tap annotated with no
way out) and 'Show tools'/'Hide tools' (the desktop eye). With the
menu as the way back, the toolsHidden cookie now applies on compact
again (desktop parity) instead of being ignored.
- The annotate servers' stop() now guards every disposal step
individually (Bun: runGuardedShutdown, mirrored inline in Pi): a
throwing agent-terminal teardown (#1314-class) no longer skips
liveProxy.stop() and the other disposals after it. Unit-tested with a
throwing disposer.
- Re-added the two regression guards dropped in the htmlHideTools ->
htmlChrome test rename: the restore commit never writes stale
pre-restore chrome values to the cookie, and the sidebar stays
reachable via Mod+B while tools are hidden.
* fix(annotate): scope the Agent TUI display reset to display settings only
The Display popover's 'Reset terminal display settings' button also called
onSideChange('left'), durably overwriting a user's chosen right/hidden
placement in config.json with no disclosure — the label scopes the reset
to font/appearance. Position is a layout preference with its own explicit
segmented control right below, so the reset no longer touches it: the
button now resets exactly the display settings through the panel's one
sanitized update path, and the popover no longer has any code path from
reset to the side.
AgentTerminalDisplayPopover is now exported with a defaultOpen test seam
(the surrounding panel needs a live WebTUI session to render it); tests
assert reset restores the display defaults without firing onSideChange,
and that the Position control remains the explicit way to change
placement.
* fix(server): live-proxy injection and config write hardening
Four confirmed pre-release QA findings, each with a test that fails on
the pre-fix source.
live-proxy: the HTML injector scanned for head markers with no notion of
comments, so a codegen banner naming <head> before the real tag captured
the bridge script into a dead comment span: never executed, annotation
silently broken, no warning. The scanner now skips comments and the
'>'-terminated markup-declaration / bogus-comment spans (doctype,
CDATA-ish, <?...>) before matching, inside the same chunk-boundary state
machine. Raw-text element contents are still not tracked; that limit and
its degraded outcome are documented in the source.
live-proxy: new URL(req.url) ran before Host validation, so a Host-less
HTTP/1.0 request threw and served Bun's internal debug page with a stack
trace. Host validation now runs first, and URL construction takes the
same 403 path on failure.
live-proxy: the text/html content-type test was case-sensitive, so a
valid TEXT/HTML response skipped injection and the framing rewrites.
config: saveConfig was an unlocked read-merge-write, so two processes
sharing a data dir dropped each other's keys while both reported success.
The read-merge-write now runs under an O_EXCL advisory lockfile with a
bounded wait and stale takeover, degrading to the old behavior with a
warning rather than ever hanging, and the write itself is temp+rename so
lock-free readers cannot observe a torn file.
Also consolidates the duplicated agent-terminal side predicate onto the
single definition in @plannotator/core.
* fix(annotate): give live app sessions their own draft slot
mode "annotate-app" resolves markdown to "" by construction (the page
lives behind the proxy, not in a string the server holds), and the
autosave draft key was contentHash of that body. Every live session on
the machine therefore collapsed to the one hash of the empty string and
shared a single draft slot: two sessions against different dev servers
read and overwrote each other's in-progress annotations, deterministically.
A live session's identity is its target, exactly as a folder session's
identity is its folder path, so the key is now derived from the target
URL (normalized through the URL parser so the same dev server recovers
its draft when spelled with or without a trailing slash). Classic file
and folder keying is untouched.
Pi has no live app mode (no annotate-app, liveApp or live-proxy outside
its vendored generated/ tree), so there is nothing to mirror there.
* Allow annotate terminal to dock on either side
* Allow annotate terminal to dock on either side
* fix(annotate): persist Agent TUI preferences through the settings registry
The Position control introduced in #1050 stored its choice in a cookie via
hand-rolled helpers that bypassed the settings registry. Every annotate
session runs on its own random port, so a cookie is scoped to one session:
the placement silently reset on the next annotate. The sibling
`plannotator-annotate-agent-terminal-default` cookie (preferred agent) had
the same gap.
Both now follow the `conventionalComments` precedent exactly:
* `agentTerminalSide` and `agentTerminalDefaultAgent` join `SETTINGS` with
serverKey/fromServer/toServer, reusing their existing cookie keys so a
user who already picked a side keeps it across the upgrade.
* `PlannotatorConfig` gains both as flat keys (only diffOptions, theme,
reviewAnalysis and prompts deep-merge in saveConfig), emitted from
`getServerConfig()` behind an `isAgentTerminalSide` guard so a
hand-edited config.json cannot advertise a side that does not exist.
* Both keys are added to the two /api/config allowlists: the Bun annotate
server and the hand-mirrored Pi one.
The side vocabulary moves to @plannotator/core/agent-terminal (widened to
include the `hidden` state added next) so the registry can reach it without
closing an import cycle through ConfigStore; the ui util keeps its public
API by re-exporting.
Regenerates the pinned guide viewer manifest, which shifts by 0.1 KB gz
because the settings registry now reaches into core/agent-terminal.
AI-assisted (Claude) under maintainer direction.
* feat(annotate): add a Hidden Agent TUI position and extract its layout
Builds on the Left/Right Position control from #1050.
Hidden (third state of the Position control)
Hidden is a durable preference that the Agent TUI is not part of this
user's layout: nothing is docked, and choosing Hidden while the terminal
is open closes it (from either surface that offers the control). It is a
default, not a lock. The rail toggle, the Shift Shift shortcut and a
message routed to the agent all still open the panel for the session, and
none of them rewrites the preference, so explicit intent wins now without
changing what happens next session. A `hidden` preference owns no dock
edge, so a session open falls back to the historic left placement.
Because the Position control lives inside the terminal's own popover, and
Hidden closes that popover along with the terminal, the same control is
now also in the Settings dialog (General tab, annotate mode). That is the
way back from Hidden, and it also answers the review note that Position
could not be preconfigured before the terminal was ever opened. It is
gated on the terminal actually being available in the session so a remote
or runtime-less annotate never offers a dead control. Both surfaces write
the same `agentTerminalSide` config value and read it through ConfigStore,
so they cannot drift.
The existing transient hide affordances (header X, resize handle click and
drag-snap, rail toggle, Shift Shift) are unchanged and stay session
scoped. A running agent still stays mounted off-layout when collapsed, so
hiding the panel never kills the PTY.
Review fixes
* Extract `getAgentTerminalLayout` from App.tsx into
packages/editor/agentTerminalLayout.ts with a table test over
{side including hidden} x {open} x {running} x {wideMode} x
{belowBreakpoint} x {rightPanelOpen}, asserting the invariants that can
actually regress: never docked on both edges, never visible below `lg` or
in wide mode, a collapsed running terminal stays mounted zero-width on its
own edge, and the right panel is suppressed exactly when a VISIBLE
right-docked terminal holds the slot.
* Fix `aiSurfaceOpen`, which still read `effectivePanelOpen &&
rightSidebarTab === 'ai'` after its siblings moved to
`isRightPanelVisible`. A right-docked terminal visually suppresses the
panel but left the Ask AI model-discovery effect firing for an invisible
surface, which is exactly the eager provider work that gate exists to
avoid. The layout computation is hoisted above the consumer so it can use
the same fact the JSX does.
* Document the right-slot invariant at both coordination sites. The
asymmetry is deliberate: the panel evicts the terminal (which keeps
running off-layout, so reopening resumes the same session), while the
terminal only suppresses the panel visually so dismissing it restores the
user's place. Symmetry would make every short terminal detour cost the
reviewer their open surface.
* Name the `useIsMobile(1024)` literal `AGENT_TERMINAL_LG_BREAKPOINT`, tied
to the panel's own `hidden lg:flex`.
* Restore `hideAgentTerminal()` in the resize hook instead of the raw
setter, and point the handle at the resolved placement.
AI-assisted (Claude) under maintainer direction.
---------
Co-authored-by: Michael Ramos <backnotprop@gmail.com>
* feat(bridge): additive live-mode gate + LIVE_BRIDGE_BOOTSTRAP
Adds the config-gated live branch to BRIDGE_SCRIPT: frame gate, pinned
parent origin, token-stamped postToParent, origin+token checks on both
inbound handlers, pinpoint-only clamp, vim and resize off, pageUrl on
ready, and coalesced page-change reporting for SPA history navigation.
With no config present (srcdoc) every branch is inert and behavior is
unchanged; the existing html-viewer suites pass unmodified as the
regression proof. LIVE_BRIDGE_BOOTSTRAP installs the annotation CSS
from the JSON config prelude before the IIFE runs. New package export
exposes the string constants without the React barrel.
* feat(ui): live-session parent side for proxied app annotation
useHtmlAnnotation gains a live option (origin + token validated before
parseBridgeMessage; token + concrete targetOrigin on every outbound
post) and a validated page-change message with onPageChange. HtmlViewer
gains src/liveSession/currentPageUrl/onPageChange: src-mode iframe with
no sandbox and no srcdoc, ready pageUrl handling, per-page restore
filtering with explicit clear-marks + re-sync on navigation, and one
postToBridge choke point for its direct posts. Annotation.pageUrl is
additive; exportAnnotations groups by page (with global numbering kept)
only when a pageUrl is present, byte-identical otherwise. AnnotationPanel
shows the page label; AnnotationToolstrip can hide the input switch.
The editor app wires mode annotate-app: full-viewport live surface,
forced pinpoint, vim off, diff/share hidden, pageUrl stamping.
* feat(server): loopback reverse proxy for live app annotation
Whole-origin mirror of a local dev server on a dedicated 127.0.0.1
port: streaming bridge injection (after the head open tag, before a
bare </head>, or appended; exactly one per document; 8-byte holdback
plus a state machine for tags split across chunks), header hygiene
(upstream Host rewrite, X-Forwarded-*, identity Accept-Encoding on
document intent only, hop-by-hop strip), CSP drop-and-replace with
frame-ancestors listing the editor origins, X-Frame-Options removal,
target-origin Location rewrite, byte-identical passthrough for assets
and encoded HTML (no injection, once-per-session diagnostic), SSE
streaming, and WebSocket passthrough with a bounded pending queue for
HMR. Host header validation runs before any upstream contact; the bind
is the literal loopback constant and the advertised-URL override is
never applied. Tests boot a fake dev server and cover injection,
hygiene, fidelity, WS echo, and the security posture.
* feat(annotate): annotate-app server mode + CLI live probe with remote hard-off
startAnnotateServer gains mode annotate-app and a liveApp option: it
throws under PLANNOTATOR_REMOTE, generates the per-session token,
composes the proxy-served bridge body (JSON config prelude with both
editor origin forms, localhost first, plus bootstrap and bridge
supplied by the caller so packages/server never imports
@plannotator/ui), starts the loopback proxy after the annotate port is
known, serves the live /api/plan payload (no rawHtml, no version
fields, sharing off), and stops the proxy with the server. Version
history and durable submission records stay excluded via the explicit
mode gate.
The CLI resolution probes loopback http URLs (3s, accept text/html)
and defaults them to live mode when the probe returns HTML; --static
forces conversion, --app forces live and fails loudly on non-loopback,
https, unreachable, or non-HTML targets; both flags are mutually
exclusive transport-shape flags never echoed in the tolerant handoff.
A live resolution under PLANNOTATOR_REMOTE is a startup failure
suggesting --static. OpenCode and Pi parsers are untouched this phase.
* test(live-annotate): protocol, server, and probe suites + smoke script + docs
htmlLiveProtocol.test.tsx covers the parent trust boundary (origin and
token rejection before parseBridgeMessage, token + targetOrigin on
every outbound post, validated page-change and ready pageUrl, per-page
restore filtering with full-list numbering) and the bridge live gate,
executed as the composed config + bootstrap + bridge body inside a
dedicated harness iframe so the srcdoc suites keep running the same
script uncontaminated in this process. annotate.test.ts gains
annotate-app cases (live payload shape, composed bridge served by the
proxy, no-history version endpoints, proxy stopped with the server,
remote rejection); annotate-live-resolution.test.ts covers the probe
matrix. The two post helpers now drop unmatched-targetOrigin posts
silently, matching browser semantics where some DOM environments throw.
Adds the manual Vite/Next smoke script and the AGENTS.md live app
annotation section (phase gate, security posture, limitations).
* test(annotate-cli): cover the CLI layer of the live app remote hard-off
Spawns the real CLI entry (async, so the in-process fake app can answer
the live probe) with PLANNOTATOR_REMOTE=1 against a loopback HTML
server and asserts the startup-failure exit with the --static hint.
Completes per-layer coverage of the three-layer hard-off (CLI exit,
server throw, unconditional loopback proxy bind).
* fix(live-annotate): harden the loopback trust boundary end to end
- isLoopbackHostname (now canonical in live-proxy.ts, re-exported by the
CLI resolution) requires localhost, ::1, or a LITERAL 127/8 IPv4
address: DNS names like 127.0.0.1.evil.example no longer classify as
loopback, so neither the default probe nor --app can start a live
proxy against an off-box origin.
- The live-eligibility probe judges the FINAL response URL: a target
that redirects off its loopback origin falls back to the static
pipeline (or fails loudly under --app) instead of opening a live
session whose iframe immediately leaves the proxy.
- WS upgrades with a browser Origin not naming the proxy itself are
refused, so a hostile page's cross-site connect is never laundered
into the origin-less shape dev servers trust as a non-browser client
(Vite CVE-2025-24010 class).
- /__plannotator__/bridge.js refuses cross-site/same-site
Sec-Fetch-Site fetches: the per-session token is no longer readable
via an off-origin script include on modern browsers.
- X-Frame-Options is stripped only on HTML responses (where
frame-ancestors replaces it); non-HTML responses keep the app's own
framing protection.
- Redirect Locations are re-anchored by loopback-host + port
equivalence instead of a string prefix: alternate loopback spellings
are now caught and lookalike ports (5173 vs 51730) pass through
untouched.
- --app on a non-URL target fails loudly instead of being silently
swallowed.
* fix(live-annotate): session correctness for SPA restores, origins, and pathful targets
- A live find-and-mark that resolves nothing keeps its record, seeded
with unresolved placeholder targets from the durable anchor/text
params, so the mutation-driven reconcile re-acquires the pin once a
lazy route or data-dependent tree renders (SPA navigation no longer
permanently drops pins). Srcdoc restores keep the fail-closed drop.
- The bridge posts every outbound message once per listed editor
origin; the browser delivers only the one matching the parent
document, so an editor opened at 127.0.0.1 instead of localhost no
longer silently loses ready and every subsequent message.
- The advertised appUrl is the proxy under its localhost spelling with
the target URL's own path and query: the framed app stays same-site
with the editor, shares the dev app's host-only localhost cookies
and storage, and a pathful target opens its page instead of the app
root. The proxy still binds the 127.0.0.1 literal.
* ci(live-annotate): run the live protocol DOM suite; document the hardened posture
htmlLiveProtocol.test.tsx is DOM-gated and was absent from the
workflow's DOM_TESTS file list, so none of its trust-boundary
assertions ran in CI. Add it, and update the live-app section of the
project docs: literal-loopback gate, probe redirect rule, WS Origin
check, bridge.js delivery gate, localhost appUrl advertisement, live
restore resilience, and the remote-mode behavior change (loopback URL
annotate under PLANNOTATOR_REMOTE now exits asking for --static
instead of silently converting).
* fix(live-annotate): absorb the v0.27 mainline into the live session surface
Post-rebase seam work after replaying the branch onto main (v0.27.4 era):
- Route the bridge's unanchored-transparency report through postToParent so
live sessions deliver it token-stamped to the listed editor origins; the
raw '*' post main introduced for srcdoc would be dropped by the live
parent's message authentication exactly where restores fail most. New
live-harness test pins the contract.
- Extend the live remote hard-off to --tailscale sessions (flag postdates
the branch): CLI startup failure + startAnnotateServer throw keyed on
tailnetPublished, matching how the annotate agent terminal treats tailnet
publication. Covered in annotate.test.ts and documented in AGENTS.md.
- Keep main's compact-touch input controls and effective mode/input values
on the HTML surface while preserving the live pinpoint-only clamps.
- Regenerate the pinned guide-viewer manifest (CSS hash moved with the new
UI classes; JS unchanged).
* feat(live-annotate): Interact/Annotate mode toggle for live app and raw HTML sessions
A live app session used to be unusable: the pinpoint capture-phase click
handler owned every click, so buttons, checkboxes, inputs, and links never
fired. One boolean mode now governs the HTML/live viewer surface:
- Interact: the bridge is fully passive. Pinpoint capture, hover outline,
drag-selection toolbar, [data-annotate] clicks, and committed-highlight
click interception are all gated behind annotateModeActive, so clicks,
forms, text selection, and SPA navigation reach the page natively.
Committed markers and highlights stay VISIBLE, and marker buttons keep
their clicks (a marker click still opens its comment).
- Annotate: classic behavior, unchanged. Live sessions annotate exclusively
via pinpoint while armed.
Control: a single bubble icon button in the editor header (icon never
changes; armed = accent + visible border, idle = transparent border of the
same width, so the box is pixel-identical in both states), plus a subtle
inset accent ring floated over the viewer while armed (pointer-transparent,
no layout shift). Keyboard: Mod+Shift+A through the shortcut registry
(html-annotate scope; the bridge mirrors the chord inside the iframe and
forwards it over the authenticated postToParent path). Esc gains a final
ladder rung: draft closes first, then the hover outline clears, then Esc
exits Annotate back to Interact (bridge posts annotate-exit; a parent-side
listener covers Esc with editor focus). The parent owns the mode and pushes
it with the same re-post-on-ready pattern as set-input-method, so it
survives live page changes, HMR reloads, and bridge re-injection without
ever reloading the iframe.
Defaults: live app sessions START in Interact; static/raw HTML sessions
START in Annotate (today's behavior preserved, and the srcdoc bridge default
keeps behavior byte-identical when no set-annotate-mode ever arrives).
Session-only state, no persistence. Vim navigation is available only while
Annotate is armed.
Covered by new bridge-harness and parent-side DOM tests in
htmlLiveProtocol.test.tsx and htmlPinpointProtocol.test.tsx: Interact
pass-through, armed capture, the Esc ladder order, mode survival across
re-injection, marker clicks in Interact, and both defaults.
* feat(live-annotate): pinpoint-armed default, always-on drag comments, comment-only HTML surfaces
Simplifies the Interact/Annotate design after live review. The new
contract replaces the previous one where they conflict:
- BOTH surfaces (raw HTML and live app) now START ARMED with pinpoint;
the live-session Interact default is gone. Esc keeps the ladder
(close draft, clear hover, then exit to Interact) and the header
toggle re-arms. The bridge also paints the pinpoint cursor at init
instead of waiting for the parent's first round trip.
- The header toggle is a PEN icon: the old bubble sat next to the
annotations-panel bubble and the two were indistinguishable. Same
box geometry (armed = accent + visible border, idle = transparent
border of identical width), aria-pressed, Mod+Shift+A, and the
armed ring over the viewer are all unchanged.
- Text drag-selection commenting is ALWAYS live on HTML/live surfaces,
in BOTH states: the selection pass is ungated from annotateModeActive
and from the pinpoint input method. In armed pinpoint, click = pin an
element and drag = select text, simultaneously; the >4px drag arming
decides which one a gesture was, a completed drag's trailing click
never re-pins (one-shot dragEndedClick), and a plain click is never
swallowed (the pass only acts on a real selection and never
preventDefaults). Esc in Interact still closes an open drag draft
before yielding to the page.
- HTML/live surfaces are COMMENT-ONLY: useHtmlAnnotation clamps
redline/quickLabel (both the host mode and a bridge-posted
modeOverride, so a hostile page cannot force a DELETION), the
selection toolbar drops Delete and quick labels behind a new
commentOnly seam on AnnotationToolbar, and the quick-label picker
portal is gone from HtmlViewer. Markdown surfaces keep the full
toolbar, and persisted DELETION annotations still restore.
- The "Show tools"/"Hide tools" header button is removed. It hid the
floating toolstrip (now gone from HTML surfaces entirely: with
comment-only plus both input paths live there is nothing left to
switch), the collapsed sidebar tab flags, and the viewer's floating
action cluster (attachments + global comment + version-diff toggle),
all of which are now always visible. htmlChrome persistence keeps
only the sidebar/panel state; an old cookie's toolsHidden flag is
read tolerantly and ignored, so a stale record cannot strand a user
with hidden chrome and no way back.
- HTML surfaces pin the viewer input method to pinpoint (the drag/
pinpoint switch is meaningless when both are live); the Alt input
switch no-ops there. Vim stays armed-only, as built.
No server, proxy, or protocol-security changes; the armed flag stays
session-only.
Tests: the live-bridge harness is reworked around the armed default
(forged-DISARM posture, drag-selection passes in armed and Interact,
the trailing-click guard), the pinpoint suite covers the comment-only
toolbar and the redline/quickLabel clamp at the trust boundary, a new
AnnotationToolbar.commentOnly seam test guards both surfaces'
toolbars, App.htmlChrome.test.tsx replaces App.htmlHideTools.test.tsx
(no tools button, stale-cookie tolerance, pen armed default), and the
htmlChrome tests cover the narrowed persisted shape.
* feat(live-annotate): collapsible floating controls cluster
The simplification removed the Hide tools toggle, which left the floating
comment/attachments cluster permanently over the page. Restore a hide
affordance on the cluster itself: a collapse chevron shrinks it to a small
expand pill in the same corner, so the page is never obstructed without a
way back. Collapsed state persists with the rest of the HTML chrome cookie
(sidebar/panel), tolerantly read. Hosts that do not wire the toggle
(readOnly viewers, review-editor panels) are unchanged.
* feat(live-annotate): header Show/Hide tools replaces the collapse pill
The collapse pill was a half measure: it left its own artifact over the
page and the sidebar tongue tabs stayed. Revert it and restore the real
thing as a header control: an eye toggle immediately left of the pen that
removes ALL floating chrome over the page from the DOM (sidebar tongue
tabs + the comment/attachments cluster), leaving nothing behind. The
toggle lives in the header, so a hidden state always has a way back,
which also makes honoring a persisted (or pre-existing) toolsHidden
cookie safe again.
Choosing the annotation mode was mouse-only: the four toolstrip buttons
were the only way to move between Markup, Comment, Redline and Label.
Shift+digit rather than mnemonic letters, because most of the obvious
keys are already claimed: bare letters are swallowed by type-to-comment,
bare digits by the label picker, Mod+digit by the browser's tab
switching, and Mod+Shift+3/4 by the macOS screenshot shortcuts, which
the OS captures above the browser. Shift+1-4 is free on macOS, Windows
and Linux, and follows the toolstrip left to right.
The handlers write through the existing handleEditorModeChange, so the
mode persists and every consumer re-renders from one call. They share
the chrome-level typing and modal guard, and additionally gate on the
memo that decides whether the toolstrip renders at all — otherwise the
mode would change silently during plan diff or with the HTML tools
hidden, with no pill to report it.
Also adds aria-pressed to the toolstrip buttons, so a keyboard-driven
mode change is announced. The labels collapse by opacity rather than
display, so they were already carrying the accessible name.
Refs #1242
Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
* feat(review): collapse linguist-generated files by default (#1317)
Code review now respects linguist-generated (and linguist-generated=true)
from .gitattributes, collapsing those diffs by default the way GitHub does.
Server (Bun + Pi mirror): a generatedFiles sidecar rides /api/diff and
/api/diff/switch, resolved through git's own attribute machinery — one
batched 'git check-attr --stdin -z' over the served patch's paths at the
review cwd, so stacked and negated rules land exactly as git resolves
them. Plain local git sessions only; PR worktrees, workspace multi-repo,
jj, GitButler, and P4 omit the sidecar (degrade to no-collapse). Shared
logic in packages/shared/generated-files.ts, vendored to Pi.
Client: generated files SEED their CodeView item collapsed (the existing
Pierre collapse state — same mechanism as commit-diff folding), render
the one-line FileHeader bar with a 'generated' tag next to the +/- counts,
and expand per file on click. Expansion is session-local App state so it
survives remounts and diff switches. Presentation-only: the diff data,
annotations, search, and Edit Mode are untouched; the file tree and
single-file tabs list generated files normally (tag, no auto-collapse).
Guide viewer manifest pin regenerated (AllFilesCodeView/FileHeader are
bundled into the guides.show viewer) from a clean frozen-lockfile install.
* feat(review): built-in generated defaults, visible collapsed strip, review-round fixes (#1317)
Round 2 on PR #1346, per maintainer review.
Built-in generated defaults (industry-standard two-layer detection):
packages/shared/generated-files.ts (vendored to Pi) now carries
DEFAULT_GENERATED_PATTERNS — lockfiles (package-lock.json, yarn.lock,
bun.lock, Cargo.lock, go.sum, ...) plus *.min.js / *.min.css / *.map —
matched against the path's last segment only. Explicit .gitattributes wins
in BOTH directions: linguist-generated (set/true) marks any file,
-linguist-generated / =false un-marks even a built-in name, unspecified
falls through to the defaults. In plain local git sessions check-attr
refines the defaults; the non-git degrade modes (piped patches, PR
worktrees, workspace, jj, GitButler, P4) now emit the sidecar from the
name-based defaults alone instead of omitting it.
Visible collapsed state: a collapsed generated card no longer renders as a
bare header — a GeneratedFileNotice strip ('Generated file collapsed',
+N/-N, 'Click to view') styled like the other below-header notices sits in
the card, and clicking it expands through the SAME reportFileCollapsed
funnel as the chevron.
Review findings:
- F1: search-match and sidebar-comment navigation expanded items without
reporting through the funnel, so those expansions died on diff switch.
Both now call syncAllCollapsedMirror + reportFileCollapsed; the funnel
invariant comment lists the navigation-driven sites.
- F2: the check-attr call gets the same 5000ms timeout as review-core's
stdin git callers, and Pi's vcs.ts stdin write gets the one-line EPIPE
guard (call-flow.ts shape) a timeout kill makes reachable.
- F3: removed the dead prevGeneratedRef + collectSetDelta leg — a changed
generated set always remounts via fileSetKey, so the delta path was
unreachable.
Tests: default-list matching (glob + directory-named-bun.lock), both-
direction precedence, non-git sidecar from defaults (dual-runtime), the
placeholder strip through the funnel, and search expansion surviving a
re-seed round-trip. AGENTS.md payload docs updated. Guide viewer manifest
pin regenerated from this clean frozen-lockfile worktree.
* 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
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
Michael Ramos