mirror of
https://github.com/Ark0N/Codeman.git
synced 2026-10-06 15:39:41 +02:00
Merge remote-tracking branch 'upstream/master' into feature/run-menu-custom-model-picker
This commit is contained in:
Devvyn
@@ -0,0 +1,131 @@
|
||||
/**
|
||||
* `WebServer.discardPartiallyBuiltSession()` against the real server object.
|
||||
*
|
||||
* The reboot-restore route calls this when a rebuild registers a session and
|
||||
* then fails to start its pane. It has to be the exact inverse of
|
||||
* `registerSessionWithLayout()` plus `setupSessionListeners()`, and it must NOT
|
||||
* be the user-initiated delete: banking the session's token totals, demoting a
|
||||
* pinned record or deleting the workspace's files would all be wrong for a
|
||||
* session that never ran.
|
||||
*
|
||||
* These tests drive the real method rather than the route, because the route
|
||||
* tests run against a mock context whose `discardPartiallyBuiltSession` is a
|
||||
* one-line stub — an earlier version of this function left four registrations
|
||||
* behind and every route test still passed.
|
||||
*
|
||||
* The retry assertion is the important one. `setupSessionListeners()` returns
|
||||
* early when `sessionListenerRefs` still holds the session id, so a discard that
|
||||
* leaves that entry makes the next attempt wire nothing at all, and the user
|
||||
* gets a tab that never shows output.
|
||||
*/
|
||||
import { mkdirSync } from 'node:fs';
|
||||
import { homedir } from 'node:os';
|
||||
import { join } from 'node:path';
|
||||
import { afterAll, afterEach, beforeAll, describe, expect, it, vi } from 'vitest';
|
||||
import { safeRmHomeTree } from './mocks/test-helpers.js';
|
||||
|
||||
import { WebServer } from '../src/web/server.js';
|
||||
import { Session } from '../src/session.js';
|
||||
import { TmuxManager } from '../src/tmux-manager.js';
|
||||
|
||||
/** Reach the private collections the discard is responsible for emptying. */
|
||||
interface ServerInternals {
|
||||
sessions: Map<string, Session>;
|
||||
sessionListenerRefs: Map<string, unknown>;
|
||||
runSummaryTrackers: Map<string, unknown>;
|
||||
registerSessionWithLayout(session: Session): Promise<void>;
|
||||
setupSessionListeners(session: Session): Promise<void>;
|
||||
discardPartiallyBuiltSession(sessionId: string): Promise<void>;
|
||||
}
|
||||
|
||||
const WORKSPACE = join(homedir(), '.codeman-test-discard');
|
||||
const SESSION_ID = 'a1b2c3d4e5f60718';
|
||||
|
||||
let server: WebServer;
|
||||
let internals: ServerInternals;
|
||||
let mux: TmuxManager;
|
||||
|
||||
function buildSession(): Session {
|
||||
return new Session({
|
||||
id: SESSION_ID,
|
||||
workingDir: WORKSPACE,
|
||||
mode: 'claude',
|
||||
name: 'rebuilt session',
|
||||
mux,
|
||||
useMux: true,
|
||||
});
|
||||
}
|
||||
|
||||
beforeAll(() => {
|
||||
mkdirSync(WORKSPACE, { recursive: true });
|
||||
// Test mode: no port is opened and no CLI is launched. One server for the file,
|
||||
// stopped at the end: the constructor registers handlers on the module-level
|
||||
// image, subagent, team and workflow watchers, and only stop() removes them.
|
||||
server = new WebServer(0, false, true);
|
||||
internals = server as unknown as ServerInternals;
|
||||
mux = new TmuxManager();
|
||||
});
|
||||
|
||||
afterEach(async () => {
|
||||
await internals.discardPartiallyBuiltSession(SESSION_ID).catch(() => {});
|
||||
});
|
||||
|
||||
afterAll(async () => {
|
||||
await server.stop().catch(() => {});
|
||||
safeRmHomeTree(WORKSPACE);
|
||||
});
|
||||
|
||||
describe('discarding a session whose pane never started', () => {
|
||||
it('takes the session back out of the server', async () => {
|
||||
const session = buildSession();
|
||||
await internals.registerSessionWithLayout(session);
|
||||
await internals.setupSessionListeners(session);
|
||||
expect(internals.sessions.has(SESSION_ID)).toBe(true);
|
||||
|
||||
await internals.discardPartiallyBuiltSession(SESSION_ID);
|
||||
expect(internals.sessions.has(SESSION_ID)).toBe(false);
|
||||
});
|
||||
|
||||
it('releases the listener registration, so a retry can wire itself again', async () => {
|
||||
const first = buildSession();
|
||||
await internals.registerSessionWithLayout(first);
|
||||
await internals.setupSessionListeners(first);
|
||||
expect(internals.sessionListenerRefs.has(SESSION_ID)).toBe(true);
|
||||
|
||||
const firstRefs = internals.sessionListenerRefs.get(SESSION_ID);
|
||||
await internals.discardPartiallyBuiltSession(SESSION_ID);
|
||||
expect(internals.sessionListenerRefs.has(SESSION_ID)).toBe(false);
|
||||
|
||||
// The retry reuses the id by design. `setupSessionListeners()` returns early
|
||||
// while the refs are still there, so a session built now would run blind: no
|
||||
// terminal output, no status updates, no exit broadcast. Asserting a DIFFERENT
|
||||
// refs object is what distinguishes wiring the retry from finding the corpse
|
||||
// of the first attempt still in place.
|
||||
const retry = buildSession();
|
||||
await internals.registerSessionWithLayout(retry);
|
||||
await internals.setupSessionListeners(retry);
|
||||
const retryRefs = internals.sessionListenerRefs.get(SESSION_ID);
|
||||
expect(retryRefs).toBeDefined();
|
||||
expect(retryRefs).not.toBe(firstRefs);
|
||||
});
|
||||
|
||||
it('stops the run-summary tracker, whose interval would otherwise keep firing', async () => {
|
||||
const session = buildSession();
|
||||
await internals.registerSessionWithLayout(session);
|
||||
await internals.setupSessionListeners(session);
|
||||
const tracker = internals.runSummaryTrackers.get(SESSION_ID) as { stop: () => void };
|
||||
expect(tracker).toBeDefined();
|
||||
// Dropping the map entry is not enough: the tracker arms a setInterval in its
|
||||
// constructor, and only stop() clears it, so a discard that merely forgot the
|
||||
// entry would leave the timer running for the life of the process.
|
||||
const stopped = vi.spyOn(tracker, 'stop');
|
||||
|
||||
await internals.discardPartiallyBuiltSession(SESSION_ID);
|
||||
expect(stopped).toHaveBeenCalled();
|
||||
expect(internals.runSummaryTrackers.has(SESSION_ID)).toBe(false);
|
||||
});
|
||||
|
||||
it('does nothing at all for a session it never registered', async () => {
|
||||
await expect(internals.discardPartiallyBuiltSession('never-existed')).resolves.toBeUndefined();
|
||||
});
|
||||
});
|
||||
@@ -48,13 +48,12 @@ describe('install.sh generated-catalogue block', () => {
|
||||
);
|
||||
});
|
||||
|
||||
it('declares every array the detection code indexes', () => {
|
||||
it('declares every array install.sh actually reads', () => {
|
||||
for (const name of [
|
||||
'CLI_IDS',
|
||||
'CLI_LABELS',
|
||||
'CLI_ENABLED',
|
||||
'CLI_KIND',
|
||||
'CLI_NPM',
|
||||
'CLI_LAUNCHER_ONLY',
|
||||
'CLI_DOCS',
|
||||
'CLI_CMD_LINUX',
|
||||
'CLI_CMD_DARWIN',
|
||||
@@ -69,6 +68,17 @@ describe('install.sh generated-catalogue block', () => {
|
||||
}
|
||||
});
|
||||
|
||||
it('declares no array install.sh never reads', () => {
|
||||
// CLI_KIND and CLI_NPM were generated and read by nothing (the .mjs/docker-hosts.ts
|
||||
// producers read the JSON's `kind`/`npmPackage` fields directly; only these two bash
|
||||
// arrays were dead). A generated-but-unread array is a maintenance trap the generator
|
||||
// itself cannot warn about — it has no reader to check against — so this pins the
|
||||
// opposite of the test above: naming what must NOT come back rather than what must.
|
||||
for (const name of ['CLI_KIND', 'CLI_NPM']) {
|
||||
expect(new RegExp(`^${name}=\\(`, 'm').test(SOURCE), `${name} is declared but nothing reads it`).toBe(false);
|
||||
}
|
||||
});
|
||||
|
||||
it('keeps no hand-written per-CLI detection behind', () => {
|
||||
// The nine `*_SEARCH_PATHS` arrays and eighteen `check_<cli>`/`get_<cli>_path` pairs are
|
||||
// what this change removes. One left behind would be a second source of truth that the
|
||||
@@ -93,6 +103,16 @@ describe('install.sh generated-catalogue block', () => {
|
||||
[]
|
||||
);
|
||||
});
|
||||
|
||||
it('keeps no dead generic-lookup helpers behind', () => {
|
||||
// _cli_index/check_cli/get_cli_path were the ungenericized precursor to the per-CLI
|
||||
// helpers above: same shape, one level of indirection, called from nowhere once the
|
||||
// catalogue-driven menu and hints stopped needing a lookup-by-id. Unlike the per-CLI
|
||||
// pairs these are exact names, not derived from the catalogue.
|
||||
for (const fn of ['_cli_index()', 'check_cli()', 'get_cli_path()']) {
|
||||
expect(CODE.includes(fn), `${fn} should have been removed as dead code`).toBe(false);
|
||||
}
|
||||
});
|
||||
});
|
||||
|
||||
describe('install.sh trust boundary', () => {
|
||||
@@ -245,3 +265,42 @@ describe('install.sh AI CLI install menu', () => {
|
||||
expect(run.status).toBe(1);
|
||||
});
|
||||
});
|
||||
|
||||
describe('install.sh detect_all_clis and a disabled entry', () => {
|
||||
// No stock entry ships disabled today, so this is characterization rather than a regression
|
||||
// pin on real data: it drives the real function in a real bash with entry 0 fabricated
|
||||
// disabled, and points its binary at `bash` — guaranteed resolvable via `command -v` — to
|
||||
// prove the entry is genuinely never PROBED (CLI_FOUND_PATH stays empty) rather than merely
|
||||
// filtered out downstream by every consumer's own `CLI_ENABLED` check.
|
||||
function driveDetect(disableEntry0: boolean) {
|
||||
const driver = `
|
||||
set -euo pipefail
|
||||
export CODEMAN_INSTALL_SH_LIB=1
|
||||
. "$1"
|
||||
k=0; while [[ $k -lt \${#CLI_ALL_BINS[@]} ]]; do CLI_ALL_BINS[$k]="codeman-test-no-such-bin-$k"; k=$((k + 1)); done
|
||||
k=0; while [[ $k -lt \${#CLI_ALL_PATHS[@]} ]]; do CLI_ALL_PATHS[$k]="/nonexistent/codeman-test/$k"; k=$((k + 1)); done
|
||||
# Point entry 0's first declared binary at something that WILL resolve, so a probe that
|
||||
# runs at all finds it.
|
||||
CLI_ALL_BINS[\${CLI_BIN_OFF[0]}]="bash"
|
||||
${disableEntry0 ? 'CLI_ENABLED[0]="0"' : ''}
|
||||
CLI_DETECT_DONE=""
|
||||
detect_all_clis
|
||||
echo "path0=[\${CLI_FOUND_PATH[0]}]"
|
||||
echo "found=$CLI_FOUND_COUNT"
|
||||
`;
|
||||
const result = spawnSync('bash', ['-c', driver, 'bash', INSTALL_SH], { encoding: 'utf-8', timeout: 30_000 });
|
||||
return { status: result.status, stdout: result.stdout ?? '', stderr: result.stderr ?? '' };
|
||||
}
|
||||
|
||||
it('probes an enabled entry (control case)', () => {
|
||||
const run = driveDetect(false);
|
||||
expect(run.stdout, run.stderr).not.toContain('path0=[]');
|
||||
expect(run.stdout).toContain('found=1');
|
||||
});
|
||||
|
||||
it('never probes a disabled entry', () => {
|
||||
const run = driveDetect(true);
|
||||
expect(run.stdout, run.stderr).toContain('path0=[]');
|
||||
expect(run.stdout).toContain('found=0');
|
||||
});
|
||||
});
|
||||
|
||||
@@ -0,0 +1,28 @@
|
||||
/**
|
||||
* The mock route context must offer everything the real one does.
|
||||
*
|
||||
* Route tests pass their context as `ctx as never`, and `tsconfig.json` includes
|
||||
* only `src/**`, so no type check ever compares the mock against the ports. A
|
||||
* port that gained a method left this mock missing it twice; both times the
|
||||
* route under test threw a TypeError inside its own catch, and the suite
|
||||
* reported a plausible-looking failure for an unrelated reason.
|
||||
*
|
||||
* So the comparison is made at runtime, against `WebServer.createRouteContext()`
|
||||
* rather than against the port types, which is what keeps it from drifting: the
|
||||
* server's own context object is the thing route modules are really given.
|
||||
*/
|
||||
import { describe, expect, it } from 'vitest';
|
||||
|
||||
import { WebServer } from '../../src/web/server.js';
|
||||
import { createMockRouteContext } from './mock-route-context.js';
|
||||
|
||||
describe('the mock route context', () => {
|
||||
it('offers every member the real route context does', () => {
|
||||
const server = new WebServer(0, false, true);
|
||||
const real = (server as unknown as { createRouteContext(): Record<string, unknown> }).createRouteContext();
|
||||
const mock = createMockRouteContext() as unknown as Record<string, unknown>;
|
||||
|
||||
const missing = Object.keys(real).filter((key) => !(key in mock));
|
||||
expect(missing, `mock-route-context.ts is missing: ${missing.join(', ')}`).toEqual([]);
|
||||
});
|
||||
});
|
||||
@@ -61,6 +61,10 @@ export function createMockRouteContext(options?: {
|
||||
setupSessionListeners: vi.fn(async () => {}),
|
||||
persistSessionState: vi.fn(),
|
||||
persistSessionStateNow: vi.fn(),
|
||||
reapplyPersistedSessionState: vi.fn(async () => {}),
|
||||
discardPartiallyBuiltSession: vi.fn(async (id: string) => {
|
||||
sessions.delete(id);
|
||||
}),
|
||||
getSessionStateWithRespawn: vi.fn((s: MockSession) => s.toState()),
|
||||
|
||||
// -- EventPort --
|
||||
@@ -149,6 +153,7 @@ export function createMockRouteContext(options?: {
|
||||
clearRespawnConfig: vi.fn(),
|
||||
updateRespawnConfig: vi.fn(),
|
||||
setHistoryLimit: vi.fn(async () => {}),
|
||||
startStatsCollection: vi.fn(),
|
||||
},
|
||||
runSummaryTrackers: new Map(),
|
||||
activePlanOrchestrators: new Map(),
|
||||
|
||||
@@ -0,0 +1,395 @@
|
||||
/**
|
||||
* @fileoverview The decision half of reboot restore, and proof that the existing
|
||||
* recovery construction path can CREATE a resumed pane.
|
||||
*
|
||||
* Three things are under test. `src/reboot-restore.ts` decides whether the
|
||||
* machine rebooted and which dead sessions may be offered back. The plan
|
||||
* registry in `src/web/reboot-restore-registry.ts` holds that offer between the
|
||||
* boot that builds it and the click that spends it. The third is the claim the
|
||||
* whole feature rests on: a `Session` built the way `restoreMuxSessions()`
|
||||
* already builds one, but given no `muxSession` and a `resumeSessionId`, creates
|
||||
* a fresh pane that resumes the old conversation. If that holds, the restore
|
||||
* needs no new session-creation service.
|
||||
*
|
||||
* `reconcileSessions()` reports every session ALIVE under vitest, so the
|
||||
* server's own boot pass cannot be reached from here. The decision logic is
|
||||
* therefore driven directly, and the construction claim is driven through a real
|
||||
* `Session` against the in-memory tmux layer vitest substitutes.
|
||||
*/
|
||||
import { mkdirSync, rmSync } from 'node:fs';
|
||||
import { homedir } from 'node:os';
|
||||
import { join } from 'node:path';
|
||||
import { afterEach, describe, expect, it } from 'vitest';
|
||||
|
||||
import { Session } from '../src/session.js';
|
||||
import { TmuxManager } from '../src/tmux-manager.js';
|
||||
import type { SessionState } from '../src/types.js';
|
||||
import {
|
||||
looksLikeHostReboot,
|
||||
newestPersistedActivity,
|
||||
planRebootRestore,
|
||||
rejectAlreadyLive,
|
||||
resolveResumeConversationId,
|
||||
type RebootRestoreEntry,
|
||||
} from '../src/reboot-restore.js';
|
||||
import { RebootRestoreRegistry } from '../src/web/reboot-restore-registry.js';
|
||||
|
||||
const HOUR = 60 * 60 * 1000;
|
||||
const NOW = 1_760_000_000_000;
|
||||
|
||||
function persistedSession(overrides: Partial<SessionState> & { id: string }): SessionState {
|
||||
return {
|
||||
// A live agent's record carries its process id; `/exit` persists null instead.
|
||||
pid: 99999,
|
||||
status: 'idle',
|
||||
workingDir: '/tmp/spike',
|
||||
currentTaskId: null,
|
||||
createdAt: NOW - 4 * HOUR,
|
||||
lastActivityAt: NOW - 2 * HOUR,
|
||||
mode: 'claude',
|
||||
...overrides,
|
||||
} as SessionState;
|
||||
}
|
||||
|
||||
describe('reboot detection', () => {
|
||||
const base = {
|
||||
livePaneCount: 0,
|
||||
deadSessionCount: 2,
|
||||
// The host came up 10 minutes ago, well after the sessions were last active.
|
||||
uptimeSeconds: 600,
|
||||
newestPersistedActivityAt: NOW - 2 * HOUR,
|
||||
now: NOW,
|
||||
};
|
||||
|
||||
it('calls it a reboot when the socket is empty and the host booted after the last activity', () => {
|
||||
expect(looksLikeHostReboot(base)).toBe(true);
|
||||
});
|
||||
|
||||
it('refuses when some panes survived, which is an ordinary server restart', () => {
|
||||
expect(looksLikeHostReboot({ ...base, livePaneCount: 3 })).toBe(false);
|
||||
});
|
||||
|
||||
it('refuses on a long-uptime host, where someone wiped the tmux socket by hand', () => {
|
||||
// Up for 30 days: the sessions were active long AFTER this boot, so the panes
|
||||
// went away for some reason other than the machine restarting.
|
||||
expect(looksLikeHostReboot({ ...base, uptimeSeconds: 30 * 24 * 60 * 60 })).toBe(false);
|
||||
});
|
||||
|
||||
it('refuses when nothing died', () => {
|
||||
expect(looksLikeHostReboot({ ...base, deadSessionCount: 0 })).toBe(false);
|
||||
});
|
||||
|
||||
it('reads the newest activity stamp across the persisted records', () => {
|
||||
const persisted = {
|
||||
a: persistedSession({ id: 'a', lastActivityAt: NOW - 5 * HOUR }),
|
||||
b: persistedSession({ id: 'b', lastActivityAt: NOW - 1 * HOUR }),
|
||||
};
|
||||
expect(newestPersistedActivity(persisted)).toBe(NOW - 1 * HOUR);
|
||||
});
|
||||
});
|
||||
|
||||
describe('which dead sessions may be rebuilt', () => {
|
||||
it('rebuilds a session that was simply running when the power went out', () => {
|
||||
const persisted = { live: persistedSession({ id: 'live', status: 'busy' }) };
|
||||
const plan = planRebootRestore(['live'], persisted, () => true);
|
||||
expect(plan.restore.map((s) => s.sessionId)).toEqual(['live']);
|
||||
});
|
||||
|
||||
it('never revives a session the user killed while pinned (COD-142 demotes it to stopped)', () => {
|
||||
const persisted = { killed: persistedSession({ id: 'killed', status: 'stopped', pinned: true }) };
|
||||
const plan = planRebootRestore(['killed'], persisted, () => true);
|
||||
expect(plan.restore).toEqual([]);
|
||||
expect(plan.skipped).toEqual([{ sessionId: 'killed', reason: 'intentionally-ended' }]);
|
||||
});
|
||||
|
||||
it('never revives a session whose record an unpinned kill already deleted', () => {
|
||||
const plan = planRebootRestore(['gone'], {}, () => true);
|
||||
expect(plan.restore).toEqual([]);
|
||||
expect(plan.skipped).toEqual([{ sessionId: 'gone', reason: 'no-persisted-record' }]);
|
||||
});
|
||||
|
||||
it('never revives a pane whose PTY-exit breaker had tripped', () => {
|
||||
const persisted = { crashy: persistedSession({ id: 'crashy', respawnBlocked: true }) };
|
||||
expect(planRebootRestore(['crashy'], persisted, () => true).skipped[0].reason).toBe('respawn-blocked');
|
||||
});
|
||||
|
||||
it('leaves remote sessions to the COD-108 reconnect watcher', () => {
|
||||
const persisted = {
|
||||
r: persistedSession({
|
||||
id: 'r',
|
||||
remote: { hostId: 'h', host: 'example.test', username: 'u', sessionName: 'n', owned: true },
|
||||
} as Partial<SessionState> & { id: string }),
|
||||
};
|
||||
expect(planRebootRestore(['r'], persisted, () => true).skipped[0].reason).toBe('remote-or-docker');
|
||||
});
|
||||
|
||||
it('leaves docker sessions alone, since the container may not be up', () => {
|
||||
const persisted = {
|
||||
d: persistedSession({ id: 'd', docker: { containerId: 'abc', caseId: 'c' } } as Partial<SessionState> & {
|
||||
id: string;
|
||||
}),
|
||||
};
|
||||
expect(planRebootRestore(['d'], persisted, () => true).skipped[0].reason).toBe('remote-or-docker');
|
||||
});
|
||||
|
||||
it('skips a CLI whose history the claude transcript reader does not understand', () => {
|
||||
const persisted = { c: persistedSession({ id: 'c', mode: 'codex' }) };
|
||||
expect(planRebootRestore(['c'], persisted, () => true).skipped[0].reason).toBe('unsupported-mode');
|
||||
});
|
||||
});
|
||||
|
||||
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 attached 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', () => {
|
||||
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 plan = planRebootRestore(['gone'], persisted, () => false);
|
||||
expect(plan.restore).toEqual([]);
|
||||
expect(plan.skipped).toEqual([{ sessionId: 'gone', reason: 'workspace-missing' }]);
|
||||
});
|
||||
|
||||
it('is judged per session, not for the batch', () => {
|
||||
const persisted = {
|
||||
kept: persistedSession({ id: 'kept', workingDir: '/tmp/still-here' }),
|
||||
gone: persistedSession({ id: 'gone', workingDir: '/tmp/deleted-repo' }),
|
||||
};
|
||||
const plan = planRebootRestore(['kept', 'gone'], persisted, (dir) => dir === '/tmp/still-here');
|
||||
expect(plan.restore.map((entry) => entry.sessionId)).toEqual(['kept']);
|
||||
expect(plan.skipped.map((s) => s.reason)).toEqual(['workspace-missing']);
|
||||
});
|
||||
});
|
||||
|
||||
describe('a conversation that came back on its own before the click', () => {
|
||||
const entry: RebootRestoreEntry = {
|
||||
sessionId: 'abc',
|
||||
workingDir: '/tmp/spike',
|
||||
mode: 'claude',
|
||||
resumeConversationId: 'conv-1',
|
||||
state: persistedSession({ id: 'abc' }),
|
||||
};
|
||||
|
||||
it('is skipped when the user resumed it by hand from the Resume list', () => {
|
||||
// Same conversation, different session id: the Resume list creates a NEW id.
|
||||
const result = rejectAlreadyLive([entry], new Set(['other']), new Set(['conv-1']));
|
||||
expect(result.restore).toEqual([]);
|
||||
expect(result.skipped).toEqual([{ sessionId: 'abc', reason: 'already-live' }]);
|
||||
});
|
||||
|
||||
it('is skipped when a session with that id is already on the board', () => {
|
||||
const result = rejectAlreadyLive([entry], new Set(['abc']), new Set());
|
||||
expect(result.skipped).toEqual([{ sessionId: 'abc', reason: 'already-live' }]);
|
||||
});
|
||||
|
||||
it('is rebuilt when neither its id nor its conversation is live', () => {
|
||||
const result = rejectAlreadyLive([entry], new Set(['other']), new Set(['conv-other']));
|
||||
expect(result.restore.map((e) => e.sessionId)).toEqual(['abc']);
|
||||
expect(result.skipped).toEqual([]);
|
||||
});
|
||||
});
|
||||
|
||||
describe('the plan the banner spends', () => {
|
||||
const all = () => true;
|
||||
const entryFor = (sessionId: string, owner?: string): RebootRestoreEntry => ({
|
||||
sessionId,
|
||||
owner,
|
||||
workingDir: '/tmp/spike',
|
||||
mode: 'claude',
|
||||
resumeConversationId: `conv-${sessionId}`,
|
||||
state: persistedSession({ id: sessionId, owner }),
|
||||
});
|
||||
|
||||
it('hands an entry to the first caller and nothing to the second', () => {
|
||||
const registry = new RebootRestoreRegistry();
|
||||
registry.set([entryFor('a'), entryFor('b')]);
|
||||
expect(registry.take(all, undefined, undefined).map((e) => e.sessionId)).toEqual(['a', 'b']);
|
||||
// The double-click: two panes on one conversation is what this prevents.
|
||||
expect(registry.take(all, undefined, undefined)).toEqual([]);
|
||||
});
|
||||
|
||||
it('spends only the ids a caller asked for', () => {
|
||||
const registry = new RebootRestoreRegistry();
|
||||
registry.set([entryFor('a'), entryFor('b')]);
|
||||
expect(registry.take(all, ['b'], undefined).map((e) => e.sessionId)).toEqual(['b']);
|
||||
expect(registry.list(all).map((e) => e.sessionId)).toEqual(['a']);
|
||||
});
|
||||
|
||||
it("shows a user their own sessions and leaves another owner's alone", () => {
|
||||
const registry = new RebootRestoreRegistry();
|
||||
registry.set([entryFor('mine', 'alice'), entryFor('theirs', 'bob')]);
|
||||
const asAlice = (owner: string | undefined) => owner === 'alice';
|
||||
expect(registry.list(asAlice).map((e) => e.sessionId)).toEqual(['mine']);
|
||||
expect(registry.take(asAlice, undefined, 'alice').map((e) => e.sessionId)).toEqual(['mine']);
|
||||
// Bob's entry is still on offer for Bob.
|
||||
expect(registry.list(() => true).map((e) => e.sessionId)).toEqual(['theirs']);
|
||||
});
|
||||
|
||||
it('puts back an entry that no pane was created for', () => {
|
||||
const registry = new RebootRestoreRegistry();
|
||||
registry.set([entryFor('a')]);
|
||||
const taken = registry.take(all, undefined, undefined);
|
||||
registry.releaseFlight(undefined, taken);
|
||||
expect(registry.list(all).map((e) => e.sessionId)).toEqual(['a']);
|
||||
});
|
||||
|
||||
it('runs one restore at a time', () => {
|
||||
const registry = new RebootRestoreRegistry();
|
||||
expect(registry.beginSpending()).toBe(true);
|
||||
expect(registry.beginSpending()).toBe(false);
|
||||
registry.endSpending();
|
||||
expect(registry.beginSpending()).toBe(true);
|
||||
});
|
||||
|
||||
it('drops what a dismiss cleared', () => {
|
||||
const registry = new RebootRestoreRegistry();
|
||||
registry.set([entryFor('a'), entryFor('b')]);
|
||||
expect(registry.clear(all)).toBe(2);
|
||||
expect(registry.list(all)).toEqual([]);
|
||||
});
|
||||
|
||||
it('forgets a plan nobody took for a day', () => {
|
||||
const registry = new RebootRestoreRegistry();
|
||||
registry.set([entryFor('a')]);
|
||||
const dayLater = Date.now() + 25 * HOUR;
|
||||
const realNow = Date.now;
|
||||
Date.now = () => dayLater;
|
||||
try {
|
||||
expect(registry.list(all)).toEqual([]);
|
||||
} finally {
|
||||
Date.now = realNow;
|
||||
}
|
||||
});
|
||||
});
|
||||
|
||||
describe('which conversation a rebuilt pane resumes', () => {
|
||||
it('prefers the chain tail, the conversation the CLI reported last', () => {
|
||||
const state = persistedSession({
|
||||
id: 'sess-1',
|
||||
resumeSessionId: 'launch-id',
|
||||
claudeSessionChain: ['launch-id', 'after-clear'],
|
||||
});
|
||||
expect(resolveResumeConversationId(state)).toBe('after-clear');
|
||||
});
|
||||
|
||||
it('falls back to the id the session originally resumed', () => {
|
||||
const state = persistedSession({ id: 'sess-1', resumeSessionId: 'resumed-id' });
|
||||
expect(resolveResumeConversationId(state)).toBe('resumed-id');
|
||||
});
|
||||
|
||||
it('falls back to the session id, which is what Claude was launched with', () => {
|
||||
expect(resolveResumeConversationId(persistedSession({ id: 'sess-1' }))).toBe('sess-1');
|
||||
});
|
||||
});
|
||||
|
||||
describe('the recovery construction path can create a resumed pane', () => {
|
||||
const workingDir = join(homedir(), 'codeman-cases', 'reboot-restore-spike');
|
||||
const sessions: Session[] = [];
|
||||
|
||||
afterEach(() => {
|
||||
for (const s of sessions.splice(0)) s.stop();
|
||||
rmSync(workingDir, { recursive: true, force: true });
|
||||
});
|
||||
|
||||
/** Built exactly as the reboot pass builds one: no `muxSession`, plus a resume id. */
|
||||
function rebuildFromPersistedState(state: SessionState, mux: TmuxManager): Session {
|
||||
mkdirSync(workingDir, { recursive: true });
|
||||
const session = new Session({
|
||||
id: state.id,
|
||||
workingDir,
|
||||
mode: state.mode,
|
||||
name: state.name,
|
||||
createdAt: state.createdAt,
|
||||
mux,
|
||||
useMux: true,
|
||||
resumeSessionId: resolveResumeConversationId(state),
|
||||
owner: state.owner,
|
||||
lastActivityAt: state.lastActivityAt,
|
||||
claudeSessionChain: state.claudeSessionChain,
|
||||
});
|
||||
sessions.push(session);
|
||||
return session;
|
||||
}
|
||||
|
||||
it('creates a NEW mux session rather than needing one to attach to', async () => {
|
||||
const mux = new TmuxManager();
|
||||
const state = persistedSession({ id: 'aaaaaaa1-1111-4111-8111-111111111111', name: 'w1-spike' });
|
||||
const session = rebuildFromPersistedState(state, mux);
|
||||
|
||||
expect(mux.getSessions()).toHaveLength(0);
|
||||
await session.startInteractive();
|
||||
|
||||
const created = mux.getSessions();
|
||||
expect(created).toHaveLength(1);
|
||||
expect(created[0].sessionId).toBe('aaaaaaa1-1111-4111-8111-111111111111');
|
||||
expect(created[0].workingDir).toBe(workingDir);
|
||||
});
|
||||
|
||||
it('comes back pointed at the conversation the pane was holding', async () => {
|
||||
const mux = new TmuxManager();
|
||||
const state = persistedSession({
|
||||
id: 'aaaaaaa2-2222-4222-8222-222222222222',
|
||||
resumeSessionId: 'launch-id',
|
||||
claudeSessionChain: ['launch-id', 'after-clear'],
|
||||
});
|
||||
const session = rebuildFromPersistedState(state, mux);
|
||||
|
||||
await session.startInteractive();
|
||||
|
||||
// The chain tail wins: a `/clear` before the reboot moved the CLI off the launch id.
|
||||
expect(session.claudeSessionId).toBe('after-clear');
|
||||
});
|
||||
|
||||
it('comes back idle, with no prompt sent and no autonomous loop armed', async () => {
|
||||
const mux = new TmuxManager();
|
||||
const state = persistedSession({
|
||||
id: 'aaaaaaa3-3333-4333-8333-333333333333',
|
||||
ralphEnabled: true,
|
||||
respawnEnabled: true,
|
||||
});
|
||||
const session = rebuildFromPersistedState(state, mux);
|
||||
|
||||
await session.startInteractive();
|
||||
|
||||
// No prompt was queued: nothing is waiting on a task. The status itself is not
|
||||
// assertable here, because the test PTY echoes and the activity detector reads
|
||||
// that echo as work; in production the pane settles once the CLI finishes booting.
|
||||
expect(session.currentTaskId).toBeNull();
|
||||
// The pass never touches the tracker, so a persisted Ralph loop stays cold.
|
||||
expect(session.ralphTracker.enabled).toBe(false);
|
||||
});
|
||||
|
||||
it('keeps the owner it was persisted with, there being no request to read one from', async () => {
|
||||
const mux = new TmuxManager();
|
||||
const state = persistedSession({ id: 'aaaaaaa4-4444-4444-8444-444444444444', owner: 'alice' });
|
||||
const session = rebuildFromPersistedState(state, mux);
|
||||
|
||||
await session.startInteractive();
|
||||
|
||||
expect(session.owner).toBe('alice');
|
||||
expect(mux.getSessions()[0].owner).toBe('alice');
|
||||
});
|
||||
});
|
||||
@@ -0,0 +1,377 @@
|
||||
/**
|
||||
* Reboot-restore route: what happens when a rebuild gets part-way and then fails.
|
||||
*
|
||||
* The other route test file deliberately uses workspaces that do not exist, so it
|
||||
* never reaches `new Session()`. This one mocks the `Session` module so the route
|
||||
* runs its whole construction path — `addSession`, `setupSessionListeners`,
|
||||
* `reapplyPersistedSessionState`, `startInteractive` — and then throws.
|
||||
*
|
||||
* The mock is the only way in. Driven against a real server, `startInteractive()`
|
||||
* does not throw for either obvious cause: the CLI resolver finds its binary by
|
||||
* absolute path rather than through PATH, and tmux falls back to another
|
||||
* directory rather than failing when it cannot enter the workspace. A mux-layer
|
||||
* failure is what is left, and it cannot be provoked from a test. Without the
|
||||
* mock this path would go unexercised, which is how the original version of this
|
||||
* route shipped a session leak the tests could not see.
|
||||
*
|
||||
* It also covers the session caps, because those too are only reachable once the
|
||||
* route is actually willing to build something.
|
||||
*/
|
||||
import { describe, it, expect, afterEach, vi, beforeEach } from 'vitest';
|
||||
import Fastify, { type FastifyInstance } from 'fastify';
|
||||
import fastifyCookie from '@fastify/cookie';
|
||||
|
||||
/** Set per test: whether the mocked `startInteractive()` rejects. */
|
||||
let startShouldThrow = false;
|
||||
/** Ordering log, so a test can assert what ran before the pane spawned. */
|
||||
const callOrder: string[] = [];
|
||||
|
||||
vi.mock('../../src/session.js', () => ({
|
||||
Session: class {
|
||||
id: string;
|
||||
mode: string;
|
||||
name?: string;
|
||||
workingDir: string;
|
||||
owner?: string;
|
||||
claudeSessionId: string | null = null;
|
||||
constructor(config: { id: string; mode?: string; name?: string; workingDir: string; owner?: string }) {
|
||||
this.id = config.id;
|
||||
this.mode = config.mode ?? 'claude';
|
||||
this.name = config.name;
|
||||
this.workingDir = config.workingDir;
|
||||
this.owner = config.owner;
|
||||
}
|
||||
async startInteractive() {
|
||||
callOrder.push('startInteractive');
|
||||
if (startShouldThrow) throw new Error('spawn claude ENOENT');
|
||||
}
|
||||
/** The mock route context projects a session through this on broadcast. */
|
||||
toState() {
|
||||
return { id: this.id, mode: this.mode, name: this.name, workingDir: this.workingDir, owner: this.owner };
|
||||
}
|
||||
},
|
||||
}));
|
||||
|
||||
const { registerRebootRestoreRoutes } = await import('../../src/web/routes/reboot-restore-routes.js');
|
||||
const { rebootRestoreRegistry } = await import('../../src/web/reboot-restore-registry.js');
|
||||
const { installRouteErrorHandler } = await import('../../src/web/route-error-handler.js');
|
||||
const { httpStatusForErrorCode } = await import('../../src/types.js');
|
||||
const { createMockRouteContext } = await import('../mocks/index.js');
|
||||
type ApiErrorCode = import('../../src/types.js').ApiErrorCode;
|
||||
type RebootRestoreEntry = import('../../src/reboot-restore.js').RebootRestoreEntry;
|
||||
type SessionState = import('../../src/types.js').SessionState;
|
||||
|
||||
/** A real directory, so the route's workspace checks pass and it reaches the build. */
|
||||
const WORKSPACE = process.cwd();
|
||||
|
||||
function offerEntry(sessionId: string, owner?: string): RebootRestoreEntry {
|
||||
return {
|
||||
sessionId,
|
||||
name: `session ${sessionId}`,
|
||||
workingDir: WORKSPACE,
|
||||
owner,
|
||||
mode: 'claude',
|
||||
resumeConversationId: `conv-${sessionId}`,
|
||||
state: {
|
||||
id: sessionId,
|
||||
pid: null,
|
||||
status: 'idle',
|
||||
workingDir: WORKSPACE,
|
||||
currentTaskId: null,
|
||||
createdAt: 1_760_000_000_000,
|
||||
mode: 'claude',
|
||||
owner,
|
||||
} as SessionState,
|
||||
};
|
||||
}
|
||||
|
||||
async function createHarness(ctx: ReturnType<typeof createMockRouteContext>): Promise<FastifyInstance> {
|
||||
const app = Fastify({ logger: false });
|
||||
await app.register(fastifyCookie);
|
||||
registerRebootRestoreRoutes(app, ctx as never);
|
||||
app.addHook('preSerialization', (req, reply, payload: unknown, done) => {
|
||||
if (!req.url.startsWith('/api')) return done(null, payload);
|
||||
if (payload === null || typeof payload !== 'object') return done(null, payload);
|
||||
const p = payload as { success?: unknown; errorCode?: unknown };
|
||||
if (p.success === false) {
|
||||
if (reply.statusCode === 200 && typeof p.errorCode === 'string') {
|
||||
reply.code(httpStatusForErrorCode(p.errorCode as ApiErrorCode));
|
||||
}
|
||||
return done(null, payload);
|
||||
}
|
||||
if (p.success === true) return done(null, payload);
|
||||
return done(null, { success: true, data: payload });
|
||||
});
|
||||
installRouteErrorHandler(app);
|
||||
await app.ready();
|
||||
return app;
|
||||
}
|
||||
|
||||
beforeEach(() => {
|
||||
startShouldThrow = false;
|
||||
callOrder.length = 0;
|
||||
});
|
||||
|
||||
afterEach(() => {
|
||||
rebootRestoreRegistry.reset();
|
||||
vi.clearAllMocks();
|
||||
});
|
||||
|
||||
describe('a rebuild that fails after the session is registered', () => {
|
||||
it('reports why it failed rather than blaming the workspace', async () => {
|
||||
startShouldThrow = true;
|
||||
rebootRestoreRegistry.set([offerEntry('a')]);
|
||||
const ctx = createMockRouteContext({ workspaceHooksEnabled: false });
|
||||
const app = await createHarness(ctx);
|
||||
|
||||
const res = (await app.inject({ method: 'POST', url: '/api/reboot-restore/restore', payload: {} })).json().data;
|
||||
expect(res.restored).toEqual([]);
|
||||
// Not `workspace-missing`: the directory is there, the agent would not start.
|
||||
expect(res.skipped).toEqual([{ sessionId: 'a', reason: 'rebuild-failed' }]);
|
||||
await app.close();
|
||||
});
|
||||
|
||||
it('does not leave a registered session with no pane behind it', async () => {
|
||||
startShouldThrow = true;
|
||||
rebootRestoreRegistry.set([offerEntry('a')]);
|
||||
const ctx = createMockRouteContext({ workspaceHooksEnabled: false });
|
||||
const app = await createHarness(ctx);
|
||||
|
||||
await app.inject({ method: 'POST', url: '/api/reboot-restore/restore', payload: {} });
|
||||
// The session reached ctx.sessions via addSession; the route has to take it
|
||||
// back out, or the board shows a tab whose pane never existed.
|
||||
expect(ctx.discardPartiallyBuiltSession).toHaveBeenCalledWith('a');
|
||||
expect(ctx.sessions.has('a')).toBe(false);
|
||||
// NOT the user-initiated delete: that would bank this session's historical
|
||||
// tokens into the lifetime totals, demote a pinned record to `stopped`, and
|
||||
// delete the workspace's .claude-images.
|
||||
expect(ctx.cleanupSession).not.toHaveBeenCalled();
|
||||
await app.close();
|
||||
});
|
||||
|
||||
it('keeps the entry on offer, so the user can fix the PATH and click again', async () => {
|
||||
startShouldThrow = true;
|
||||
rebootRestoreRegistry.set([offerEntry('a')]);
|
||||
const ctx = createMockRouteContext({ workspaceHooksEnabled: false });
|
||||
const app = await createHarness(ctx);
|
||||
|
||||
await app.inject({ method: 'POST', url: '/api/reboot-restore/restore', payload: {} });
|
||||
const left = (await app.inject({ method: 'GET', url: '/api/reboot-restore' })).json().data;
|
||||
expect(left.sessions.map((s: { id: string }) => s.id)).toEqual(['a']);
|
||||
await app.close();
|
||||
});
|
||||
});
|
||||
|
||||
describe('a rebuild that succeeds', () => {
|
||||
it('re-applies the persisted state before the record is written again', async () => {
|
||||
rebootRestoreRegistry.set([offerEntry('a')]);
|
||||
const ctx = createMockRouteContext({ workspaceHooksEnabled: false });
|
||||
const app = await createHarness(ctx);
|
||||
|
||||
const res = (await app.inject({ method: 'POST', url: '/api/reboot-restore/restore', payload: {} })).json().data;
|
||||
expect(res.restored.map((s: { id: string }) => s.id)).toEqual(['a']);
|
||||
// A session built from a record carries none of the pin, token totals or
|
||||
// custom-model selection, so persisting it first would replace the fuller
|
||||
// record with the reduced one.
|
||||
expect(ctx.reapplyPersistedSessionState).toHaveBeenCalled();
|
||||
const reapplyOrder = (ctx.reapplyPersistedSessionState as ReturnType<typeof vi.fn>).mock.invocationCallOrder[0];
|
||||
const persistOrder = (ctx.persistSessionState as ReturnType<typeof vi.fn>).mock.invocationCallOrder[0];
|
||||
expect(reapplyOrder).toBeLessThan(persistOrder);
|
||||
await app.close();
|
||||
});
|
||||
|
||||
it('shapes the pane before it spawns, and restores the history after', async () => {
|
||||
rebootRestoreRegistry.set([offerEntry('a')]);
|
||||
const ctx = createMockRouteContext({ workspaceHooksEnabled: false });
|
||||
(ctx.reapplyPersistedSessionState as ReturnType<typeof vi.fn>).mockImplementation(
|
||||
async (_s: unknown, _saved: unknown, phase: string) => {
|
||||
callOrder.push(`reapply:${phase}`);
|
||||
}
|
||||
);
|
||||
(ctx.setupSessionListeners as ReturnType<typeof vi.fn>).mockImplementation(async () => {
|
||||
callOrder.push('setupSessionListeners');
|
||||
});
|
||||
const app = await createHarness(ctx);
|
||||
|
||||
await app.inject({ method: 'POST', url: '/api/reboot-restore/restore', payload: {} });
|
||||
// `setupSessionListeners()` READS the image-watcher flag that `before-spawn`
|
||||
// restores, so the phase has to precede it or the session comes back
|
||||
// reporting the watcher as on with nothing watching. The custom-model
|
||||
// environment has to reach the process, and the token totals must not land
|
||||
// on a session whose pane never started.
|
||||
expect(callOrder).toEqual([
|
||||
'reapply:before-spawn',
|
||||
'setupSessionListeners',
|
||||
'startInteractive',
|
||||
'reapply:after-spawn',
|
||||
]);
|
||||
await app.close();
|
||||
});
|
||||
|
||||
it('tells every other board about the rebuilt session', async () => {
|
||||
rebootRestoreRegistry.set([offerEntry('a')]);
|
||||
const ctx = createMockRouteContext({ workspaceHooksEnabled: false });
|
||||
const app = await createHarness(ctx);
|
||||
|
||||
await app.inject({ method: 'POST', url: '/api/reboot-restore/restore', payload: {} });
|
||||
expect(ctx.broadcast).toHaveBeenCalledWith('session:created', expect.anything());
|
||||
await app.close();
|
||||
});
|
||||
|
||||
it('spends the entry, so it is no longer on offer', async () => {
|
||||
rebootRestoreRegistry.set([offerEntry('a')]);
|
||||
const ctx = createMockRouteContext({ workspaceHooksEnabled: false });
|
||||
const app = await createHarness(ctx);
|
||||
|
||||
await app.inject({ method: 'POST', url: '/api/reboot-restore/restore', payload: {} });
|
||||
const left = (await app.inject({ method: 'GET', url: '/api/reboot-restore' })).json().data;
|
||||
expect(left.sessions).toEqual([]);
|
||||
await app.close();
|
||||
});
|
||||
});
|
||||
|
||||
describe('the session caps', () => {
|
||||
it('counts the sessions it is itself creating, not just the ones it started with', async () => {
|
||||
rebootRestoreRegistry.set([offerEntry('a'), offerEntry('b')]);
|
||||
const ctx = createMockRouteContext({ workspaceHooksEnabled: false });
|
||||
// One seat short of the documented maximum of 50, counting the session the
|
||||
// mock context seeds. A check that ran once before the loop would restore
|
||||
// BOTH entries; only a per-iteration check refuses the second.
|
||||
for (let i = 0; i < 48; i += 1) {
|
||||
ctx.sessions.set(`filler-${i}`, { id: `filler-${i}`, owner: undefined } as never);
|
||||
}
|
||||
const app = await createHarness(ctx);
|
||||
|
||||
const res = (await app.inject({ method: 'POST', url: '/api/reboot-restore/restore', payload: {} })).json().data;
|
||||
expect(res.restored.map((s: { id: string }) => s.id)).toEqual(['a']);
|
||||
expect(res.skipped).toEqual([{ sessionId: 'b', reason: 'capacity-reached' }]);
|
||||
|
||||
// Refused rather than lost: closing a session and clicking again works.
|
||||
const left = (await app.inject({ method: 'GET', url: '/api/reboot-restore' })).json().data;
|
||||
expect(left.sessions.map((s: { id: string }) => s.id)).toEqual(['b']);
|
||||
await app.close();
|
||||
});
|
||||
|
||||
it('refuses every entry when the board is already at the cap', async () => {
|
||||
rebootRestoreRegistry.set([offerEntry('a'), offerEntry('b')]);
|
||||
const ctx = createMockRouteContext({ workspaceHooksEnabled: false });
|
||||
for (let i = 0; i < 50; i += 1) {
|
||||
ctx.sessions.set(`filler-${i}`, { id: `filler-${i}`, owner: undefined } as never);
|
||||
}
|
||||
const app = await createHarness(ctx);
|
||||
|
||||
const res = (await app.inject({ method: 'POST', url: '/api/reboot-restore/restore', payload: {} })).json().data;
|
||||
expect(res.restored).toEqual([]);
|
||||
expect(res.skipped.map((s: { reason: string }) => s.reason)).toEqual(['capacity-reached', 'capacity-reached']);
|
||||
await app.close();
|
||||
});
|
||||
});
|
||||
|
||||
describe('a failure before any entry is considered', () => {
|
||||
it('returns the whole plan rather than spending it', async () => {
|
||||
rebootRestoreRegistry.set([offerEntry('a'), offerEntry('b')]);
|
||||
const ctx = createMockRouteContext({ workspaceHooksEnabled: false });
|
||||
(ctx.getWorkspaceHooksEnabled as ReturnType<typeof vi.fn>).mockRejectedValue(new Error('settings unreadable'));
|
||||
const app = await createHarness(ctx);
|
||||
|
||||
const res = await app.inject({ method: 'POST', url: '/api/reboot-restore/restore', payload: {} });
|
||||
expect(res.statusCode).toBeGreaterThanOrEqual(500);
|
||||
|
||||
// The plan cannot be rebuilt once boot has pruned the records, so a throw
|
||||
// anywhere in the route has to hand the entries back.
|
||||
const left = (await app.inject({ method: 'GET', url: '/api/reboot-restore' })).json().data;
|
||||
expect(left.sessions.map((s: { id: string }) => s.id).sort()).toEqual(['a', 'b']);
|
||||
await app.close();
|
||||
});
|
||||
|
||||
it('releases the single flight, so the next click is not refused', async () => {
|
||||
rebootRestoreRegistry.set([offerEntry('a')]);
|
||||
const ctx = createMockRouteContext({ workspaceHooksEnabled: false });
|
||||
(ctx.getWorkspaceHooksEnabled as ReturnType<typeof vi.fn>).mockRejectedValue(new Error('settings unreadable'));
|
||||
const app = await createHarness(ctx);
|
||||
|
||||
await app.inject({ method: 'POST', url: '/api/reboot-restore/restore', payload: {} });
|
||||
expect(rebootRestoreRegistry.beginSpending(undefined)).toBe(true);
|
||||
rebootRestoreRegistry.endSpending(undefined);
|
||||
await app.close();
|
||||
});
|
||||
});
|
||||
|
||||
describe('a dismiss that lands while a restore is running', () => {
|
||||
it('wins when an admin is restoring the entries and their owner dismisses', async () => {
|
||||
const theirs = offerEntry('theirs', 'bob');
|
||||
rebootRestoreRegistry.set([theirs]);
|
||||
// An admin may spend another user's entries, so the caller doing the restore
|
||||
// and the owner of what is being restored are different people.
|
||||
const taken = rebootRestoreRegistry.take(() => true, undefined, 'admin');
|
||||
expect(taken.map((e) => e.sessionId)).toEqual(['theirs']);
|
||||
|
||||
// Bob dismisses his own banner. Nothing of his is in the plan any more, and
|
||||
// the restore is running under a different name than his.
|
||||
rebootRestoreRegistry.clear((owner) => owner === 'bob');
|
||||
rebootRestoreRegistry.releaseFlight('admin', taken);
|
||||
|
||||
expect(rebootRestoreRegistry.list(() => true)).toEqual([]);
|
||||
});
|
||||
|
||||
it('wins when an admin dismisses everything mid-restore', async () => {
|
||||
rebootRestoreRegistry.set([offerEntry('theirs', 'bob')]);
|
||||
const taken = rebootRestoreRegistry.take(() => true, undefined, 'admin');
|
||||
|
||||
rebootRestoreRegistry.clear(() => true);
|
||||
rebootRestoreRegistry.releaseFlight('admin', taken);
|
||||
|
||||
expect(rebootRestoreRegistry.list(() => true)).toEqual([]);
|
||||
});
|
||||
|
||||
it('wins, rather than being undone when the route hands its entries back', async () => {
|
||||
rebootRestoreRegistry.set([offerEntry('a')]);
|
||||
const ctx = createMockRouteContext({ workspaceHooksEnabled: false });
|
||||
// The user clicks Dismiss while the restore is between its take and its
|
||||
// return. Driven through the ROUTE, so removing the generation argument from
|
||||
// the route would make this fail.
|
||||
(ctx.getWorkspaceHooksEnabled as ReturnType<typeof vi.fn>).mockImplementation(async () => {
|
||||
rebootRestoreRegistry.clear(() => true);
|
||||
throw new Error('settings unreadable');
|
||||
});
|
||||
const app = await createHarness(ctx);
|
||||
|
||||
await app.inject({ method: 'POST', url: '/api/reboot-restore/restore', payload: {} });
|
||||
|
||||
const left = (await app.inject({ method: 'GET', url: '/api/reboot-restore' })).json().data;
|
||||
expect(left.sessions).toEqual([]);
|
||||
await app.close();
|
||||
});
|
||||
|
||||
it('reaches an in-flight restore the dismisser can see, even once its entries are taken', async () => {
|
||||
const mine = offerEntry('mine', 'alice');
|
||||
rebootRestoreRegistry.set([mine]);
|
||||
const taken = rebootRestoreRegistry.take((owner) => owner === 'alice', undefined, 'alice');
|
||||
expect(taken).toHaveLength(1);
|
||||
|
||||
// The plan is empty now, so the dismiss has nothing of Alice's left in the
|
||||
// plan; it has to reach the entry the restore is holding.
|
||||
rebootRestoreRegistry.clear((owner) => owner === 'alice');
|
||||
rebootRestoreRegistry.releaseFlight('alice', taken);
|
||||
|
||||
expect(rebootRestoreRegistry.list(() => true)).toEqual([]);
|
||||
});
|
||||
|
||||
it('does not reach another owner, whose unspent entries still come back', async () => {
|
||||
const mine = offerEntry('mine', 'alice');
|
||||
const theirs = offerEntry('theirs', 'bob');
|
||||
rebootRestoreRegistry.set([mine, theirs]);
|
||||
|
||||
// Bob is mid-restore, holding his own entry.
|
||||
const bobsTaken = rebootRestoreRegistry.take((owner) => owner === 'bob', undefined, 'bob');
|
||||
expect(bobsTaken.map((e) => e.sessionId)).toEqual(['theirs']);
|
||||
|
||||
// Alice dismisses her own banner meanwhile.
|
||||
rebootRestoreRegistry.clear((owner) => owner === 'alice');
|
||||
|
||||
// Bob's restore finishes and hands his entry back. Alice's dismiss covered
|
||||
// her entries, not his, so his offer survives.
|
||||
rebootRestoreRegistry.releaseFlight('bob', bobsTaken);
|
||||
expect(rebootRestoreRegistry.list(() => true).map((e) => e.sessionId)).toEqual(['theirs']);
|
||||
});
|
||||
});
|
||||
@@ -0,0 +1,248 @@
|
||||
/**
|
||||
* Reboot-restore route tests (src/web/routes/reboot-restore-routes.ts) via
|
||||
* app.inject(), no live port.
|
||||
*
|
||||
* Every entry these tests put on offer names a workspace that does not exist, so
|
||||
* the route's click-time workspace check rejects it before any `Session` is
|
||||
* constructed. That keeps the tests on the route's own guards — taking, scoping,
|
||||
* single-flighting and re-checking — and leaves pane creation to
|
||||
* test/reboot-restore.test.ts, which drives a real `Session` for it.
|
||||
*
|
||||
* The routes read the process-wide `rebootRestoreRegistry` singleton, so every
|
||||
* test resets it; a leaked entry would bleed into the next one.
|
||||
*/
|
||||
import { describe, it, expect, afterEach, beforeEach } from 'vitest';
|
||||
import { mkdtempSync, rmSync } from 'node:fs';
|
||||
import { tmpdir } from 'node:os';
|
||||
import { join } from 'node:path';
|
||||
import Fastify, { type FastifyInstance } from 'fastify';
|
||||
import fastifyCookie from '@fastify/cookie';
|
||||
import { registerRebootRestoreRoutes } from '../../src/web/routes/reboot-restore-routes.js';
|
||||
import { rebootRestoreRegistry } from '../../src/web/reboot-restore-registry.js';
|
||||
import { installRouteErrorHandler } from '../../src/web/route-error-handler.js';
|
||||
import { httpStatusForErrorCode, type ApiErrorCode } from '../../src/types.js';
|
||||
import { createMockRouteContext } from '../mocks/index.js';
|
||||
import type { RebootRestoreEntry } from '../../src/reboot-restore.js';
|
||||
import type { SessionState } from '../../src/types.js';
|
||||
|
||||
async function createHarness(authUser?: { username: string; role: 'admin' | 'user' }): Promise<FastifyInstance> {
|
||||
return createHarnessWithCtx(createMockRouteContext(), authUser);
|
||||
}
|
||||
|
||||
async function createHarnessWithCtx(
|
||||
ctx: ReturnType<typeof createMockRouteContext>,
|
||||
authUser?: { username: string; role: 'admin' | 'user' }
|
||||
): Promise<FastifyInstance> {
|
||||
const app = Fastify({ logger: false });
|
||||
await app.register(fastifyCookie);
|
||||
if (authUser) {
|
||||
app.addHook('onRequest', async (req) => {
|
||||
(req as unknown as { authUser: typeof authUser }).authUser = authUser;
|
||||
});
|
||||
}
|
||||
registerRebootRestoreRoutes(app, ctx as never);
|
||||
|
||||
app.addHook('preSerialization', (req, reply, payload: unknown, done) => {
|
||||
if (!req.url.startsWith('/api')) return done(null, payload);
|
||||
if (payload === null || typeof payload !== 'object') return done(null, payload);
|
||||
const p = payload as { success?: unknown; errorCode?: unknown };
|
||||
if (p.success === false) {
|
||||
if (reply.statusCode === 200 && typeof p.errorCode === 'string') {
|
||||
reply.code(httpStatusForErrorCode(p.errorCode as ApiErrorCode));
|
||||
}
|
||||
return done(null, payload);
|
||||
}
|
||||
if (p.success === true) return done(null, payload);
|
||||
return done(null, { success: true, data: payload });
|
||||
});
|
||||
|
||||
installRouteErrorHandler(app);
|
||||
await app.ready();
|
||||
return app;
|
||||
}
|
||||
|
||||
/** An entry whose workspace is deliberately absent, so no pane is ever created. */
|
||||
function offerEntry(sessionId: string, owner?: string): RebootRestoreEntry {
|
||||
return {
|
||||
sessionId,
|
||||
name: `session ${sessionId}`,
|
||||
workingDir: `/tmp/codeman-reboot-restore-missing/${sessionId}`,
|
||||
owner,
|
||||
mode: 'claude',
|
||||
resumeConversationId: `conv-${sessionId}`,
|
||||
state: {
|
||||
id: sessionId,
|
||||
pid: null,
|
||||
status: 'idle',
|
||||
workingDir: `/tmp/codeman-reboot-restore-missing/${sessionId}`,
|
||||
currentTaskId: null,
|
||||
createdAt: 1_760_000_000_000,
|
||||
mode: 'claude',
|
||||
owner,
|
||||
} as SessionState,
|
||||
};
|
||||
}
|
||||
|
||||
afterEach(() => {
|
||||
rebootRestoreRegistry.reset();
|
||||
});
|
||||
|
||||
describe('GET /api/reboot-restore', () => {
|
||||
it('reports nothing when no reboot left anything behind', async () => {
|
||||
const app = await createHarness();
|
||||
const res = await app.inject({ method: 'GET', url: '/api/reboot-restore' });
|
||||
expect(res.statusCode).toBe(200);
|
||||
expect(res.json().data.sessions).toEqual([]);
|
||||
await app.close();
|
||||
});
|
||||
|
||||
it('names what is on offer, and says the scrollback is not coming back', async () => {
|
||||
rebootRestoreRegistry.set([offerEntry('a'), offerEntry('b')]);
|
||||
const app = await createHarness();
|
||||
const body = (await app.inject({ method: 'GET', url: '/api/reboot-restore' })).json().data;
|
||||
expect(body.sessions.map((s: { id: string }) => s.id)).toEqual(['a', 'b']);
|
||||
expect(body.scrollbackRestored).toBe(false);
|
||||
await app.close();
|
||||
});
|
||||
|
||||
it('never carries the persisted record itself to the browser', async () => {
|
||||
rebootRestoreRegistry.set([offerEntry('a', 'alice')]);
|
||||
const app = await createHarness({ username: 'alice', role: 'admin' });
|
||||
const body = (await app.inject({ method: 'GET', url: '/api/reboot-restore' })).json().data;
|
||||
expect(Object.keys(body.sessions[0]).sort()).toEqual(['id', 'mode', 'name', 'owner', 'workingDir']);
|
||||
expect(body.sessions[0].state).toBeUndefined();
|
||||
await app.close();
|
||||
});
|
||||
});
|
||||
|
||||
describe('POST /api/reboot-restore/restore', () => {
|
||||
it('reports a workspace that is gone, and keeps offering it in case it comes back', async () => {
|
||||
rebootRestoreRegistry.set([offerEntry('a')]);
|
||||
const app = await createHarness();
|
||||
|
||||
const first = (await app.inject({ method: 'POST', url: '/api/reboot-restore/restore', payload: {} })).json().data;
|
||||
expect(first.restored).toEqual([]);
|
||||
expect(first.skipped).toEqual([{ sessionId: 'a', reason: 'workspace-missing' }]);
|
||||
|
||||
// Nothing was built, so the entry goes back: a repo can be restored from a
|
||||
// backup between two clicks, and losing the offer would be unrecoverable.
|
||||
const left = (await app.inject({ method: 'GET', url: '/api/reboot-restore' })).json().data;
|
||||
expect(left.sessions.map((s: { id: string }) => s.id)).toEqual(['a']);
|
||||
await app.close();
|
||||
});
|
||||
|
||||
it('never re-offers a conversation that is already open', async () => {
|
||||
const entry = offerEntry('a');
|
||||
rebootRestoreRegistry.set([entry]);
|
||||
const app = await createHarness();
|
||||
const ctx = createMockRouteContext({ sessionId: entry.sessionId });
|
||||
// A session with that id is live, which is what the Resume list would produce.
|
||||
const liveApp = await createHarnessWithCtx(ctx);
|
||||
|
||||
const res = (await liveApp.inject({ method: 'POST', url: '/api/reboot-restore/restore', payload: {} })).json().data;
|
||||
expect(res.skipped).toEqual([{ sessionId: 'a', reason: 'already-live' }]);
|
||||
|
||||
// Unlike a missing workspace, this one is dropped: it cannot stop being true.
|
||||
const left = (await liveApp.inject({ method: 'GET', url: '/api/reboot-restore' })).json().data;
|
||||
expect(left.sessions).toEqual([]);
|
||||
await liveApp.close();
|
||||
await app.close();
|
||||
});
|
||||
|
||||
it('spends only the sessions the click named', async () => {
|
||||
rebootRestoreRegistry.set([offerEntry('a'), offerEntry('b')]);
|
||||
const app = await createHarness();
|
||||
|
||||
const res = await app.inject({
|
||||
method: 'POST',
|
||||
url: '/api/reboot-restore/restore',
|
||||
payload: { sessionIds: ['b'] },
|
||||
});
|
||||
expect(res.json().data.skipped).toEqual([{ sessionId: 'b', reason: 'workspace-missing' }]);
|
||||
|
||||
// 'a' was never taken, and 'b' came back because no pane was built for it.
|
||||
const left = (await app.inject({ method: 'GET', url: '/api/reboot-restore' })).json().data;
|
||||
expect(left.sessions.map((s: { id: string }) => s.id).sort()).toEqual(['a', 'b']);
|
||||
await app.close();
|
||||
});
|
||||
|
||||
it('refuses a body it does not recognise rather than guessing', async () => {
|
||||
const app = await createHarness();
|
||||
const res = await app.inject({
|
||||
method: 'POST',
|
||||
url: '/api/reboot-restore/restore',
|
||||
payload: { sessionIds: 'not-an-array' },
|
||||
});
|
||||
expect(res.statusCode).toBeGreaterThanOrEqual(400);
|
||||
await app.close();
|
||||
});
|
||||
|
||||
it('turns a second concurrent restore away rather than interleaving it', async () => {
|
||||
rebootRestoreRegistry.set([offerEntry('a')]);
|
||||
// Claimed by a restore already in flight for this same owner (undefined in
|
||||
// single-user mode, which is what the harness runs as).
|
||||
expect(rebootRestoreRegistry.beginSpending(undefined)).toBe(true);
|
||||
const app = await createHarness();
|
||||
const res = await app.inject({ method: 'POST', url: '/api/reboot-restore/restore', payload: {} });
|
||||
expect(res.statusCode).toBe(409);
|
||||
rebootRestoreRegistry.endSpending(undefined);
|
||||
await app.close();
|
||||
});
|
||||
});
|
||||
|
||||
describe('POST /api/reboot-restore/restore: multi-user workspace confinement', () => {
|
||||
const saved: Record<string, string | undefined> = {};
|
||||
let realDir: string;
|
||||
|
||||
beforeEach(() => {
|
||||
saved.CODEMAN_MULTIUSER = process.env.CODEMAN_MULTIUSER;
|
||||
process.env.CODEMAN_MULTIUSER = '1';
|
||||
// This branch sits AFTER the existsSync check, so the workspace has to be
|
||||
// real for the confinement rule to be the thing that rejects the entry.
|
||||
realDir = mkdtempSync(join(tmpdir(), 'codeman-reboot-restore-real-'));
|
||||
});
|
||||
|
||||
afterEach(() => {
|
||||
if (saved.CODEMAN_MULTIUSER === undefined) delete process.env.CODEMAN_MULTIUSER;
|
||||
else process.env.CODEMAN_MULTIUSER = saved.CODEMAN_MULTIUSER;
|
||||
rmSync(realDir, { recursive: true, force: true });
|
||||
});
|
||||
|
||||
it("refuses a workspace outside the OWNER's case space, and leaves it on offer", async () => {
|
||||
const entry = offerEntry('a', 'alice');
|
||||
entry.workingDir = realDir;
|
||||
(entry.state as { workingDir: string }).workingDir = realDir;
|
||||
rebootRestoreRegistry.set([entry]);
|
||||
|
||||
// An admin does the clicking. The confinement is still resolved against
|
||||
// alice, the entry's OWNER: `isWorkingDirAllowed` waves an admin through, so
|
||||
// reading the caller here would hand an admin the power to rebuild another
|
||||
// user's session anywhere on the box.
|
||||
const app = await createHarness({ username: 'root-user', role: 'admin' });
|
||||
const res = await app.inject({ method: 'POST', url: '/api/reboot-restore/restore', payload: {} });
|
||||
|
||||
expect(res.statusCode).toBe(200);
|
||||
expect(res.json().data.restored).toEqual([]);
|
||||
expect(res.json().data.skipped).toEqual([{ sessionId: 'a', reason: 'workspace-forbidden' }]);
|
||||
|
||||
// A withdrawn grant can be given back, so unlike `already-live` this is not
|
||||
// the permanent kind of refusal and the entry stays claimable.
|
||||
const left = (await app.inject({ method: 'GET', url: '/api/reboot-restore' })).json().data;
|
||||
expect(left.sessions.map((s: { id: string }) => s.id)).toEqual(['a']);
|
||||
await app.close();
|
||||
});
|
||||
});
|
||||
|
||||
describe('POST /api/reboot-restore/dismiss', () => {
|
||||
it('drops the offer and leaves the banner with nothing to show', async () => {
|
||||
rebootRestoreRegistry.set([offerEntry('a'), offerEntry('b')]);
|
||||
const app = await createHarness();
|
||||
|
||||
const res = await app.inject({ method: 'POST', url: '/api/reboot-restore/dismiss', payload: {} });
|
||||
expect(res.json().data.dismissed).toBe(2);
|
||||
|
||||
const after = (await app.inject({ method: 'GET', url: '/api/reboot-restore' })).json().data;
|
||||
expect(after.sessions).toEqual([]);
|
||||
await app.close();
|
||||
});
|
||||
});
|
||||
@@ -49,6 +49,39 @@ function loadTerminalUiHarness(mode: string) {
|
||||
return { app, writes };
|
||||
}
|
||||
|
||||
/**
|
||||
* Swap in a terminal whose write() parses ASYNCHRONOUSLY, the way xterm.js does.
|
||||
*
|
||||
* The real renderer queues the chunk and applies it later, firing the write
|
||||
* callback once it has been parsed; a redraw that addresses a row past the
|
||||
* viewport (Codex's status line) drags the viewport to the live bottom at that
|
||||
* point, not when write() returns. `parse()` runs that pending work.
|
||||
*/
|
||||
function attachAsyncParsingTerminal(app: any, opts: { viewportY: number; baseY: number }) {
|
||||
const buffer = { viewportY: opts.viewportY, baseY: opts.baseY };
|
||||
const pending: Array<() => void> = [];
|
||||
app.terminal.buffer = { active: buffer };
|
||||
app.terminal.write = vi.fn((_data: string, callback?: () => void) => {
|
||||
pending.push(() => {
|
||||
buffer.viewportY = buffer.baseY; // the redraw lands
|
||||
callback?.();
|
||||
});
|
||||
});
|
||||
app.terminal.scrollToLine = vi.fn((line: number) => {
|
||||
buffer.viewportY = line;
|
||||
});
|
||||
app.terminal.scrollToBottom = vi.fn(() => {
|
||||
buffer.viewportY = buffer.baseY;
|
||||
});
|
||||
return {
|
||||
buffer,
|
||||
parse: () => {
|
||||
const queued = pending.splice(0, pending.length);
|
||||
for (const run of queued) run();
|
||||
},
|
||||
};
|
||||
}
|
||||
|
||||
function loadAppHarness() {
|
||||
const dir = resolve(import.meta.dirname, '../src/web/public');
|
||||
const fetchMock = vi.fn();
|
||||
@@ -337,20 +370,111 @@ describe('terminal flush budget', () => {
|
||||
|
||||
it('restores the user scroll position when Codex Working redraws move the viewport', () => {
|
||||
const { app } = loadTerminalUiHarness('codex');
|
||||
const buffer = { viewportY: 40, baseY: 100 };
|
||||
app.terminal.buffer = { active: buffer };
|
||||
app.terminal.write = vi.fn(() => {
|
||||
buffer.viewportY = buffer.baseY;
|
||||
});
|
||||
app.terminal.scrollToLine = vi.fn((line: number) => {
|
||||
buffer.viewportY = line;
|
||||
});
|
||||
const { buffer, parse } = attachAsyncParsingTerminal(app, { viewportY: 40, baseY: 100 });
|
||||
app._wasAtBottomBeforeWrite = true;
|
||||
app._lastUserScrollUpAt = 0;
|
||||
app.pendingWrites.push('\x1b[55;1H\x1b[2m• Working (6s)');
|
||||
|
||||
app.flushPendingWrites();
|
||||
parse();
|
||||
|
||||
expect(buffer.viewportY).toBe(40);
|
||||
});
|
||||
|
||||
// Issue #358. xterm.js parses on its own schedule, so the buffer still holds
|
||||
// the pre-write viewport the instant write() returns: restoring there compared
|
||||
// the anchor against itself, did nothing, and left the redraw free to drag the
|
||||
// viewport to the live bottom a tick later. The previous regression passed
|
||||
// because its write mock moved the viewport synchronously, which real xterm
|
||||
// never does. These drive the callback explicitly instead.
|
||||
it('restores the history anchor only AFTER xterm has parsed the write (#358)', () => {
|
||||
const { app } = loadTerminalUiHarness('codex');
|
||||
const { buffer, parse } = attachAsyncParsingTerminal(app, { viewportY: 40, baseY: 100 });
|
||||
app.pendingWrites.push('\x1b[55;1H\x1b[2m• Working (6s)');
|
||||
|
||||
app.flushPendingWrites();
|
||||
// Nothing has parsed yet, so nothing may have been restored yet either.
|
||||
expect(app.terminal.scrollToLine).not.toHaveBeenCalled();
|
||||
expect(buffer.viewportY).toBe(40);
|
||||
|
||||
parse();
|
||||
|
||||
expect(app.terminal.scrollToLine).toHaveBeenCalledWith(40);
|
||||
expect(buffer.viewportY).toBe(40);
|
||||
});
|
||||
|
||||
it('holds the anchor across consecutive Codex redraws', () => {
|
||||
const { app } = loadTerminalUiHarness('codex');
|
||||
const { buffer, parse } = attachAsyncParsingTerminal(app, { viewportY: 40, baseY: 100 });
|
||||
|
||||
for (const frame of ['\x1b[55;1H\x1b[2m• Working (6s)', '\x1b[55;1H\x1b[2m• Working (7s)']) {
|
||||
app.pendingWrites.push(frame);
|
||||
app.flushPendingWrites();
|
||||
parse();
|
||||
expect(buffer.viewportY).toBe(40);
|
||||
}
|
||||
});
|
||||
|
||||
it('holds the anchor across a chunked write whose remainder is deferred', () => {
|
||||
const { app } = loadTerminalUiHarness('codex');
|
||||
const { buffer, parse } = attachAsyncParsingTerminal(app, { viewportY: 40, baseY: 100 });
|
||||
// Over the 32KB codex frame budget, so the flush defers a remainder and the
|
||||
// second chunk goes out from the write callback's reschedule.
|
||||
app.pendingWrites.push('x'.repeat(40000));
|
||||
|
||||
app.flushPendingWrites();
|
||||
parse();
|
||||
expect(buffer.viewportY).toBe(40);
|
||||
|
||||
app.flushPendingWrites();
|
||||
parse();
|
||||
expect(buffer.viewportY).toBe(40);
|
||||
expect(app.pendingWrites).toHaveLength(0);
|
||||
});
|
||||
|
||||
it('drops the anchor when the user switched sessions before the write parsed', () => {
|
||||
// The anchor indexes the buffer it came from. selectSession() resets the
|
||||
// terminal and chunk-loads a different scrollback, so replaying row 40 into
|
||||
// that one is a jump to an arbitrary place, not a restore. Only reachable now
|
||||
// that the restore runs a parse later than the write.
|
||||
const { app } = loadTerminalUiHarness('codex');
|
||||
const { buffer, parse } = attachAsyncParsingTerminal(app, { viewportY: 40, baseY: 100 });
|
||||
app.pendingWrites.push('\x1b[55;1H\x1b[2m• Working (6s)');
|
||||
|
||||
app.flushPendingWrites();
|
||||
app.activeSessionId = 'session-2'; // the user clicked another tab
|
||||
parse();
|
||||
|
||||
expect(app.terminal.scrollToLine).not.toHaveBeenCalled();
|
||||
expect(buffer.viewportY).toBe(buffer.baseY);
|
||||
});
|
||||
|
||||
it('drops the anchor while a buffer load is replaying history', () => {
|
||||
const { app } = loadTerminalUiHarness('codex');
|
||||
const { parse } = attachAsyncParsingTerminal(app, { viewportY: 40, baseY: 100 });
|
||||
app.pendingWrites.push('\x1b[55;1H\x1b[2m• Working (6s)');
|
||||
|
||||
app.flushPendingWrites();
|
||||
app._isLoadingBuffer = true; // chunkedTerminalWrite owns the viewport now
|
||||
parse();
|
||||
|
||||
expect(app.terminal.scrollToLine).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('does not bounce off the bottom when the sticky flag and an anchor disagree', () => {
|
||||
// _wasAtBottomBeforeWrite is captured at the frame's first batchTerminalWrite
|
||||
// and the anchor at flush time, so a scroll-up in between leaves both live.
|
||||
// The anchor wins: scrolling to the bottom and back would be a visible jump.
|
||||
const { app } = loadTerminalUiHarness('codex');
|
||||
const { buffer, parse } = attachAsyncParsingTerminal(app, { viewportY: 40, baseY: 100 });
|
||||
app._wasAtBottomBeforeWrite = true;
|
||||
app._lastUserScrollUpAt = 0;
|
||||
app.pendingWrites.push('\x1b[55;1H\x1b[2m• Working (6s)');
|
||||
|
||||
app.flushPendingWrites();
|
||||
parse();
|
||||
|
||||
expect(app.terminal.scrollToBottom).not.toHaveBeenCalled();
|
||||
expect(buffer.viewportY).toBe(40);
|
||||
});
|
||||
});
|
||||
|
||||
@@ -9,7 +9,10 @@
|
||||
* (stopPropagation, not stopImmediatePropagation) does not silence it;
|
||||
* - a `composed: true` insertText preceded by a keydown — the shape Chrome on
|
||||
* Android delivers — is dropped by xterm and recovered by us, exactly once;
|
||||
* - a keystroke xterm DOES handle is delivered exactly once, not twice.
|
||||
* - a keystroke xterm DOES handle is delivered exactly once, not twice;
|
||||
* - a character committed in the SAME page task as Enter reaches the send
|
||||
* path ahead of the `\r`, which is the ordering the zero-delay timer
|
||||
* alone cannot produce.
|
||||
*
|
||||
* Browser-driven, so it is excluded from `npm run test:ci` like the other
|
||||
* Playwright suites. Run locally:
|
||||
@@ -146,6 +149,84 @@ describe('orphaned terminal input recovery wiring', () => {
|
||||
expect(second.sent.join('')).toBe('z');
|
||||
});
|
||||
|
||||
/**
|
||||
* The batched shape an Android soft keyboard actually delivers when the user
|
||||
* taps the last character and then Enter: the character's keydown, its
|
||||
* `composed: true` insertText, and Enter's keydown all land in ONE page task,
|
||||
* before any zero-delay timer can run.
|
||||
*
|
||||
* This is the ordering half of the fix, and the half the unit harness cannot
|
||||
* reach: the unit tests prove WHICH candidate is forwarded, this proves WHEN.
|
||||
* Resolving the pending candidate only on its 0 ms timer loses the character
|
||||
* outright here, because by the time that timer runs xterm has already
|
||||
* emitted the `\r` and bumped the canonical counter past the candidate's
|
||||
* snapshot, so it stands down. Draining at the next keydown, from xterm's
|
||||
* custom key handler (which runs before xterm processes that key), puts the
|
||||
* character on the wire ahead of the `\r`.
|
||||
*/
|
||||
async function batchedCommitThenEnter(data: string) {
|
||||
return page.evaluate(async (text) => {
|
||||
const app = (window as any).app;
|
||||
const textarea = document.querySelector('.xterm-helper-textarea') as HTMLTextAreaElement;
|
||||
const originalSessionId = app.activeSessionId;
|
||||
const originalLocalEcho = app._localEchoEnabled;
|
||||
const originalSendInput = app._sendInputAsync;
|
||||
const originalPendingInput = app._pendingInput;
|
||||
const originalLastKeystrokeTime = app._lastKeystrokeTime;
|
||||
const sent: string[] = [];
|
||||
|
||||
try {
|
||||
app.activeSessionId = 'cod388-browser-batched';
|
||||
app._localEchoEnabled = false;
|
||||
app._pendingInput = '';
|
||||
app._lastKeystrokeTime = 0;
|
||||
app._sendInputAsync = (_sessionId: string, chunk: string) => sent.push(chunk);
|
||||
textarea.focus();
|
||||
|
||||
// One task, no awaits between the three dispatches.
|
||||
const charDown = new KeyboardEvent('keydown', {
|
||||
key: 'Unidentified',
|
||||
bubbles: true,
|
||||
cancelable: true,
|
||||
composed: true,
|
||||
});
|
||||
Object.defineProperties(charDown, { keyCode: { value: 65 }, which: { value: 65 } });
|
||||
textarea.dispatchEvent(charDown);
|
||||
|
||||
textarea.value = text;
|
||||
textarea.dispatchEvent(
|
||||
new InputEvent('input', { data: text, inputType: 'insertText', bubbles: true, composed: true })
|
||||
);
|
||||
|
||||
const enterDown = new KeyboardEvent('keydown', {
|
||||
key: 'Enter',
|
||||
code: 'Enter',
|
||||
bubbles: true,
|
||||
cancelable: true,
|
||||
composed: true,
|
||||
});
|
||||
Object.defineProperties(enterDown, { keyCode: { value: 13 }, which: { value: 13 } });
|
||||
textarea.dispatchEvent(enterDown);
|
||||
|
||||
await new Promise((resolve) => setTimeout(resolve, 80));
|
||||
return { wire: sent.join('') };
|
||||
} finally {
|
||||
app.activeSessionId = originalSessionId;
|
||||
app._localEchoEnabled = originalLocalEcho;
|
||||
app._sendInputAsync = originalSendInput;
|
||||
app._pendingInput = originalPendingInput;
|
||||
app._lastKeystrokeTime = originalLastKeystrokeTime;
|
||||
textarea.value = '';
|
||||
}
|
||||
}, data);
|
||||
}
|
||||
|
||||
it('delivers a character committed in the same task as Enter BEFORE the carriage return', async () => {
|
||||
const { wire } = await batchedCommitThenEnter('o');
|
||||
// Not '\r' (character lost, the defect) and not '\ro' (recovered too late).
|
||||
expect(wire).toBe('o\r');
|
||||
});
|
||||
|
||||
it('sends nothing for a keydown that produces no input event', async () => {
|
||||
const { sent } = await keystroke({ data: 'q', dispatchInput: false, keyCode: 65 });
|
||||
expect(sent).toEqual([]);
|
||||
|
||||
@@ -218,6 +218,52 @@ describe('orphaned terminal input recovery', () => {
|
||||
expect(reads).toEqual([]);
|
||||
});
|
||||
|
||||
it('delivers the last character BEFORE the Enter that submits it (defect 4)', () => {
|
||||
// Android soft keyboards commit the last character and send the Enter key in
|
||||
// ONE InputConnection transaction, so the `input` event and the Enter keydown
|
||||
// are processed before any zero-delay timer runs. Two things then went wrong
|
||||
// with a candidate that only resolved on its timer:
|
||||
//
|
||||
// 1. ORDER — xterm emits '\r' synchronously from the Enter keydown, and the
|
||||
// local-echo composer submits `pendingText` right there. The recovered
|
||||
// character arrived one macrotask too late to be part of the prompt.
|
||||
// 2. LOSS — that '\r' bumps the canonical counter, so by the time the
|
||||
// candidate resolved, `canonicalCount > snapshot` read as "xterm spoke
|
||||
// for this keystroke" and stood the recovery down. The character was
|
||||
// dropped outright: every message sent from the phone lost its last
|
||||
// character.
|
||||
//
|
||||
// Resolving pending candidates synchronously at the NEXT keydown fixes both:
|
||||
// the counter still holds the value it had when that candidate was created,
|
||||
// and the byte reaches the composer ahead of the Enter.
|
||||
const h = harness();
|
||||
h.keydown();
|
||||
h.input('o');
|
||||
expect(h.emitted).toEqual([]);
|
||||
|
||||
h.keydown({ key: 'Enter' });
|
||||
expect(h.emitted).toEqual(['o']);
|
||||
|
||||
// xterm now emits '\r' for the Enter. The already-resolved candidate must
|
||||
// not fire a second time when its timer is flushed.
|
||||
h.controller.notifyCanonicalData();
|
||||
h.flushTimers();
|
||||
expect(h.emitted).toEqual(['o']);
|
||||
expect(h.pendingTimers()).toBe(0);
|
||||
});
|
||||
|
||||
it('still stands down at the next keydown when xterm spoke for the candidate', () => {
|
||||
// The synchronous resolve must not become a "forward everything" path: a
|
||||
// keystroke xterm delivered itself is still a duplicate if recovered.
|
||||
const h = harness();
|
||||
h.keydown();
|
||||
h.input('x');
|
||||
h.controller.notifyCanonicalData();
|
||||
h.keydown({ key: 'Enter' });
|
||||
h.flushTimers();
|
||||
expect(h.emitted).toEqual([]);
|
||||
});
|
||||
|
||||
it('ignores input events that are not committed text', () => {
|
||||
const h = harness();
|
||||
for (const inputType of ['insertCompositionText', 'deleteContentBackward', 'insertLineBreak', 'insertFromPaste']) {
|
||||
|
||||
Reference in New Issue
Block a user