mirror of
https://github.com/Ark0N/Codeman.git
synced 2026-10-02 13:39:41 +02:00
resumeHistorySession() creates the resumed row in its own mode via a modeConfigKey map (opencode/pi/grok/omp -> continueSession, deepseek -> resumeSession) and retires the old row afterward. codex, gemini and antigravity were missing from that map, so resuming one of their rows started a brand-new session with NO continuation while still deleting the row it came from -- silent data loss dressed as the duplicate-row fix. Gate row retirement on continuesSomething (true only for modes that actually got a continuation config) instead of wiring an unverified sessionId->native-conversation-id assumption for the three affected CLIs. DELETE /api/sessions/:id reimplemented the ownership 404 check inline in two places instead of going through findSessionOrFail, and its persisted-only-session branch never broadcast session:deleted, so other open tabs kept the retired row until their next unrelated fetch. Extract the shared 404 into sessionNotFoundError(), add findPersistedSessionOrFail() alongside findSessionOrFail() in route-helpers.ts (same ownership contract, returns a SessionState instead of a live Session), and use both from the route instead of inline checks. Add the missing broadcast.
144 lines
5.7 KiB
TypeScript
144 lines
5.7 KiB
TypeScript
/**
|
|
* @fileoverview Upstream review fix (Ark0N/Codeman#353, PR #3): resumeHistorySession()
|
|
* threads the row's own mode through session creation via a `modeConfigKey` map
|
|
* (opencode/pi/grok/omp → `continueSession: true`), then retires the old row via
|
|
* DELETE. codex/gemini/antigravity were missing from that map, so resuming one of
|
|
* their rows created a session with NO continuation while still deleting the row
|
|
* it came from — data loss dressed as a fix. The correction: only retire the row
|
|
* when the new session actually continues something.
|
|
*
|
|
* Loaded via `vm` against a stub CodemanApp, same harness as resume-name.test.ts.
|
|
* `fetch` is a shared mutable stub so each test can inspect exactly which requests
|
|
* fired without a real network/server.
|
|
*/
|
|
|
|
import { readFileSync } from 'node:fs';
|
|
import { resolve } from 'node:path';
|
|
import vm from 'node:vm';
|
|
import { describe, expect, it, vi, beforeEach } from 'vitest';
|
|
|
|
/* eslint-disable @typescript-eslint/no-explicit-any */
|
|
|
|
/** The fetch the vm's shipping code calls; swapped per test (see beforeEach). */
|
|
let currentFetch: (...args: unknown[]) => unknown = () => {
|
|
throw new Error('fetch not stubbed for this test');
|
|
};
|
|
|
|
function loadTerminalUiPrototype(): Record<string, (...args: unknown[]) => unknown> {
|
|
const source = readFileSync(resolve(import.meta.dirname, '../src/web/public/terminal-ui.js'), 'utf8');
|
|
const context = vm.createContext({
|
|
console,
|
|
CodemanApp: class CodemanApp {},
|
|
setInterval: vi.fn(),
|
|
clearInterval: vi.fn(),
|
|
setTimeout,
|
|
clearTimeout,
|
|
requestAnimationFrame: vi.fn(),
|
|
document: { addEventListener: vi.fn(), getElementById: vi.fn(() => null) },
|
|
window: { addEventListener: vi.fn(), removeEventListener: vi.fn() },
|
|
fetch: (...args: unknown[]) => currentFetch(...args),
|
|
});
|
|
vm.runInContext(`${source}\nglobalThis.__proto = CodemanApp.prototype;`, context);
|
|
return (context as { __proto: Record<string, (...args: unknown[]) => unknown> }).__proto;
|
|
}
|
|
|
|
const proto = loadTerminalUiPrototype();
|
|
|
|
function makeApp() {
|
|
return {
|
|
terminal: { clear: vi.fn(), writeln: vi.fn(), focus: vi.fn() },
|
|
cases: [],
|
|
resumeHistorySession: proto.resumeHistorySession as (...args: unknown[]) => Promise<void>,
|
|
_closeFolderHistoryModal: vi.fn(),
|
|
_resolveResumeName: () => 'w1-case',
|
|
loadAppSettingsFromStorage: () => ({}),
|
|
getCaseSettings: () => ({}),
|
|
buildEnvOverrides: () => ({}),
|
|
getEffortSetting: () => undefined,
|
|
selectSession: vi.fn(async () => {}),
|
|
};
|
|
}
|
|
|
|
/** DELETE calls the fetch mock recorded. */
|
|
function deleteCalls(fetchMock: ReturnType<typeof vi.fn>): string[] {
|
|
return fetchMock.mock.calls
|
|
.filter(([, opts]: [string, { method?: string }]) => opts?.method === 'DELETE')
|
|
.map(([url]: [string]) => url);
|
|
}
|
|
|
|
/** POST /api/sessions body the fetch mock recorded. */
|
|
function createBody(fetchMock: ReturnType<typeof vi.fn>): any {
|
|
const call = fetchMock.mock.calls.find(([url]: [string]) => url === '/api/sessions');
|
|
return call ? JSON.parse((call[1] as { body: string }).body) : undefined;
|
|
}
|
|
|
|
function stubFetch(newSessionId: string): ReturnType<typeof vi.fn> {
|
|
const fetchMock = vi.fn(async (url: string) => {
|
|
if (url === '/api/sessions') {
|
|
return { json: async () => ({ success: true, data: { session: { id: newSessionId } } }) };
|
|
}
|
|
return { json: async () => ({ success: true }) };
|
|
});
|
|
currentFetch = fetchMock;
|
|
return fetchMock;
|
|
}
|
|
|
|
describe('resumeHistorySession: row retirement is gated on actual continuation', () => {
|
|
let fetchMock: ReturnType<typeof vi.fn>;
|
|
|
|
beforeEach(() => {
|
|
fetchMock = stubFetch('new-session-id');
|
|
});
|
|
|
|
it.each(['codex', 'gemini', 'antigravity'])(
|
|
'does NOT retire the old row for %s (no continuation is wired for it)',
|
|
async (mode) => {
|
|
const app = makeApp();
|
|
await app.resumeHistorySession.call(app, 'old-id', '/repo', 'w1-repo', mode);
|
|
|
|
expect(createBody(fetchMock)).toMatchObject({ mode });
|
|
expect(createBody(fetchMock).codexConfig).toBeUndefined();
|
|
expect(createBody(fetchMock).geminiConfig).toBeUndefined();
|
|
expect(createBody(fetchMock).antigravityConfig).toBeUndefined();
|
|
expect(deleteCalls(fetchMock)).toEqual([]);
|
|
}
|
|
);
|
|
|
|
it.each([
|
|
['opencode', 'openCodeConfig'],
|
|
['pi', 'piConfig'],
|
|
['grok', 'grokConfig'],
|
|
['omp', 'ompConfig'],
|
|
])('retires the old row for %s (continueSession is wired via %s)', async (mode, configKey) => {
|
|
const app = makeApp();
|
|
await app.resumeHistorySession.call(app, 'old-id', '/repo', 'w1-repo', mode);
|
|
|
|
expect(createBody(fetchMock)[configKey]).toEqual({ continueSession: true });
|
|
expect(deleteCalls(fetchMock)).toEqual(['/api/sessions/old-id?killMux=true']);
|
|
});
|
|
|
|
it('retires the old row for deepseek (resumeSession is wired)', async () => {
|
|
const app = makeApp();
|
|
await app.resumeHistorySession.call(app, 'old-id', '/repo', 'w1-repo', 'deepseek');
|
|
|
|
expect(createBody(fetchMock).deepSeekConfig).toEqual({ resumeSession: true });
|
|
expect(deleteCalls(fetchMock)).toEqual(['/api/sessions/old-id?killMux=true']);
|
|
});
|
|
|
|
it('never retires a claude row (resumeSessionId is a claudeSessionId, not a Codeman row id)', async () => {
|
|
const app = makeApp();
|
|
await app.resumeHistorySession.call(app, 'claude-uuid', '/repo', 'w1-repo', 'claude');
|
|
|
|
expect(createBody(fetchMock)).toMatchObject({ mode: 'claude', resumeSessionId: 'claude-uuid' });
|
|
expect(deleteCalls(fetchMock)).toEqual([]);
|
|
});
|
|
|
|
it('never retires when the new session id equals the old one (no-op resume)', async () => {
|
|
fetchMock = stubFetch('same-id');
|
|
const app = makeApp();
|
|
await app.resumeHistorySession.call(app, 'same-id', '/repo', 'w1-repo', 'omp');
|
|
|
|
expect(deleteCalls(fetchMock)).toEqual([]);
|
|
});
|
|
});
|