Skip to content

Commit c6188ef

Browse files
thymikeethymikee
committed
fix(apple-runner): restart without resending a command written then lost
A connect-shaped failure keeps its restart, but the command is sent again on the restarted runner only with proof the first send could not have run it: the attempt provably wrote nothing (a connect refused before the write, dispatched no), the readiness preflight gave up before the write, or the command is read-only by its runner trait. A mutating command whose first attempt may have written it now fails with reason runner_reply_lost and dispatched unknown after the restart; a restarted runner's journal is empty, so no status answer can prove the first send did not run. A read-only command's restart failure discloses no: a read has no side effect to repeat. dispatch-disclosure.json: ios-runner.transport.written-then-lost keeps unknown with a trigger that says the command is not resent; ios-runner.transport.read-only-written-then-lost is new and discloses no. Tests (runner-lifecycle-dispatch-disclosure, real connect loop over a stubbed fetch and simctl curl exit 28, restart succeeds): - written-then-lost: tap restarts the runner, is not sent on the restarted runner, fails unknown with reason runner_reply_lost. Mutation: make canResendAfterRestart return true -> fails (the tap is replayed and the request succeeds). - read-only-written-then-lost: snapshot is sent again on the restarted runner and its failure discloses no. Mutations: drop the read-only term from canResendAfterRestart -> fails (no resend); drop the read-only branch of discloseRestartDispatch -> fails (unknown). runner-command-retry: four mutating-restart fixtures now carry the proof a real refused connect stamps (dispatched no); without it they would test the removed replay.
1 parent 1eb0efc commit c6188ef

6 files changed

Lines changed: 157 additions & 35 deletions

File tree

