Skip to content

Commit 7320348

Browse files
cameroncookecodex
andcommitted
fix(snapshot-tests): Guard CLI snapshot process output
Fail CLI domain snapshot invocations when the CLI process itself fails, is terminated, has no exit status, or emits unexpected stderr. This keeps fixtures focused on domain tool output while preventing hidden process-level noise from being ignored. Co-Authored-By: OpenAI Codex <codex@openai.com>
1 parent f68db5f commit 7320348

2 files changed

Lines changed: 73 additions & 11 deletions

File tree

src/snapshot-tests/__tests__/json-harness-error-state.test.ts

Lines changed: 37 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
import { describe, expect, it } from 'vitest';
2-
import { resolveCliJsonSnapshotErrorState } from '../harness.ts';
2+
import { assertCliSnapshotProcessResult, resolveCliJsonSnapshotErrorState } from '../harness.ts';
33
import { resolveMcpSnapshotErrorState } from '../mcp-harness.ts';
44
import type { StructuredOutputEnvelope } from '../../types/structured-output.ts';
55

@@ -17,6 +17,42 @@ const errorEnvelope: StructuredOutputEnvelope<null> = {
1717
error: 'Failed',
1818
};
1919

20+
describe('CLI snapshot process result guard', () => {
21+
it('accepts completed domain invocations without process stderr', () => {
22+
expect(() =>
23+
assertCliSnapshotProcessResult(
24+
{ error: undefined, signal: null, status: 1, stderr: '' },
25+
'tool',
26+
),
27+
).not.toThrow();
28+
});
29+
30+
it('rejects process stderr so domain snapshots cannot hide user-visible noise', () => {
31+
expect(() =>
32+
assertCliSnapshotProcessResult(
33+
{ error: undefined, signal: null, status: 0, stderr: 'warning\n' },
34+
'tool',
35+
),
36+
).toThrow('CLI process emitted unexpected stderr for tool:\nwarning');
37+
});
38+
39+
it('rejects failed process execution before snapshot matching', () => {
40+
expect(() =>
41+
assertCliSnapshotProcessResult(
42+
{ error: new Error('spawn failed'), signal: null, status: null, stderr: '' },
43+
'tool',
44+
),
45+
).toThrow('CLI process failed for tool: spawn failed');
46+
47+
expect(() =>
48+
assertCliSnapshotProcessResult(
49+
{ error: undefined, signal: 'SIGTERM', status: null, stderr: '' },
50+
'tool',
51+
),
52+
).toThrow('CLI process for tool was terminated by signal SIGTERM.');
53+
});
54+
});
55+
2056
describe('JSON snapshot harness error state', () => {
2157
it('uses CLI process status and envelope.didError when they agree', () => {
2258
expect(resolveCliJsonSnapshotErrorState(0, successEnvelope, 'tool')).toBe(false);

src/snapshot-tests/harness.ts

Lines changed: 36 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -56,6 +56,34 @@ function runSnapshotCli(
5656
});
5757
}
5858

59+
function readProcessOutput(output: string | Buffer | null | undefined): string {
60+
return typeof output === 'string' ? output : (output?.toString('utf8') ?? '');
61+
}
62+
63+
export function assertCliSnapshotProcessResult(
64+
result: Pick<ReturnType<typeof spawnSync>, 'error' | 'signal' | 'status' | 'stderr'>,
65+
label: string,
66+
): void {
67+
if (result.error) {
68+
throw new Error(`CLI process failed for ${label}: ${result.error.message}`);
69+
}
70+
71+
if (result.signal) {
72+
throw new Error(`CLI process for ${label} was terminated by signal ${result.signal}.`);
73+
}
74+
75+
if (result.status === null) {
76+
throw new Error(
77+
`CLI process exit status was null for ${label}; the process may have timed out or been killed by a signal.`,
78+
);
79+
}
80+
81+
const stderr = readProcessOutput(result.stderr).trim();
82+
if (stderr.length > 0) {
83+
throw new Error(`CLI process emitted unexpected stderr for ${label}:\n${stderr}`);
84+
}
85+
}
86+
5987
function parseStructuredEnvelope(
6088
stdout: string,
6189
label: string,
@@ -107,9 +135,10 @@ export async function createSnapshotHarness(
107135
throw new Error(`Tool '${cliToolName}' in workflow '${workflow}' is not CLI-available`);
108136
}
109137

138+
const label = `${workflow}/${cliToolName}`;
110139
const result = runSnapshotCli(workflow, cliToolName, args, 'text', options);
111-
const stdout =
112-
typeof result.stdout === 'string' ? result.stdout : (result.stdout?.toString('utf8') ?? '');
140+
assertCliSnapshotProcessResult(result, label);
141+
const stdout = readProcessOutput(result.stdout);
113142

114143
return {
115144
text: normalizeSnapshotOutput(stdout),
@@ -141,19 +170,16 @@ export async function createCliJsonSnapshotHarness(
141170
throw new Error(`Tool '${cliToolName}' in workflow '${workflow}' is not CLI-available`);
142171
}
143172

173+
const label = `${workflow}/${cliToolName}`;
144174
const result = runSnapshotCli(workflow, cliToolName, args, 'json', options);
145-
const stdout =
146-
typeof result.stdout === 'string' ? result.stdout : (result.stdout?.toString('utf8') ?? '');
147-
const envelope = parseStructuredEnvelope(stdout, `${workflow}/${cliToolName}`);
175+
assertCliSnapshotProcessResult(result, label);
176+
const stdout = readProcessOutput(result.stdout);
177+
const envelope = parseStructuredEnvelope(stdout, label);
148178

149179
return {
150180
text: formatStructuredEnvelopeFixture(envelope),
151181
rawText: stdout,
152-
isError: resolveCliJsonSnapshotErrorState(
153-
result.status,
154-
envelope,
155-
`${workflow}/${cliToolName}`,
156-
),
182+
isError: resolveCliJsonSnapshotErrorState(result.status, envelope, label),
157183
structuredEnvelope: envelope,
158184
};
159185
}

0 commit comments

Comments
 (0)