Skip to content

Commit f83bdfe

Browse files
committed
fix(apple-runner): a canceled request is not a runner that stopped answering
The marker says the preflight gave up, and one of the things a preflight gives up on is a caller that stopped waiting. `createRequestCanceledError` answers `COMMAND_FAILED` too, so the rule written last round matched a cancellation exactly as well as a wedged runner: on a session that was ready and still working, the cancel invalidated the session and replayed the canceled command against a fresh boot nobody was going to wait for. The main branch rethrew cancellations before any restart branch could see them; the marker rule made the marker the only question, and the marker does not distinguish the two shapes it is applied to. The exception belongs on the rule, where the verdict is, rather than on the call site that happens to notice: a marked error restarts the session unless the typed reason on it says the request was canceled. `isRequestCanceledDetails` is exported from the owning type so the rule reads that reason instead of restating the literal, and `isRequestCanceledError` now asks it. Deleting the conjunct turns both the classification test and the lifecycle test red — the lifecycle one pins the exact shape from the report: ready session, marked cancellation, error rethrown, no invalidate called, no second boot.
1 parent 9b4a130 commit f83bdfe

5 files changed

Lines changed: 60 additions & 5 deletions

File tree

CHANGELOG.md

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,9 @@
2222
`pressButton`, `activateApp`).
2323
- Changed (iOS runner): what recovers a stuck runner is decided by the recorded error, not by its
2424
wording. A readiness preflight marks the error it gives up with, and that marker is now the whole
25-
test for restarting the session and replaying the command. Two message checks decided it before,
25+
test for restarting the session and replaying the command — except for a request that was canceled,
26+
which that same catch also marks: a command nobody is going to send again has no restart to spend,
27+
and the session it would tear down may be one that still works. Two message checks decided it before,
2628
and a preflight reaches the caller in whatever shape its connect loop ended with — "Runner did not
2729
accept connection", "Runner endpoint probe failed", a killed `simctl` fallback, a post that ran out
2830
of its budget — so only some of those restarted and the rest failed the command. The other half is

packages/kernel/src/errors.ts

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -262,10 +262,19 @@ export function createRequestCanceledError(details?: AppErrorDetails, cause?: un
262262
);
263263
}
264264

