mirror of
https://github.com/Ark0N/Codeman.git
synced 2026-10-03 05:59:43 +02:00
fix(sessions): correct what the missing-pid rule actually recognises
A second real reboot disproved the mechanism the previous commit was built on. Typing `/exit` does not persist `pid: null`, and the session was restored anyway. The pid a session record carries is its `tmux attach-session` process, not the agent. `/exit` ends the CLI inside the pane, `remain-on-exit` keeps the pane, and the attach process stays alive throughout — so Codeman's PTY never exits, no exit handler runs, and the record keeps both its pid and `status: 'idle'`. The lifecycle log for the session that came back shows created, started, stale_cleaned and recovered, with no exit event at all, which is the proof: Codeman never learned the agent was gone. So nothing durable distinguishes an exited agent from a session that was idle when the power went, and this pass restores both. Ark0N/Codeman#446 is about making Codeman notice the dead pane; contrary to what the previous commit's message claimed, this genuinely does wait on that. Until a record can say the agent is gone, the user dismisses or closes those sessions. The rule itself is kept, because a record with no attach process does describe a session that never started or whose pane died outright, and refusing it is right. Only its documentation was wrong. The module header, the branch comment and the test names now say what it recognises instead of claiming the case it cannot see. 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
5108a24bf0
commit
62ceb4e87b
+24
-16
@@ -19,12 +19,19 @@
|
||||
* 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.
|
||||
* ⚠️ Ending the AGENT rather than the session is a shape this module CANNOT
|
||||
* recognise today, and a reboot restores it. `/exit` ends the CLI inside the
|
||||
* pane, `remain-on-exit` keeps the pane, and the PTY Codeman owns is the
|
||||
* `tmux attach-session` process, which stays alive throughout — so no exit
|
||||
* handler runs, no lifecycle `exit` is logged, and the record keeps both its pid
|
||||
* and `status: 'idle'`. Nothing durable distinguishes it from a session that was
|
||||
* simply idle when the power went. Ark0N/Codeman#446 covers making Codeman
|
||||
* notice the dead pane; until a record can say the agent is gone, this pass will
|
||||
* offer those sessions back, and the user dismisses or closes them.
|
||||
*
|
||||
* The `pid` check below is therefore NOT that rule. It refuses a record whose
|
||||
* attach process was already gone, which is a session that never started or
|
||||
* whose pane died outright.
|
||||
*
|
||||
* @dependencies types (SessionState), config/cli-registry
|
||||
* @consumedby web/server (plan build at boot), web/routes/reboot-restore-routes
|
||||
@@ -172,17 +179,18 @@ export function planRebootRestore(
|
||||
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.
|
||||
// No attach process when the record was last written: the session never
|
||||
// started, or its pane died outright rather than its agent exiting inside a
|
||||
// surviving pane. Either way there was nothing running to bring back.
|
||||
//
|
||||
// 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.
|
||||
// ⚠️ This does NOT catch a session the user ended with `/exit`. See the
|
||||
// module header: that leaves the pid in place, because the pid is the tmux
|
||||
// attach process and `remain-on-exit` keeps it alive.
|
||||
//
|
||||
// Conservative on purpose. 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;
|
||||
}
|
||||
|
||||
@@ -138,17 +138,18 @@ 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.
|
||||
describe('a session with no attach process in its record', () => {
|
||||
it('is refused, because there was nothing running to bring back', () => {
|
||||
// A session that never started, or whose pane died outright. NOT a session
|
||||
// the user ended with `/exit`: that keeps its pid, because the pid is the
|
||||
// tmux attach process and `remain-on-exit` keeps the pane alive.
|
||||
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', () => {
|
||||
it('still restores the session beside it that was attached when the power went', () => {
|
||||
const persisted = {
|
||||
exited: persistedSession({ id: 'exited', pid: null }),
|
||||
running: persistedSession({ id: 'running', pid: 4242 }),
|
||||
|
||||
Reference in New Issue
Block a user