Skip to content

fix(mod): restart a dead Ask-this-session bridge; one watcher per launch across processes - #1727

Merged
backnotprop merged 3 commits into
mainfrom
fix/mod-bridge-restart
Oct 6, 2026
Merged

backnotprop merged 3 commits into
mainfrom
fix/mod-bridge-restart

Conversation

@backnotprop

@backnotprop backnotprop commented Oct 6, 2026 •

Copy link
Copy Markdown
Owner

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.fetch gives 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 when launch.bridge was 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, closing or ended.
  • On 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.
  • 401/403/404/405/503 (wrong token, an older CLI without the bridge, AI off) and closing stop the loop for good.
  • A poll answered superseded: true waits 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 --continue while 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 lease: watcher.json in 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, the plannotator tool and ExitPlanMode all take the lease at once. The lease is renewed every 5 s, is taken over when it is 20 s old, and session.end releases it.
  • One claim per launch: before settling, a process makes the launch's settled/ directory (mkdir has exactly one winner) and writes its id to settled/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.
  • The other process: a process that loses the claim, finds a claim by someone else, or finds the launch's stdin gone forgets the launch quietly. Its status line clears.
  • Plan approvals: every ExitPlanMode re-reads the approval from $.store, and ExitPlanMode and the tool adopt this session's launches the other process started.
  • Store: 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 $.store when nothing is left of them:

    • any record whose directory was cleaned up (no stdin);
    • for other sessions, a server that died without a decision;
    • for other sessions older than 14 days, any whose server is gone, decision or not.

    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.md is unchanged.

  • Debug log: PLANNOTATOR_MOD_DEBUG=1 now 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:

  • on 2.1.290, --continue keeps the session id;
  • the fork paths (--fork-session, /clear) do not reattach; this is a known limitation, and re-adopting reviews across a fork is a follow-up;
  • how the lease and claim work, the pruning and the append-only log.

The Ask-this-session section describes the restart and the backoff after superseded.

Tests

  • bun test apps/hook/hooks/mod: 115 pass. New tests:
    • a bridge loop reports failures (with or without a poll that got through), refused for 404, ended, and a 10-15 s backoff after superseded;
    • the controller restarts a dead bridge after 5 s, then 10 s while the server stays silent, and stops retrying once it answers;
    • a 404 is never polled again;
    • with two instances on one file map, only the watcher delivers and the other forgets the review quietly;
    • two instances that check at the same time still deliver once;
    • the second instance takes over from a stale lease, and at once from a released lease;
    • persist keeps the other process's records;
    • restore pruning;
    • the claim, prune and debug-append scripts run for real with /bin/sh (one winner of two concurrent claims).
  • Mutation checks: with the restart, lease, claim or persist merge removed, the matching tests fail.
  • bun test apps/hook/server: 400 pass.
  • tsc -p apps/hook/hooks/mod/tsconfig.json and bun run typecheck: clean.
  • scripts/test-claude-code-mod.sh (claude plugin test), with a temporary CLAUDE_CONFIG_DIR and PLANNOTATOR_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.

  • This branch:
    • the bridge gave up at about 3 minutes;
    • the debug log shows "starting again in 5000 ms";
    • after SIGCONT, /api/ai/capabilities reported ready and stayed ready;
    • an Ask AI question was answered by the session.
  • origin/main: ready for about 30 s after SIGCONT, then gone for good. Ask AI answered session_gone.
  • Two processes (this branch): I started claude --continue beside the running process. It reattached and stayed a watcher.
    • Both processes used about 0.1% CPU.
    • A submitted decision was delivered once, by the watching process.
    • The other process logged "settled by another Claude Code process" and its status line cleared.
    • The shared debug log has lines from both processes and no NUL bytes.

…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.
@backnotprop

Copy link
Copy Markdown
Owner Author

