diff --git a/COMMS/sidecar-app-store/21-rejection-backstop.md b/COMMS/sidecar-app-store/21-rejection-backstop.md new file mode 100644 index 00000000..3e68bab5 --- /dev/null +++ b/COMMS/sidecar-app-store/21-rejection-backstop.md @@ -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. diff --git a/src/servers/sidecar/claude/user-instance.ts b/src/servers/sidecar/claude/user-instance.ts index 050baeff..6783c539 100644 --- a/src/servers/sidecar/claude/user-instance.ts +++ b/src/servers/sidecar/claude/user-instance.ts @@ -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);