265+
/**
266+
* The typed reason of a canceled request, for a caller holding the details rather than the error:
267+
* a rule table that matches on details needs the same fact {@link isRequestCanceledError} reads, and
268+
* must not restate the literal.
269+
*/
270+
export function isRequestCanceledDetails(details: AppErrorDetails | undefined): boolean {
271+
return details?.reason === REQUEST_CANCELED_REASON;
272+
}
273+
265274
export function isRequestCanceledError(error: unknown): boolean {
266275
if (!(error instanceof AppError)) return false;
267276
if (error.code !== 'COMMAND_FAILED') return false;
268-
if (error.details?.reason === REQUEST_CANCELED_REASON) return true;
277+
if (isRequestCanceledDetails(error.details)) return true;
269278
// Owned debt: canceled errors that crossed a wire without their details keep
270279
// the message; do not add new message sniffs beside it.
271280
return error.message === REQUEST_CANCELED_MESSAGE;

packages/platform-apple/src/runner/__tests__/runner-error-classification.test.ts

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
import assert from 'node:assert/strict';
22
import { test } from 'vitest';
3-
import { AppError } from '@agent-device/kernel/errors';
3+
import { AppError, createRequestCanceledError } from '@agent-device/kernel/errors';
44
import {
55
RUNNER_ERROR_RULES,
66
isRetryableRunnerError,
@@ -108,6 +108,14 @@ test('the preflight marker alone decides the restart', () => {
108108
shouldRestartRunnerAfterReadinessPreflight(commandFailed('Runner readiness refused')),
109109
false,
110110
);
111+
// The same catch marks a caller that stopped waiting. That mark is not a runner that stopped
112+
// answering: the command was canceled, so no restart has a request left to serve.
113+
assert.equal(
114+
shouldRestartRunnerAfterReadinessPreflight(
115+
createRequestCanceledError({ runnerReadinessPreflightFailed: true }),
116+
),
117+
false,
118+
);
111119
});
112120

113121
test('a deadline on its own earns no recovery verdict', () => {

packages/platform-apple/src/runner/__tests__/runner-lifecycle-readiness-preflight.test.ts

Lines changed: 30 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,10 @@
11
import assert from 'node:assert/strict';
22
import { beforeEach, test, vi } from 'vitest';
3-
import { AppError } from '@agent-device/kernel/errors';
3+
import {
4+
AppError,
5+
createRequestCanceledError,
6+
isRequestCanceledError,
7+
} from '@agent-device/kernel/errors';
48
import { appleRunnerTestHost } from '../test-host.ts';
59
import type { RunnerXctestrunArtifact } from '../runner-xctestrun.ts';
610
import { IOS_SIMULATOR } from './device-fixtures.ts';
@@ -178,6 +182,31 @@ test('a failed readiness probe without the marker does not restart the session',
178182
assert.equal(mockEnsureRunnerSession.mock.calls.length, 1);
179183
});
180184

185+
test('a cancellation during the readiness preflight does not restart the session it canceled', async () => {
186+
const session = makeRunnerSession({ port: 8100, ready: true });
187+
188+
// The preflight's catch marks whatever it was waiting on when it gave up, and one of the things
189+
// it waits on is a caller that stopped waiting. That mark describes a walkaway, not a wedged
190+
// runner, and a session that is ready is the one the caller just left: restarting it would boot a
191+
// runner for a command nobody is going to send again, and take down a session that still works.
192+
mockEnsureRunnerSession.mockResolvedValueOnce(session);
193+
mockExecuteRunnerCommandWithSession.mockRejectedValueOnce(
194+
createRequestCanceledError({ runnerReadinessPreflightFailed: true, command: 'tap' }),
195+
);
196+
197+
await assert.rejects(
198+
() => runAppleRunnerCommand(IOS_SIMULATOR, { command: 'tap', x: 120, y: 240 }),
199+
(error: unknown) => {
200+
assert.ok(error instanceof AppError);
201+
assert.ok(isRequestCanceledError(error));
202+
return true;
203+
},
204+
);
205+
assert.equal(mockInvalidateRunnerSession.mock.calls.length, 0);
206+
assert.equal(mockEnsureRunnerSession.mock.calls.length, 1);
207+
assert.equal(mockExecuteRunnerCommandWithSession.mock.calls.length, 1);
208+
});
209+
181210
test('a boot that exited early does not wipe a restored runner artifact', async () => {
182211
const restoredSession = makeRunnerSession({
183212
port: 8100,

packages/platform-apple/src/runner/runner-contract.ts

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
import {
22
AppError,
33
createRequestCanceledError,
4+
isRequestCanceledDetails,
45
toAppErrorCode,
56
type AppErrorCode,
67
type AppErrorDetails,
@@ -168,8 +169,14 @@ type RunnerErrorMatch = {
168169
const hasRetriableFlag: RunnerErrorDetailsMatch = (details) => details.retriable === true;
169170
const hasUsbmuxDeviceUnattached: RunnerErrorDetailsMatch = (details) =>
170171
details.usbmuxDeviceAttached === false;
172+
/**
173+
* The preflight marks whatever it was waiting on when it stopped, and one of the things it waits on
174+
* is a caller that stopped waiting. A canceled request is not a wedged runner: the restart this
175+
* marker authorises would boot a runner for a command nobody is going to send again. Every abort in
176+
* the connect loop normalizes to the typed canceled reason before it reaches here.
177+
*/
171178
const hasReadinessPreflightFailure: RunnerErrorDetailsMatch = (details) =>
172-
details.runnerReadinessPreflightFailed === true;
179+
details.runnerReadinessPreflightFailed === true && !isRequestCanceledDetails(details);
173180

174181
type RunnerErrorVerdicts = {
175182
/** isRetryableRunnerError: transport error worth a same-session resend. */

0 commit comments

Comments
 (0)