mirror of
https://github.com/Ark0N/Codeman.git
synced 2026-09-30 12:39:42 +02:00
fix(sessions): never restore a session whose agent was already exited
Found by a real reboot, which is the first thing to catch it. Typing `/exit` ends the CLI process and leaves the session record behind, and the process-exit handler persists `pid: null` with `status: 'idle'` before anything else runs. By status alone that is indistinguishable from a session sitting idle when the power went, so the boot pass offered those sessions back and a click spawned the agents the user had deliberately closed — the exact case the eligibility rule exists to exclude. The absent pid is what tells the two apart, and the plan step now refuses a record without one, under its own `not-running` reason so the boot log says why. On a healthy board every running session carries a pid; a record with none describes an agent that is already gone. Deliberately the conservative direction. 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. The opposite error spawns processes nobody asked for. Ark0N/Codeman#446 covers the dead panes those exits leave behind, but this does not wait on it: the rule belongs here whether or not the record's shape changes later. Refs #411 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 5
parent
5f55f9cb65
commit
5108a24bf0
+24
-1
@@ -19,6 +19,13 @@
|
|||||||
* touching its status, so a pinned session a reboot killed still reads `idle` or
|
* touching its status, so a pinned session a reboot killed still reads `idle` or
|
||||||
* `busy` and stays eligible.
|
* `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
|
* @dependencies types (SessionState), config/cli-registry
|
||||||
* @consumedby web/server (plan build at boot), web/routes/reboot-restore-routes
|
* @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.
|
* 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
|
* `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
|
* 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.
|
* "try again after closing something", the other means the CLI would not start.
|
||||||
@@ -99,6 +106,7 @@ export interface RebootRestoreRejection {
|
|||||||
reason:
|
reason:
|
||||||
| 'no-persisted-record'
|
| 'no-persisted-record'
|
||||||
| 'intentionally-ended'
|
| 'intentionally-ended'
|
||||||
|
| 'not-running'
|
||||||
| 'respawn-blocked'
|
| 'respawn-blocked'
|
||||||
| 'remote-or-docker'
|
| 'remote-or-docker'
|
||||||
| 'unsupported-mode'
|
| 'unsupported-mode'
|
||||||
@@ -163,6 +171,21 @@ export function planRebootRestore(
|
|||||||
skipped.push({ sessionId, reason: 'intentionally-ended' });
|
skipped.push({ sessionId, reason: 'intentionally-ended' });
|
||||||
continue;
|
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) {
|
if (state.respawnBlocked === true) {
|
||||||
// The crash-loop breaker tripped on this pane. Re-creating it restarts the loop.
|
// The crash-loop breaker tripped on this pane. Re-creating it restarts the loop.
|
||||||
skipped.push({ sessionId, reason: 'respawn-blocked' });
|
skipped.push({ sessionId, reason: 'respawn-blocked' });
|
||||||
|
|||||||
@@ -39,6 +39,7 @@ const NOW = 1_760_000_000_000;
|
|||||||
|
|
||||||
function persistedSession(overrides: Partial<SessionState> & { id: string }): SessionState {
|
function persistedSession(overrides: Partial<SessionState> & { id: string }): SessionState {
|
||||||
return {
|
return {
|
||||||
|
// A live agent's record carries its process id; `/exit` persists null instead.
|
||||||
pid: 99999,
|
pid: 99999,
|
||||||
status: 'idle',
|
status: 'idle',
|
||||||
workingDir: '/tmp/spike',
|
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', () => {
|
describe('a workspace that is no longer on disk', () => {
|
||||||
it('is kept out of the offer, so a click cannot scaffold a deleted repo', () => {
|
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' }) };
|
const persisted = { gone: persistedSession({ id: 'gone', workingDir: '/tmp/deleted-repo' }) };
|
||||||
|
|||||||
Reference in New Issue
Block a user