diff --git a/src/utils/omp-cli-resolver.ts b/src/utils/omp-cli-resolver.ts index 5da8e1b9..269b64d2 100644 --- a/src/utils/omp-cli-resolver.ts +++ b/src/utils/omp-cli-resolver.ts @@ -76,7 +76,11 @@ function probeOmpVersion(binPath: string): string | null { type OmpVersionProbe = (binPath: string) => string | null; -function createOmpResolver(host?: CliResolverHost, versionProbe: OmpVersionProbe = probeOmpVersion, now?: () => number) { +function createOmpResolver( + host?: CliResolverHost, + versionProbe: OmpVersionProbe = probeOmpVersion, + now?: () => number +) { return createCliExecutableResolver( { binary: 'omp', diff --git a/src/web/public/terminal-ui.js b/src/web/public/terminal-ui.js index a3feb0fc..47182165 100644 --- a/src/web/public/terminal-ui.js +++ b/src/web/public/terminal-ui.js @@ -2969,6 +2969,16 @@ Object.assign(CodemanApp.prototype, { // Start interactive await fetch(`/api/sessions/${newSessionId}/interactive`, { method: 'POST' }); + // Retire the row being resumed: a non-claude "resume" is really a brand + // new Codeman session pointed at the same directory (there is no id to + // reattach to), so without this every resume leaves the old row behind + // as a duplicate — click it 3 times, see the same name 3 times. Claude + // rows are left alone: `sessionId` there is a claudeSessionId, which + // usually has no live/persisted Codeman session of its own to delete. + if (effectiveMode !== 'claude' && sessionId !== newSessionId) { + fetch(`/api/sessions/${sessionId}?killMux=true`, { method: 'DELETE' }).catch(() => {}); + } + this.terminal.writeln(`\x1b[90m Session ${name} ready\x1b[0m`); await this.selectSession(newSessionId); this.terminal.focus(); diff --git a/src/web/routes/session-routes.ts b/src/web/routes/session-routes.ts index 99498a70..80fef58b 100644 --- a/src/web/routes/session-routes.ts +++ b/src/web/routes/session-routes.ts @@ -1134,8 +1134,31 @@ export function registerSessionRoutes( const query = req.query as { killMux?: string }; const killMux = query.killMux !== 'false'; // Default to true - // Security: owner-scoped lookup 404s foreign/missing sessions uniformly (no existence leak, no cross-user kill). - const session = findSessionOrFail(ctx, id, req); + // A resumed/detached-but-never-live row (e.g. a non-claude "Resume" that + // relaunched into a NEW session and wants to retire the old one it can no + // longer reattach to) has no entry in ctx.sessions at all — only in + // persisted state. Fall back to removing that persisted record directly + // rather than 404ing: the caller means "make this row go away", and a + // stale duplicate row is exactly what's left behind otherwise. Pinned + // sessions keep their existing demote-not-delete protection. + const session = ctx.sessions.get(id); + if (!session) { + const persisted = ctx.store.getSession(id); + if (!persisted || !canAccessOwned(getAuthUser(req), persisted.owner)) { + throw Object.assign(new Error(`Session ${id} not found`), { + statusCode: 404, + body: createErrorResponse(ApiErrorCode.NOT_FOUND, `Session ${id} not found`), + }); + } + ctx.store.demoteOrRemoveSession(id); + return {}; + } + if (req && !canAccessOwned(getAuthUser(req), session.owner)) { + throw Object.assign(new Error(`Session ${id} not found`), { + statusCode: 404, + body: createErrorResponse(ApiErrorCode.NOT_FOUND, `Session ${id} not found`), + }); + } await ctx.cleanupSession(session.id, killMux, 'user_delete'); return {}; diff --git a/test/mocks/mock-route-context.ts b/test/mocks/mock-route-context.ts index 7dd4222e..8a1bd884 100644 --- a/test/mocks/mock-route-context.ts +++ b/test/mocks/mock-route-context.ts @@ -86,6 +86,7 @@ export function createMockRouteContext(options?: { getSession: vi.fn(), setSession: vi.fn(), removeSession: vi.fn(), + demoteOrRemoveSession: vi.fn(() => 'removed' as const), getSettings: vi.fn(() => ({})), setSettings: vi.fn(), getRalphLoopState: vi.fn(() => ({})), diff --git a/test/routes/session-routes.test.ts b/test/routes/session-routes.test.ts index dbe8efa2..dfdc3505 100644 --- a/test/routes/session-routes.test.ts +++ b/test/routes/session-routes.test.ts @@ -341,6 +341,32 @@ describe('session-routes', () => { const body = JSON.parse(res.body); expect(body.success).toBe(false); }); + + it('removes a persisted-only session (not live) via the state store, without touching cleanupSession', async () => { + vi.mocked(harness.ctx.store.getSession).mockReturnValueOnce({ + id: 'ghost-session', + owner: undefined, + } as never); + const res = await harness.app.inject({ + method: 'DELETE', + url: '/api/sessions/ghost-session', + }); + expect(res.statusCode).toBe(200); + const body = JSON.parse(res.body); + expect(body.success).toBe(true); + expect(harness.ctx.store.demoteOrRemoveSession).toHaveBeenCalledWith('ghost-session'); + expect(harness.ctx.cleanupSession).not.toHaveBeenCalled(); + }); + + it('404s a persisted-only session id the state store does not recognize either', async () => { + vi.mocked(harness.ctx.store.getSession).mockReturnValueOnce(null); + const res = await harness.app.inject({ + method: 'DELETE', + url: '/api/sessions/truly-nonexistent', + }); + expect(res.statusCode).toBe(404); + expect(harness.ctx.store.demoteOrRemoveSession).not.toHaveBeenCalled(); + }); }); // ========== DELETE /api/sessions (delete all) ==========