* fix(review): stabilize JJ line-of-work bases
* fix(review): scope switch-handler context adoption to gitbutler and jj
Maintainer review follow-ups: keep plain-git sessions serving their
launch-frozen context on reload (the recomputed worktree context still
rides the switch response transiently), truncate frozen 40-hex commit
ids in the jj-line header label, and mark the two deliberate test pins.
---------
Co-authored-by: Michael Ramos <mdramos8@gmail.com>
Adds two session-only flags to plannotator review, parsed in the shared
parser so every host inherits them together:
- --base <ref> opens the session against a caller-chosen compare target
(branch, origin/<branch>, tag, SHA, HEAD~N), probed with git rev-parse
--verify --end-of-options before the server starts so a typo'd ref is a
startup error with near-match suggestions instead of a silently
mislabelled merge-base->HEAD diff.
- --diff-type <id> opens the session in one of the nine flat git diff
modes (REVIEW_OPEN_DIFF_TYPES, pinned against GIT_DIFF_TYPES).
The flags are a seed, never a setting: nothing writes config.json or any
review cookie, and the UI stays fully mutable. Validation is pure in
packages/shared/review-open-state.ts (provider matrix errors on
jj/GitButler/P4/workspace/PR mode, promote-with-notice when the saved
default is base-irrelevant, fatal explicit contradiction).
A flagged base rides explicitBase semantics: the new initialBaseExplicit
server option (both runtimes) seeds baseExplicitlyChosen, suppressing the
startup origin/* upgrade and canonicalization, and openStatePinned rides
/api/diff so the client neither offers the first-run setup dialog (its
one-time cookie is NOT consumed) nor runs the panel-pair self-heal for a
pinned session. The since-base dropdown label now renders from the live
active base, matching the adjacent base picker.
Coverage: Bun CLI, opencode-review bridge, OpenCode embedded plugin, and
the Pi extension (re-vendored; strict validation on the slash-command
path only, programmatic callers unchanged). Skills, command stubs, help
text, and docs updated across every host surface.
parseReviewArgs now reports argument-shape problems through an always-present
errors[] field instead of letting unknown dash-prefixed tokens fall into the
ignored positional list. All four host surfaces refuse to start a session on
a parse error: the CLI and the opencode-review bridge exit 1 with the errors
on stderr, and the Pi and OpenCode plugins notify through their hosts. Plain
non-dashed words stay tolerated for slash-command hosts that forward raw user
prose. The loop is index-based so value-taking flags can consume their value
token.
Gates /api/doc reads on realpath containment: a path must resolve inside the project both as written and after symlink resolution, else 403. Consolidates the duplicated doc-read logic from both server runtimes into one shared module (packages/shared/doc-resolve.ts, vendored to Pi).
Claude-Session: https://claude.ai/code/session_019GV6EKtzh8Pf9GA2rrBLNf
Closes#1456. Adds plannotator review --json emitting one { decision, message } record from the same builder as the plaintext output, routes Amp solely on the decision field, and removes the substring classifier that could silently drop feedback mentioning phrases like 'no feedback'. Invalid or legacy CLI output produces a recoverable update notice, never a guessed decision.
Claude-Session: https://claude.ai/code/session_019GV6EKtzh8Pf9GA2rrBLNf
- Anonymize the real project paths and ticket prefix in session-log
test fixtures (slug expectations updated to match).
- Untrack scripts/convert-themes.ts: a one-shot migration script
hardcoding a path into an unrelated private project; its output in
packages/ui/themes/ is already committed.
- Delete the three unreferenced sprite_package_*/index.html preview
pages, which also shipped in the @plannotator/ui npm tarball via the
wholesale directory entries in files.
* feat(review): add POST /api/code-nav/hover in both runtimes
Tier 0 of the token hover card: the hover pipeline is the ripgrep search
/resolve already runs, plus three cheap enrichments read off the same
machinery.
- packages/shared/code-nav.ts: definition patterns carry the kind they
prove (alternations split one-per-kind, so definition-vs-reference
classification is byte-identical and classifyMatch becomes a thin
wrapper over classifyMatchDetailed); scanDocComment, buildSignature and
resolveCodeNavHover; an additive timeoutMs option on resolveCodeNav so
hover can ask for 3s while /resolve keeps its 5s; an optional readFile
member on CodeNavRuntime so /resolve callers stay unchanged.
- Both review servers gain /api/code-nav/hover behind the same guard
stack as /resolve. /resolve itself is untouched.
The doc scan is conservative by construction: per-language, blank-line
separated, capped, and null for an unknown language. Returning nothing
always beats returning garbage.
* feat(review): token hover cards in the code-review diff
Resting the pointer on a symbol opens a card with where it is defined, an
approximate signature, its doc comment if the scan found a real one, and a
sample of its references. Every location on the card routes into the same
References panel Cmd+click opens.
- utils/stitchTokenIdentifier: rebuilds one identifier from the token spans
Shiki fragmented it into, using each span's data-char column to prove
adjacency. It stops at dots (rg searches with --word-regexp, where a
dotted path matches nothing) and refuses keywords and one-character
names, which is what keeps most hovers off the wire entirely.
- hooks/useTokenHover: 350ms dwell before any request exists, one in-flight
request aborted by its successor, a 30-entry LRU flushed whenever the diff
snapshot changes, a 250ms leave grace so the card's own links are
reachable, and a scroll/wheel cancel because the anchor rect is stale the
moment the pane moves. An unavailable backend, a failure, a timeout and a
thin answer all render nothing, silently: a hover is an idle gesture and
must never nag.
- components/TokenHoverCard: portaled to body so it escapes the Dockview
overflow and stacking context; anchored below the token, flipped above
when the viewport would clip it. It shows what the search found and
nothing it did not, so uncertainty is a second location line rather than
a description of the ranking.
- Wiring: two optional props beside onCodeNavRequest in both diff views,
passed only when the existing live-workspace gate AND the new cookie-only
"Token hover cards" setting are on. Off means no listeners, no requests
and no card in the tree. Alt+click joins Cmd+click as an unadvertised
alias into the References panel; the meta/ctrl branch is unchanged.
The guides.show viewer manifest moves with this: AllFilesCodeView is in the
portable viewer's graph, so its new optional props shift the bundle hash.
* fix(review): correct token hover supersession, scroll and doc-scan defects
Review findings, each with the regression test that fails without the fix.
- An open card could be rewritten by a NEIGHBOUR's answer: drifting onto an
adjacent token launched its request, and returning to the open card took
the same-key early return without reclaiming the active key, so the
neighbour's answer still passed the landing check. The early return now
reclaims the key, kills the pending dwell, and aborts a foreign request.
- Re-entering a token inside the leave grace re-armed the dwell while that
token's own request was still in flight, spawning a second ripgrep for an
answer already on its way. The dwell now joins the in-flight request, and
the answer anchors to the span the pointer is on now.
- Scrolling INSIDE the card closed it, which made the signature block's
horizontal scroller unreadable by the gesture meant to read it. The
cancel now ignores events originating in the card's own subtree; a pane
scroll still closes it.
- A below-threshold answer for a different token left the previous token's
card standing over a symbol the reviewer had already left.
- The doc scan rendered tooling directives as documentation. Directives are
dropped from BOTH ENDS of the comment run — eslint-disable, @ts-*,
prettier-ignore, biome-ignore, istanbul ignore, noqa, type: ignore and
triple-slash references — because the commonest real position is the line
immediately above the definition, which is the trailing end of the run as
collected. Never from the middle: a directive surrounded by prose sits
inside documentation we would have to interpret to cut safely. A run that
is nothing but directives returns null, and prose that merely mentions a
directive is untouched.
Also: an answer whose token has been recycled out of the DOM opens no card
(a detached rect is 0,0 and would pin it to the viewport corner); a flipped
card is clamped to the top edge; the card is a tooltip, not a dialog; a
location click describes the CLICKED location rather than forwarding the
hover's charStart and language into another file; definition.preview stays
declared but unpopulated until a consumer exists; the overflow line regains
its leading ellipsis and now renders under the banned-vocabulary sweep.
Portable viewer: the hover prop is inverted to (props, filePath) so the two
diff views import nothing new and stitching lives in App. The read-only
guide viewer bundle no longer carries the stitcher or the request builder.
Applies the PR5 stage-review rulings:
M0: approve-time feedback is no longer appended raw after the approved
prompt ("no changes requested" beside a change-request-shaped export read as
a contradiction). composeReviewApprovedMessage now resolves the new
DEFAULT_REVIEW_APPROVED_WITH_NOTES_PROMPT (configurable as
prompts.review.approvedWithNotes; field added to the config review section),
which frames the notes as non-blocking guidance and says not to revise or
reopen. Signature is now (runtime, feedback, config) so one function fixes
all four consumers; re-vendored to Pi; prompts.test.ts pins the framing,
the config override, the byte-identical bare approval, and the legacy
placeholder filter; cli-bridge.test.ts asserts the bridge routes through
the composer.
M1: fail-closed approval-notes handshake for the OpenCode CLI bridge. The
plugin declares supportsApprovalNotes: true on the opencode-review stdin
JSON; the binary adverts approvalNotesSupported for opencode ONLY when the
declaration is present, so a new binary + old plugin (advert in the binary,
delivery in the independently-versioned plugin) renders no approve-carrying
items instead of silently dropping the reviewer's note.
supportsReviewApprovalNotes stays the seam; documented at both ends beside
the existing version-skew reasoning. Pinned end to end by
apps/hook/server/opencode-review-advert.test.ts, which spawns the real
entrypoint and reads /api/diff: stdin without the declaration serves false,
with it true.
m1: applyPRResponse re-applies the advert from the PR-family payloads
(pr-diff-scope, pr-switch, load-full-diff), so the client stays in lockstep
with whatever diff payload it last applied — the "whole diff family" comment
is now literally true.
m2: buildReviewApprovalBody folds a note in ahead of the export when
annotations also ride, so a future combined item cannot lose data; pinned
with a pure assertion.
i1: AGENTS.md corrected — the standalone dev server emits raw decision JSON
with unfiltered feedback and does not route through the composer; the
consumer list, framing, and handshake are now described accurately.
Claude-Session: https://claude.ai/code/session_01Drrzd1x4EfnH9N3z7nNwo9
The four waitForDecision consumers that threw result.feedback away on the
approved branch (spec §6.3) now emit composeReviewApprovedMessage — the
approved prompt, then the note when one rides the decision:
1. Claude Code CLI `plannotator review` (apps/hook/server/index.ts); the
amp/droid plugins relay its stdout and inherit the delivery.
2. OpenCode native (apps/opencode-plugin/commands.ts) — also fixes the
delivery gate: it rode on the LGTM placeholder making feedback truthy, so
with the placeholder gone a bare approval would have been silently dropped;
the gate is now `feedback || approved`.
3. OpenCode CLI bridge (buildReviewPromptFromBridgeOutcome) — the CLI's JSON
record always carried the feedback; the bridge stops discarding it.
4. Pi (apps/pi-extension/index.ts) via the vendored prompts module.
Each consumer's startReviewServer call now passes the matching advert:
supportsReviewApprovalNotes(origin) for the hook CLI (new seam in
apps/hook/server/review-output.ts — every origin shares the one stdout
relay today), Boolean(sessionId) for OpenCode native (no session, no
delivery — the annotate precedent), unconditional true for Pi and the
standalone dev server (which already emitted feedback on approve).
Claude-Session: https://claude.ai/code/session_01Drrzd1x4EfnH9N3z7nNwo9
Both runtimes gain an `approvalNotesSupported` option on the review server
(default false) echoed on every diff payload — /api/diff, /api/diff/switch,
/api/pr-diff-scope, /api/pr-switch — so the advert survives a diff switch
(spec §6.4). An old caller that never passes the option advertises false and
the client keeps rendering no approve-carrying items.
packages/shared/prompts.ts gains composeReviewApprovedMessage (vendored to Pi
with the rest of prompts.ts): the one composer every review decision consumer
emits approvals through — the approved prompt, then the approve-time feedback
when the decision carries any. It filters the legacy
'LGTM - no changes requested.' placeholder so a new consumer reading an old
built client's approval never appends filler the reviewer did not write.
Dual-runtime tests: advert default false / true-and-survives-switch on both
servers; approve-time feedback reaches waitForDecision byte-identical and
archives as approved-with-notes; a bare approval (post-placeholder shape)
archives as lgtm with no sidecar — the decision the placeholder made
unreachable (spec §6.2 fact 1).
Claude-Session: https://claude.ai/code/session_01Drrzd1x4EfnH9N3z7nNwo9
* feat(server): archive every submitted review to a durable local feedback store
Submitted feedback was only as durable as the agent session that asked for
it. Code review persisted nothing at all: /api/feedback deleted the draft,
settled the decision promise, and if the invoking agent had already timed
out the review existed nowhere (the failure #678 fixed for annotate). Plan
decisions only reached plans/ while the client-side planSave setting was on,
and repeat decisions on one plan overwrote each other. Annotate kept the
#678 record for single local files only.
Every submission now appends one record to
${PLANNOTATOR_DATA_DIR}/feedback/{project}/index.jsonl, plus a
records/{stamp}-{surface}-{decision}.md sidecar when it carries content,
written at decision settlement time inside the servers so all nine agent
frontends are covered by two implementations.
Surfaces wired in both runtimes: plan approve and deny, code review
/api/feedback (Send Feedback, Approve, LGTM) and /api/exit, annotate submit,
approve and exit. Bare approvals, LGTMs and dismissals are decision-only
JSONL lines with no sidecar.
Records are cheap by design. Code review carries diff identity (vcsType,
diffType, base, gitRef, snapshotId, cwd, PR metadata, changed-file count,
patch byte count) and never the patch bytes; plan records carry the decision
text plus a reference to the history/{project}/{slug}/NNN.md version the
decision was made on rather than a second copy of the plan. Annotation
provenance (source, author) is preserved, so external, review-agent and
WebMCP findings stay tagged and source == null selects the reviewer's own
comments.
The shared module never throws: an archive failure is logged, degrades
silently for the user, and keeps the annotation draft as the recovery copy.
The append happens before deleteDraft, generalizing the #678 ordering.
Controlled by PLANNOTATOR_FEEDBACK_HISTORY / feedbackHistory (default on).
PLANNOTATOR_ANNOTATE_HISTORY=0 additionally suppresses records for every
annotate surface, so the documented stateless-annotate promise still holds.
"feedback" is added to PURGE_OWNED_TOP_LEVEL so uninstall purge removes it.
AI-assisted (Claude) under maintainer direction.
* fix(server): stop the feedback archive from writing into the real data dir in tests
Review findings on the durable feedback archive.
1. The archive is default-on, and most server tests boot a real plan, review,
or annotate server without redirecting PLANNOTATOR_DATA_DIR, so `bun test`
deposited records in the contributor's own ~/.plannotator/feedback (24 files
across 12 buckets from two test files alone) on CI and every machine. A new
bunfig test preload, tests/setup/feedback-archive-off.ts, turns the archive
off for the suite; the archive's own tests opt back in inside their test
bodies, which is also how they exercise the opt-out. Those tests now use
distinctive project names and remove the annotate history they leave in the
real data dir, since storage.ts fixes its data directory at import time.
2. PR reviews bucketed under feedback/pr-<n>/. PR mode never sets gitContext
and --local points agentCwd at a pool/pr-<n> checkout, so deriving the
project from the review cwd was wrong. ReviewServerOptions now takes a
`project` option, mirroring the annotate server, preferred over the cwd
derivation on both runtimes; the Claude Code, OpenCode, and Pi entry points
pass their already-computed detectProjectName() result.
3. changedFiles overcounted renames: extractChangedFiles unions the a/ and b/
sides so a reader can resolve either path. The record now counts b-side
paths through countChangedFiles, so a rename is one file.
4. Docs: the feedback archive is added to the privacy page and
PLANNOTATOR_FEEDBACK_HISTORY (plus PLANNOTATOR_ANNOTATE_HISTORY) to the
environment variables reference. The overclaim that every submitted review
is archived is corrected: a review posted straight to GitHub or GitLab
through /api/pr-action is not archived locally yet. Three behaviors are now
written down: O_APPEND is not atomic on NFS or SMB and a genuine interleave
damages both records that raced, folder-session records carry the folder
path rather than the open document, and URL-session records store the full
URL including its query string.
5. Pi parity: the Node mirror now has the failed-archive-write test (the one
invariant its handler copies by hand) and the PR-mode bucketing test.
Comments only, no behavior change: the pool checkout recorded in
target.review.cwd can be cleaned up before anyone reads the record, and
getPlanVersionPath resolves the data directory storage.ts captured at import
while the archive resolves it per call.
AI-assisted (Claude) under maintainer direction.
* docs(server): make the feedback index an explicit multi-client contract
plannotator-tui will append to the same feedback/{project}/index.jsonl with
client "plannotator-tui", so the module's stance of "a client tool may emit
this shape under its own clients/ namespace" is out of date. The index is one
shared source of records, labeled by client.
1. The module docstring and the FEEDBACK_RECORD_CLIENT comment now describe the
shared index: several tools append to the same file, separated by `client`;
plannotator-tui is a known second writer, herdr-annotate is reserved, and
`client` is an open set rather than an enum to validate against.
2. Two optional fields are declared so v1 reserves their names across clients:
target.agent ({ host, session, transcript }) for surfaces whose subject is
an agent session rather than a file or a diff, and top-level clientVersion.
Neither is populated here. clientVersion stays unset deliberately: there is
no runtime-agnostic version constant in packages/shared, and reading
package.json from a vendored module would be a new filesystem dependency
for cosmetic data.
3. Sidecar naming is documented at the naming site and in AGENTS.md: other
clients suffix their id ({stamp}-{surface}-{decision}-plannotator-tui.md),
so recordFile values carrying such suffixes are valid and nothing may parse
a sidecar name. Nothing in this repo did: every consumer treats recordFile
as an opaque handle and no test pins a filename pattern. A new test appends
a foreign line (unknown client, unknown fields, suffixed recordFile) and
pins that the reader keeps it.
4. Honesty fix to the atomicity comments, in code and in AGENTS.md:
appendFileSync loops internally, so "one write syscall" was wrong even on a
local filesystem. The real model is that a line-sized buffer handed to a
single append-mode write completes without interleaving in practice
locally, with the reader's skip-unparsable tolerance as the backstop and
the NFS/SMB caveat unchanged.
5. Exhausting the sidecar collision counter now throws a named error instead of
re-throwing a bare EEXIST, so the server log says what actually happened:
100 taken names in one millisecond means a stopped clock or a runaway
writer, not a transient disk problem.
6. AGENTS.md and the parseFeedbackIndex doc state the reader contract: lines
are gated on a numeric `v` and unparsable ones are skipped, so analyzers
that depend on v1 semantics should filter v <= 1 themselves. Fields are
added, never repurposed, so a v2 would mean a real shape change.
AI-assisted (Claude) under maintainer direction.
* fix(ai): stop leaking opencode serve processes
Every server boot with the opencode CLI on PATH eagerly called the
provider's fetchModels() to fill the Ask AI dropdown, which spawned (or
attached to) an 'opencode serve' on the shared default port 4096. Dispose
only ran on the clean decision path, so Ctrl-C orphaned the child, and
every later session attached to the orphan and piled unevictable
per-directory instances into it (multi-GB over a day of normal use).
Three changes:
- Lazy start: opencode model discovery moves onto the same deferred
provider initializer Codex uses. Nothing spawns until the user
activates opencode in Ask AI (?activate= from the model picker, or the
first opencode session). The picker still lists the provider with an
empty model list pre-activation, exactly like Codex.
- Own server per process: spawn with port 0 (OS-assigned; the SDK reads
the real URL from the child's listening line) and never attach to a
server we did not spawn. An explicitly configured port is honored.
- Exit cleanup: a process 'exit' handler closes the spawned server
(SIGINT/SIGTERM are routed through process.exit by the CLI), removed
again on dispose. No SIGHUP listener, preserving nohup.
Both runtimes; regression tests mock the SDK so no real server spawns.
* fix(ai): close review findings on the opencode lifecycle
Independent review of the leak fix found two holes, both now closed and
regression-tested against the mocked SDK:
- A failure after the spawn (client construction) left the child running
and its exit handler registered, and because the handler read
this.server late instead of capturing its own server, a retry's second
spawn made the first unreachable by any cleanup. doStart now captures
the server in its handler closure and reaps child + handler on any
post-spawn failure.
- dispose() during an in-flight spawn was a no-op the completing spawn
then undid, resurrecting a disposed provider with a live child and a
fresh exit handler. dispose() now bumps a start epoch; a spawn that
completes past its epoch reaps its own server and rejects, and the
provider remains restartable afterwards.
Also documents the OpenCode transport (per-process server, deferred
discovery) beside the Codex note in AGENTS.md.
The e2e case from #1444 spawns the real CLI, whose module load imports
../dist/index.html and ../dist/review.html. Dev machines always have
those built, but CI's test job does not build the apps, so the spawn
died at import time with 'Cannot find module' before the guard ran and
the assertion on the unknown-command message failed, breaking main.
The test now drops empty placeholders for any missing artifact before
spawning and removes exactly what it created afterwards; a real dist is
never touched (verified with sentinel content). The guard under test
exits before either page is served, so placeholder content is
irrelevant.
A typo'd subcommand fell through the dispatcher to the plan-hook path,
which reads the hook payload from stdin. From a terminal that stdin never
closes, so `plannotator annotatte README.md` hung with no output instead
of reporting the typo.
Reject an unrecognized first token before the dispatcher runs: print the
offending word, the nearest documented command, and a pointer to --help,
then exit 1. Flags and the no-argument hook invocation are left alone so
the real hook path is untouched.
* fix(opencode): show the session URL on OpenCode 2's native command path
On OpenCode 2 a remote session's URL was invisible. runNativeCommand builds
its bridge client with createV2BridgeClient, which deliberately has no tui
domain, so toastPlannotatorUrl optional-chained to a no-op; both URL delivery
paths (the CLI stderr forwarder and the ready-file poller) route through it.
The V2 client's app.log is console.error, and OpenCode discards a server
plugin's stderr under both default launch modes (packages/cli/src/services/
standalone.ts uses stderr: "ignore" unless OPENCODE_PRINT_LOGS=1). Remote mode
also suppresses the browser, so /plannotator-review showed the user nothing at
all and presented as a hang.
Deliver the URL as a visible transcript notice instead. createSessionUrlNotifier
duck-types ctx.session.synthetic and exposes it to cli-bridge as notifyUrl, a
seam toastPlannotatorUrl prefers over the toast when present; OpenCode 1 clients
carry no notifyUrl and keep their real toast unchanged. The notice is posted
with resume: false, which upstream skips the wake for, so nothing starts a model
turn, and it carries the URL in both text and description because the TUI drops
a synthetic row whose description is empty and renders the description rather
than the text. Everything is guarded: a host without session.synthetic, or a
call with no session, gets no notifier and falls back to today's log-only
behavior, and a rejecting synthetic is caught and leaves the URL retryable by
the other delivery path.
The README's remedy line claimed remote sessions should read the URL from the
OpenCode log, which was never true; it now describes the transcript notice and
names OPENCODE_PRINT_LOGS=1 for older hosts.
Also fixes two bugs in the OpenCode 2 native-command smoke:
- scripts/opencode2-native-commands-smoke.sh looked for a node_modules/.bin/
opencode binary. @opencode-ai/cli publishes opencode2 on every dist-tag, so
the script failed before it started a server. It now tries both names and
reports which it looked for.
- The command-ownership check read /api/command once, immediately after
activation, racing the reclaim schedule whose last tick lands about 15.5s
later. Under PLANNOTATOR_SMOKE_EXPECT_NATIVE=1 that reported a shadowing bug
the reclaim had simply not reached yet. It now polls to a 30s deadline
(PLANNOTATOR_SMOKE_COMMAND_TIMEOUT_MS), still only after /api/plugin reports
the plugin loaded.
AI-assisted (Claude) under maintainer direction.
* fix(opencode): deliver the session URL on OpenCode 2's plan review path too
The first commit fixed only the native command path. The plan path builds its
own client (createV2Client, typed as { app: { agents, log } } with no notifier),
so a remote OpenCode 2 user who reached a review through submit_plan still never
saw the URL: no browser is opened for them and the plugin's console output is
discarded by the host.
The plan path now builds the same bridge client the command path uses, with
toolContext.sessionID, so it carries notifyUrl whenever the host exposes
session.synthetic. That covers both runtimes: the CLI runtime already prefers
notifyUrl inside toastPlannotatorUrl, and the embedded runtime's previously
empty logReady hook is now createPlanReadyNotifier.
That hook still does not log. app.log is console.error, the same stderr
handleServerReady already printed the URL to, so logging there would duplicate
the line in remote mode and add a stray one locally, which is why the hook was
empty. The transcript notice is a different surface, and it is the only one a
remote reviewer can see. Without session.synthetic the hook stays silent exactly
as before.
createV2Client is gone: it duplicated the bridge client's URL-deduped app.log
verbatim, and nothing else used it.
Three tests on the plan path (delivers the notice; stays silent and does not
re-log without synthetic; catches a rejecting notice) plus one that pins the two
wiring seams at source level, since the notifier tests all pass while the plan
path is wired to nothing, which is the shape the bug had.
Also from review: console.error is stubbed across the V2 URL delivery block, so
those tests no longer print URL lines into the suite output. The README bullet
now says the notice covers every way a session opens rather than slash commands
alone.
AI-assisted (Claude) under maintainer direction.
* feat(opencode): restore the slash commands on OpenCode 2
OpenCode's V2 plugin API gained native command execution upstream
(anomalyco/opencode issue #2185, PR #44765): ctx.command.transform lets a
plugin add a command whose execute callback fully owns the invocation. That
shape currently ships on the beta and dev dist-tags of @opencode-ai/plugin
while next and latest still carry the older context, so the capability is
duck-typed at runtime and never imported. On a host that exposes it the V2
adapter registers /plannotator-review, /plannotator-annotate and
/plannotator-last and runs the same handleCliCommand machinery OpenCode 1
uses, passing the raw argument tail straight through to the CLI. On a host
without it nothing new is registered and behavior is byte-identical to before.
Also wires ctx.session.switchAgent (same API generation, same probe) so an
agent switch chosen in the review UI is applied instead of only warned about,
and accepts both agent.list() response shapes: the HTTP client types it as a
{ location, data } envelope while the in-process plugin domain answers with a
bare array, where reading .data threw and silently emptied the agent list.
The shared command stubs get model-mediated fallback bodies for OpenCode 2
hosts on the stale channels. They carry no shell interpolation on purpose:
OpenCode 1 evaluates a template's !`...` before the V1 plugin's
command.execute.before hook can clear the parts, so a bang template there
would launch a second Plannotator session on every OC1 invocation. A source
level test pins that.
AI-assisted (Claude) under maintainer direction.
* fix(opencode): probe the command draft and reclaim the names from the stubs
Review found the capability probe was wrong in the direction that matters.
ctx.command.transform exists on pre-#44765 hosts too: our own pinned
@opencode-ai/plugin@0.0.0-next-16775 declares CommandDraft as
{ list, get, update, remove } with no add. The probe therefore returned true on
next and latest, draft.add was undefined, and because transforms are stored and
replayed the TypeError landed in the batched reload flush and aborted it before
commit, plausibly taking every command registration on the host down with it.
Capability is now read from the draft handed to the callback, which is the only
witness, and the registration call is wrapped so no transform rejection can fail
plugin setup.
The stubs also shadowed the native definitions on new hosts. Command definitions
land in a name-keyed map where add is Map.set, transforms replay in registration
order, and OpenCode's own ConfigCommandPlugin activates in the post group after
package plugins while scanning the exact directory the installer writes the
three stubs to. A setup-time registration is therefore always overwritten on a
normal install. The plugin now re-registers the same transform once activation
settles, so its definitions are last in the replay order, and calls
ctx.command.reload() explicitly because a late registration only adds its reload
to the already-flushed boot batch. Ownership is read back from
ctx.command.list() by description, which is why the native descriptions and the
stub frontmatter are deliberately distinct. If the reclaim cannot run the stubs
keep the names and the commands still work through their fallback bodies.
Also: a failing switchAgent no longer costs the reviewer their feedback on the
command path, feedback is delivered as "queue" rather than replaying the
invocation's admission mode minutes later when a steer would land mid-turn, and
the agent-list comment no longer asserts a bare-array response that could not be
reproduced upstream (accepting both shapes is still right, since reading .data
blindly throws into a catch that degrades silently).
Tests: the real old-host draft shape registers nothing and throws nothing, the
shadowing contest is modelled against upstream's replay semantics, the OpenCode 1
parts-clearing invariant is pinned for all three commands in both plan-agent and
manual mode now that the stubs carry real instructions, and the V2 smoke asserts
the plugin did not activate as failed and that all three commands resolve. The
smoke now also installs the stubs into its sandbox config dir so the contest
actually happens there. scripts/opencode2-native-commands-smoke.sh runs the same
smoke against a dev-channel build with native commands required; CI cannot,
because it pins a next build.
AI-assisted (Claude) under maintainer direction.
* fix(opencode): keep the reclaim ticking and stop an unbuilt checkout failing setup
The reclaim ended the loop when the draft-probe flag read false, but that flag
only flips when the transform replays, which under boot batching is the flush
after every plugin has loaded. Plannotator loads before the post-group config
plugins, so the first tick legitimately reads false and the loop exited for
good: the reclaim was inert in exactly the shape production has. The tick is
skipped now instead, with a test that flips the flag between ticks.
The V1 entry called resolveBundledHtmlPath synchronously during plugin
construction, outside the .catch that was there to absorb a missing asset, so an
unbuilt checkout threw out of construction before any code path that needs the
HTML. The Test workflow runs bun test with no build step, so the new OpenCode 1
interception tests failed there. Both preloads are guarded; the lazy getters
still raise a clear error if something actually needs the file.
The smoke's failed-plugin guard read entry.state.status, but Plugin.Info carries
status and error at the top level, so a failed activation slipped through.
Reads the top level first and keeps the nested one as a fallback.
Comment corrections: State.batch clears its active flag before flushing, so a
late transform registration materializes on its own; the explicit reload() is
redundant-but-defensive rather than required. The reclaim schedule is a list of
deltas the loop awaits in turn, so the ticks land near 0.3s, 1.5s, 5.5s and
15.5s, not at the raw numbers.
AI-assisted (Claude) under maintainer direction.
* feat(review): mark files viewed as you scroll past them
Reviewers reading the all-files diff top to bottom had to check every file
off by hand. Now a file marks itself viewed when the reviewer MOVES ON from
it, after its content was actually on screen long enough to have been read.
Arriving at a file never marks it; leaving it downward does.
- All-files surface: a file marks when the reader scrolls past it (its
successor has reached the viewport top, so it genuinely scrolled out above)
and has accumulated at least 1000ms as the reported reading file. Dwell is
cumulative per diff snapshot, so bouncing between two files still accrues,
while a momentum flick to the bottom marks nothing. The last file, which can
never scroll out above, marks on reaching the end of the diff.
- Single-file panel: opening a file never marks it; navigating away after the
same dwell floor does. Keyboard file navigation drives the same panel
switches, so keyboard-only parity is automatic.
- Collapsed cards never mark. Generated files seed collapsed, so nobody
reviews a lockfile by scrolling past its folded header.
- Un-viewing a file suppresses auto-view for it until it is marked viewed by
hand again. That set rides the review draft as an additive optional field.
- Inert inside the Guided Review takeover and on a commit detour, where the
files on screen are not the change under review.
- A viewed file whose patch changes under a refresh loses its checkmark, but
only while auto-view is on, so the off state stays byte-identical to today.
- PR sessions batch the marks into one /api/pr-viewed request rather than one
per file.
The setting is reviewAutoViewed, cookie-only and on by default, with two off
switches: Settings > Git and a row in the file-list gear popover. The first
time auto-view actually fires, a toast says so and offers Turn off; using
either switch consumes that one-time notice.
The decision core is pure and clock-injected (utils/autoViewed.ts), the
binding is a hook (hooks/useAutoViewed.ts), and AllFilesCodeView only gains
one optional emission callback on the rAF path it already runs. No server
changes in either runtime.
AI-assisted (Claude) under maintainer direction.
* fix(review): scope auto-mark-viewed to the transitions it was meant for
Four review findings on the auto-mark-viewed branch.
Rule 5 fired on EVERY applied diff switch, not just the staleness refresh.
The review app funnels every transition through one apply path, so entering
the Commits detour (the rail auto-opens HEAD), switching base branch, and
toggling hide-whitespace all un-viewed files whose per-path patch text
legitimately differs, which contradicts both Rule 4's "a commit detour is
inert" and Rule 5's own rationale. The apply path now goes through
resolveDiffSwitchUnviews, which requires the caller to opt in
(`contentRefresh`) and re-checks the identity of the diff on top of that:
same selection, same base, and never a commit-family type on either side.
Only the staleness refresh and the post-fetch base refresh opt in. The pure
delta resolver is unchanged. A source-level test pins which call sites may
opt in, since that is where the guarantee actually lives.
The at-bottom branch fired on the mount tick. A diff shorter than the
viewport is at-bottom from the very first report, and that report is the
mount seed, so the file on screen marked itself about a second later with
zero interaction and fired the first-time toast at a motionless page. It now
requires a real scroll event on the current file set.
Staging a file marked it viewed without clearing auto-view suppression,
unlike v, the header button and the tree row, so a file the reviewer
un-viewed and later staged stayed permanently off-limits to auto-view.
Dwell accrued while the setting was off, so enabling mid-read could mark the
current file instantly on time the reviewer spent with the feature
deliberately disabled. Disabled is now fully inert: the clock does not
accrue, and enabling starts a fresh one rather than replaying the gap.
AI-assisted (Claude) under maintainer direction.
* chore: refresh pinned guide viewer manifest after merging main
README gains a Herdr Annotate section after Annotate HTML Artifacts: the
banner SVG and a TUI screenshot side by side, install one-liner, standalone
Plannotator TUI pointer, and a header link row entry anchoring to it.
Landing page: slim announce bar under the nav linking to the plugin repo, a
"watch the demo" strip above the capabilities section linking to the X demo
post, and the Workspaces waitlist pill redrawn as a blueprint chip (plan-grid
fill, corner registration marks) replacing the rounded dot pill.
Also fixes the hero shimmer under Firefox forced colors: Firefox drops the
author gradient but leaves color: transparent standing, so the shimmer word
and the command list rendered invisible when "Override the colors specified
by the page: Always" (or OS high contrast) was active. The gradient is now
declared once for both call sites behind @supports, forced-colors mode gets
CanvasText with the animation stopped, and prefers-reduced-motion freezes
the shimmer at a legible mid-palette slice.
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.
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.
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 context filter stripped delivered framing from mid-history at phase
transitions, shifting every later message and invalidating the provider's
cached prefix (88 of 119 messages re-billed in the reporter's session).
History is now append-only: delivered framing stays, and stale instructions
are neutralized by superseding language in the phase templates plus the
existing plan-mode-off countermand.
Fixes#1380
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.
Corrects staleness that landed with the live local app annotation work
(#1352) and the configurable Agent TUI placement (#1050).
- AGENTS.md "Session shape" no longer claims drag selection is disabled
in live mode. Drag-select commenting is always live on HTML and live
surfaces, in both the armed and Interact states.
- Adds the shared HTML/live interaction model to the canonical
"## Annotation System" section: pinpoint armed by default, the Esc
ladder, pen and Mod+Shift+A re-arm, comment-only clamping at the
postMessage trust boundary, no toolstrip, and the header eye toggle.
- Documents the agentTerminalSide and agentTerminalDefaultAgent
config-only settings, including registry precedence.
- Corrects the plan-review shortcut scope list to its actual 12 files.
- Marketing docs: documents live local app annotation on the annotate
command page and the remote-mode refusal in the env var reference.
- Fixes a stale code comment in App.tsx that contradicted the code
restoring the toolsHidden flag 17 lines below it.
* 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.
#1357 made the panel defer to the app's stored theme mode and seeded
System only when no mode was stored. That helped first-time panels and
nobody else: every panel opened before it already stored `dark`, written
by ThemeProvider on its first mount rather than chosen by anyone, so the
seed never fired and the panel stayed dark in a light IDE. That is issue
#1053 exactly, still broken for the users who reported it.
There is no provenance in the store to read: it is one flat cookie string
in globalState with no timestamps and no per-cookie metadata, and the app
writes the same `plannotator-theme=dark` whether the user picked Dark or
never opened the theme settings. The migration leans on the three signals
that do exist. `light` and `system` are values the auto-seed cannot
produce, so they are choices and are never touched. A new
`plannotator-vscode-seed` marker, written on every load, makes the
re-seed run at most once per store, so a Dark picked afterwards is
permanent. And a mode the user actually picked is recorded server-side in
~/.plannotator/config.json by configStore.set, which configStore.init
applies over the cookie and writes back, so a real choice outranks the
seed and re-asserts itself in the same page load.
What remains is a Dark that exists only as a cookie with nothing in
config.json behind it. That is indistinguishable from the auto-seed and
is reset once: invisible in a dark IDE, and in a light IDE one re-pick
makes it stick for good.
Also fixes the type error #1357 shipped in applyPanelCookieDefaults and
adds the extension's own tsc to CI, which had never run there.
Co-authored-by: Michael Ramos <backnotprop@gmail.com>
* 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>
The capability-absent warning told every host to update Pi, but forks
that never implemented ctx.isProjectTrusted (oh-my-pi) also hit this
path, and update Pi is wrong advice there. Neither Pi's nor oh-my-pi's
extension context exposes a host name or version, so the two audiences
cannot be reliably told apart at runtime. The warning now states the
capability gap without guessing the host, and says what still works:
bundled and global config load regardless (only project-local config is
trust-gated in loadPlannotatorConfig).
Fail-closed behavior is unchanged: capability absent still skips
.pi/plannotator.json, a host-provided true is still honored verbatim
(the oh-my-pi shim in can1357/oh-my-pi#7958 will work unmodified), and
a throwing trustFn still propagates. Tests pin all four paths.
Reported by @materemias in #1353.
The theme bridge wrote VS Code's colors as inline custom properties on
<html>, the same element ThemeProvider stamps `theme-<palette>` and
`light` on, and it forced the `light` class to the IDE's theme kind. An
inline property outranks every `.theme-*` rule, so picking Light in a
dark IDE produced dark VS Code tokens sitting under a `.light` class,
and any palette chosen in Plannotator's settings was painted over.
The bridge now reconciles instead of applying once on arrival: VS Code
colors are only painted while the user is on the default palette and the
app is already rendering the IDE's light/dark side, anything it painted
is removed the moment that stops holding, and it no longer writes the
`light` class except to map System onto the IDE's theme kind.
Panels that have never stored a mode seed System, so a first-time user
in a light IDE still gets a light panel now that the bridge does not
force the mode.
Reported by @it-sha.
Graeme Folk