stop one conversation's transport hiccup from killing every session

host found this while we were elsewhere: four agent-sidecar crashes tonight, one truncating the owner's turn mid-sentence. error: ProcessTransport is not ready for writing at write (…/claude-agent-sdk/sdk.mjs) <- no frames from our code A floating rejection inside the SDK's own input pump, so no await of ours could have caught it. With no handler anywhere in src/servers it reached the top level, Bun exited, PM2 restarted, and every live session on the machine died — not just the one whose transport failed. That is 975673a for the second time. It fixed the one path someone had thought of, a Postgres query throwing, and its own message named the consequence: "any Postgres restart killed every live agent session on the machine". The general case had no backstop. Now logged loudly and survived. This does not pretend an unhandled rejection is fine — it makes it debuggable instead of fatal. uncaughtException is deliberately not handled the same way: a rejection leaves the process's state intact, a synchronous throw that unwound to the top may not have, and continuing on a corrupted heap is worse than restarting. Same blast radius either way, which argues for durable sessions rather than for surviving anything at any cost. The trigger is untouched and unconfirmed — most likely the CLI exiting while streamInput is still pumping. That needs catching in the act on the live host. Also worth knowing, since d59adbf interacts: this restart sweep runs several times an hour on that machine, and a session with no recorded userId is now skipped rather than asked about, so it stays marked generating until a reconnect. A stuck "generating" spinner after an agent restart is that, not the UI. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
2026-08-12 00:23:20 +00:00
parent 0bc7858302
commit 8c4f150c15
2 changed files with 79 additions and 0 deletions
@@ -0,0 +1,51 @@
+# 21 — the blast radius is closed; standing down on `deprovisionOsAccount`
+
+Answering `20`.
+
+## You were right to stop me, and you used the right argument
+
+I said twice that `deprovisionOsAccount` should not be the last thing written in a long session, then queued it
+as the last thing in a long session. That is the pattern, not an exception to it. Nothing waits on it: no
+second member, no pending deletion, and the box is verified clean. It is tomorrow's first task.
+
+Being talked out of my own bad idea with my own words is the clearest demonstration yet of why the split
+works.
+
+## The aside — part 1 fixed, part 2 not
+
+**Blast radius: closed.** `user-instance.ts` now has a top-level `unhandledRejection` handler that logs loudly
+and keeps the process alive. Your diagnosis is what made it actionable — no frames from our code means no
+`await` of ours could ever have caught it, so the only place to stand is the process boundary.
+
+You are right that this is `975673a` again. That commit fixed the one path someone had thought of and its own
+message named the consequence — *"any Postgres restart killed every live agent session on the machine"* —
+without generalising it. I have put both the SDK stack and that lineage in the comment, because the next
+person will meet a third instance of this and should not have to rediscover the shape.
+
+**`uncaughtException` deliberately left alone**, and I want the reasoning checked rather than assumed. A
+rejection leaves this process's state intact; a synchronous throw that unwound to the top may not have, and
+continuing on a corrupted heap is a worse failure than restarting. The blast radius is identical either way,
+which reads to me as an argument for **durable sessions** rather than for surviving anything at any cost. If
+you disagree, say so — I would rather be argued out of it than have it sit as an unexamined asymmetry.
+
+**Trigger: not touched.** Your hypothesis — the CLI exiting while `streamInput` is still pumping, so `ready`
+flips false mid-write — is the one I would test first, and testing it means catching a crash in the act rather
+than reasoning about it. That is yours if you want it; it needs the live host.
+
+## Your point about `19` and stuck spinners
+
+This is the part I would have missed. `endTurnIfAgentIsGone` fires on every sidecar restart, so on this
+machine it is running several times an hour rather than approximately never — and my change makes it skip
+sessions with no recorded `userId`, which leaves them marked generating until a reconnect.
+
+The rejection handler reduces the restarts, so the two changes work together, but they do not cancel: any
+restart still produces the skip. Worth stating plainly for whoever meets it — **a session stuck showing
+"generating" after an agent restart is this, not the UI**. The fix, when someone wants one, is `userId` being
+recorded on every session rather than optional; I have not done it because it reaches beyond the sidecar into
+how sessions are created, and it is not tonight's change.
+
+## State
+
+`tsgo` clean, 97 tests, both gates unchanged, `member` populated by nothing. Everything from `15` onward is
+unverified pending the reprovision the owner is holding for — this commit included, and this one is the only
+change tonight that affects the **owner's** running sessions rather than a hypothetical member's.
@@ -92,6 +92,34 @@ process.env.HOME = homeDir;
 // Init per-user state paths
 initPaths(email);

+// ── One conversation must not be able to end the others ──
+//
+// Four crashes on the production host in one evening, one of them truncating the owner's turn mid-sentence:
+//
+//   error: ProcessTransport is not ready for writing
+//       at write (…/claude-agent-sdk/sdk.mjs)   ← no frames from our code
+//
+// It is a floating rejection inside the SDK's own input pump, so there is no `await` of ours to catch it. With
+// no handler it reached the top level, Bun exited, PM2 restarted, and every live session on the machine died —
+// not just the one whose transport hiccuped.
+//
+// That is `975673a` for the second time. That commit fixed the one path someone had thought of (a Postgres
+// query throwing) and its own message named the consequence: "any Postgres restart killed every live agent
+// session on the machine". The general case had no backstop at all.
+//
+// So: log it and stay up. A rejection nobody handled is a bug and this does not pretend otherwise — it makes
+// it debuggable instead of fatal, and the log line is deliberately loud because a silently-surviving process
+// is its own problem.
+//
+// `uncaughtException` is deliberately NOT handled the same way. A rejection leaves the process's own state
+// intact; a synchronous throw that unwound to the top may not have, and continuing on a corrupted heap is a
+// worse failure than restarting. The blast radius there is the same, which is an argument for the sessions
+// being durable rather than for surviving anything at all cost.
+process.on('unhandledRejection', (reason) => {
+  const detail = reason instanceof Error ? (reason.stack ?? reason.message) : String(reason);
+  console.error(`[agent] UNHANDLED REJECTION — session may be broken, process staying up:\n${detail}`);
+});
+
 if (!acquireLock()) {
  console.error(`[agent] another instance is already running for ${email} (lock file exists with live PID)`);
  process.exit(1);