plan: chat session lifetime — the two idle timers and what to do about them
Investigation only, no code. The sidecar already protects sessions with background work in flight (pendingTasks disarms its idle GC); officer's hour-long timer knows nothing about that and kills them anyway, and does not survive its own restart. Plan is to have officer release its binding instead of killing, and to add a list verb so running sessions can be found after a restart. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
@@ -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<string>` (`: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.
|
||||
Reference in New Issue
Block a user