diff --git a/docs/chat-session-lifetime.md b/docs/chat-session-lifetime.md new file mode 100644 index 00000000..9b14eed7 --- /dev/null +++ b/docs/chat-session-lifetime.md @@ -0,0 +1,119 @@ +# Chat session lifetime — findings and plan + +Written 2026-08-09, before implementation. The investigation is done; the code is not. + +**The trigger.** A laptop ran out of battery overnight while an agent on the home lab was running a +15-minute background loop (review new commits, restart the server). The session should have survived +until morning. Officer would have killed it about an hour after the browser socket dropped. + +## What is actually there — verified, not remembered + +**Two independent idle policies exist, in two processes, answering different questions.** + +### The sidecar's — the real one + +`src/servers/sidecar/claude/claude-manager.ts` + +- `IDLE_TIMEOUT_MS = 30 * 60 * 1000` (`:39`) — thirty minutes with **no new turn**. +- `armIdle` (`:284-295`) is a **heartbeat, not a one-shot**: on expiry it re-checks and re-arms if + `isGenerating || pendingTasks.size > 0`, so it can never collect a session that is still working. +- `pendingTasks: Set` (`:174`) holds background tasks started but not yet notified. + `task:started` adds the id and **clears the idle timer outright** (`:406-411`); `task:notification` + removes it and re-arms only if nothing else is outstanding (`:413-414`). +- Its own comment states the intent: *"never GC a session that's mid-turn or still has background tasks + running — a long silent `run_in_background` job would otherwise be killed along with its pending + `task_notification`."* + +So **the sidecar already detects long-running background work with no declaration from the user.** The +"mark a session permanent" feature is not needed for this case. + +There is also `stallTimer` for a turn that claims to be generating but has emitted nothing, and +`SEND_TIMEOUT_MS = 30 * 60 * 1000` (`:10`). + +### Officer's — the blunt one + +`src/servers/api/chat/websocket.ts`, `src/servers/api/chat/session-manager.ts` + +- `IDLE_TIMEOUT_MS = 60 * 60 * 1000` (`websocket.ts:48`). +- Armed by the socket **closing**, not by it being unresponsive: `close(ws)` → `detachWs` + + `setIdleTimeout` (`websocket.ts:151-163`). Unresponsiveness matters only upstream — Bun closes a WS + idle for 60s and there is a per-connection heartbeat, so a dead laptop loses the socket about a minute + in, and *that* starts the hour. +- Expiry runs `deleteSession` (`session-manager.ts:74-102`), which calls `_sidecarUnsub()` then + `_claudeKill()` → `sidecar.killClaude(sessionId)`. +- `attachWs` clears the timer (`session-manager.ts:104-116`), so reconnecting cancels it cleanly. + +**Three defects follow:** + +1. It kills sessions the sidecar has deliberately protected. Officer has no view of `pendingTasks`. +2. It does not survive `pm2 restart officer` — a `setTimeout` on an in-memory record, in the process + designed to bounce. If the browser never returns, nothing re-arms it. +3. On a flat battery the sidecar's 30 minutes normally fires first anyway, so officer's hour is mostly + redundant *except* in the one case where it does damage — a session with background work, which the + sidecar keeps and officer kills. + +### What is already fine + +`pm2 restart officer` **does not kill running sessions.** Sidecars are PM2 peers, not children; the +agent keeps generating and keeps committing to `chat_session_events`. Only officer's in-memory binding +dies, and `adoptOrphanedSession` (`websocket.ts:559`) rebuilds it — including the session-scoped +subscription, without which a reconnected client replays and then goes silent for the rest of the turn. + +## Step 1 — officer's timer releases instead of kills + +Officer's idle timer is doing two unrelated jobs: garbage-collecting its own binding (its business) and +terminating the agent (the sidecar's). Split them and leave the agent's lifetime to the process that +already reasons about it correctly. + +**Rejected alternative:** teaching officer about `pendingTasks` over the protocol. That re-implements +the sidecar's heartbeat in a second place, which is how these two drifted apart to begin with. + +**The blocker.** `src/servers/channels/send-claude-code.ts:65-69` welds them together: + +```ts +return { kill: () => { sidecar.killClaude(params.sessionKey); unsub(); } } +``` + +`unsub` is a closure reachable only *through* `kill`, so officer cannot let go without killing. + +**The seam already exists.** `_sidecarUnsub` is declared (`types.ts:310`) and called separately in +`deleteSession` (`session-manager.ts:83-84`) — and **nothing ever assigns it**. Populating it is the fix. + +Work: + +1. `send-claude-code.ts` — return `{ kill, detach }`; `kill` stays as-is, `detach` is `unsub` alone. +2. Same for the OpenCode path (`websocket.ts:452` assigns `handle.kill` there too). +3. `websocket.ts:361` / `:452` — also assign `session._sidecarUnsub = handle.detach`. +4. `session-manager.ts` — add `releaseSession(sessionId)`: clear the idle timer, call `_sidecarUnsub`, + drop from `sessions`/`userSessions`. **Do not** call `_claudeKill`. +5. `setIdleTimeout`'s callback → `releaseSession`, not `deleteSession`. +6. Leave `deleteSession` alone — explicit disconnect (`handleDisconnect`) must still kill. + +Check while doing it: `adoptOrphanedSession` notes that `handleChat` treats an absent `_claudeKill` as +"first turn of this session" and would open a **second** subscription, delivering every message twice. +Make sure a released-then-readopted session cannot land in that state. + +Consequence to accept: after an hour a returning browser goes through the adopt path rather than finding +a live record. That path already runs on every officer restart. + +## Step 2 — enumerate live sessions + +After an officer restart a live session is invisible until a browser reconnects to it *by id*; adoption +is on-demand only. **There is no list verb in the sidecar protocol** (checked). + +Add `claude:list` returning each live `sessionKey` with `isGenerating` and `pendingTasks.size`, so +officer can answer "what is running right now" and surface it. + +**Do not try to persist a running session.** A live session is a running `query()` with an open stream; +it cannot be serialised. The durable part — the output — is already in `chat_session_events`, which is +what makes replay work. + +**Hard limit:** if the *agent sidecar* restarts, a mid-turn is lost regardless. Officer restarts are +survivable; `officer-agent` restarts are not. + +## Deliberately not decided + +Keeping an **idle** session alive — nothing generating, no background work — is a separate decision and +the only part that would need the user to mark anything. Steps 1 and 2 cover sessions with work in +flight, which is the case that actually bit. A 15-minute loop re-arms the sidecar's 30-minute timer on +every turn, so it never idles out on its own.