‎contracts/fixtures/dispatch-disclosure.json‎

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -238,9 +238,15 @@
238238
{
239239
"id": "ios-runner.transport.written-then-lost",
240240
"producer": "ios-runner",
241-
"trigger": "a connect attempt posted the command and then timed out (fetch deadline, simctl curl exit 28); the failure keeps the runner_connect_refused restart verdict, and the restart that replays it failed",
241+
"trigger": "a connect attempt posted a mutating command and then timed out (fetch deadline, simctl curl exit 28); the runner is restarted, and the command is not sent again because nothing proves the first send did not run",
242242
"dispatched": "unknown"
243243
},
244+
{
245+
"id": "ios-runner.transport.read-only-written-then-lost",
246+
"producer": "ios-runner",
247+
"trigger": "a connect attempt posted a read-only command and then timed out (fetch deadline, simctl curl exit 28); the runner is restarted and the read is sent again, a read has no side effect, and the resend failed",
248+
"dispatched": "no"
249+
},
244250
{
245251
"id": "ios-runner.pre-send.readiness-preflight-after-restart",
246252
"producer": "ios-runner",

‎packages/platform-apple/src/runner/__tests__/runner-command-retry.test.ts‎

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -51,6 +51,10 @@ import { prepareIosRunner, runAppleRunnerCommand } from '../runner-client.ts';
5151
import { resetRunnerRecycleLedgerForTests } from '../runner-recycle-ledger.ts';
5252
import type { RunnerXctestrunArtifact } from '../runner-xctestrun.ts';
5353

54+
/** Every connect attempt was refused before a byte was written, so a restart may resend the command. */
55+
const unwrittenConnectRefusal = (): AppError =>
56+
runnerConnectFailure('runner_connect_refused', undefined, { dispatched: 'no' });
57+
5458
const requestCancellation = createTestRequestCancellation();
5559
const { markRequestCanceled, clearRequestCanceled, isRequestCanceled } = requestCancellation;
5660

@@ -306,7 +310,7 @@ test('mutating commands restart stale ready sessions when the preflight probe ne
306310

307311
mockEnsureRunnerSession.mockResolvedValueOnce(staleSession).mockResolvedValueOnce(freshSession);
308312
mockExecuteRunnerCommandWithSession
309-
.mockRejectedValueOnce(runnerConnectFailure('runner_connect_refused'))
313+
.mockRejectedValueOnce(unwrittenConnectRefusal())
310314
.mockResolvedValueOnce({ message: 'tapped' });
311315

312316
const result = await runAppleRunnerCommand(IOS_SIMULATOR, { command: 'tap', x: 120, y: 240 });
@@ -329,7 +333,7 @@ test('mutating commands retry startup sessions with stale bundle cleanup', async
329333

330334
mockEnsureRunnerSession.mockResolvedValueOnce(startupSession).mockResolvedValueOnce(freshSession);
331335
mockExecuteRunnerCommandWithSession
332-
.mockRejectedValueOnce(runnerConnectFailure('runner_connect_refused'))
336+
.mockRejectedValueOnce(unwrittenConnectRefusal())
333337
.mockResolvedValueOnce({ message: 'tapped' });
334338

335339
const result = await runAppleRunnerCommand(IOS_SIMULATOR, { command: 'tap', x: 120, y: 240 });
@@ -814,7 +818,7 @@ test('mutating commands invalidate the retry session without replaying again', a
814818

815819
mockEnsureRunnerSession.mockResolvedValueOnce(staleSession).mockResolvedValueOnce(freshSession);
816820
mockExecuteRunnerCommandWithSession
817-
.mockRejectedValueOnce(runnerConnectFailure('runner_connect_refused'))
821+
.mockRejectedValueOnce(unwrittenConnectRefusal())
818822
.mockRejectedValueOnce(new AppError('COMMAND_FAILED', 'fetch failed'))
819823
.mockResolvedValueOnce({ lifecycleState: 'notAccepted' });
820824

@@ -1192,7 +1196,7 @@ test('a later command in the same request cannot pay for a second recycle boot',
11921196

11931197
mockEnsureRunnerSession.mockResolvedValueOnce(staleSession).mockResolvedValueOnce(freshSession);
11941198
mockExecuteRunnerCommandWithSession
1195-
.mockRejectedValueOnce(runnerConnectFailure('runner_connect_refused'))
1199+
.mockRejectedValueOnce(unwrittenConnectRefusal())
11961200
.mockResolvedValueOnce({ message: 'tapped' });
11971201

11981202
// First command consumes the request's only recycle via restart-and-replay.

‎packages/platform-apple/src/runner/__tests__/runner-lifecycle-dispatch-disclosure.test.ts‎

Lines changed: 70 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,8 @@ vi.mock('../runner-session.ts', async () => {
3939
});
4040

4141
import { runAppleRunnerCommand } from '../runner-client.ts';
42+
import { RUNNER_REPLY_LOST_REASON } from '../runner-command-recovery.ts';
43+
import type { RunnerCommand } from '../runner-contract.ts';
4244
import { resetRunnerRecycleLedgerForTests } from '../runner-recycle-ledger.ts';
4345
import { waitForRunner } from '../runner-startup-transport.ts';
4446

@@ -72,12 +74,9 @@ const readinessPreflightFailure = (): AppError =>
7274

7375
/**
7476
* The first attempt runs the real connect loop against a simulator whose every fetch fails the
75-
* same way and whose simctl curl fallback exits with `curlExitCode`; the restart it earns fails.
77+
* same way and whose simctl curl fallback exits with `curlExitCode`.
7678
*/
77-
async function connectLoopThenFailedRestart(transport: {
78-
fetchFailure: () => Error;
79-
curlExitCode: number;
80-
}): Promise<unknown> {
79+
function stubConnectLoopFailure(transport: { fetchFailure: () => Error; curlExitCode: number }) {
8180
vi.stubGlobal(
8281
'fetch',
8382
vi.fn(async () => {
@@ -91,13 +90,21 @@ async function connectLoopThenFailedRestart(transport: {
9190
stderr: `curl exited ${transport.curlExitCode}`,
9291
})),
9392
});
94-
mockEnsureRunnerSession
95-
.mockResolvedValueOnce(makeRunnerSession())
96-
.mockRejectedValueOnce(new AppError('COMMAND_FAILED', 'runner restart failed'));
9793
mockExecuteRunnerCommandWithSession.mockImplementationOnce(
9894
async (device, session, command) =>
9995
await waitForRunner(device, session.port, command, undefined, 400),
10096
);
97+
}
98+
99+
/** A connect loop that fails as `transport` says, then a restart that fails. */
100+
async function connectLoopThenFailedRestart(transport: {
101+
fetchFailure: () => Error;
102+
curlExitCode: number;
103+
}): Promise<unknown> {
104+
stubConnectLoopFailure(transport);
105+
mockEnsureRunnerSession
106+
.mockResolvedValueOnce(makeRunnerSession())
107+
.mockRejectedValueOnce(new AppError('COMMAND_FAILED', 'runner restart failed'));
101108
try {
102109
return await tap();
103110
} catch (error) {
@@ -107,6 +114,38 @@ async function connectLoopThenFailedRestart(transport: {
107114
}
108115
}
109116

117+
/** A fetch deadline, then a simctl curl that timed out after its POST: the command may have run. */
118+
const writtenThenLost = {
119+
fetchFailure: () => new AppError('COMMAND_FAILED', 'Runner command deadline exceeded'),
120+
curlExitCode: 28,
121+
};
122+
123+
/**
124+
* `command` is written and its reply lost on the first runner, the runner restarts, and the
125+
* restarted runner answers every command with `restartedAnswer`.
126+
*/
127+
async function writtenThenLostThenRestarted(
128+
command: RunnerCommand,
129+
restartedAnswer: () => Promise<Record<string, unknown>>,
130+
): Promise<Record<string, unknown>> {
131+
stubConnectLoopFailure(writtenThenLost);
132+
mockEnsureRunnerSession
133+
.mockResolvedValueOnce(makeRunnerSession())
134+
.mockResolvedValueOnce(makeRunnerSession({ port: 8101 }));
135+
mockExecuteRunnerCommandWithSession.mockImplementation(restartedAnswer);
136+
try {
137+
return await runAppleRunnerCommand(IOS_SIMULATOR, command);
138+
} finally {
139+
assert.equal(mockEnsureRunnerSession.mock.calls.length, 2, 'the runner is restarted');
140+
}
141+
}
142+
143+
function sendsOnRestartedRunner(): number {
144+
return mockExecuteRunnerCommandWithSession.mock.calls.filter(
145+
([, session]) => session.port === 8101,
146+
).length;
147+
}
148+
110149
const DRIVERS: Record<string, () => Promise<unknown>> = {
111150
'ios-runner.pre-send.session-start-failed': async () => {
112151
mockEnsureRunnerSession.mockRejectedValueOnce(
@@ -124,11 +163,29 @@ const DRIVERS: Record<string, () => Promise<unknown>> = {
124163
}),
125164
curlExitCode: 7,
126165
}),
127-
'ios-runner.transport.written-then-lost': () =>
128-
connectLoopThenFailedRestart({
129-
fetchFailure: () => new AppError('COMMAND_FAILED', 'Runner command deadline exceeded'),
130-
curlExitCode: 28,
131-
}),
166+
'ios-runner.transport.written-then-lost': async () => {
167+
try {
168+
return await writtenThenLostThenRestarted(
169+
{ command: 'tap', x: 120, y: 240 },
170+
async () => ({}),
171+
);
172+
} catch (error) {
173+
assert.ok(error instanceof AppError);
174+
assert.equal(error.details?.reason, RUNNER_REPLY_LOST_REASON);
175+
assert.equal(error.details?.runnerRestarted, true);
176+
assert.equal(sendsOnRestartedRunner(), 0, 'the tap is not sent again');
177+
throw error;
178+
}
179+
},
180+
'ios-runner.transport.read-only-written-then-lost': async () => {
181+
try {
182+
return await writtenThenLostThenRestarted({ command: 'snapshot' }, async () => {
183+
throw new AppError('COMMAND_FAILED', 'runner resend failed', { dispatched: 'unknown' });
184+
});
185+
} finally {
186+
assert.equal(sendsOnRestartedRunner(), 1, 'the read is sent again');
187+
}
188+
},
132189
'ios-runner.pre-send.readiness-preflight-after-restart': async () => {
133190
mockEnsureRunnerSession
134191
.mockResolvedValueOnce(makeRunnerSession())

‎packages/platform-apple/src/runner/runner-command-recovery.ts‎

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,31 @@ const RUNNER_STATUS_RECOVERY_TIMEOUT_MS = 3_000;
5353
*/
5454
export const RUNNER_REPLY_LOST_REASON = 'runner_reply_lost';
5555

56+
/**
57+
* The command was written and its reply lost, and the runner was restarted without resending it:
58+
* a restarted runner's journal cannot say whether the first send ran.
59+
*/
60+
export function buildRunnerRestartedWithoutResendError(
61+
command: RunnerCommand,
62+
transportError: AppError,
63+
options: AppleRunnerCommandOptions,
64+
): AppError {
65+
return new AppError(
66+
'COMMAND_FAILED',
67+
`Runner command "${command.command}" may have run before its reply was lost, so agent-device restarted the runner without sending it again.`,
68+
{
69+
command: command.command,
70+
commandId: command.commandId,
71+
reason: RUNNER_REPLY_LOST_REASON,
72+
recovery: 'runner_restarted_without_resend',
73+
hint: `Run snapshot -i to inspect the current UI and check whether "${command.command}" took effect before you retry it.`,
74+
logPath: options.logPath,
75+
transportError: transportError.message,
76+
},
77+
transportError,
78+
);
79+
}
80+
5681
export async function handleRunnerTransportErrorAfterCommandSend(params: {
5782
device: DeviceInfo;
5883
session: RunnerSession;

‎packages/platform-apple/src/runner/runner-error-classification.ts‎

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -115,7 +115,10 @@ type RunnerErrorVerdicts = {
115115
connectRetry?: boolean;
116116
/** Session-fatal classification: invalidate the cached runner session with this reason. */
117117
sessionFatalReason?: string;
118-
/** Connect-shaped failure before the command was sent: restart the session and replay. */
118+
/**
119+
* Connect-shaped failure: restart the session. The command is resent only when the attempt
120+
* provably wrote nothing or the command is read-only.
121+
*/
119122
restartBeforeSend?: boolean;
120123
/** Readiness preflight gave up before the command was written: restart the session and replay. */
121124
restartAfterReadinessPreflight?: boolean;
@@ -610,9 +613,9 @@ export function resolveRunnerFatalErrorReason(error: unknown): string | undefine
610613
}
611614

612615
/**
613-
* A connect-shaped failure that surfaced before the command was sent: restart
614-
* the runner session and replay the command, rather than probing a runner
615-
* that never accepted the connection.
616+
* A connect-shaped failure: restart the runner session rather than probing a runner that never
617+
* accepted the connection. Whether the command is resent on the restarted runner is decided by
618+
* what proves the first attempt could not have run it.
616619
*/
617620
export function shouldRestartRunnerBeforeCommandSend(error: unknown): boolean {
618621
return runnerErrorVerdict(error, 'restartBeforeSend') ?? false;

‎packages/platform-apple/src/runner/runner-lifecycle.ts‎

Lines changed: 40 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,11 @@ import type {
4242
AppleRunnerPrepareResult,
4343
} from './runner-provider.ts';
4444
import { markRunnerXctestrunArtifactBadForRun } from './runner-xctestrun.ts';
45-
import { handleRunnerTransportErrorAfterCommandSend } from './runner-command-recovery.ts';
45+
import {
46+
buildRunnerRestartedWithoutResendError,
47+
handleRunnerTransportErrorAfterCommandSend,
48+
} from './runner-command-recovery.ts';
49+
import { isReadOnlyRunnerCommand } from './runner-command-traits.ts';
4650
import {
4751
buildRunnerRecycleBudgetExhaustedError,
4852
commitRunnerRecycle,
@@ -360,6 +364,7 @@ async function executeRunnerCommandAttempt(
360364
options,
361365
signal,
362366
restartReason: 'runner_connect_failed_before_command_send',
367+
firstAttemptError: appErr,
363368
firstAttemptUnwritten: isRunnerPreSendRefusal(appErr),
364369
});
365370
}
@@ -373,6 +378,7 @@ async function executeRunnerCommandAttempt(
373378
signal,
374379
restartReason: 'runner_readiness_preflight_failed_before_command_send',
375380
recoveredDiagnosticPhase: 'ios_runner_readiness_preflight_recovered',
381+
firstAttemptError: appErr,
376382
firstAttemptUnwritten: true,
377383
});
378384
}
@@ -405,10 +411,8 @@ async function restartSessionAndRunCommand(params: {
405411
| 'runner_connect_failed_before_command_send'
406412
| 'runner_readiness_preflight_failed_before_command_send';
407413
recoveredDiagnosticPhase?: string;
408-
/**
409-
* The failed first attempt provably never wrote the command. When it may have, the replay can
410-
* double-send, and no failure of this restart may claim `no`.
411-
*/
414+
firstAttemptError: AppError;
415+
/** The failed first attempt provably never wrote the command. */
412416
firstAttemptUnwritten: boolean;
413417
}): Promise<Record<string, unknown>> {
414418
const { device, command, options, signal, restartReason } = params;
@@ -419,7 +423,7 @@ async function restartSessionAndRunCommand(params: {
419423
if (!tryBeginRunnerRecycle(recycleKey)) {
420424
throw discloseDispatch(
421425
buildRunnerRecycleBudgetExhaustedError(command, options),
422-
params.firstAttemptUnwritten ? 'no' : 'unknown',
426+
canResendAfterRestart(params) ? 'no' : 'unknown',
423427
);
424428
}
425429
await invalidateRunnerSession(params.session, restartReason);
@@ -430,6 +434,13 @@ async function restartSessionAndRunCommand(params: {
430434
throw markRunnerRestartError(error, params);
431435
});
432436
commitRunnerRecycle(recycleKey);
437+
if (!canResendAfterRestart(params)) {
438+
throw markRunnerRestartError(
439+
buildRunnerRestartedWithoutResendError(command, params.firstAttemptError, options),
440+
params,
441+
restartedSession,
442+
);
443+
}
433444
try {
434445
const recovered = await executeRunnerCommandWithSession(
435446
device,
@@ -474,6 +485,20 @@ async function restartSessionAndRunCommand(params: {
474485
}
475486
}
476487

488+
type RunnerRestartResendEvidence = Pick<
489+
Parameters<typeof restartSessionAndRunCommand>[0],
490+
'command' | 'firstAttemptUnwritten'
491+
>;
492+
493+
/**
494+
* Whether sending the command again on the restarted runner cannot run it twice: the first attempt
495+
* provably wrote nothing, or the command is read-only by its runner trait. A restarted runner's
496+
* journal is empty, so its `status` cannot prove the first send did not run.
497+
*/
498+
function canResendAfterRestart(evidence: RunnerRestartResendEvidence): boolean {
499+
return evidence.firstAttemptUnwritten || isReadOnlyRunnerCommand(evidence.command);
500+
}
501+
477502
function markRunnerRestartError(
478503
error: unknown,
479504
params: Pick<
@@ -500,21 +525,23 @@ function markRunnerRestartError(
500525
},
501526
error.cause ?? error,
502527
);
503-
return discloseRestartDispatch(marked, params.firstAttemptUnwritten, restartedSession);
528+
return discloseRestartDispatch(marked, params, restartedSession);
504529
}
505530

506531
/**
507-
* A restart that never replayed says what the first attempt did; a replay after a first attempt
508-
* that may have written the command cannot claim `no` for the two sends together. After an unwritten
509-
* first attempt the replay's own verdict stands; without one, only a pre-send refusal is `no`.
532+
* A read-only command has no side effect to repeat, so its failure is `no` however many sends it
533+
* took. A mutating command whose first attempt may have written it is `unknown`: it is never resent.
534+
* After an unwritten first attempt, a restart that never replayed is `no`, and a replay's own verdict
535+
* stands; without one, only a pre-send refusal is `no`.
510536
*/
511537
function discloseRestartDispatch(
512538
error: AppError,
513-
firstAttemptUnwritten: boolean,
539+
evidence: RunnerRestartResendEvidence,
514540
restartedSession: RunnerSession | undefined,
515541
): AppError {
516-
if (!restartedSession) return discloseDispatch(error, firstAttemptUnwritten ? 'no' : 'unknown');
517-
if (!firstAttemptUnwritten) return discloseDispatch(error, 'unknown');
542+
if (isReadOnlyRunnerCommand(evidence.command)) return discloseDispatch(error, 'no');
543+
if (!evidence.firstAttemptUnwritten) return discloseDispatch(error, 'unknown');
544+
if (!restartedSession) return discloseDispatch(error, 'no');
518545
return discloseUnclassifiedDispatch(error, isRunnerPreSendRefusal(error) ? 'no' : 'unknown');
519546
}
520547

0 commit comments

Comments
 (0)