I pushed ba6b1b8 to address the review. Three changes supersede what the description says about two processes:

  • Delivery claim: there is now one claim per launch, not a rename of each file. The mod makes a settled/ directory in the launch (mkdir has exactly one winner) and writes the claimant's id to settled/by.
  • Lease: it now goes to the process the person used most recently, instead of staying with the first process.
  • Pruning: it now runs after restore, so it no longer delays session start.
  1. One claim per launch, plus an in-flight flag per launch. A process that finds a claim made by another process forgets the launch quietly, whichever file it found. Cleanup removes stdin first and keeps settled/ while feedback.md keeps the directory, so a claim made after cleanup starts loses. Without the in-flight flag, two overlapping checks in one process could each win the claim, because the claim names the instance. New tests:

    • a decision on disk as both a record and an exit code, with both processes checking at once;
    • a claim made by another process stops this one, whichever file it finds;
    • two overlapping ticks in one process.

    With the flag removed, the overlapping-ticks test fails.

  2. The lease follows the person. These actions update touchedAt and take the lease at once:

    • restoring a launch;
    • a prompt typed into that process (composer);
    • a slash command;
    • the plannotator tool;
    • ExitPlanMode.

    A live holder keeps the lease only against an older or equal touch. The watcher re-reads the lease right before it settles a launch. Every ExitPlanMode re-reads the approval from $.store, so an approval received or already used by the other process is honored once. ExitPlanMode and the tool also adopt launches the other process started on this session.

    New tests:

    • the process that restored later receives the decision;
    • typing in the first process takes the reviews back;
    • a plan approval reaches the process in use, and its ExitPlanMode passes;
    • an approval received by the other process passes once.
  3. Lost claim answer. settled/by names the claimant. If the process call timed out after the claim was made, the same instance wins the claim again on the next tick and delivers; there is a test for this. The window that remains, a crash between the claim and $.prompt.submit, is documented in AGENTS.md.

  4. Pruning:

    • It runs fire-and-forget after restore, with a 5 s cap.
    • New --expired group: another session's launch older than 14 days whose server is gone is pruned, even with a decision on disk.
    • The store is read again before the write, so a record another process adds meanwhile stays. The test covers this.
  5. Debug log rotation: it now runs under a mkdir lock (debug.log.rotating; a lock older than one minute is removed) and checks the size again inside the lock. The test checks the result: the old log is kept, no line is lost, and no lock is left behind. It does not reproduce the race on every run, because the window is narrow.

Checks:

  • bun test apps/hook/hooks/mod: 123 pass.
  • tsc -p apps/hook/hooks/mod/tsconfig.json and bun run typecheck: no errors.
  • scripts/test-claude-code-mod.sh with a temp CLAUDE_CONFIG_DIR: 17 pass.
  • Live check of two processes on Claude Code 2.1.290:
    • Process A opened /plannotator-annotate; then claude --continue started B.
    • B took the lease at restore. A stepped down within 3 s and backed off once after superseded.
    • Both processes used about 0.1% CPU.
    • An Ask AI question was answered in B.
    • A submitted decision was delivered once, in B. A logged "settled by another Claude Code process", and its status line cleared.
    • The launch directory was fully cleaned. The shared debug log has lines from both processes and no NUL bytes.

…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.
@backnotprop

Copy link
Copy Markdown
Owner Author

19b91ed closes the gap where a decision is stranded while Claude goes idle.

$.prompt.submit waits for Claude to be idle, so a claimed decision can wait through all of Claude's current turn. If the claimant quit in that time, the decision was left with nobody to deliver it.

What the claimant does now:

  • It keeps the launch's record in $.store while it waits.
  • It renews its watcher lease every 5 s while it waits.
  • It writes settled/delivered once $.prompt.submit returns. A close by Claude, where there is nothing to deliver, also writes it.
  • It releases the lease at session.end.

What other processes check. The check covers a launch that is claimed, not marked delivered, and still has its stdin and its decision on disk (result.json, or stdout with exit 0 for an older CLI). It runs at restore of the same session, when ExitPlanMode or the tool adopts a launch from the store, and in the tick for a launch this process watched when it saw the claim.

  • Claimant's lease fresh: the process keeps waiting.
  • Lease released, or stale for more than 60 s: the process logs one line plus a toast, "A decision for <subject> arrived but wasn't delivered — it's saved in <dir>/result.json". It then writes settled/reported, so the line appears once.

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 claimSettlement comment.

Tests:

  • A claimant quits while Claude is busy: the next restore reports once, pointing at the file. A third restore says nothing, and nothing is submitted.
  • A claimant still waiting is not reported. Once it delivers, settled/delivered exists, the record is gone and nothing is said.
  • A claimant killed while busy (no release): the process that watched the launch reports once, after the lease has been stale for 60 s.
  • The prune script keeps a stranded claim and drops a delivered one.
  • Mutation checks: removing the delivered marker or the release at session.end makes these tests fail.

Checks:

  • Mod tests: 126 pass.
  • Mod typecheck and bun run typecheck: no errors.
  • Mod harness (temp CLAUDE_CONFIG_DIR): 17 pass.
  • I did not run a live repro of this case.

@backnotprop
backnotprop merged commit 2a330ab into main Oct 6, 2026
28 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant