fix(mod): restart a dead Ask-this-session bridge; one watcher per launch across processes - #1727
Conversation
…nch across processes Bridge restart: the pull-bridge loop gave up after six failed polls (~3 minutes of a paused server or a sleeping laptop, since $.http.fetch gives up after 30 s) and nothing started another, so Ask AI said the session was gone for good. run() now reports why it ended; on failures the controller clears the handle and starts a new loop with the same token after 5 s, doubling to 60 s. 401/403/404/405/503 and closing stop it for good. A poll answered superseded waits 10-15 s instead of taking the server back at once. Two processes on one session (claude --continue while the first runs): a watcher lease (watcher.json in the launch dir, renewed every 5 s, stale after 20 s, released on session.end) picks one process to run the bridge and deliver. Delivery is claimed by renaming result.json / exit / pid to <file>.claimed (one winner), so a lease race never delivers twice; the loser forgets the launch quietly instead of showing "waiting for you" forever. persist() keeps this session's records it did not launch. Housekeeping: stored launch records with nothing left (cleaned-up directory; or another session's dead server with no decision) are pruned at restore. The debug log is appended (O_APPEND, rotated past 1 MiB) with a per-process tag instead of being rewritten from each process's buffer, which clobbered lines and left NUL bytes.
…ow-ups Review fixes for #1727: - One settlement claim per launch: claimArgv makes the launch's `settled/` directory (mkdir: one winner) and writes the claimant's id to `settled/by`, whichever file settles it (result record, exit code with an older CLI's stdout, dead pid). The per-file renames let one process deliver the record while another delivered the exit code's stdout copy. cleanupArgv removes stdin first and keeps `settled/` while feedback.md keeps the directory, so a claim made after cleanup loses. - A per-launch in-flight flag: the 1 s timer never waits for a slow check, and two checks of one launch in one process both won its claim. - The watcher lease follows the person: restoring a launch, a typed prompt (composer), a slash command, the plannotator tool and ExitPlanMode touch the process's launches and take the lease at once (`touchedAt` in watcher.json; a live holder keeps it only against an older or equal touch). The watcher re-reads the lease right before it settles. Every ExitPlanMode re-reads the approval from $.store and adopts this session's launches another process started, so plan review works from the process in use. - A claim whose process call timed out is the claimant's: it wins again on the next tick and delivers. The remaining window (death between the claim and $.prompt.submit) is documented. - Pruning runs after restore, off the session-start path (5 s cap), adds a 14-day expiry for other sessions' launches whose server is gone (decision or not), and re-reads the store before writing so records added meanwhile stay. - Debug log rotation runs under a mkdir lock with the size re-checked inside it, so two writers never rotate twice.
|
I pushed ba6b1b8 to address the review. Three changes supersede what the description says about two processes:
Checks:
|
…ring it $.prompt.submit waits for Claude to be idle, so a claimed decision can wait for the whole of Claude's current turn. A claimant that quit in that time left settled/ behind with nobody delivering the decision, and a later restore skipped it silently while result.json was still on disk. The claimant now keeps the launch's record in the store and renews its lease while it waits, writes settled/delivered once $.prompt.submit returns, and releases the lease at session.end. A restore of the session, and any process that saw the claim while watching the launch, checks a claimed, undelivered launch whose decision is still on disk: while its claimant's lease is fresh it waits; once it is released or 60 s stale it logs once, with a toast, where the decision is saved and writes settled/reported. It never re-delivers: the claimant may still be waiting to deliver, so only a report is strictly at most once. Pruning keeps such records until they are reported (or expire). AGENTS.md now says the window is the whole wait for Claude to go idle.
|
19b91ed closes the gap where a decision is stranded while Claude goes idle.
What the claimant does now:
What other processes check. The check covers a launch that is claimed, not marked delivered, and still has its
No re-delivery. The mod only reports the decision; it never delivers it again. A process that seems dead can still finish its pending submit (for example a laptop that wakes from sleep), so re-delivering could not be strictly at most once. Pruning keeps a claimed, undelivered record until it is reported or expires. Docs. AGENTS.md now says the window is the whole wait for Claude to go idle, not one process call. The same wording is in the Tests:
Checks:
|
Fixes two bugs in the Claude Code mod's "Ask this session" and decision delivery.
1. Ask AI said "session gone" for good after the server was unreachable for ~3 minutes
The bridge loop (
apps/hook/hooks/mod/bridge.ts) stopped after six failed polls.$.http.fetchgives up after 30 s, so a sleeping laptop or a paused server reached that limit in about three minutes. The controller started a bridge only whenlaunch.bridgewas empty, and it never cleared a dead handle. After that, Ask AI said the session was gone, but decisions still arrived through the 1 s result-file timer.Changes:
run()now returns why the loop ended (BridgeEnd):failures,refused,closingorended.failures, the controller clears the handle. The timer starts a new loop with the same token after 5 s. The wait doubles up to 60 s while the server stays silent, and goes back to 5 s after a loop that got through.closingstop the loop for good.superseded: truewaits 10-15 s before it polls again. Before, it polled again at once, which started a busy loop with the other client.2. Two Claude processes on one session id
claude --continuewhile the first process still runs gives two processes the same session id, and both reattached the same launches. Their polls superseded each other in a loop (about 46-49% CPU each). After one process delivered the decision, the other showed "waiting for you" forever, and both could deliver.Changes (updated in ba6b1b8 after review):
watcher.jsonin the launch directory selects one process to run the bridge and deliver. The process the person used most recently wins it: restoring a launch, a typed prompt, a slash command, theplannotatortool and ExitPlanMode all take the lease at once. The lease is renewed every 5 s, is taken over when it is 20 s old, andsession.endreleases it.settled/directory (mkdir has exactly one winner) and writes its id tosettled/by. This holds whichever file settles the launch, so a race for the lease cannot deliver twice. A claim whose process call timed out is won again by the same claimant. A per-launch in-flight flag stops overlapping checks.stdingone forgets the launch quietly. Its status line clears.$.store, and ExitPlanMode and the tool adopt this session's launches the other process started.persist()keeps records of this session that the other process launched.Housekeeping
Pruning after restore (off the session-start path, 5 s cap; the store is read again before the write): launch records older than a minute are removed from
$.storewhen nothing is left of them:stdin);A live server, or a decision that a resumed session would still deliver, keeps its record. This session's dead servers are left to the timer, which reports them.
feedback.mdis unchanged.Debug log:
PLANNOTATOR_MOD_DEBUG=1now appends to the log (O_APPEND through/bin/sh, rotated past 1 MiB under a lock) and tags each line with the process. Before, each process rewrote the whole file from its own buffer, so concurrent processes overwrote each other's lines and left NUL bytes.Docs
The "Detached launch" section of AGENTS.md (CLAUDE.md is a symlink to it) now says:
--continuekeeps the session id;--fork-session,/clear) do not reattach; this is a known limitation, and re-adopting reviews across a fork is a follow-up;The Ask-this-session section describes the restart and the backoff after
superseded.Tests
bun test apps/hook/hooks/mod: 115 pass. New tests:failures(with or without a poll that got through),refusedfor 404,ended, and a 10-15 s backoff aftersuperseded;persistkeeps the other process's records;/bin/sh(one winner of two concurrent claims).bun test apps/hook/server: 400 pass.tsc -p apps/hook/hooks/mod/tsconfig.jsonandbun run typecheck: clean.scripts/test-claude-code-mod.sh(claude plugin test), with a temporaryCLAUDE_CONFIG_DIRandPLANNOTATOR_DATA_DIR: 17 pass.Live repro
Claude Code 2.1.290 with a scripted fake Messages API and the 0.28.4 CLI. I ran two sessions side by side: one with this branch's mod, one with origin/main's. Each opened
/plannotator-annotate notes.md, then I sent SIGSTOP to both review servers for 4.5 minutes and then SIGCONT./api/ai/capabilitiesreportedreadyand stayedready;readyfor about 30 s after SIGCONT, thengonefor good. Ask AI answeredsession_gone.claude --continuebeside the running process. It reattached and stayed a watcher.