From 35a0217ccdcb2e2bc8ec230d2c61286761aabbbe Mon Sep 17 00:00:00 2001 From: Aamer Akhter Date: Sun, 21 Jun 2026 15:34:08 -0400 Subject: [PATCH] COD-142 retain pin when a pinned session is killed A killed session was full-deleted from state.json (removeSession), dropping the COD-139 pinned/pinnedAt fields, so the session vanished from the session-manager pinned group. cleanupStaleSessions also reaped any persisted record with no live session on boot, which would have wiped a preserved pin on the next restart. Fix (state-store): - demoteOrRemoveSession(id): on kill, demote a *pinned* record to a lightweight stopped record (status=stopped, pid=null, pin retained) instead of deleting; unpinned records are removed as before. - cleanupStaleSessions skips pinned records so the pin survives restart. - server _doCleanupSession calls demoteOrRemoveSession on the killMux path (shutdown path unchanged). Restoration iterates live mux sessions, not state.json, so a stopped+ pinned record is never auto-revived. Unit-tested on the real StateStore path (state-store.test.ts +4); session-cleanup/session-pin regress green. (cherry picked from commit 86f183eacfc3f2f6ac28499fb1ae2d21eef2bbed) --- src/state-store.ts | 20 ++++++++++++++ src/web/server.ts | 2 +- test/state-store.test.ts | 60 ++++++++++++++++++++++++++++++++++++++++ 3 files changed, 81 insertions(+), 1 deletion(-) diff --git a/src/state-store.ts b/src/state-store.ts index 792e4a21..444e4ea3 100644 --- a/src/state-store.ts +++ b/src/state-store.ts @@ -488,6 +488,25 @@ export class StateStore { this.save(); } + /** + * COD-142: Remove a session's persisted record on kill UNLESS it is pinned. + * A pinned session is demoted to a lightweight `stopped` record (pin retained) + * so it stays visible in the session-manager pinned group and survives restart. + * Unpinned sessions are fully removed (unchanged behavior). + * @returns 'preserved' if demoted to stopped+pinned, 'removed' if deleted, 'absent' if no record existed. + */ + demoteOrRemoveSession(id: string): 'preserved' | 'removed' | 'absent' { + const existing = this.state.sessions[id]; + if (!existing) return 'absent'; + if (existing.pinned === true) { + // Demote in place: keep identity/resume fields + pin, mark stopped, clear live runtime. + this.setSession(id, { ...existing, status: 'stopped', pid: null }); + return 'preserved'; + } + this.removeSession(id); + return 'removed'; + } + /** * Cleans up stale sessions from state that don't have corresponding active sessions. * @param activeSessionIds - Set of currently active session IDs @@ -502,6 +521,7 @@ export class StateStore { for (const sessionId of allSessionIds) { if (!activeSessionIds.has(sessionId)) { + if (this.state.sessions[sessionId]?.pinned === true) continue; // COD-142: pinned records persist even with no live session const name = this.state.sessions[sessionId]?.name; cleaned.push({ id: sessionId, name }); delete this.state.sessions[sessionId]; diff --git a/src/web/server.ts b/src/web/server.ts index 791d5921..acb0b4cd 100644 --- a/src/web/server.ts +++ b/src/web/server.ts @@ -1185,7 +1185,7 @@ export class WebServer extends EventEmitter { // Only remove from state.json if we're also killing the mux session. // When killMux=false (server shutdown), preserve state for recovery. if (killMux) { - this.store.removeSession(sessionId); + this.store.demoteOrRemoveSession(sessionId); } } diff --git a/test/state-store.test.ts b/test/state-store.test.ts index 78ed87da..56b522df 100644 --- a/test/state-store.test.ts +++ b/test/state-store.test.ts @@ -141,6 +141,66 @@ describe('StateStore', () => { }); }); + describe('demoteOrRemoveSession and pinned cleanup (COD-142)', () => { + it('should preserve a pinned session as a stopped record on kill', () => { + const store = new StateStore(testFilePath); + const pinnedAt = Date.now(); + store.setSession('pinned-1', { + ...createMockSessionState('pinned-1'), + name: 'My Pinned Session', + workingDir: '/tmp/pinned', + pinned: true, + pinnedAt, + }); + + const result = store.demoteOrRemoveSession('pinned-1'); + + expect(result).toBe('preserved'); + const preserved = store.getSession('pinned-1'); + expect(preserved).not.toBeNull(); + expect(preserved?.status).toBe('stopped'); + expect(preserved?.pid).toBeNull(); + expect(preserved?.pinned).toBe(true); + expect(preserved?.pinnedAt).toBe(pinnedAt); + expect(preserved?.name).toBe('My Pinned Session'); + expect(preserved?.workingDir).toBe('/tmp/pinned'); + }); + + it('should fully remove an unpinned session on kill', () => { + const store = new StateStore(testFilePath); + store.setSession('plain-1', createMockSessionState('plain-1')); + + const result = store.demoteOrRemoveSession('plain-1'); + + expect(result).toBe('removed'); + expect(store.getSession('plain-1')).toBeNull(); + }); + + it('should report absent for an unknown session id', () => { + const store = new StateStore(testFilePath); + + expect(store.demoteOrRemoveSession('does-not-exist')).toBe('absent'); + }); + + it('should keep pinned records during cleanupStaleSessions but reap unpinned ones', () => { + const store = new StateStore(testFilePath); + store.setSession('pinned-1', { + ...createMockSessionState('pinned-1'), + pinned: true, + pinnedAt: Date.now(), + }); + store.setSession('plain-1', createMockSessionState('plain-1')); + + const result = store.cleanupStaleSessions(new Set()); + + expect(result.count).toBe(1); + expect(result.cleaned.map((c) => c.id)).toEqual(['plain-1']); + expect(result.cleaned.some((c) => c.id === 'pinned-1')).toBe(false); + expect(store.getSession('pinned-1')).not.toBeNull(); + expect(store.getSession('plain-1')).toBeNull(); + }); + }); + describe('task operations', () => { it('should set and get tasks', () => { const store = new StateStore(testFilePath);