Skip to content

Commit 9a5e318

Browse files
committed
refactor: add policy invariants and command guard tests
Change-Id: I317c32b10c7bd62ac82784bfb9d3d6c791d373b9 Signed-off-by: Thomas Kosiewski <tk@coder.com>
1 parent 6e9ec0a commit 9a5e318

20 files changed

Lines changed: 260 additions & 314 deletions

CONTEXT.md

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,9 @@ A **Session** where callers should reconstruct renderer state from persisted rep
3030
A **Terminal Session** whose persisted directory may be removed by garbage collection.
3131
_Avoid_: Deletable session
3232

33+
**Destroyed Status Check**:
34+
A convenience policy predicate for the single `destroyed` **Session Status** value. It is not a separate lifecycle classification.
35+
3336
## Relationships
3437

3538
- A **Session** has exactly one **Session Status** at a time.

src/cli/commands/gc.ts

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,10 @@ import type { CommandContext } from '../context.js';
66
import { emitSuccess } from '../output.js';
77
import { reconcileSession } from '../../host/lifecycle.js';
88
import { ERROR_CODES, makeCliError } from '../../protocol/errors.js';
9-
import { isCollectableSessionStatus } from '../../protocol/sessionStatusPolicy.js';
9+
import {
10+
isCollectableSessionStatus,
11+
isTerminalSessionStatus,
12+
} from '../../protocol/sessionStatusPolicy.js';
1013
import type { SessionRecord } from '../../protocol/schemas.js';
1114
import { readManifestIfExists } from '../../storage/manifests.js';
1215
import { manifestPath, sessionDir } from '../../storage/sessionPaths.js';
@@ -144,8 +147,8 @@ function wasReconciledFromStaleHost(
144147
manifestAfter: SessionRecord,
145148
): boolean {
146149
return (
147-
!isCollectableSessionStatus(manifestBefore.status) &&
148-
isCollectableSessionStatus(manifestAfter.status)
150+
!isTerminalSessionStatus(manifestBefore.status) &&
151+
isTerminalSessionStatus(manifestAfter.status)
149152
);
150153
}
151154

src/cli/commands/inspect.ts

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ import { countEventLogEntries } from '../../host/eventLog.js';
1313
import { reconcileSession } from '../../host/lifecycle.js';
1414
import { sendRpc } from '../../host/rpcClient.js';
1515
import {
16+
isCommandableSessionStatus,
1617
isLiveHostEligibleSessionStatus,
1718
isOfflineReplayEligibleSessionStatus,
1819
} from '../../protocol/sessionStatusPolicy.js';
@@ -36,8 +37,10 @@ interface CommandOptions {
3637

3738
function computeUptime(session: SessionRecord): number {
3839
const createdAt = Date.parse(session.createdAt);
39-
const endAt =
40-
session.status === 'running' ? Date.now() : Date.parse(session.updatedAt);
40+
// Matches pre-existing behavior: only running sessions show live uptime.
41+
const endAt = isCommandableSessionStatus(session.status)
42+
? Date.now()
43+
: Date.parse(session.updatedAt);
4144

4245
return Math.max(0, endAt - createdAt);
4346
}

src/cli/commands/mark.ts

Lines changed: 2 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -5,16 +5,13 @@ import { emitSuccess } from '../output.js';
55
import { sendRpc } from '../../host/rpcClient.js';
66
import { MarkResultSchema } from '../../protocol/messages.js';
77
import { ERROR_CODES, makeCliError } from '../../protocol/errors.js';
8-
import {
9-
isCommandableSessionStatus,
10-
isDestroyedSessionStatus,
11-
} from '../../protocol/sessionStatusPolicy.js';
128
import { readManifestIfExists } from '../../storage/manifests.js';
139
import {
1410
manifestPath,
1511
sessionDir,
1612
socketPath,
1713
} from '../../storage/sessionPaths.js';
14+
import { assertSessionCommandable } from '../sessionGuards.js';
1815

1916
export type { MarkResult } from '../../protocol/messages.js';
2017

@@ -41,25 +38,7 @@ export async function runMarkCommand(options: CommandOptions): Promise<void> {
4138
});
4239
}
4340

44-
if (isDestroyedSessionStatus(manifest.status)) {
45-
throw makeCliError(ERROR_CODES.SESSION_ALREADY_DESTROYED, {
46-
message: `Session "${options.sessionId}" is already destroyed.`,
47-
details: {
48-
sessionId: options.sessionId,
49-
status: manifest.status,
50-
},
51-
});
52-
}
53-
54-
if (!isCommandableSessionStatus(manifest.status)) {
55-
throw makeCliError(ERROR_CODES.SESSION_NOT_RUNNING, {
56-
message: `Session "${options.sessionId}" is not running.`,
57-
details: {
58-
sessionId: options.sessionId,
59-
status: manifest.status,
60-
},
61-
});
62-
}
41+
assertSessionCommandable(manifest, options.sessionId);
6342

