Add external presenter support - #1147
Open
backnotprop wants to merge 5 commits into
Open
Conversation
Conflict resolutions: - packages/server/annotate.ts: carried main's #1143 client-lease cleanup (clientLease.cancel() + closeSessions()) into the branch's memoized async stop closure, ahead of server.stop, keeping the presenter dismiss after server stop. A HEAD-favoring resolve would have regressed the abandoned-gate dismissal. - apps/pi-extension/index.ts: kept main's sessionAlive latch (383ac97) assigned synchronously before the branch's awaited stopActivePlannotatorBrowserSessions(), so the stale-ctx guard is not delayed by the presenter dismiss budget. - apps/pi-extension/plannotator-browser.ts: composed main's .catch + console.error on the fire-and-forget browser open with the branch's presenter presentation flow. - apps/pi-extension/vendor.sh: union of both module lists (main's annotate-client-lease + annotate-decision, branch's presenter). Claude-Session: https://claude.ai/code/session_01H5KQWqXqjrPxyxUNso1QHS
Re-review follow-ups on the external presenter feature: - annotate server keeps the force-stop, with a lease-stream flush first. Non-force was considered per review guidance, but the server runs with idleTimeout 0 and a connected UI holds heartbeat SSE streams (external annotations), so a graceful stop pends until the client disconnects while presenter hosts only close the client pane after the dismiss that runs once stop resolves. Bun also ignores stop(true) after a graceful stop has begun, ruling out graceful-then-force. A brief flush after clientLease.closeSessions() lets lease clients observe a clean end-of-stream instead of ECONNRESET (annotate.test.ts lease test). - strict annotate gates publish the stdout decision record (and result file) BEFORE server cleanup, and a rejected cleanup routes to the documented environment-failure exit 2. Previously a stopServer() rejection exited with no record at all, which --require-approval callers would misread as "the reviewer did not approve". - the shutdown coordinator's cleanup budget is now PRESENTER_DISMISS_TIMEOUT_MS + 2s so a timing-out dismiss resolves inside the budget instead of racing the force-exit, and presenter children are tracked and SIGKILLed synchronously on process exit — the unref'd SIGTERM->SIGKILL escalation (now 2s) could never fire before the CLI exited, leaving hung presenters orphaned. - pi's session_shutdown catches (logs, never rethrows) failures from stopActivePlannotatorBrowserSessions(), which can reject with an AggregateError — same bug class as 383ac97. Claude-Session: https://claude.ai/code/session_01H5KQWqXqjrPxyxUNso1QHS
Adds the env var + config.json key to the repo env-var table (AGENTS.md, the CLAUDE.md symlink source) and the marketing environment-variables reference, plus a config-file note in getting-started/configuration. Documents that PLANNOTATOR_SKIP_BROWSER_OPEN=1 wins over the presenter. Claude-Session: https://claude.ai/code/session_01H5KQWqXqjrPxyxUNso1QHS
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What changed
Why
Terminal environments such as Herdr already have their own browser surface. This lets them display Plannotator's existing page there without moving approvals, feedback, or agent behavior out of Plannotator.
The presenter is a small executable configured in
config.json. Plannotator sends it one JSON request to present the URL and another to dismiss the returned handle. Plannotator does not depend on Herdr.Verification
Companion plugin: https://github.com/plannotator/herdr-plannotator
Known residuals
sessionAliveplus the browser-session start gate); restructuring it is out of scope here and the stale-ctx guard from Pi 0.82.1 stale-ctx guard crashes the extension after plan approval in print mode #1140 covers the teardown paths it does not.presentaborted mid-flight can leave its child running for up to the present timeout window (roughly 5-15s) before termination escalates; the exit-time SIGKILL bounds this to the lifetime of the CLI process and it is accepted for now.