Skip to content

Commit 8bc51cd

Browse files
thymikeethymikee
committed
refactor(ios): runner error classification as data; recovery tested at the transport seam (#1631)
Error-retry classification lived in four places consulted from one catch dispatch: two overlapping substring chains in runner-contract.ts, a code-keyed fatality chain in runner-session.ts, and an inline composite in runner-lifecycle.ts. RUNNER_ERROR_RULES is now the one declaration (mirroring RUNNER_COMMAND_TRAIT_MANIFEST): each rule names an error shape and its verdicts across the four axes (retryable, connect-retry, session-fatal reason, restart-before-send); the predicates keep their signatures and derive from the table. One deliberate widening, noted at the declaration: restart-before-send matching is now case-insensitive like the other axes (the message is our own transport literal). runner-command-recovery.ts gets its first direct coverage — through the real stack: a scripted fake iOS runner (local HTTP server) stands in for the XCTest runner process, executeRunnerCommandWithSession and the transport fetch run for real, and invalidation is observed through the module's existing invalidateSession parameter. Seven scenarios (retained response, runner-reported failure, in-flight, unknown lifecycle, probe failure, missing command id, read-only completed-without-response) plus an 11-case classification suite. Zero vi.mocks of runner internals.
1 parent 58a9d4b commit 8bc51cd

6 files changed

Lines changed: 502 additions & 35 deletions

File tree

‎src/platforms/apple/core/__tests__/fake-runner-server.ts‎

Lines changed: 80 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,80 @@
1+
import http from 'node:http';
2+
import type { AddressInfo } from 'node:net';
3+
4+
/**
5+
* A deterministic fake iOS runner: a local HTTP server standing in for the
6+
* XCTest runner process, scripted per request. Recovery/retry suites drive the
7+
* REAL send stack (executeRunnerCommandWithSession → transport fetch) against
8+
* it instead of vi-mocking internal functions — the #1631 testing seam. Each
9+
* incoming request consumes the next scripted response; running out of script
10+
* fails loudly rather than improvising.
11+
*/
12+
13+
export type FakeRunnerResponse =
14+
| { kind: 'ok'; data: Record<string, unknown> }
15+
| { kind: 'runnerError'; code: string; message: string }
16+
| { kind: 'hangUp' };
17+
18+
export type FakeRunnerRequest = {
19+
command: string;
20+
body: Record<string, unknown>;
21+
};
22+
23+
export type FakeRunnerServer = {
24+
port: number;
25+
requests: FakeRunnerRequest[];
26+
close: () => Promise<void>;
27+
};
28+
29+
export async function startFakeRunnerServer(
30+
script: FakeRunnerResponse[],
31+
): Promise<FakeRunnerServer> {
32+
const remaining = [...script];
33+
const requests: FakeRunnerRequest[] = [];
34+
const server = http.createServer((req, res) => {
35+
let raw = '';
36+
req.on('data', (chunk) => {
37+
raw += chunk;
38+
});
39+
req.on('end', () => {
40+
const body = parseBody(raw);
41+
requests.push({ command: String(body.command ?? ''), body });
42+
const next = remaining.shift();
43+
if (!next) {
44+
res.statusCode = 500;
45+
res.end(JSON.stringify({ ok: false, error: { message: 'fake runner script exhausted' } }));
46+
return;
47+
}
48+
if (next.kind === 'hangUp') {
49+
res.destroy();
50+
return;
51+
}
52+
if (next.kind === 'runnerError') {
53+
res.setHeader('content-type', 'application/json');
54+
res.end(JSON.stringify({ ok: false, error: { code: next.code, message: next.message } }));
55+
return;
56+
}
57+
res.setHeader('content-type', 'application/json');
58+
res.end(JSON.stringify({ ok: true, data: next.data }));
59+
});
60+
});
61+
await new Promise<void>((resolve) => server.listen(0, '127.0.0.1', resolve));
62+
const port = (server.address() as AddressInfo).port;
63+
return {
64+
port,
65+
requests,
66+
close: () =>
67+
new Promise<void>((resolve, reject) =>
68+
server.close((error) => (error ? reject(error) : resolve())),
69+
),
70+
};
71+
}
72+
73+
function parseBody(raw: string): Record<string, unknown> {
74+
try {
75+
const parsed: unknown = JSON.parse(raw);
76+
return parsed && typeof parsed === 'object' ? (parsed as Record<string, unknown>) : {};
77+
} catch {
78+
return {};
79+
}
80+
}

‎src/platforms/apple/core/__tests__/runner-command-recovery.test.ts‎

Lines changed: 151 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,151 @@
1+
import assert from 'node:assert/strict';
2+
import { afterEach, test, vi } from 'vitest';
3+
import { AppError } from '@agent-device/kernel/errors';
4+
import { IOS_SIMULATOR } from '../../../../__tests__/test-utils/device-fixtures.ts';
5+
import type { ExecResult } from '../../../../utils/exec.ts';
6+
import { handleRunnerTransportErrorAfterCommandSend } from '../runner/runner-command-recovery.ts';
7+
import type { RunnerCommand } from '../runner/runner-contract.ts';
8+
import type { RunnerSession } from '../runner/runner-session.ts';
9+
import {
10+
startFakeRunnerServer,
11+
type FakeRunnerResponse,
12+
type FakeRunnerServer,
13+
} from './fake-runner-server.ts';
14+
15+
// Recovery driven through the REAL stack: real executeRunnerCommandWithSession,
16+
// real transport fetch, a scripted fake runner on localhost, and the
17+
// invalidateSession parameter the module already exposes as its seam. The only
18+
// faked thing is the runner process itself (#1631).
19+
20+
let server: FakeRunnerServer | undefined;
21+
22+
afterEach(async () => {
23+
await server?.close();
24+
server = undefined;
25+
});
26+
27+
function makeRunnerSession(port: number): RunnerSession {
28+
return {
29+
sessionId: `fake:${port}`,
30+
device: IOS_SIMULATOR,
31+
deviceId: IOS_SIMULATOR.id,
32+
port,
33+
xctestrunPath: '/tmp/fake.xctestrun',
34+
jsonPath: '/tmp/fake.json',
35+
testPromise: new Promise<ExecResult>(() => {}),
36+
child: { pid: process.pid, exitCode: null },
37+
ready: true,
38+
};
39+
}
40+
41+
function tapCommand(commandId = 'cmd-1'): RunnerCommand {
42+
return { command: 'tap', x: 10, y: 10, commandId } as RunnerCommand;
43+
}
44+
45+
async function runRecovery(params: {
46+
script: FakeRunnerResponse[];
47+
command?: RunnerCommand;
48+
transportError?: AppError;
49+
}): Promise<{
50+
result: Promise<Record<string, unknown>>;
51+
session: RunnerSession;
52+
invalidate: ReturnType<typeof vi.fn>;
53+
transportError: AppError;
54+
}> {
55+
server = await startFakeRunnerServer(params.script);
56+
const session = makeRunnerSession(server.port);
57+
const invalidate = vi.fn(async () => {});
58+
const transportError = params.transportError ?? new AppError('COMMAND_FAILED', 'socket hang up');
59+
const result = handleRunnerTransportErrorAfterCommandSend({
60+
device: IOS_SIMULATOR,
61+
session,
62+
command: params.command ?? tapCommand(),
63+
transportError,
64+
options: {},
65+
signal: undefined,
66+
invalidationReason: 'transport_error_after_command_send',
67+
invalidateSession: invalidate,
68+
});
69+
return { result, session, invalidate, transportError };
70+
}
71+
72+
test('a completed command with a retained response recovers without invalidation', async () => {
73+
const { result, invalidate } = await runRecovery({
74+
script: [
75+
{
76+
kind: 'ok',
77+
data: {
78+
lifecycleState: 'completed',
79+
lifecycleResponseJson: JSON.stringify({ ok: true, data: { tapped: true } }),
80+
},
81+
},
82+
],
83+
});
84+
85+
assert.deepEqual(await result, { tapped: true });
86+
assert.equal(invalidate.mock.calls.length, 0);
87+
assert.equal(server?.requests[0]?.command, 'status');
88+
assert.equal(server?.requests[0]?.body.statusCommandId, 'cmd-1');
89+
});
90+
91+
test('a runner-reported failure surfaces without invalidating the session', async () => {
92+
const { result, invalidate } = await runRecovery({
93+
script: [
94+
{ kind: 'ok', data: { lifecycleState: 'failed', lifecycleErrorMessage: 'tap failed' } },
95+
],
96+
});
97+
98+
await assert.rejects(result, (error: unknown) => error instanceof AppError);
99+
assert.equal(invalidate.mock.calls.length, 0);
100+
});
101+
102+
test('a command still in flight surfaces without invalidating the session', async () => {
103+
const { result, invalidate } = await runRecovery({
104+
script: [{ kind: 'ok', data: { lifecycleState: 'started' } }],
105+
});
106+
107+
await assert.rejects(result, (error: unknown) => error instanceof AppError);
108+
assert.equal(invalidate.mock.calls.length, 0);
109+
});
110+
111+
test('an unknown lifecycle state invalidates the session and says so', async () => {
112+
const { result, invalidate } = await runRecovery({
113+
script: [{ kind: 'ok', data: { lifecycleState: 'zombie' } }],
114+
});
115+
116+
await assert.rejects(result, (error: unknown) => {
117+
return error instanceof AppError && error.message.includes('invalidated the runner session');
118+
});
119+
assert.equal(invalidate.mock.calls.length, 1);
120+
assert.equal(invalidate.mock.calls[0]?.[1], 'transport_error_after_command_send');
121+
});
122+
123+
test('a failing status probe retains the invalidation and rethrows the transport error', async () => {
124+
const { result, invalidate, transportError } = await runRecovery({
125+
script: [{ kind: 'runnerError', code: 'COMMAND_FAILED', message: 'status probe exploded' }],
126+
});
127+
128+
await assert.rejects(result, (error: unknown) => error === transportError);
129+
assert.equal(invalidate.mock.calls.length, 1);
130+
});
131+
132+
test('a command without an id cannot be probed: invalidate and rethrow', async () => {
133+
const { result, invalidate, transportError } = await runRecovery({
134+
script: [],
135+
command: { command: 'tap', x: 10, y: 10 } as RunnerCommand,
136+
});
137+
138+
await assert.rejects(result, (error: unknown) => error === transportError);
139+
assert.equal(invalidate.mock.calls.length, 1);
140+
assert.equal(server?.requests.length, 0);
141+
});
142+
143+
test('a completed read-only command without a retained response rethrows without invalidation', async () => {
144+
const { result, invalidate, transportError } = await runRecovery({
145+
script: [{ kind: 'ok', data: { lifecycleState: 'completed' } }],
146+
command: { command: 'snapshot', commandId: 'cmd-ro' } as RunnerCommand,
147+
});
148+
149+
await assert.rejects(result, (error: unknown) => error === transportError);
150+
assert.equal(invalidate.mock.calls.length, 0);
151+
});

‎src/platforms/apple/core/__tests__/runner-error-classification.test.ts‎

Lines changed: 112 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,112 @@
1+
import assert from 'node:assert/strict';
2+
import { test } from 'vitest';
3+
import { AppError } from '@agent-device/kernel/errors';
4+
import {
5+
RUNNER_ERROR_RULES,
6+
isRetryableRunnerError,
7+
resolveRunnerFatalErrorReason,
8+
shouldRestartRunnerBeforeCommandSend,
9+
shouldRetryRunnerConnectError,
10+
} from '../runner/runner-contract.ts';
11+
12+
function commandFailed(message: string, details?: Record<string, unknown>): AppError {
13+
return new AppError('COMMAND_FAILED', message, details);
14+
}
15+
16+
test('every rule carries a unique reason', () => {
17+
const reasons = RUNNER_ERROR_RULES.map((rule) => rule.reason);
18+
assert.equal(new Set(reasons).size, reasons.length);
19+
});
20+
21+
// --- retryable axis (isRetryableRunnerError) ---
22+
23+
test('transport-shaped failures are retryable', () => {
24+
for (const message of [
25+
'Runner did not accept connection on port 8100',
26+
'fetch failed',
27+
'connect ECONNREFUSED 127.0.0.1:8100',
28+
'socket hang up',
29+
]) {
30+
assert.equal(isRetryableRunnerError(commandFailed(message)), true, message);
31+
}
32+
});
33+
34+
test('boot-shaped failures are not retryable', () => {
35+
assert.equal(isRetryableRunnerError(commandFailed('xcodebuild exited early (code 65)')), false);
36+
assert.equal(
37+
isRetryableRunnerError(commandFailed('Device is busy (Connecting to Simulator)')),
38+
false,
39+
);
40+
});
41+
42+
test('an explicitly retriable flag wins over any message denial', () => {
43+
const flagged = commandFailed('xcodebuild exited early', { retriable: true });
44+
assert.equal(isRetryableRunnerError(flagged), true);
45+
});
46+
47+
test('retryable requires an AppError with COMMAND_FAILED', () => {
48+
assert.equal(isRetryableRunnerError(new Error('fetch failed')), false);
49+
assert.equal(isRetryableRunnerError(new AppError('DEVICE_NOT_FOUND', 'fetch failed')), false);
50+
});
51+
52+
// --- connect-retry axis (shouldRetryRunnerConnectError) ---
53+
54+
test('connect loop keeps waiting by default, including for unknown errors', () => {
55+
assert.equal(
56+
shouldRetryRunnerConnectError(commandFailed('Runner did not accept connection')),
57+
true,
58+
);
59+
assert.equal(shouldRetryRunnerConnectError(new Error('anything')), true);
60+
assert.equal(shouldRetryRunnerConnectError(new AppError('INVALID_ARGS', 'nope')), true);
61+
});
62+
63+
test('connect loop stops for terminal verdicts', () => {
64+
assert.equal(shouldRetryRunnerConnectError(commandFailed('xcodebuild exited early')), false);
65+
const unattached = new AppError('DEVICE_NOT_FOUND', 'device not attached', {
66+
usbmuxDeviceAttached: false,
67+
});
68+
assert.equal(shouldRetryRunnerConnectError(unattached), false);
69+
// The same code without the usbmux evidence keeps waiting.
70+
assert.equal(shouldRetryRunnerConnectError(new AppError('DEVICE_NOT_FOUND', 'gone')), true);
71+
});
72+
73+
// --- session-fatal axis (resolveRunnerFatalErrorReason) ---
74+
75+
test('session-fatal codes map to their invalidation reasons', () => {
76+
assert.equal(
77+
resolveRunnerFatalErrorReason(new AppError('IOS_AX_SNAPSHOT_FAILED', 'ax root failed')),
78+
'ax_snapshot_failure',
79+
);
80+
assert.equal(
81+
resolveRunnerFatalErrorReason(new AppError('XCTEST_RECORDED_FAILURE', 'recorded failure')),
82+
'xctest_recorded_failure',
83+
);
84+
assert.equal(
85+
resolveRunnerFatalErrorReason(new AppError('RUNNER_WEDGED', 'main thread stuck')),
86+
'runner_main_thread_wedged',
87+
);
88+
});
89+
90+
test('ordinary errors are never session-fatal', () => {
91+
assert.equal(resolveRunnerFatalErrorReason(commandFailed('socket hang up')), undefined);
92+
assert.equal(resolveRunnerFatalErrorReason(new Error('boom')), undefined);
93+
});
94+
95+
// --- restart-before-send axis (shouldRestartRunnerBeforeCommandSend) ---
96+
97+
test('a refused connection before send restarts the session, case-insensitively', () => {
98+
assert.equal(
99+
shouldRestartRunnerBeforeCommandSend(commandFailed('Runner did not accept connection')),
100+
true,
101+
);
102+
assert.equal(
103+
shouldRestartRunnerBeforeCommandSend(commandFailed('runner did not accept connection')),
104+
true,
105+
);
106+
});
107+
108+
test('a terminal connect verdict refuses the restart even when the message matches', () => {
109+
const both = commandFailed('xcodebuild exited early: runner did not accept connection');
110+
assert.equal(shouldRestartRunnerBeforeCommandSend(both), false);
111+
assert.equal(shouldRestartRunnerBeforeCommandSend(commandFailed('socket hang up')), false);
112+
});

0 commit comments

Comments
 (0)