diff --git a/CONTEXT.md b/CONTEXT.md new file mode 100644 index 0000000..46d7015 --- /dev/null +++ b/CONTEXT.md @@ -0,0 +1,51 @@ +# agent-tty + +`agent-tty` manages long-lived terminal sessions and preserves enough state to inspect, replay, and clean them up safely. + +## Language + +**Session**: +A long-lived PTY-backed terminal instance owned by `agent-tty`. + +**Session Status**: +The lifecycle state of a **Session**: `running`, `exiting`, `exited`, `failed`, `destroying`, or `destroyed`. + +**Active Session**: +A **Session** whose host-side lifecycle may still be in progress. + +**Terminal Session**: +A **Session** whose host-side lifecycle has reached a final persisted state. +_Avoid_: Finished session + +**Commandable Session**: +A **Session** that can still accept user input and control commands. + +**Live Host Eligible Session**: +A **Session** where callers should ask the live session host for fresh state. + +**Offline Replay Eligible Session**: +A **Session** where callers should reconstruct renderer state from persisted replay data. + +**Collectable Session**: +A **Terminal Session** whose persisted directory may be removed by garbage collection. +_Avoid_: Deletable session + +**Destroyed Status Check**: +A convenience policy predicate for the single `destroyed` **Session Status** value. It is not a separate lifecycle classification. + +## Relationships + +- A **Session** has exactly one **Session Status** at a time. +- A `running` **Session** is **Active**, **Commandable**, and **Live Host Eligible**. +- An `exiting` **Session** is **Active** and **Live Host Eligible**, but not **Commandable**. +- A `destroying` **Session** is **Active** and **Offline Replay Eligible**, but not **Terminal** or **Collectable**. +- `exited`, `failed`, and `destroyed` **Sessions** are **Terminal**, **Offline Replay Eligible**, and **Collectable**. + +## Example dialogue + +> **Dev:** "Can garbage collection remove a **destroying** **Session**?" +> **Domain expert:** "No. It is still an **Active Session**, even though renderer inspection should use **Offline Replay** instead of the live host." + +## Flagged ambiguities + +- "Active" and "offline replay eligible" are independent classifications: `destroying` is both **Active** and **Offline Replay Eligible**. diff --git a/src/cli/commands/gc.ts b/src/cli/commands/gc.ts index 7249c60..74b5c21 100644 --- a/src/cli/commands/gc.ts +++ b/src/cli/commands/gc.ts @@ -6,6 +6,10 @@ import type { CommandContext } from '../context.js'; import { emitSuccess } from '../output.js'; import { reconcileSession } from '../../host/lifecycle.js'; import { ERROR_CODES, makeCliError } from '../../protocol/errors.js'; +import { + isCollectableSessionStatus, + isTerminalSessionStatus, +} from '../../protocol/sessionStatusPolicy.js'; import type { SessionRecord } from '../../protocol/schemas.js'; import { readManifestIfExists } from '../../storage/manifests.js'; import { manifestPath, sessionDir } from '../../storage/sessionPaths.js'; @@ -142,9 +146,10 @@ function wasReconciledFromStaleHost( manifestBefore: SessionRecord, manifestAfter: SessionRecord, ): boolean { - const isTerminal = (s: string): boolean => - s === 'exited' || s === 'failed' || s === 'destroyed'; - return !isTerminal(manifestBefore.status) && isTerminal(manifestAfter.status); + return ( + !isTerminalSessionStatus(manifestBefore.status) && + isTerminalSessionStatus(manifestAfter.status) + ); } function shouldSkipForAge( @@ -335,11 +340,7 @@ export async function gcSessions( } const sessionId = manifestAfter.sessionId; - const isTerminalAfter = - manifestAfter.status === 'exited' || - manifestAfter.status === 'failed' || - manifestAfter.status === 'destroyed'; - if (!isTerminalAfter) { + if (!isCollectableSessionStatus(manifestAfter.status)) { result.skippedSessions.push({ sessionId, reason: 'session host is still alive', @@ -384,11 +385,10 @@ export async function gcSessions( continue; } - const isFinalTerminal = - finalManifest?.status === 'exited' || - finalManifest?.status === 'failed' || - finalManifest?.status === 'destroyed'; - if (finalManifest !== null && !isFinalTerminal) { + if ( + finalManifest !== null && + !isCollectableSessionStatus(finalManifest.status) + ) { result.skippedSessions.push({ sessionId, reason: 'session restarted between check and delete', diff --git a/src/cli/commands/inspect.ts b/src/cli/commands/inspect.ts index 08daa7a..e236b53 100644 --- a/src/cli/commands/inspect.ts +++ b/src/cli/commands/inspect.ts @@ -12,6 +12,11 @@ import type { CommandContext } from '../context.js'; import { countEventLogEntries } from '../../host/eventLog.js'; import { reconcileSession } from '../../host/lifecycle.js'; import { sendRpc } from '../../host/rpcClient.js'; +import { + isCommandableSessionStatus, + isLiveHostEligibleSessionStatus, + isOfflineReplayEligibleSessionStatus, +} from '../../protocol/sessionStatusPolicy.js'; import { deriveTerminationCategory } from '../../protocol/terminationCategory.js'; import { ERROR_CODES, makeCliError } from '../../protocol/errors.js'; import { emitSuccess } from '../output.js'; @@ -32,8 +37,10 @@ interface CommandOptions { function computeUptime(session: SessionRecord): number { const createdAt = Date.parse(session.createdAt); - const endAt = - session.status === 'running' ? Date.now() : Date.parse(session.updatedAt); + // Matches pre-existing behavior: only running sessions show live uptime. + const endAt = isCommandableSessionStatus(session.status) + ? Date.now() + : Date.parse(session.updatedAt); return Math.max(0, endAt - createdAt); } @@ -54,23 +61,6 @@ function formatArtifactKinds(byKind: Record): string { const RENDERER_BACKEND = 'ghostty-web'; -function usesOfflineReplay(sessionStatus: SessionStatus): boolean { - switch (sessionStatus) { - case 'running': - case 'exiting': - return false; - case 'exited': - case 'failed': - case 'destroying': - case 'destroyed': - return true; - default: { - const exhaustiveStatus: never = sessionStatus; - throw new Error(`unexpected session status: ${String(exhaustiveStatus)}`); - } - } -} - function deriveRendererRuntimeSummary(options: { usedOfflineReplay: boolean; sessionStatus: SessionStatus; @@ -84,7 +74,7 @@ function deriveRendererRuntimeSummary(options: { }; } - if (usesOfflineReplay(options.sessionStatus)) { + if (isOfflineReplayEligibleSessionStatus(options.sessionStatus)) { return { backend: RENDERER_BACKEND, mode: 'offline-replay', @@ -146,8 +136,7 @@ function formatSessionLines(result: InspectResult): string[] { ); if ( - session.status !== 'running' && - session.status !== 'exiting' && + !isLiveHostEligibleSessionStatus(session.status) && result.terminationCategory !== undefined ) { lines.push(`Termination: ${result.terminationCategory}`); @@ -181,8 +170,8 @@ export async function runInspectCommand( }); } - const isOffline = usesOfflineReplay(session.status); - if (!isOffline) { + const isLiveHostEligible = isLiveHostEligibleSessionStatus(session.status); + if (isLiveHostEligible) { try { const rawResult: unknown = await sendRpc( socketPath(sessionDirectory), diff --git a/src/cli/commands/mark.ts b/src/cli/commands/mark.ts index 535ea87..3e28e7f 100644 --- a/src/cli/commands/mark.ts +++ b/src/cli/commands/mark.ts @@ -11,6 +11,7 @@ import { sessionDir, socketPath, } from '../../storage/sessionPaths.js'; +import { assertSessionCommandable } from '../sessionGuards.js'; export type { MarkResult } from '../../protocol/messages.js'; @@ -37,25 +38,7 @@ export async function runMarkCommand(options: CommandOptions): Promise { }); } - if (manifest.status === 'destroyed') { - throw makeCliError(ERROR_CODES.SESSION_ALREADY_DESTROYED, { - message: `Session "${options.sessionId}" is already destroyed.`, - details: { - sessionId: options.sessionId, - status: manifest.status, - }, - }); - } - - if (manifest.status !== 'running') { - throw makeCliError(ERROR_CODES.SESSION_NOT_RUNNING, { - message: `Session "${options.sessionId}" is not running.`, - details: { - sessionId: options.sessionId, - status: manifest.status, - }, - }); - } + assertSessionCommandable(manifest, options.sessionId); const rawResult: unknown = await sendRpc( socketPath(sessionDirectory), diff --git a/src/cli/commands/paste.ts b/src/cli/commands/paste.ts index bb5f9df..23d4957 100644 --- a/src/cli/commands/paste.ts +++ b/src/cli/commands/paste.ts @@ -10,6 +10,7 @@ import { socketPath, } from '../../storage/sessionPaths.js'; import { resolveCommandInputText } from './inputSource.js'; +import { assertSessionCommandable } from '../sessionGuards.js'; export interface PasteResult { [key: string]: never; @@ -54,25 +55,7 @@ export async function runPasteCommand(options: CommandOptions): Promise { }); } - if (manifest.status === 'destroyed') { - throw makeCliError(ERROR_CODES.SESSION_ALREADY_DESTROYED, { - message: `Session "${options.sessionId}" is already destroyed.`, - details: { - sessionId: options.sessionId, - status: manifest.status, - }, - }); - } - - if (manifest.status !== 'running') { - throw makeCliError(ERROR_CODES.SESSION_NOT_RUNNING, { - message: `Session "${options.sessionId}" is not running.`, - details: { - sessionId: options.sessionId, - status: manifest.status, - }, - }); - } + assertSessionCommandable(manifest, options.sessionId); await sendRpc(socketPath(sessionDirectory), 'paste', { text, diff --git a/src/cli/commands/resize.ts b/src/cli/commands/resize.ts index ddf3a09..0486bb3 100644 --- a/src/cli/commands/resize.ts +++ b/src/cli/commands/resize.ts @@ -9,6 +9,7 @@ import { sessionDir, socketPath, } from '../../storage/sessionPaths.js'; +import { assertSessionCommandable } from '../sessionGuards.js'; export interface ResizeResult { cols: number; @@ -39,25 +40,7 @@ export async function runResizeCommand(options: CommandOptions): Promise { }); } - if (manifest.status === 'destroyed') { - throw makeCliError(ERROR_CODES.SESSION_ALREADY_DESTROYED, { - message: `Session "${options.sessionId}" is already destroyed.`, - details: { - sessionId: options.sessionId, - status: manifest.status, - }, - }); - } - - if (manifest.status !== 'running') { - throw makeCliError(ERROR_CODES.SESSION_NOT_RUNNING, { - message: `Session "${options.sessionId}" is not running.`, - details: { - sessionId: options.sessionId, - status: manifest.status, - }, - }); - } + assertSessionCommandable(manifest, options.sessionId); if ( !Number.isInteger(options.cols) || diff --git a/src/cli/commands/run.ts b/src/cli/commands/run.ts index 29599f8..42fa8ed 100644 --- a/src/cli/commands/run.ts +++ b/src/cli/commands/run.ts @@ -12,6 +12,7 @@ import { socketPath, } from '../../storage/sessionPaths.js'; import { resolveCommandInputText } from './inputSource.js'; +import { assertSessionCommandable } from '../sessionGuards.js'; interface CommandOptions { context: CommandContext; @@ -63,25 +64,7 @@ export async function runRunCommand(options: CommandOptions): Promise { }); } - if (manifest.status === 'destroyed') { - throw makeCliError(ERROR_CODES.SESSION_ALREADY_DESTROYED, { - message: `Session "${options.sessionId}" is already destroyed.`, - details: { - sessionId: options.sessionId, - status: manifest.status, - }, - }); - } - - if (manifest.status !== 'running') { - throw makeCliError(ERROR_CODES.SESSION_NOT_RUNNING, { - message: `Session "${options.sessionId}" is not running.`, - details: { - sessionId: options.sessionId, - status: manifest.status, - }, - }); - } + assertSessionCommandable(manifest, options.sessionId); const noWait = !options.wait; const rpcParams: Record = { diff --git a/src/cli/commands/screenshot.ts b/src/cli/commands/screenshot.ts index c93ca6e..0e9f2fe 100644 --- a/src/cli/commands/screenshot.ts +++ b/src/cli/commands/screenshot.ts @@ -255,6 +255,9 @@ export async function runScreenshotCommand( let rawResult: unknown; let invalidResultMessage = 'Unexpected response from host'; + // Snapshot and screenshot intentionally keep their narrower legacy live-RPC + // gate. `exiting` sessions are live-host eligible for inspect, but these + // commands preserve their existing offline-replay capture behavior. if (manifest.status === 'running') { try { rawResult = await sendRpc(socketPath(sessionDirectory), 'screenshot', { diff --git a/src/cli/commands/send-keys.ts b/src/cli/commands/send-keys.ts index 2d517c2..28f4dd5 100644 --- a/src/cli/commands/send-keys.ts +++ b/src/cli/commands/send-keys.ts @@ -11,6 +11,7 @@ import { sessionDir, socketPath, } from '../../storage/sessionPaths.js'; +import { assertSessionCommandable } from '../sessionGuards.js'; export type { SendKeysResult } from '../../protocol/messages.js'; @@ -39,25 +40,7 @@ export async function runSendKeysCommand( }); } - if (manifest.status === 'destroyed') { - throw makeCliError(ERROR_CODES.SESSION_ALREADY_DESTROYED, { - message: `Session "${options.sessionId}" is already destroyed.`, - details: { - sessionId: options.sessionId, - status: manifest.status, - }, - }); - } - - if (manifest.status !== 'running') { - throw makeCliError(ERROR_CODES.SESSION_NOT_RUNNING, { - message: `Session "${options.sessionId}" is not running.`, - details: { - sessionId: options.sessionId, - status: manifest.status, - }, - }); - } + assertSessionCommandable(manifest, options.sessionId); const rawResult: unknown = await sendRpc( socketPath(sessionDirectory), diff --git a/src/cli/commands/signal.ts b/src/cli/commands/signal.ts index 49620ec..b613dc6 100644 --- a/src/cli/commands/signal.ts +++ b/src/cli/commands/signal.ts @@ -9,6 +9,7 @@ import { sessionDir, socketPath, } from '../../storage/sessionPaths.js'; +import { assertSessionCommandable } from '../sessionGuards.js'; const ALLOWED_SIGNALS = [ 'SIGTERM', @@ -47,25 +48,7 @@ export async function runSignalCommand(options: CommandOptions): Promise { }); } - if (manifest.status === 'destroyed') { - throw makeCliError(ERROR_CODES.SESSION_ALREADY_DESTROYED, { - message: `Session "${options.sessionId}" is already destroyed.`, - details: { - sessionId: options.sessionId, - status: manifest.status, - }, - }); - } - - if (manifest.status !== 'running') { - throw makeCliError(ERROR_CODES.SESSION_NOT_RUNNING, { - message: `Session "${options.sessionId}" is not running.`, - details: { - sessionId: options.sessionId, - status: manifest.status, - }, - }); - } + assertSessionCommandable(manifest, options.sessionId); if ( !ALLOWED_SIGNALS.includes( diff --git a/src/cli/commands/snapshot.ts b/src/cli/commands/snapshot.ts index 4358a06..0ca01c8 100644 --- a/src/cli/commands/snapshot.ts +++ b/src/cli/commands/snapshot.ts @@ -312,6 +312,9 @@ export async function runSnapshotCommand( } let result: SnapshotResult; + // Snapshot and screenshot intentionally keep their narrower legacy live-RPC + // gate. `exiting` sessions are live-host eligible for inspect, but these + // commands preserve their existing offline-replay capture behavior. if (manifest.status === 'running') { try { result = await runRpcSnapshot( diff --git a/src/cli/commands/type.ts b/src/cli/commands/type.ts index c84aa54..322f4e8 100644 --- a/src/cli/commands/type.ts +++ b/src/cli/commands/type.ts @@ -10,6 +10,7 @@ import { socketPath, } from '../../storage/sessionPaths.js'; import { resolveCommandInputText } from './inputSource.js'; +import { assertSessionCommandable } from '../sessionGuards.js'; export interface TypeResult { [key: string]: never; @@ -57,25 +58,7 @@ export async function runTypeCommand(options: CommandOptions): Promise { }); } - if (manifest.status === 'destroyed') { - throw makeCliError(ERROR_CODES.SESSION_ALREADY_DESTROYED, { - message: `Session "${options.sessionId}" is already destroyed.`, - details: { - sessionId: options.sessionId, - status: manifest.status, - }, - }); - } - - if (manifest.status !== 'running') { - throw makeCliError(ERROR_CODES.SESSION_NOT_RUNNING, { - message: `Session "${options.sessionId}" is not running.`, - details: { - sessionId: options.sessionId, - status: manifest.status, - }, - }); - } + assertSessionCommandable(manifest, options.sessionId); await sendRpc(socketPath(sessionDirectory), 'type', { text, diff --git a/src/cli/commands/wait.ts b/src/cli/commands/wait.ts index c58b7ac..0eb710f 100644 --- a/src/cli/commands/wait.ts +++ b/src/cli/commands/wait.ts @@ -14,6 +14,7 @@ import { WaitForRenderResultSchema, WaitResultSchema, } from '../../protocol/schemas.js'; +import { isTerminalSessionStatus } from '../../protocol/sessionStatusPolicy.js'; import { withOfflineReplayRenderer } from '../../replay/offlineReplay.js'; import { readManifestIfExists } from '../../storage/manifests.js'; import { @@ -21,6 +22,7 @@ import { sessionDir, socketPath, } from '../../storage/sessionPaths.js'; +import { assertSessionCommandable } from '../sessionGuards.js'; interface CommandOptions { context: CommandContext; @@ -345,12 +347,7 @@ export async function runWaitCommand(options: CommandOptions): Promise { const manifestStatus = manifest.status; - if ( - options.waitForExit && - (manifestStatus === 'exited' || - manifestStatus === 'failed' || - manifestStatus === 'destroyed') - ) { + if (options.waitForExit && isTerminalSessionStatus(manifestStatus)) { const result: WaitResult = { timedOut: false, ...(manifest.exitCode === null ? {} : { exitCode: manifest.exitCode }), @@ -365,24 +362,8 @@ export async function runWaitCommand(options: CommandOptions): Promise { return; } - if (manifest.status === 'destroyed') { - throw makeCliError(ERROR_CODES.SESSION_ALREADY_DESTROYED, { - message: `Session "${options.sessionId}" is already destroyed.`, - details: { - sessionId: options.sessionId, - status: manifest.status, - }, - }); - } - - if (!options.waitForExit && manifestStatus !== 'running') { - throw makeCliError(ERROR_CODES.SESSION_NOT_RUNNING, { - message: `Session "${options.sessionId}" is not running.`, - details: { - sessionId: options.sessionId, - status: manifest.status, - }, - }); + if (!options.waitForExit) { + assertSessionCommandable(manifest, options.sessionId); } const effectiveTimeout = options.timeout ?? DEFAULT_WAIT_TIMEOUT_MS; diff --git a/src/cli/sessionGuards.ts b/src/cli/sessionGuards.ts new file mode 100644 index 0000000..e0a76d1 --- /dev/null +++ b/src/cli/sessionGuards.ts @@ -0,0 +1,43 @@ +import { ERROR_CODES, makeCliError } from '../protocol/errors.js'; +import { + isCommandableSessionStatus, + isDestroyedSessionStatus, +} from '../protocol/sessionStatusPolicy.js'; +import type { SessionRecord } from '../protocol/schemas.js'; +import { invariant } from '../util/assert.js'; + +/** + * Throws SESSION_ALREADY_DESTROYED for destroyed sessions and + * SESSION_NOT_RUNNING for other non-commandable session statuses. + */ +export function assertSessionCommandable( + manifest: Pick, + sessionId: string, +): void { + invariant( + typeof sessionId === 'string' && sessionId.length > 0, + 'sessionId must be non-empty', + ); + + if (isDestroyedSessionStatus(manifest.status)) { + throw makeCliError(ERROR_CODES.SESSION_ALREADY_DESTROYED, { + message: `Session "${sessionId}" is already destroyed.`, + details: { + sessionId, + status: manifest.status, + }, + }); + } + + if (!isCommandableSessionStatus(manifest.status)) { + throw makeCliError(ERROR_CODES.SESSION_NOT_RUNNING, { + // Preserve the legacy public error text while centralizing the + // commandable-session policy behind this guard. + message: `Session "${sessionId}" is not running.`, + details: { + sessionId, + status: manifest.status, + }, + }); + } +} diff --git a/src/host/hostMain.ts b/src/host/hostMain.ts index c8d837d..a30cc18 100644 --- a/src/host/hostMain.ts +++ b/src/host/hostMain.ts @@ -21,6 +21,7 @@ import { createPty } from '../pty/createPty.js'; import { encodeKey } from '../pty/keyEncoder.js'; import { encodePaste } from '../pty/pasteEncoder.js'; import { ERROR_CODES, makeCliError } from '../protocol/errors.js'; +import { isCommandableSessionStatus } from '../protocol/sessionStatusPolicy.js'; import type { MarkParams, PasteParams, @@ -122,8 +123,18 @@ function normalizeExitSignal(signal: number | null): string | null { return signal === null || signal === 0 ? null : String(signal); } -function isSessionRunning(state: SessionState): boolean { - return state.snapshot().status === 'running'; +function isSessionCommandable(state: SessionState): boolean { + return isCommandableSessionStatus(state.snapshot().status); +} + +function assertSessionCommandable(state: SessionState): void { + if (!isSessionCommandable(state)) { + throw makeCliError(ERROR_CODES.SESSION_NOT_RUNNING, { + // Preserve the legacy RPC wire contract: errors include only code and + // message, so this host-side guard remains message-only. + message: 'Session is not running.', + }); + } } function rethrowAsync(error: unknown): void { @@ -597,7 +608,7 @@ export async function runHost(sessionId: string): Promise { const startIdlePolling = (): void => { if ( idleTimeoutMs <= 0 || - !isSessionRunning(state) || + !isSessionCommandable(state) || idleTimeoutHandle !== null ) { return; @@ -605,7 +616,7 @@ export async function runHost(sessionId: string): Promise { const checkIntervalMs = Math.min(idleTimeoutMs, IDLE_CHECK_CAP_MS); idleTimeoutHandle = setInterval(() => { - if (!isSessionRunning(state)) { + if (!isSessionCommandable(state)) { clearIdleTimeout(); return; } @@ -626,7 +637,7 @@ export async function runHost(sessionId: string): Promise { shutdownPromise = (async () => { try { clearIdleTimeout(); - if (isSessionRunning(state)) { + if (isSessionCommandable(state)) { pty.kill(); state.requestDestroy(); await writeManifest(mPath, state.snapshot()); @@ -902,11 +913,7 @@ export async function runHost(sessionId: string): Promise { type: async (params: unknown) => { const { text } = params as TypeParams; - if (!isSessionRunning(state)) { - throw makeCliError(ERROR_CODES.SESSION_NOT_RUNNING, { - message: 'Session is not running.', - }); - } + assertSessionCommandable(state); invariant(typeof text === 'string', 'type text must be a string'); pty.write(text); @@ -917,11 +924,7 @@ export async function runHost(sessionId: string): Promise { mark: async (params: unknown) => { const { label } = params as MarkParams; - if (!isSessionRunning(state)) { - throw makeCliError(ERROR_CODES.SESSION_NOT_RUNNING, { - message: 'Session is not running.', - }); - } + assertSessionCommandable(state); invariant(typeof label === 'string', 'mark label must be a string'); lastActivityAt = Date.now(); @@ -931,11 +934,7 @@ export async function runHost(sessionId: string): Promise { paste: async (params: unknown) => { const { text } = params as PasteParams; - if (!isSessionRunning(state)) { - throw makeCliError(ERROR_CODES.SESSION_NOT_RUNNING, { - message: 'Session is not running.', - }); - } + assertSessionCommandable(state); invariant( typeof text === 'string' && text.length > 0, @@ -950,11 +949,7 @@ export async function runHost(sessionId: string): Promise { run: async (params: unknown) => { const { command, noWait, timeoutMs } = params as RunParams; - if (!isSessionRunning(state)) { - throw makeCliError(ERROR_CODES.SESSION_NOT_RUNNING, { - message: 'Session is not running.', - }); - } + assertSessionCommandable(state); invariant( typeof command === 'string' && command.length > 0, @@ -1047,11 +1042,7 @@ export async function runHost(sessionId: string): Promise { sendKeys: async (params: unknown) => { const { keys } = params as SendKeysParams; - if (!isSessionRunning(state)) { - throw makeCliError(ERROR_CODES.SESSION_NOT_RUNNING, { - message: 'Session is not running.', - }); - } + assertSessionCommandable(state); invariant( Array.isArray(keys) && keys.length > 0, @@ -1098,11 +1089,7 @@ export async function runHost(sessionId: string): Promise { resize: async (params: unknown) => { const { cols, rows } = params as ResizeParams; - if (!isSessionRunning(state)) { - throw makeCliError(ERROR_CODES.SESSION_NOT_RUNNING, { - message: 'Session is not running.', - }); - } + assertSessionCommandable(state); invariant( Number.isInteger(cols) && cols > 0, @@ -1123,11 +1110,7 @@ export async function runHost(sessionId: string): Promise { signal: async (params: unknown) => { const { signal } = params as SignalParams; - if (!isSessionRunning(state)) { - throw makeCliError(ERROR_CODES.SESSION_NOT_RUNNING, { - message: 'Session is not running.', - }); - } + assertSessionCommandable(state); invariant( typeof signal === 'string' && signal.length > 0, @@ -1209,11 +1192,7 @@ export async function runHost(sessionId: string): Promise { return result; }); } else { - if (!isSessionRunning(state)) { - throw makeCliError(ERROR_CODES.SESSION_NOT_RUNNING, { - message: 'Session is not running.', - }); - } + assertSessionCommandable(state); const idleDuration = idleMs ?? 0; invariant( @@ -1537,7 +1516,7 @@ export async function runHost(sessionId: string): Promise { await writeManifest(mPath, state.snapshot()); await mkdir(dirname(sPath), { recursive: true }); - if (!isSessionRunning(state)) { + if (!isSessionCommandable(state)) { await initiateShutdown(); return; } @@ -1545,7 +1524,7 @@ export async function runHost(sessionId: string): Promise { rpcListenPromise = rpcServer.listen(); await rpcListenPromise; - if (!isSessionRunning(state)) { + if (!isSessionCommandable(state)) { await initiateShutdown(); } } catch (error) { diff --git a/src/host/lifecycle.ts b/src/host/lifecycle.ts index 9457bb3..1b423c7 100644 --- a/src/host/lifecycle.ts +++ b/src/host/lifecycle.ts @@ -8,6 +8,11 @@ import { ulid } from 'ulid'; import { CliError } from '../cli/errors.js'; import { ERROR_CODES, makeCliError } from '../protocol/errors.js'; +import { + isActiveSessionStatus, + isDestroyedSessionStatus, + isTerminalSessionStatus, +} from '../protocol/sessionStatusPolicy.js'; import type { FailureOrigin, SessionRecord } from '../protocol/schemas.js'; import { ensureHome, resolveHome } from '../storage/home.js'; import { @@ -139,22 +144,6 @@ function assertStringRecord( } } -function isSessionTerminal(record: SessionRecord): boolean { - return ( - record.status === 'exited' || - record.status === 'failed' || - record.status === 'destroyed' - ); -} - -function isSessionActive(record: SessionRecord): boolean { - return ( - record.status === 'running' || - record.status === 'exiting' || - record.status === 'destroying' - ); -} - function isProcessAlive(pid: number | null): boolean { if (pid === null) { return false; @@ -270,7 +259,7 @@ async function waitForTerminalManifest( for (let attempt = 0; attempt < maxAttempts; attempt += 1) { const manifest = await readManifest(manifestFile); - if (isSessionTerminal(manifest)) { + if (isTerminalSessionStatus(manifest.status)) { return manifest; } @@ -477,14 +466,14 @@ export async function destroySession( getSessionPaths(sessionId); const manifest = await readSessionManifestOrThrow(sessionId, manifestFile); - if (manifest.status === 'destroyed') { + if (isDestroyedSessionStatus(manifest.status)) { throw makeCliError(ERROR_CODES.SESSION_ALREADY_DESTROYED, { message: `Session "${sessionId}" is already destroyed.`, details: { sessionId }, }); } - if (isSessionTerminal(manifest)) { + if (isTerminalSessionStatus(manifest.status)) { const destroyedManifest: SessionRecord = { ...manifest, status: 'destroyed', @@ -511,7 +500,7 @@ export async function destroySession( sessionId, manifestFile, ); - if (isSessionTerminal(reconciledManifest)) { + if (isTerminalSessionStatus(reconciledManifest.status)) { return; } @@ -539,7 +528,7 @@ export async function destroySession( sessionId, manifestFile, ); - if (isSessionTerminal(reconciledManifest)) { + if (isTerminalSessionStatus(reconciledManifest.status)) { return; } @@ -556,7 +545,7 @@ export async function destroySession( sessionId, manifestFile, ); - if (isSessionTerminal(reconciledManifest)) { + if (isTerminalSessionStatus(reconciledManifest.status)) { return; } @@ -604,7 +593,7 @@ export async function listSessions( continue; } - if (isSessionActive(manifest)) { + if (isActiveSessionStatus(manifest.status)) { try { await reconcileSession(sessionDirectory); manifest = await readManifestIfExists(manifestFile); @@ -617,7 +606,7 @@ export async function listSessions( } } - if (all !== true && isSessionTerminal(manifest)) { + if (all !== true && isTerminalSessionStatus(manifest.status)) { continue; } @@ -640,7 +629,7 @@ export async function reconcileSession( const manifestFile = manifestPath(sessionDirectory); const manifest = await readManifestIfExists(manifestFile); - if (manifest === null || isSessionTerminal(manifest)) { + if (manifest === null || isTerminalSessionStatus(manifest.status)) { return; } diff --git a/src/protocol/sessionStatusPolicy.ts b/src/protocol/sessionStatusPolicy.ts new file mode 100644 index 0000000..6581762 --- /dev/null +++ b/src/protocol/sessionStatusPolicy.ts @@ -0,0 +1,133 @@ +import type { SessionStatus } from './schemas.js'; + +import { invariant } from '../util/assert.js'; + +// Canonical truth table for session lifecycle classifications. +// See CONTEXT.md for the vocabulary these predicates implement. +export interface SessionStatusPolicy { + readonly active: boolean; + readonly terminal: boolean; + readonly commandable: boolean; + readonly liveHostEligible: boolean; + readonly offlineReplayEligible: boolean; + readonly collectable: boolean; + readonly destroyed: boolean; +} + +const SESSION_STATUS_POLICIES = { + running: { + active: true, + terminal: false, + commandable: true, + liveHostEligible: true, + offlineReplayEligible: false, + collectable: false, + destroyed: false, + }, + exiting: { + active: true, + terminal: false, + commandable: false, + liveHostEligible: true, + offlineReplayEligible: false, + collectable: false, + destroyed: false, + }, + exited: { + active: false, + terminal: true, + commandable: false, + liveHostEligible: false, + offlineReplayEligible: true, + collectable: true, + destroyed: false, + }, + failed: { + active: false, + terminal: true, + commandable: false, + liveHostEligible: false, + offlineReplayEligible: true, + collectable: true, + destroyed: false, + }, + destroying: { + active: true, + terminal: false, + commandable: false, + liveHostEligible: false, + offlineReplayEligible: true, + collectable: false, + destroyed: false, + }, + destroyed: { + active: false, + terminal: true, + commandable: false, + liveHostEligible: false, + offlineReplayEligible: true, + collectable: true, + destroyed: true, + }, +} satisfies Record; + +for (const [status, policy] of Object.entries(SESSION_STATUS_POLICIES)) { + invariant( + policy.active !== policy.terminal, + `${status} sessions must be exactly one of active or terminal`, + ); + invariant( + !policy.collectable || policy.terminal, + `${status} collectable sessions must be terminal`, + ); + invariant( + !policy.commandable || policy.active, + `${status} commandable sessions must be active`, + ); + invariant( + policy.liveHostEligible !== policy.offlineReplayEligible, + `${status} sessions must use exactly one renderer state source`, + ); +} + +export function getSessionStatusPolicy( + status: SessionStatus, +): SessionStatusPolicy { + invariant( + Object.hasOwn(SESSION_STATUS_POLICIES, status), + `unknown session status: ${status}`, + ); + return SESSION_STATUS_POLICIES[status]; +} + +export function isActiveSessionStatus(status: SessionStatus): boolean { + return getSessionStatusPolicy(status).active; +} + +export function isTerminalSessionStatus(status: SessionStatus): boolean { + return getSessionStatusPolicy(status).terminal; +} + +export function isCommandableSessionStatus(status: SessionStatus): boolean { + return getSessionStatusPolicy(status).commandable; +} + +export function isLiveHostEligibleSessionStatus( + status: SessionStatus, +): boolean { + return getSessionStatusPolicy(status).liveHostEligible; +} + +export function isOfflineReplayEligibleSessionStatus( + status: SessionStatus, +): boolean { + return getSessionStatusPolicy(status).offlineReplayEligible; +} + +export function isCollectableSessionStatus(status: SessionStatus): boolean { + return getSessionStatusPolicy(status).collectable; +} + +export function isDestroyedSessionStatus(status: SessionStatus): boolean { + return getSessionStatusPolicy(status).destroyed; +} diff --git a/test/unit/commands/mark.test.ts b/test/unit/commands/mark.test.ts index a1ca754..5752a39 100644 --- a/test/unit/commands/mark.test.ts +++ b/test/unit/commands/mark.test.ts @@ -49,7 +49,7 @@ const TEST_CONTEXT = { } as const; function createSessionRecord( - status: 'running' | 'exiting' | 'exited' = 'running', + status: 'running' | 'exiting' | 'exited' | 'destroyed' = 'running', ) { return { version: 1, @@ -127,6 +127,29 @@ describe('mark command', () => { expect(mocks.sendRpc).not.toHaveBeenCalled(); }); + it('throws SESSION_ALREADY_DESTROYED when the session is destroyed', async () => { + mocks.readManifestIfExists.mockResolvedValue( + createSessionRecord('destroyed'), + ); + + await expect( + runMarkCommand({ + context: TEST_CONTEXT, + json: false, + sessionId: 'session-01', + label: 'checkpoint', + }), + ).rejects.toMatchObject({ + code: ERROR_CODES.SESSION_ALREADY_DESTROYED, + message: 'Session "session-01" is already destroyed.', + details: { + sessionId: 'session-01', + status: 'destroyed', + }, + }); + expect(mocks.sendRpc).not.toHaveBeenCalled(); + }); + it('throws SESSION_NOT_FOUND when the session does not exist', async () => { mocks.readManifestIfExists.mockResolvedValue(null); diff --git a/test/unit/commands/type.test.ts b/test/unit/commands/type.test.ts index 3e3ea12..caab8eb 100644 --- a/test/unit/commands/type.test.ts +++ b/test/unit/commands/type.test.ts @@ -1,5 +1,7 @@ import { beforeEach, describe, expect, it, vi } from 'vitest'; +import { ERROR_CODES } from '../../../src/protocol/errors.js'; + const mocks = vi.hoisted(() => ({ emitSuccess: vi.fn(), sendRpc: vi.fn(), @@ -47,7 +49,7 @@ const TEST_CONTEXT = { } as const; function createSessionRecord( - status: 'running' | 'exiting' | 'exited' = 'running', + status: 'running' | 'exiting' | 'exited' | 'destroyed' = 'running', ) { return { version: 1, @@ -180,4 +182,27 @@ describe('type command', () => { { text: '\n' }, ); }); + + it('throws SESSION_ALREADY_DESTROYED when the session is destroyed', async () => { + mocks.readManifestIfExists.mockResolvedValue( + createSessionRecord('destroyed'), + ); + + await expect( + runTypeCommand({ + context: TEST_CONTEXT, + json: false, + sessionId: 'session-01', + text: 'hello', + }), + ).rejects.toMatchObject({ + code: ERROR_CODES.SESSION_ALREADY_DESTROYED, + message: 'Session "session-01" is already destroyed.', + details: { + sessionId: 'session-01', + status: 'destroyed', + }, + }); + expect(mocks.sendRpc).not.toHaveBeenCalled(); + }); }); diff --git a/test/unit/commands/wait.test.ts b/test/unit/commands/wait.test.ts index 5a161ee..8507e16 100644 --- a/test/unit/commands/wait.test.ts +++ b/test/unit/commands/wait.test.ts @@ -54,7 +54,7 @@ const TEST_CONTEXT = { } as const; function createSessionRecord( - status: 'running' | 'exited' = 'running', + status: 'running' | 'exited' | 'destroyed' = 'running', exitCode: number | null = null, ) { return { @@ -335,6 +335,43 @@ describe('wait command', () => { ); }); + it('returns immediately for --exit waits on already-terminal sessions', async () => { + mocks.readManifestIfExists.mockResolvedValue( + createSessionRecord('exited', 42), + ); + + await runWaitCommand(createOptions({ waitForExit: true })); + + expect(mocks.sendRpc).not.toHaveBeenCalled(); + expect(mocks.emitSuccess).toHaveBeenCalledWith({ + command: 'wait', + json: false, + result: { + timedOut: false, + exitCode: 42, + }, + lines: ['Process exited with code 42.'], + }); + }); + + it('omits exitCode for --exit waits on terminal sessions without an exit code', async () => { + mocks.readManifestIfExists.mockResolvedValue( + createSessionRecord('exited', null), + ); + + await runWaitCommand(createOptions({ waitForExit: true })); + + expect(mocks.sendRpc).not.toHaveBeenCalled(); + expect(mocks.emitSuccess).toHaveBeenCalledWith({ + command: 'wait', + json: false, + result: { + timedOut: false, + }, + lines: ['Wait condition met.'], + }); + }); + it('routes --idle-ms waits to the legacy wait RPC', async () => { const result = { timedOut: false }; mocks.sendRpc.mockResolvedValue(result); @@ -606,6 +643,24 @@ describe('wait command', () => { expect(mocks.sendRpc).not.toHaveBeenCalled(); }); + it('throws SESSION_ALREADY_DESTROYED for idle waits on destroyed sessions', async () => { + mocks.readManifestIfExists.mockResolvedValue( + createSessionRecord('destroyed'), + ); + + await expect( + runWaitCommand(createOptions({ idleMs: 500 })), + ).rejects.toMatchObject({ + code: ERROR_CODES.SESSION_ALREADY_DESTROYED, + message: 'Session "session-01" is already destroyed.', + details: { + sessionId: 'session-01', + status: 'destroyed', + }, + }); + expect(mocks.sendRpc).not.toHaveBeenCalled(); + }); + it('surfaces render wait errors when the session is no longer running', async () => { mocks.readManifestIfExists.mockResolvedValue( createSessionRecord('exited', 0), diff --git a/test/unit/protocol/sessionStatusPolicy.test.ts b/test/unit/protocol/sessionStatusPolicy.test.ts new file mode 100644 index 0000000..4c5d473 --- /dev/null +++ b/test/unit/protocol/sessionStatusPolicy.test.ts @@ -0,0 +1,92 @@ +import { describe, expect, it } from 'vitest'; + +import { + getSessionStatusPolicy, + isActiveSessionStatus, + isCollectableSessionStatus, + isCommandableSessionStatus, + isDestroyedSessionStatus, + isLiveHostEligibleSessionStatus, + isOfflineReplayEligibleSessionStatus, + isTerminalSessionStatus, +} from '../../../src/protocol/sessionStatusPolicy.js'; +import type { SessionStatus } from '../../../src/protocol/schemas.js'; +import { SessionStatusSchema } from '../../../src/protocol/schemas.js'; + +const POLICY_FIELDS = [ + 'active', + 'terminal', + 'commandable', + 'liveHostEligible', + 'offlineReplayEligible', + 'collectable', + 'destroyed', +] as const; + +const EXPECTED_STATUS_SETS = { + active: ['running', 'exiting', 'destroying'], + terminal: ['exited', 'failed', 'destroyed'], + commandable: ['running'], + liveHostEligible: ['running', 'exiting'], + offlineReplayEligible: ['exited', 'failed', 'destroying', 'destroyed'], + collectable: ['exited', 'failed', 'destroyed'], + destroyed: ['destroyed'], +} satisfies Record<(typeof POLICY_FIELDS)[number], readonly SessionStatus[]>; + +function expectStatuses( + predicate: (status: SessionStatus) => boolean, + expectedStatuses: readonly SessionStatus[], +): void { + expect(SessionStatusSchema.options.filter(predicate).toSorted()).toEqual( + expectedStatuses.toSorted(), + ); +} + +describe('session status policy', () => { + it('classifies every session status into documented status sets', () => { + expectStatuses(isActiveSessionStatus, EXPECTED_STATUS_SETS.active); + expectStatuses(isTerminalSessionStatus, EXPECTED_STATUS_SETS.terminal); + expectStatuses( + isCommandableSessionStatus, + EXPECTED_STATUS_SETS.commandable, + ); + expectStatuses( + isLiveHostEligibleSessionStatus, + EXPECTED_STATUS_SETS.liveHostEligible, + ); + expectStatuses( + isOfflineReplayEligibleSessionStatus, + EXPECTED_STATUS_SETS.offlineReplayEligible, + ); + expectStatuses( + isCollectableSessionStatus, + EXPECTED_STATUS_SETS.collectable, + ); + expectStatuses(isDestroyedSessionStatus, EXPECTED_STATUS_SETS.destroyed); + }); + + it('returns complete policy objects that agree with the named predicates', () => { + for (const status of SessionStatusSchema.options) { + const policy = getSessionStatusPolicy(status); + expect(Object.keys(policy).toSorted()).toEqual(POLICY_FIELDS.toSorted()); + expect(policy.active).toBe(isActiveSessionStatus(status)); + expect(policy.terminal).toBe(isTerminalSessionStatus(status)); + expect(policy.commandable).toBe(isCommandableSessionStatus(status)); + expect(policy.liveHostEligible).toBe( + isLiveHostEligibleSessionStatus(status), + ); + expect(policy.offlineReplayEligible).toBe( + isOfflineReplayEligibleSessionStatus(status), + ); + expect(policy.collectable).toBe(isCollectableSessionStatus(status)); + expect(policy.destroyed).toBe(isDestroyedSessionStatus(status)); + } + }); + + it('preserves the destroying status split between active and offline replay', () => { + expect(isActiveSessionStatus('destroying')).toBe(true); + expect(isTerminalSessionStatus('destroying')).toBe(false); + expect(isOfflineReplayEligibleSessionStatus('destroying')).toBe(true); + expect(isCollectableSessionStatus('destroying')).toBe(false); + }); +});