6443
const rawResult: unknown = await sendRpc(
6544
socketPath(sessionDirectory),

src/cli/commands/paste.ts

Lines changed: 2 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -3,17 +3,14 @@ import type { CommandContext } from '../context.js';
33
import { emitSuccess } from '../output.js';
44
import { sendRpc } from '../../host/rpcClient.js';
55
import { ERROR_CODES, makeCliError } from '../../protocol/errors.js';
6-
import {
7-
isCommandableSessionStatus,
8-
isDestroyedSessionStatus,
9-
} from '../../protocol/sessionStatusPolicy.js';
106
import { readManifestIfExists } from '../../storage/manifests.js';
117
import {
128
manifestPath,
139
sessionDir,
1410
socketPath,
1511
} from '../../storage/sessionPaths.js';
1612
import { resolveCommandInputText } from './inputSource.js';
13+
import { assertSessionCommandable } from '../sessionGuards.js';
1714

1815
export interface PasteResult {
1916
[key: string]: never;
@@ -58,25 +55,7 @@ export async function runPasteCommand(options: CommandOptions): Promise<void> {
5855
});
5956
}
6057

61-
if (isDestroyedSessionStatus(manifest.status)) {
62-
throw makeCliError(ERROR_CODES.SESSION_ALREADY_DESTROYED, {
63-
message: `Session "${options.sessionId}" is already destroyed.`,
64-
details: {
65-
sessionId: options.sessionId,
66-
status: manifest.status,
67-
},
68-
});
69-
}
70-
71-
if (!isCommandableSessionStatus(manifest.status)) {
72-
throw makeCliError(ERROR_CODES.SESSION_NOT_RUNNING, {
73-
message: `Session "${options.sessionId}" is not running.`,
74-
details: {
75-
sessionId: options.sessionId,
76-
status: manifest.status,
77-
},
78-
});
79-
}
58+
assertSessionCommandable(manifest, options.sessionId);
8059

