From 6e9ec0abc82a507531fc0b9a866cc66cdd0e2e65 Mon Sep 17 00:00:00 2001 From: Thomas Kosiewski Date: Wed, 29 Apr 2026 11:58:44 +0200 Subject: [PATCH 1/2] refactor: centralize session status policy Change-Id: Ieb854aba647511d3797d6c5319cc31d06eb20446 Signed-off-by: Thomas Kosiewski --- CONTEXT.md | 48 +++++++ src/cli/commands/gc.ts | 23 ++-- src/cli/commands/inspect.ts | 30 ++--- src/cli/commands/mark.ts | 8 +- src/cli/commands/paste.ts | 8 +- src/cli/commands/resize.ts | 8 +- src/cli/commands/run.ts | 8 +- src/cli/commands/send-keys.ts | 8 +- src/cli/commands/signal.ts | 8 +- src/cli/commands/type.ts | 8 +- src/cli/commands/wait.ts | 16 +-- src/host/hostMain.ts | 3 +- src/host/lifecycle.ts | 39 ++---- src/protocol/sessionStatusPolicy.ts | 127 ++++++++++++++++++ .../unit/protocol/sessionStatusPolicy.test.ts | 107 +++++++++++++++ 15 files changed, 366 insertions(+), 83 deletions(-) create mode 100644 CONTEXT.md create mode 100644 src/protocol/sessionStatusPolicy.ts create mode 100644 test/unit/protocol/sessionStatusPolicy.test.ts diff --git a/CONTEXT.md b/CONTEXT.md new file mode 100644 index 0000000..5ba0731 --- /dev/null +++ b/CONTEXT.md @@ -0,0 +1,48 @@ +# 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 + +## 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..de333bc 100644 --- a/src/cli/commands/gc.ts +++ b/src/cli/commands/gc.ts @@ -6,6 +6,7 @@ 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 } 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 +143,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 ( + !isCollectableSessionStatus(manifestBefore.status) && + isCollectableSessionStatus(manifestAfter.status) + ); } function shouldSkipForAge( @@ -335,11 +337,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 +382,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..1a1692b 100644 --- a/src/cli/commands/inspect.ts +++ b/src/cli/commands/inspect.ts @@ -12,6 +12,10 @@ 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 { + 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'; @@ -54,23 +58,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 +71,7 @@ function deriveRendererRuntimeSummary(options: { }; } - if (usesOfflineReplay(options.sessionStatus)) { + if (isOfflineReplayEligibleSessionStatus(options.sessionStatus)) { return { backend: RENDERER_BACKEND, mode: 'offline-replay', @@ -146,8 +133,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 +167,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..7efc7d4 100644 --- a/src/cli/commands/mark.ts +++ b/src/cli/commands/mark.ts @@ -5,6 +5,10 @@ import { emitSuccess } from '../output.js'; import { sendRpc } from '../../host/rpcClient.js'; import { MarkResultSchema } from '../../protocol/messages.js'; import { ERROR_CODES, makeCliError } from '../../protocol/errors.js'; +import { + isCommandableSessionStatus, + isDestroyedSessionStatus, +} from '../../protocol/sessionStatusPolicy.js'; import { readManifestIfExists } from '../../storage/manifests.js'; import { manifestPath, @@ -37,7 +41,7 @@ export async function runMarkCommand(options: CommandOptions): Promise { }); } - if (manifest.status === 'destroyed') { + if (isDestroyedSessionStatus(manifest.status)) { throw makeCliError(ERROR_CODES.SESSION_ALREADY_DESTROYED, { message: `Session "${options.sessionId}" is already destroyed.`, details: { @@ -47,7 +51,7 @@ export async function runMarkCommand(options: CommandOptions): Promise { }); } - if (manifest.status !== 'running') { + if (!isCommandableSessionStatus(manifest.status)) { throw makeCliError(ERROR_CODES.SESSION_NOT_RUNNING, { message: `Session "${options.sessionId}" is not running.`, details: { diff --git a/src/cli/commands/paste.ts b/src/cli/commands/paste.ts index bb5f9df..708f126 100644 --- a/src/cli/commands/paste.ts +++ b/src/cli/commands/paste.ts @@ -3,6 +3,10 @@ import type { CommandContext } from '../context.js'; import { emitSuccess } from '../output.js'; import { sendRpc } from '../../host/rpcClient.js'; import { ERROR_CODES, makeCliError } from '../../protocol/errors.js'; +import { + isCommandableSessionStatus, + isDestroyedSessionStatus, +} from '../../protocol/sessionStatusPolicy.js'; import { readManifestIfExists } from '../../storage/manifests.js'; import { manifestPath, @@ -54,7 +58,7 @@ export async function runPasteCommand(options: CommandOptions): Promise { }); } - if (manifest.status === 'destroyed') { + if (isDestroyedSessionStatus(manifest.status)) { throw makeCliError(ERROR_CODES.SESSION_ALREADY_DESTROYED, { message: `Session "${options.sessionId}" is already destroyed.`, details: { @@ -64,7 +68,7 @@ export async function runPasteCommand(options: CommandOptions): Promise { }); } - if (manifest.status !== 'running') { + if (!isCommandableSessionStatus(manifest.status)) { throw makeCliError(ERROR_CODES.SESSION_NOT_RUNNING, { message: `Session "${options.sessionId}" is not running.`, details: { diff --git a/src/cli/commands/resize.ts b/src/cli/commands/resize.ts index ddf3a09..ab4e7d0 100644 --- a/src/cli/commands/resize.ts +++ b/src/cli/commands/resize.ts @@ -3,6 +3,10 @@ import type { CommandContext } from '../context.js'; import { emitSuccess } from '../output.js'; import { sendRpc } from '../../host/rpcClient.js'; import { ERROR_CODES, makeCliError } from '../../protocol/errors.js'; +import { + isCommandableSessionStatus, + isDestroyedSessionStatus, +} from '../../protocol/sessionStatusPolicy.js'; import { readManifestIfExists } from '../../storage/manifests.js'; import { manifestPath, @@ -39,7 +43,7 @@ export async function runResizeCommand(options: CommandOptions): Promise { }); } - if (manifest.status === 'destroyed') { + if (isDestroyedSessionStatus(manifest.status)) { throw makeCliError(ERROR_CODES.SESSION_ALREADY_DESTROYED, { message: `Session "${options.sessionId}" is already destroyed.`, details: { @@ -49,7 +53,7 @@ export async function runResizeCommand(options: CommandOptions): Promise { }); } - if (manifest.status !== 'running') { + if (!isCommandableSessionStatus(manifest.status)) { throw makeCliError(ERROR_CODES.SESSION_NOT_RUNNING, { message: `Session "${options.sessionId}" is not running.`, details: { diff --git a/src/cli/commands/run.ts b/src/cli/commands/run.ts index 29599f8..feb0ca2 100644 --- a/src/cli/commands/run.ts +++ b/src/cli/commands/run.ts @@ -5,6 +5,10 @@ import { sendRpc } from '../../host/rpcClient.js'; import type { RunResult } from '../../protocol/messages.js'; import { RunResultSchema } from '../../protocol/messages.js'; import { ERROR_CODES, makeCliError } from '../../protocol/errors.js'; +import { + isCommandableSessionStatus, + isDestroyedSessionStatus, +} from '../../protocol/sessionStatusPolicy.js'; import { readManifestIfExists } from '../../storage/manifests.js'; import { manifestPath, @@ -63,7 +67,7 @@ export async function runRunCommand(options: CommandOptions): Promise { }); } - if (manifest.status === 'destroyed') { + if (isDestroyedSessionStatus(manifest.status)) { throw makeCliError(ERROR_CODES.SESSION_ALREADY_DESTROYED, { message: `Session "${options.sessionId}" is already destroyed.`, details: { @@ -73,7 +77,7 @@ export async function runRunCommand(options: CommandOptions): Promise { }); } - if (manifest.status !== 'running') { + if (!isCommandableSessionStatus(manifest.status)) { throw makeCliError(ERROR_CODES.SESSION_NOT_RUNNING, { message: `Session "${options.sessionId}" is not running.`, details: { diff --git a/src/cli/commands/send-keys.ts b/src/cli/commands/send-keys.ts index 2d517c2..e79d0b6 100644 --- a/src/cli/commands/send-keys.ts +++ b/src/cli/commands/send-keys.ts @@ -5,6 +5,10 @@ import { emitSuccess } from '../output.js'; import { sendRpc } from '../../host/rpcClient.js'; import { SendKeysResultSchema } from '../../protocol/messages.js'; import { ERROR_CODES, makeCliError } from '../../protocol/errors.js'; +import { + isCommandableSessionStatus, + isDestroyedSessionStatus, +} from '../../protocol/sessionStatusPolicy.js'; import { readManifestIfExists } from '../../storage/manifests.js'; import { manifestPath, @@ -39,7 +43,7 @@ export async function runSendKeysCommand( }); } - if (manifest.status === 'destroyed') { + if (isDestroyedSessionStatus(manifest.status)) { throw makeCliError(ERROR_CODES.SESSION_ALREADY_DESTROYED, { message: `Session "${options.sessionId}" is already destroyed.`, details: { @@ -49,7 +53,7 @@ export async function runSendKeysCommand( }); } - if (manifest.status !== 'running') { + if (!isCommandableSessionStatus(manifest.status)) { throw makeCliError(ERROR_CODES.SESSION_NOT_RUNNING, { message: `Session "${options.sessionId}" is not running.`, details: { diff --git a/src/cli/commands/signal.ts b/src/cli/commands/signal.ts index 49620ec..bf83bef 100644 --- a/src/cli/commands/signal.ts +++ b/src/cli/commands/signal.ts @@ -3,6 +3,10 @@ import type { CommandContext } from '../context.js'; import { emitSuccess } from '../output.js'; import { sendRpc } from '../../host/rpcClient.js'; import { ERROR_CODES, makeCliError } from '../../protocol/errors.js'; +import { + isCommandableSessionStatus, + isDestroyedSessionStatus, +} from '../../protocol/sessionStatusPolicy.js'; import { readManifestIfExists } from '../../storage/manifests.js'; import { manifestPath, @@ -47,7 +51,7 @@ export async function runSignalCommand(options: CommandOptions): Promise { }); } - if (manifest.status === 'destroyed') { + if (isDestroyedSessionStatus(manifest.status)) { throw makeCliError(ERROR_CODES.SESSION_ALREADY_DESTROYED, { message: `Session "${options.sessionId}" is already destroyed.`, details: { @@ -57,7 +61,7 @@ export async function runSignalCommand(options: CommandOptions): Promise { }); } - if (manifest.status !== 'running') { + if (!isCommandableSessionStatus(manifest.status)) { throw makeCliError(ERROR_CODES.SESSION_NOT_RUNNING, { message: `Session "${options.sessionId}" is not running.`, details: { diff --git a/src/cli/commands/type.ts b/src/cli/commands/type.ts index c84aa54..3a58961 100644 --- a/src/cli/commands/type.ts +++ b/src/cli/commands/type.ts @@ -3,6 +3,10 @@ import type { CommandContext } from '../context.js'; import { emitSuccess } from '../output.js'; import { sendRpc } from '../../host/rpcClient.js'; import { ERROR_CODES, makeCliError } from '../../protocol/errors.js'; +import { + isCommandableSessionStatus, + isDestroyedSessionStatus, +} from '../../protocol/sessionStatusPolicy.js'; import { readManifestIfExists } from '../../storage/manifests.js'; import { manifestPath, @@ -57,7 +61,7 @@ export async function runTypeCommand(options: CommandOptions): Promise { }); } - if (manifest.status === 'destroyed') { + if (isDestroyedSessionStatus(manifest.status)) { throw makeCliError(ERROR_CODES.SESSION_ALREADY_DESTROYED, { message: `Session "${options.sessionId}" is already destroyed.`, details: { @@ -67,7 +71,7 @@ export async function runTypeCommand(options: CommandOptions): Promise { }); } - if (manifest.status !== 'running') { + if (!isCommandableSessionStatus(manifest.status)) { throw makeCliError(ERROR_CODES.SESSION_NOT_RUNNING, { message: `Session "${options.sessionId}" is not running.`, details: { diff --git a/src/cli/commands/wait.ts b/src/cli/commands/wait.ts index c58b7ac..9a9a122 100644 --- a/src/cli/commands/wait.ts +++ b/src/cli/commands/wait.ts @@ -14,6 +14,11 @@ import { WaitForRenderResultSchema, WaitResultSchema, } from '../../protocol/schemas.js'; +import { + isCommandableSessionStatus, + isDestroyedSessionStatus, + isTerminalSessionStatus, +} from '../../protocol/sessionStatusPolicy.js'; import { withOfflineReplayRenderer } from '../../replay/offlineReplay.js'; import { readManifestIfExists } from '../../storage/manifests.js'; import { @@ -345,12 +350,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,7 +365,7 @@ export async function runWaitCommand(options: CommandOptions): Promise { return; } - if (manifest.status === 'destroyed') { + if (isDestroyedSessionStatus(manifest.status)) { throw makeCliError(ERROR_CODES.SESSION_ALREADY_DESTROYED, { message: `Session "${options.sessionId}" is already destroyed.`, details: { @@ -375,7 +375,7 @@ export async function runWaitCommand(options: CommandOptions): Promise { }); } - if (!options.waitForExit && manifestStatus !== 'running') { + if (!options.waitForExit && !isCommandableSessionStatus(manifestStatus)) { throw makeCliError(ERROR_CODES.SESSION_NOT_RUNNING, { message: `Session "${options.sessionId}" is not running.`, details: { diff --git a/src/host/hostMain.ts b/src/host/hostMain.ts index c8d837d..4281cda 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, @@ -123,7 +124,7 @@ function normalizeExitSignal(signal: number | null): string | null { } function isSessionRunning(state: SessionState): boolean { - return state.snapshot().status === 'running'; + return isCommandableSessionStatus(state.snapshot().status); } function rethrowAsync(error: unknown): void { 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..e16ad76 --- /dev/null +++ b/src/protocol/sessionStatusPolicy.ts @@ -0,0 +1,127 @@ +import type { SessionStatus } from './schemas.js'; + +import { invariant } from '../util/assert.js'; + +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.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 cannot use live-host and offline-replay rendering at once`, + ); +} + +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/protocol/sessionStatusPolicy.test.ts b/test/unit/protocol/sessionStatusPolicy.test.ts new file mode 100644 index 0000000..dc2ffdc --- /dev/null +++ b/test/unit/protocol/sessionStatusPolicy.test.ts @@ -0,0 +1,107 @@ +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 EXPECTED_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>; + +describe('session status policy', () => { + it('classifies every session status explicitly', () => { + expect(Object.keys(EXPECTED_POLICIES).sort()).toEqual( + [...SessionStatusSchema.options].sort(), + ); + + for (const status of SessionStatusSchema.options) { + expect(getSessionStatusPolicy(status)).toEqual(EXPECTED_POLICIES[status]); + } + }); + + it('exposes named predicates for callers', () => { + for (const status of SessionStatusSchema.options) { + const expected = EXPECTED_POLICIES[status]; + expect(isActiveSessionStatus(status)).toBe(expected.active); + expect(isTerminalSessionStatus(status)).toBe(expected.terminal); + expect(isCommandableSessionStatus(status)).toBe(expected.commandable); + expect(isLiveHostEligibleSessionStatus(status)).toBe( + expected.liveHostEligible, + ); + expect(isOfflineReplayEligibleSessionStatus(status)).toBe( + expected.offlineReplayEligible, + ); + expect(isCollectableSessionStatus(status)).toBe(expected.collectable); + expect(isDestroyedSessionStatus(status)).toBe(expected.destroyed); + } + }); + + 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); + }); +}); From 999d42c2c23ba8585d8322f5fbb3e90085dca5ed Mon Sep 17 00:00:00 2001 From: Thomas Kosiewski Date: Wed, 29 Apr 2026 12:45:20 +0200 Subject: [PATCH 2/2] refactor: add policy invariants and command guard tests Change-Id: I317c32b10c7bd62ac82784bfb9d3d6c791d373b9 Signed-off-by: Thomas Kosiewski --- CONTEXT.md | 3 + src/cli/commands/gc.ts | 9 +- src/cli/commands/inspect.ts | 7 +- src/cli/commands/mark.ts | 25 +--- src/cli/commands/paste.ts | 25 +--- src/cli/commands/resize.ts | 25 +--- src/cli/commands/run.ts | 25 +--- src/cli/commands/screenshot.ts | 3 + src/cli/commands/send-keys.ts | 25 +--- src/cli/commands/signal.ts | 25 +--- src/cli/commands/snapshot.ts | 3 + src/cli/commands/type.ts | 25 +--- src/cli/commands/wait.ts | 27 +--- src/cli/sessionGuards.ts | 43 ++++++ src/host/hostMain.ts | 70 ++++----- src/protocol/sessionStatusPolicy.ts | 10 +- test/unit/commands/mark.test.ts | 25 +++- test/unit/commands/type.test.ts | 27 +++- test/unit/commands/wait.test.ts | 57 +++++++- .../unit/protocol/sessionStatusPolicy.test.ts | 133 ++++++++---------- 20 files changed, 278 insertions(+), 314 deletions(-) create mode 100644 src/cli/sessionGuards.ts diff --git a/CONTEXT.md b/CONTEXT.md index 5ba0731..46d7015 100644 --- a/CONTEXT.md +++ b/CONTEXT.md @@ -30,6 +30,9 @@ A **Session** where callers should reconstruct renderer state from persisted rep 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. diff --git a/src/cli/commands/gc.ts b/src/cli/commands/gc.ts index de333bc..74b5c21 100644 --- a/src/cli/commands/gc.ts +++ b/src/cli/commands/gc.ts @@ -6,7 +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 } from '../../protocol/sessionStatusPolicy.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'; @@ -144,8 +147,8 @@ function wasReconciledFromStaleHost( manifestAfter: SessionRecord, ): boolean { return ( - !isCollectableSessionStatus(manifestBefore.status) && - isCollectableSessionStatus(manifestAfter.status) + !isTerminalSessionStatus(manifestBefore.status) && + isTerminalSessionStatus(manifestAfter.status) ); } diff --git a/src/cli/commands/inspect.ts b/src/cli/commands/inspect.ts index 1a1692b..e236b53 100644 --- a/src/cli/commands/inspect.ts +++ b/src/cli/commands/inspect.ts @@ -13,6 +13,7 @@ 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'; @@ -36,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); } diff --git a/src/cli/commands/mark.ts b/src/cli/commands/mark.ts index 7efc7d4..3e28e7f 100644 --- a/src/cli/commands/mark.ts +++ b/src/cli/commands/mark.ts @@ -5,16 +5,13 @@ import { emitSuccess } from '../output.js'; import { sendRpc } from '../../host/rpcClient.js'; import { MarkResultSchema } from '../../protocol/messages.js'; import { ERROR_CODES, makeCliError } from '../../protocol/errors.js'; -import { - isCommandableSessionStatus, - isDestroyedSessionStatus, -} from '../../protocol/sessionStatusPolicy.js'; import { readManifestIfExists } from '../../storage/manifests.js'; import { manifestPath, sessionDir, socketPath, } from '../../storage/sessionPaths.js'; +import { assertSessionCommandable } from '../sessionGuards.js'; export type { MarkResult } from '../../protocol/messages.js'; @@ -41,25 +38,7 @@ export async function runMarkCommand(options: CommandOptions): Promise { }); } - if (isDestroyedSessionStatus(manifest.status)) { - throw makeCliError(ERROR_CODES.SESSION_ALREADY_DESTROYED, { - message: `Session "${options.sessionId}" is already destroyed.`, - details: { - sessionId: options.sessionId, - status: manifest.status, - }, - }); - } - - if (!isCommandableSessionStatus(manifest.status)) { - 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 708f126..23d4957 100644 --- a/src/cli/commands/paste.ts +++ b/src/cli/commands/paste.ts @@ -3,10 +3,6 @@ import type { CommandContext } from '../context.js'; import { emitSuccess } from '../output.js'; import { sendRpc } from '../../host/rpcClient.js'; import { ERROR_CODES, makeCliError } from '../../protocol/errors.js'; -import { - isCommandableSessionStatus, - isDestroyedSessionStatus, -} from '../../protocol/sessionStatusPolicy.js'; import { readManifestIfExists } from '../../storage/manifests.js'; import { manifestPath, @@ -14,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; @@ -58,25 +55,7 @@ export async function runPasteCommand(options: CommandOptions): Promise { }); } - if (isDestroyedSessionStatus(manifest.status)) { - throw makeCliError(ERROR_CODES.SESSION_ALREADY_DESTROYED, { - message: `Session "${options.sessionId}" is already destroyed.`, - details: { - sessionId: options.sessionId, - status: manifest.status, - }, - }); - } - - if (!isCommandableSessionStatus(manifest.status)) { - 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 ab4e7d0..0486bb3 100644 --- a/src/cli/commands/resize.ts +++ b/src/cli/commands/resize.ts @@ -3,16 +3,13 @@ import type { CommandContext } from '../context.js'; import { emitSuccess } from '../output.js'; import { sendRpc } from '../../host/rpcClient.js'; import { ERROR_CODES, makeCliError } from '../../protocol/errors.js'; -import { - isCommandableSessionStatus, - isDestroyedSessionStatus, -} from '../../protocol/sessionStatusPolicy.js'; import { readManifestIfExists } from '../../storage/manifests.js'; import { manifestPath, sessionDir, socketPath, } from '../../storage/sessionPaths.js'; +import { assertSessionCommandable } from '../sessionGuards.js'; export interface ResizeResult { cols: number; @@ -43,25 +40,7 @@ export async function runResizeCommand(options: CommandOptions): Promise { }); } - if (isDestroyedSessionStatus(manifest.status)) { - throw makeCliError(ERROR_CODES.SESSION_ALREADY_DESTROYED, { - message: `Session "${options.sessionId}" is already destroyed.`, - details: { - sessionId: options.sessionId, - status: manifest.status, - }, - }); - } - - if (!isCommandableSessionStatus(manifest.status)) { - 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 feb0ca2..42fa8ed 100644 --- a/src/cli/commands/run.ts +++ b/src/cli/commands/run.ts @@ -5,10 +5,6 @@ import { sendRpc } from '../../host/rpcClient.js'; import type { RunResult } from '../../protocol/messages.js'; import { RunResultSchema } from '../../protocol/messages.js'; import { ERROR_CODES, makeCliError } from '../../protocol/errors.js'; -import { - isCommandableSessionStatus, - isDestroyedSessionStatus, -} from '../../protocol/sessionStatusPolicy.js'; import { readManifestIfExists } from '../../storage/manifests.js'; import { manifestPath, @@ -16,6 +12,7 @@ import { socketPath, } from '../../storage/sessionPaths.js'; import { resolveCommandInputText } from './inputSource.js'; +import { assertSessionCommandable } from '../sessionGuards.js'; interface CommandOptions { context: CommandContext; @@ -67,25 +64,7 @@ export async function runRunCommand(options: CommandOptions): Promise { }); } - if (isDestroyedSessionStatus(manifest.status)) { - throw makeCliError(ERROR_CODES.SESSION_ALREADY_DESTROYED, { - message: `Session "${options.sessionId}" is already destroyed.`, - details: { - sessionId: options.sessionId, - status: manifest.status, - }, - }); - } - - if (!isCommandableSessionStatus(manifest.status)) { - 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 e79d0b6..28f4dd5 100644 --- a/src/cli/commands/send-keys.ts +++ b/src/cli/commands/send-keys.ts @@ -5,16 +5,13 @@ import { emitSuccess } from '../output.js'; import { sendRpc } from '../../host/rpcClient.js'; import { SendKeysResultSchema } from '../../protocol/messages.js'; import { ERROR_CODES, makeCliError } from '../../protocol/errors.js'; -import { - isCommandableSessionStatus, - isDestroyedSessionStatus, -} from '../../protocol/sessionStatusPolicy.js'; import { readManifestIfExists } from '../../storage/manifests.js'; import { manifestPath, sessionDir, socketPath, } from '../../storage/sessionPaths.js'; +import { assertSessionCommandable } from '../sessionGuards.js'; export type { SendKeysResult } from '../../protocol/messages.js'; @@ -43,25 +40,7 @@ export async function runSendKeysCommand( }); } - if (isDestroyedSessionStatus(manifest.status)) { - throw makeCliError(ERROR_CODES.SESSION_ALREADY_DESTROYED, { - message: `Session "${options.sessionId}" is already destroyed.`, - details: { - sessionId: options.sessionId, - status: manifest.status, - }, - }); - } - - if (!isCommandableSessionStatus(manifest.status)) { - 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 bf83bef..b613dc6 100644 --- a/src/cli/commands/signal.ts +++ b/src/cli/commands/signal.ts @@ -3,16 +3,13 @@ import type { CommandContext } from '../context.js'; import { emitSuccess } from '../output.js'; import { sendRpc } from '../../host/rpcClient.js'; import { ERROR_CODES, makeCliError } from '../../protocol/errors.js'; -import { - isCommandableSessionStatus, - isDestroyedSessionStatus, -} from '../../protocol/sessionStatusPolicy.js'; import { readManifestIfExists } from '../../storage/manifests.js'; import { manifestPath, sessionDir, socketPath, } from '../../storage/sessionPaths.js'; +import { assertSessionCommandable } from '../sessionGuards.js'; const ALLOWED_SIGNALS = [ 'SIGTERM', @@ -51,25 +48,7 @@ export async function runSignalCommand(options: CommandOptions): Promise { }); } - if (isDestroyedSessionStatus(manifest.status)) { - throw makeCliError(ERROR_CODES.SESSION_ALREADY_DESTROYED, { - message: `Session "${options.sessionId}" is already destroyed.`, - details: { - sessionId: options.sessionId, - status: manifest.status, - }, - }); - } - - if (!isCommandableSessionStatus(manifest.status)) { - 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 3a58961..322f4e8 100644 --- a/src/cli/commands/type.ts +++ b/src/cli/commands/type.ts @@ -3,10 +3,6 @@ import type { CommandContext } from '../context.js'; import { emitSuccess } from '../output.js'; import { sendRpc } from '../../host/rpcClient.js'; import { ERROR_CODES, makeCliError } from '../../protocol/errors.js'; -import { - isCommandableSessionStatus, - isDestroyedSessionStatus, -} from '../../protocol/sessionStatusPolicy.js'; import { readManifestIfExists } from '../../storage/manifests.js'; import { manifestPath, @@ -14,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; @@ -61,25 +58,7 @@ export async function runTypeCommand(options: CommandOptions): Promise { }); } - if (isDestroyedSessionStatus(manifest.status)) { - throw makeCliError(ERROR_CODES.SESSION_ALREADY_DESTROYED, { - message: `Session "${options.sessionId}" is already destroyed.`, - details: { - sessionId: options.sessionId, - status: manifest.status, - }, - }); - } - - if (!isCommandableSessionStatus(manifest.status)) { - 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 9a9a122..0eb710f 100644 --- a/src/cli/commands/wait.ts +++ b/src/cli/commands/wait.ts @@ -14,11 +14,7 @@ import { WaitForRenderResultSchema, WaitResultSchema, } from '../../protocol/schemas.js'; -import { - isCommandableSessionStatus, - isDestroyedSessionStatus, - isTerminalSessionStatus, -} from '../../protocol/sessionStatusPolicy.js'; +import { isTerminalSessionStatus } from '../../protocol/sessionStatusPolicy.js'; import { withOfflineReplayRenderer } from '../../replay/offlineReplay.js'; import { readManifestIfExists } from '../../storage/manifests.js'; import { @@ -26,6 +22,7 @@ import { sessionDir, socketPath, } from '../../storage/sessionPaths.js'; +import { assertSessionCommandable } from '../sessionGuards.js'; interface CommandOptions { context: CommandContext; @@ -365,24 +362,8 @@ export async function runWaitCommand(options: CommandOptions): Promise { return; } - if (isDestroyedSessionStatus(manifest.status)) { - throw makeCliError(ERROR_CODES.SESSION_ALREADY_DESTROYED, { - message: `Session "${options.sessionId}" is already destroyed.`, - details: { - sessionId: options.sessionId, - status: manifest.status, - }, - }); - } - - if (!options.waitForExit && !isCommandableSessionStatus(manifestStatus)) { - 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 4281cda..a30cc18 100644 --- a/src/host/hostMain.ts +++ b/src/host/hostMain.ts @@ -123,10 +123,20 @@ function normalizeExitSignal(signal: number | null): string | null { return signal === null || signal === 0 ? null : String(signal); } -function isSessionRunning(state: SessionState): boolean { +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 { process.nextTick(() => { throw error; @@ -598,7 +608,7 @@ export async function runHost(sessionId: string): Promise { const startIdlePolling = (): void => { if ( idleTimeoutMs <= 0 || - !isSessionRunning(state) || + !isSessionCommandable(state) || idleTimeoutHandle !== null ) { return; @@ -606,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; } @@ -627,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()); @@ -903,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); @@ -918,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(); @@ -932,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, @@ -951,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, @@ -1048,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, @@ -1099,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, @@ -1124,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, @@ -1210,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( @@ -1538,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; } @@ -1546,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/protocol/sessionStatusPolicy.ts b/src/protocol/sessionStatusPolicy.ts index e16ad76..6581762 100644 --- a/src/protocol/sessionStatusPolicy.ts +++ b/src/protocol/sessionStatusPolicy.ts @@ -2,6 +2,8 @@ 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; @@ -70,6 +72,10 @@ const SESSION_STATUS_POLICIES = { } 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`, @@ -79,8 +85,8 @@ for (const [status, policy] of Object.entries(SESSION_STATUS_POLICIES)) { `${status} commandable sessions must be active`, ); invariant( - !(policy.liveHostEligible && policy.offlineReplayEligible), - `${status} sessions cannot use live-host and offline-replay rendering at once`, + policy.liveHostEligible !== policy.offlineReplayEligible, + `${status} sessions must use exactly one renderer state source`, ); } 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 index dc2ffdc..4c5d473 100644 --- a/test/unit/protocol/sessionStatusPolicy.test.ts +++ b/test/unit/protocol/sessionStatusPolicy.test.ts @@ -13,88 +13,73 @@ import { import type { SessionStatus } from '../../../src/protocol/schemas.js'; import { SessionStatusSchema } from '../../../src/protocol/schemas.js'; -const EXPECTED_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>; +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 explicitly', () => { - expect(Object.keys(EXPECTED_POLICIES).sort()).toEqual( - [...SessionStatusSchema.options].sort(), + 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, ); - - for (const status of SessionStatusSchema.options) { - expect(getSessionStatusPolicy(status)).toEqual(EXPECTED_POLICIES[status]); - } + 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('exposes named predicates for callers', () => { + it('returns complete policy objects that agree with the named predicates', () => { for (const status of SessionStatusSchema.options) { - const expected = EXPECTED_POLICIES[status]; - expect(isActiveSessionStatus(status)).toBe(expected.active); - expect(isTerminalSessionStatus(status)).toBe(expected.terminal); - expect(isCommandableSessionStatus(status)).toBe(expected.commandable); - expect(isLiveHostEligibleSessionStatus(status)).toBe( - expected.liveHostEligible, + 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(isOfflineReplayEligibleSessionStatus(status)).toBe( - expected.offlineReplayEligible, + expect(policy.offlineReplayEligible).toBe( + isOfflineReplayEligibleSessionStatus(status), ); - expect(isCollectableSessionStatus(status)).toBe(expected.collectable); - expect(isDestroyedSessionStatus(status)).toBe(expected.destroyed); + expect(policy.collectable).toBe(isCollectableSessionStatus(status)); + expect(policy.destroyed).toBe(isDestroyedSessionStatus(status)); } });