diff --git a/src/reboot-restore.ts b/src/reboot-restore.ts index e226138c..d295a34d 100644 --- a/src/reboot-restore.ts +++ b/src/reboot-restore.ts @@ -19,6 +19,13 @@ * touching its status, so a pinned session a reboot killed still reads `idle` or * `busy` and stays eligible. * + * Ending the AGENT rather than the session leaves a third shape, and it is the one + * a real reboot caught this module getting wrong. `/exit` ends the CLI process + * while the session record survives, and the process-exit handler persists + * `pid: null` with `status: 'idle'` — indistinguishable by status from a session + * that was merely idle when the power went. The absent pid is what tells them + * apart, so a record without one is refused. + * * @dependencies types (SessionState), config/cli-registry * @consumedby web/server (plan build at boot), web/routes/reboot-restore-routes * @@ -89,7 +96,7 @@ export function resolveResumeConversationId(state: SessionState): string { /** * Why one session was passed over. Reported for logging and shown to the user. * - * The first six are decided before anything is built. `capacity-reached` and + * The first seven are decided before anything is built. `capacity-reached` and * `rebuild-failed` can only happen once a click is spending the plan, and they * are the two the banner must not confuse with a missing workspace: one means * "try again after closing something", the other means the CLI would not start. @@ -99,6 +106,7 @@ export interface RebootRestoreRejection { reason: | 'no-persisted-record' | 'intentionally-ended' + | 'not-running' | 'respawn-blocked' | 'remote-or-docker' | 'unsupported-mode' @@ -163,6 +171,21 @@ export function planRebootRestore( skipped.push({ sessionId, reason: 'intentionally-ended' }); continue; } + if (state.pid === null || state.pid === undefined) { + // The agent had already exited when the machine went down: `/exit` ends the + // process, and its exit handler persists `pid: null` with `status: 'idle'` + // before anything else can. Status alone cannot tell that apart from a + // session that was simply sitting idle when the power went, so without this + // a reboot restore spawns the agents the user deliberately closed — the + // exact case the eligibility rule exists to exclude. + // + // A heuristic, and deliberately the conservative one. A session that somehow + // persisted no pid while genuinely running is not offered, and its + // conversation stays reachable from the Resume list, which is where every + // session would be without this feature. + skipped.push({ sessionId, reason: 'not-running' }); + continue; + } if (state.respawnBlocked === true) { // The crash-loop breaker tripped on this pane. Re-creating it restarts the loop. skipped.push({ sessionId, reason: 'respawn-blocked' }); diff --git a/test/reboot-restore.test.ts b/test/reboot-restore.test.ts index aaf9a83e..5ed989fd 100644 --- a/test/reboot-restore.test.ts +++ b/test/reboot-restore.test.ts @@ -39,6 +39,7 @@ const NOW = 1_760_000_000_000; function persistedSession(overrides: Partial & { id: string }): SessionState { return { + // A live agent's record carries its process id; `/exit` persists null instead. pid: 99999, status: 'idle', workingDir: '/tmp/spike', @@ -137,6 +138,32 @@ describe('which dead sessions may be rebuilt', () => { }); }); +describe('a session whose agent had already exited', () => { + it('is refused, because `/exit` leaves the record reading idle with no pid', () => { + // What the process-exit handler persists: the CLI is gone, the record is not, + // and its status is indistinguishable from a session that was merely idle. + const persisted = { exited: persistedSession({ id: 'exited', status: 'idle', pid: null }) }; + const plan = planRebootRestore(['exited'], persisted, () => true); + expect(plan.restore).toEqual([]); + expect(plan.skipped).toEqual([{ sessionId: 'exited', reason: 'not-running' }]); + }); + + it('still restores the session beside it that was running when the power went', () => { + const persisted = { + exited: persistedSession({ id: 'exited', pid: null }), + running: persistedSession({ id: 'running', pid: 4242 }), + }; + const plan = planRebootRestore(['exited', 'running'], persisted, () => true); + expect(plan.restore.map((entry) => entry.sessionId)).toEqual(['running']); + expect(plan.skipped.map((s) => s.reason)).toEqual(['not-running']); + }); + + it('refuses a record with no pid field at all', () => { + const persisted = { odd: persistedSession({ id: 'odd', pid: undefined as unknown as null }) }; + expect(planRebootRestore(['odd'], persisted, () => true).skipped[0].reason).toBe('not-running'); + }); +}); + describe('a workspace that is no longer on disk', () => { it('is kept out of the offer, so a click cannot scaffold a deleted repo', () => { const persisted = { gone: persistedSession({ id: 'gone', workingDir: '/tmp/deleted-repo' }) };