8160
await sendRpc(socketPath(sessionDirectory), 'paste', {
8261
text,

src/cli/commands/resize.ts

Lines changed: 2 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -3,16 +3,13 @@ import type { CommandContext } from '../context.js';
33
import { emitSuccess } from '../output.js';
44
import { sendRpc } from '../../host/rpcClient.js';
55
import { ERROR_CODES, makeCliError } from '../../protocol/errors.js';
6-
import {
7-
isCommandableSessionStatus,
8-
isDestroyedSessionStatus,
9-
} from '../../protocol/sessionStatusPolicy.js';
106
import { readManifestIfExists } from '../../storage/manifests.js';
117
import {
128
manifestPath,
139
sessionDir,
1410
socketPath,
1511
} from '../../storage/sessionPaths.js';
12+
import { assertSessionCommandable } from '../sessionGuards.js';
1613

1714
export interface ResizeResult {
1815
cols: number;
@@ -43,25 +40,7 @@ export async function runResizeCommand(options: CommandOptions): Promise<void> {
4340
});
4441
}
4542

46-
if (isDestroyedSessionStatus(manifest.status)) {
47-
throw makeCliError(ERROR_CODES.SESSION_ALREADY_DESTROYED, {
48-
message: `Session "${options.sessionId}" is already destroyed.`,
49-
details: {
50-
sessionId: options.sessionId,
51-
status: manifest.status,
52-
},
53-
});
54-
}
55-
56-
if (!isCommandableSessionStatus(manifest.status)) {
57-
throw makeCliError(ERROR_CODES.SESSION_NOT_RUNNING, {
58-
message: `Session "${options.sessionId}" is not running.`,
59-
details: {
60-
sessionId: options.sessionId,
61-
status: manifest.status,
62-
},
63-
});
64-
}
43+
assertSessionCommandable(manifest, options.sessionId);
6544

6645
if (
6746
!Number.isInteger(options.cols) ||

src/cli/commands/run.ts

Lines changed: 2 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -5,17 +5,14 @@ import { sendRpc } from '../../host/rpcClient.js';
55
import type { RunResult } from '../../protocol/messages.js';
66
import { RunResultSchema } from '../../protocol/messages.js';
77
import { ERROR_CODES, makeCliError } from '../../protocol/errors.js';
8-
import {
9-
isCommandableSessionStatus,
10-
isDestroyedSessionStatus,
11-
} from '../../protocol/sessionStatusPolicy.js';
128
import { readManifestIfExists } from '../../storage/manifests.js';
139
import {
1410
manifestPath,
1511
sessionDir,
1612
socketPath,
1713
} from '../../storage/sessionPaths.js';
1814
import { resolveCommandInputText } from './inputSource.js';
15+
import { assertSessionCommandable } from '../sessionGuards.js';
1916

2017
interface CommandOptions {
2118
context: CommandContext;
@@ -67,25 +64,7 @@ export async function runRunCommand(options: CommandOptions): Promise<void> {
6764
});
6865
}
6966

70-
if (isDestroyedSessionStatus(manifest.status)) {
71-
throw makeCliError(ERROR_CODES.SESSION_ALREADY_DESTROYED, {
72-
message: `Session "${options.sessionId}" is already destroyed.`,
73-
details: {
74-
sessionId: options.sessionId,
75-
status: manifest.status,
76-
},
77-
});
78-
}
79-
80-
if (!isCommandableSessionStatus(manifest.status)) {
81-
throw makeCliError(ERROR_CODES.SESSION_NOT_RUNNING, {
82-
message: `Session "${options.sessionId}" is not running.`,
83-
details: {
84-
sessionId: options.sessionId,
85-
status: manifest.status,
86-
},
87-
});
88-
}
67+
assertSessionCommandable(manifest, options.sessionId);
8968

9069
const noWait = !options.wait;
9170
const rpcParams: Record<string, unknown> = {

src/cli/commands/screenshot.ts

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -255,6 +255,9 @@ export async function runScreenshotCommand(
255255
let rawResult: unknown;
256256
let invalidResultMessage = 'Unexpected response from host';
257257

258+
// Snapshot and screenshot intentionally keep their narrower legacy live-RPC
259+
// gate. `exiting` sessions are live-host eligible for inspect, but these
260+
// commands preserve their existing offline-replay capture behavior.
258261
if (manifest.status === 'running') {
259262
try {
260263
rawResult = await sendRpc(socketPath(sessionDirectory), 'screenshot', {

src/cli/commands/send-keys.ts

Lines changed: 2 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -5,16 +5,13 @@ import { emitSuccess } from '../output.js';
55
import { sendRpc } from '../../host/rpcClient.js';
66
import { SendKeysResultSchema } from '../../protocol/messages.js';
77
import { ERROR_CODES, makeCliError } from '../../protocol/errors.js';
8-
import {
9-
isCommandableSessionStatus,
10-
isDestroyedSessionStatus,
11-
} from '../../protocol/sessionStatusPolicy.js';
128
import { readManifestIfExists } from '../../storage/manifests.js';
139
import {
1410
manifestPath,
1511
sessionDir,
1612
socketPath,
1713
} from '../../storage/sessionPaths.js';
14+
import { assertSessionCommandable } from '../sessionGuards.js';
1815

1916
export type { SendKeysResult } from '../../protocol/messages.js';
2017

@@ -43,25 +40,7 @@ export async function runSendKeysCommand(
4340
});
4441
}
4542

46-
if (isDestroyedSessionStatus(manifest.status)) {
47-
throw makeCliError(ERROR_CODES.SESSION_ALREADY_DESTROYED, {
48-
message: `Session "${options.sessionId}" is already destroyed.`,
49-
details: {
50-
sessionId: options.sessionId,
51-
status: manifest.status,
52-
},
53-
});
54-
}
55-
56-
if (!isCommandableSessionStatus(manifest.status)) {
57-
throw makeCliError(ERROR_CODES.SESSION_NOT_RUNNING, {
58-
message: `Session "${options.sessionId}" is not running.`,
59-
details: {
60-
sessionId: options.sessionId,
61-
status: manifest.status,
62-
},
63-
});
64-
}
43+
assertSessionCommandable(manifest, options.sessionId);
6544

6645
const rawResult: unknown = await sendRpc(
6746
socketPath(sessionDirectory),

src/cli/commands/signal.ts

Lines changed: 2 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -3,16 +3,13 @@ import type { CommandContext } from '../context.js';
33
import { emitSuccess } from '../output.js';
44
import { sendRpc } from '../../host/rpcClient.js';
55
import { ERROR_CODES, makeCliError } from '../../protocol/errors.js';
6-
import {
7-
isCommandableSessionStatus,
8-
isDestroyedSessionStatus,
9-
} from '../../protocol/sessionStatusPolicy.js';
106
import { readManifestIfExists } from '../../storage/manifests.js';
117
import {
128
manifestPath,
139
sessionDir,
1410
socketPath,
1511
} from '../../storage/sessionPaths.js';
12+
import { assertSessionCommandable } from '../sessionGuards.js';
1613

1714
const ALLOWED_SIGNALS = [
1815
'SIGTERM',
@@ -51,25 +48,7 @@ export async function runSignalCommand(options: CommandOptions): Promise<void> {
5148
});
5249
}
5350

54-
if (isDestroyedSessionStatus(manifest.status)) {
55-
throw makeCliError(ERROR_CODES.SESSION_ALREADY_DESTROYED, {
56-
message: `Session "${options.sessionId}" is already destroyed.`,
57-
details: {
58-
sessionId: options.sessionId,
59-
status: manifest.status,
60-
},
61-
});
62-
}
63-
64-
if (!isCommandableSessionStatus(manifest.status)) {
65-
throw makeCliError(ERROR_CODES.SESSION_NOT_RUNNING, {
66-
message: `Session "${options.sessionId}" is not running.`,
67-
details: {
68-
sessionId: options.sessionId,
69-
status: manifest.status,
70-
},
71-
});
72-
}
51+
assertSessionCommandable(manifest, options.sessionId);
7352

7453
if (
7554
!ALLOWED_SIGNALS.includes(

0 commit comments

Comments
 (0)