diff --git a/contracts/fixtures/dispatch-disclosure.json b/contracts/fixtures/dispatch-disclosure.json index 81564e2a58..acf40bb0f2 100644 --- a/contracts/fixtures/dispatch-disclosure.json +++ b/contracts/fixtures/dispatch-disclosure.json @@ -256,7 +256,13 @@ { "id": "ios-runner.transport.written-then-lost", "producer": "ios-runner", - "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", + "trigger": "a mutating command was posted and the runner process died before replying; the status probe fails, the session is invalidated, and the command is not sent again (details.reason runner_reply_lost)", + "dispatched": "unknown" + }, + { + "id": "ios-runner.transport.read-only-written-then-lost", + "producer": "ios-runner", + "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, and the resend failed; the first send may have run, so the runner reports unknown, and the daemon's read-only rule (daemon.read-only-command) reports no to the caller", "dispatched": "unknown" }, { @@ -289,6 +295,12 @@ "trigger": "transport lost; status probe reports lifecycleState completed without a readable retained reply", "dispatched": "unknown" }, + { + "id": "ios-runner.status.read-only-completed-without-retained-reply", + "producer": "ios-runner", + "trigger": "transport lost on a read-only command; status probe reports lifecycleState completed without a readable retained reply; the runner reports unknown, and the daemon's read-only rule (daemon.read-only-command) reports no to the caller", + "dispatched": "unknown" + }, { "id": "ios-runner.status.accepted", "producer": "ios-runner", diff --git a/docs/adr/0011-interaction-guarantee-contract.md b/docs/adr/0011-interaction-guarantee-contract.md index 4f91843195..dab99333ee 100644 --- a/docs/adr/0011-interaction-guarantee-contract.md +++ b/docs/adr/0011-interaction-guarantee-contract.md @@ -192,6 +192,18 @@ repeat. A producer that runs several device inputs inside one bound operation Each row names its driver file by id prefix, and that file drives the real producer. +The Apple runner does not resend a mutating command whose first send may have +run: a restart resends only a command the first attempt provably did not write, +or a read-only one. A mutation whose reply stays lost (the runner dies +mid-command, or status recovery finds neither a retained result nor a runner +answer) fails with `reason: runner_reply_lost` and `dispatched: unknown` (rows +`ios-runner.transport.written-then-lost`, +`ios-runner.status.completed-without-retained-reply`, +`ios-runner.status.accepted`, `ios-runner.status.started`, +`ios-runner.status.notAccepted`, `ios-runner.status.probe-failed`, and +`ios-runner.status.unavailable`). The `ios-runner.status.failed*` rows carry the +runner's own answer instead. A read keeps its transport error and is resent. + Remaining gaps: a failure before the router's locked scope (session resolution, lock acquisition, lease and daemon-policy admission) never reaches the disclosure and carries no `dispatched`. It sends nothing, but a consumer diff --git a/packages/platform-apple/src/runner/__tests__/fake-runner-server.ts b/packages/platform-apple/src/runner/__tests__/fake-runner-server.ts index b96acf5634..9b4298605c 100644 --- a/packages/platform-apple/src/runner/__tests__/fake-runner-server.ts +++ b/packages/platform-apple/src/runner/__tests__/fake-runner-server.ts @@ -13,7 +13,11 @@ import type { AddressInfo } from 'node:net'; export type FakeRunnerResponse = | { kind: 'ok'; data: Record } | { kind: 'runnerError'; code: string; message: string } - | { kind: 'hangUp' }; + | { kind: 'hangUp' } + /** Hangs up on this request and every later one for the command: the entry is never consumed. */ + | { kind: 'hangUpAlways' } + /** Hangs up and stops listening, as a runner process that died mid-command. */ + | { kind: 'exit' }; export type FakeRunnerRequest = { command: string; @@ -44,6 +48,7 @@ export async function startFakeRunnerServer( : Object.fromEntries(Object.entries(script).map(([key, list]) => [key, [...list]])); const remaining = sequential ?? []; const requests: FakeRunnerRequest[] = []; + let stopped: Promise | undefined; const server = http.createServer((req, res) => { let raw = ''; req.on('data', (chunk) => { @@ -53,24 +58,18 @@ export async function startFakeRunnerServer( const body = parseBody(raw); requests.push({ command: String(body.command ?? ''), body }); const next = byCommand - ? (byCommand[String(body.command ?? '')]?.shift() ?? { kind: 'ok' as const, data: {} }) - : remaining.shift(); - if (!next) { - res.statusCode = 500; - res.end(JSON.stringify({ ok: false, error: { message: 'fake runner script exhausted' } })); - return; - } - if (next.kind === 'hangUp') { + ? (takeScriptedResponse(byCommand[String(body.command ?? '')]) ?? { + kind: 'ok' as const, + data: {}, + }) + : takeScriptedResponse(remaining); + if (next?.kind === 'exit') { res.destroy(); + stopped ??= new Promise((resolve) => server.close(() => resolve())); + server.closeAllConnections(); return; } - if (next.kind === 'runnerError') { - res.setHeader('content-type', 'application/json'); - res.end(JSON.stringify({ ok: false, error: { code: next.code, message: next.message } })); - return; - } - res.setHeader('content-type', 'application/json'); - res.end(JSON.stringify({ ok: true, data: next.data })); + writeFakeRunnerResponse(res, next); }); }); await new Promise((resolve) => server.listen(0, '127.0.0.1', resolve)); @@ -79,12 +78,41 @@ export async function startFakeRunnerServer( port, requests, close: () => + stopped ?? new Promise((resolve, reject) => server.close((error) => (error ? reject(error) : resolve())), ), }; } +/** The next scripted reply; a `hangUpAlways` entry stays at the head of its queue. */ +function takeScriptedResponse( + queue: FakeRunnerResponse[] | undefined, +): FakeRunnerResponse | undefined { + return queue?.[0]?.kind === 'hangUpAlways' ? queue[0] : queue?.shift(); +} + +function writeFakeRunnerResponse( + res: http.ServerResponse, + next: Exclude | undefined, +): void { + if (!next) { + res.statusCode = 500; + res.end(JSON.stringify({ ok: false, error: { message: 'fake runner script exhausted' } })); + return; + } + if (next.kind === 'hangUp' || next.kind === 'hangUpAlways') { + res.destroy(); + return; + } + res.setHeader('content-type', 'application/json'); + if (next.kind === 'runnerError') { + res.end(JSON.stringify({ ok: false, error: { code: next.code, message: next.message } })); + return; + } + res.end(JSON.stringify({ ok: true, data: next.data })); +} + function parseBody(raw: string): Record { try { const parsed: unknown = JSON.parse(raw); diff --git a/packages/platform-apple/src/runner/__tests__/runner-command-recovery.test.ts b/packages/platform-apple/src/runner/__tests__/runner-command-recovery.test.ts index 24f847f53c..8749668dfd 100644 --- a/packages/platform-apple/src/runner/__tests__/runner-command-recovery.test.ts +++ b/packages/platform-apple/src/runner/__tests__/runner-command-recovery.test.ts @@ -6,6 +6,7 @@ import { IOS_SIMULATOR } from './device-fixtures.ts'; import type { ExecResult } from '@agent-device/host-kit/command'; import { handleRunnerTransportErrorAfterCommandSend } from '../runner-command-recovery.ts'; import type { RunnerCommand } from '../runner-contract.ts'; +import { RUNNER_REPLY_LOST_REASON } from '../runner-error-classification.ts'; import type { RunnerSession } from '../runner-session.ts'; import { startFakeRunnerServer, @@ -122,22 +123,53 @@ test('an unknown lifecycle state invalidates the session and says so', async () assert.equal(invalidate.mock.calls[0]?.[1], 'transport_error_after_command_send'); }); -test('a failing status probe retains the invalidation and rethrows the transport error', async () => { +test('notAccepted from a restarted runner fails the lost command as unknown, naming the lost reply', async () => { + const { result, invalidate } = await runRecovery({ + script: [{ kind: 'ok', data: { lifecycleState: 'notAccepted' } }], + }); + + await assert.rejects(result, (error: unknown) => { + assert.ok(error instanceof AppError); + assert.equal(error.details?.reason, RUNNER_REPLY_LOST_REASON); + assert.equal(error.details?.dispatched, 'unknown'); + return true; + }); + assert.equal(invalidate.mock.calls.length, 1); +}); + +test('a failing status probe retains the invalidation and names the lost reply', async () => { const { result, invalidate, transportError } = await runRecovery({ script: [{ kind: 'runnerError', code: 'COMMAND_FAILED', message: 'status probe exploded' }], + transportError: new AppError('COMMAND_FAILED', 'socket hang up', { reason: 'socket_reset' }), }); - await assert.rejects(result, (error: unknown) => error === transportError); + await assert.rejects(result, (error: unknown) => { + assert.ok(error instanceof AppError); + assert.notEqual(error.message, transportError.message); + assert.equal(error.details?.transportError, transportError.message); + assert.equal(error.cause, transportError); + assert.equal(error.details?.reason, RUNNER_REPLY_LOST_REASON); + assert.equal(error.details?.transportReason, 'socket_reset'); + assert.equal(error.details?.recovery, 'status_probe_failed'); + assert.equal(error.details?.dispatched, 'unknown'); + return true; + }); assert.equal(invalidate.mock.calls.length, 1); }); -test('a command without an id cannot be probed: invalidate and rethrow', async () => { +test('a command without an id cannot be probed: invalidate and name the lost reply', async () => { const { result, invalidate, transportError } = await runRecovery({ script: [], command: { command: 'tap', x: 10, y: 10 } as RunnerCommand, }); - await assert.rejects(result, (error: unknown) => error === transportError); + await assert.rejects(result, (error: unknown) => { + assert.ok(error instanceof AppError); + assert.equal(error.cause, transportError); + assert.equal(error.details?.reason, RUNNER_REPLY_LOST_REASON); + assert.equal(error.details?.recovery, 'status_recovery_unavailable'); + return true; + }); assert.equal(invalidate.mock.calls.length, 1); assert.equal(server?.requests.length, 0); }); diff --git a/packages/platform-apple/src/runner/__tests__/runner-command-retry.test.ts b/packages/platform-apple/src/runner/__tests__/runner-command-retry.test.ts index 478e65ed06..6ed3423486 100644 --- a/packages/platform-apple/src/runner/__tests__/runner-command-retry.test.ts +++ b/packages/platform-apple/src/runner/__tests__/runner-command-retry.test.ts @@ -5,6 +5,7 @@ import { createTestRequestCancellation, makeRunnerSession, runnerConnectFailure, + unwrittenConnectRefusal, } from './runner-session-fixtures.ts'; import { AppError } from '@agent-device/kernel/errors'; import { Deadline } from '../host.ts'; @@ -49,6 +50,7 @@ vi.mock('../runner-xctestrun.ts', async () => { import { prepareIosRunner, runAppleRunnerCommand } from '../runner-client.ts'; import { resetRunnerRecycleLedgerForTests } from '../runner-recycle-ledger.ts'; +import { RUNNER_REPLY_LOST_REASON } from '../runner-error-classification.ts'; import type { RunnerXctestrunArtifact } from '../runner-xctestrun.ts'; const requestCancellation = createTestRequestCancellation(); @@ -306,7 +308,7 @@ test('mutating commands restart stale ready sessions when the preflight probe ne mockEnsureRunnerSession.mockResolvedValueOnce(staleSession).mockResolvedValueOnce(freshSession); mockExecuteRunnerCommandWithSession - .mockRejectedValueOnce(runnerConnectFailure('runner_connect_refused')) + .mockRejectedValueOnce(unwrittenConnectRefusal()) .mockResolvedValueOnce({ message: 'tapped' }); const result = await runAppleRunnerCommand(IOS_SIMULATOR, { command: 'tap', x: 120, y: 240 }); @@ -329,7 +331,7 @@ test('mutating commands retry startup sessions with stale bundle cleanup', async mockEnsureRunnerSession.mockResolvedValueOnce(startupSession).mockResolvedValueOnce(freshSession); mockExecuteRunnerCommandWithSession - .mockRejectedValueOnce(runnerConnectFailure('runner_connect_refused')) + .mockRejectedValueOnce(unwrittenConnectRefusal()) .mockResolvedValueOnce({ message: 'tapped' }); const result = await runAppleRunnerCommand(IOS_SIMULATOR, { command: 'tap', x: 120, y: 240 }); @@ -474,10 +476,9 @@ test('mutating commands keep invalidating when status recovery probe fails', asy await assert.rejects( () => runAppleRunnerCommand(IOS_SIMULATOR, { command: 'tap', x: 120, y: 240 }), (error: unknown) => { - // A failed status probe re-throws the original transport error, not the probe's own. assert.ok(error instanceof AppError); - assert.equal(error.code, 'COMMAND_FAILED'); - assert.equal(error.message, 'fetch failed'); + assert.equal(error.details?.reason, RUNNER_REPLY_LOST_REASON); + assert.equal(error.details?.transportError, 'fetch failed'); return true; }, ); @@ -814,7 +815,7 @@ test('mutating commands invalidate the retry session without replaying again', a mockEnsureRunnerSession.mockResolvedValueOnce(staleSession).mockResolvedValueOnce(freshSession); mockExecuteRunnerCommandWithSession - .mockRejectedValueOnce(runnerConnectFailure('runner_connect_refused')) + .mockRejectedValueOnce(unwrittenConnectRefusal()) .mockRejectedValueOnce(new AppError('COMMAND_FAILED', 'fetch failed')) .mockResolvedValueOnce({ lifecycleState: 'notAccepted' }); @@ -970,10 +971,9 @@ test('sequence invalidates the session when the status probe fails', async () => steps: [{ kind: 'tap', x: 1, y: 2 }], }), (error: unknown) => { - // A failed status probe re-throws the original transport error, not the probe's own. assert.ok(error instanceof AppError); - assert.equal(error.code, 'COMMAND_FAILED'); - assert.equal(error.message, 'fetch failed'); + assert.equal(error.details?.reason, RUNNER_REPLY_LOST_REASON); + assert.equal(error.details?.transportError, 'fetch failed'); return true; }, ); @@ -1189,10 +1189,9 @@ test('a later command in the same request cannot pay for a second recycle boot', const requestId = 'req-restart-cap'; const staleSession = makeRunnerSession({ port: 8100, state: 'ready' }); const freshSession = makeRunnerSession({ port: 8101, state: 'starting' }); - mockEnsureRunnerSession.mockResolvedValueOnce(staleSession).mockResolvedValueOnce(freshSession); mockExecuteRunnerCommandWithSession - .mockRejectedValueOnce(runnerConnectFailure('runner_connect_refused')) + .mockRejectedValueOnce(unwrittenConnectRefusal()) .mockResolvedValueOnce({ message: 'tapped' }); // First command consumes the request's only recycle via restart-and-replay. diff --git a/packages/platform-apple/src/runner/__tests__/runner-dispatch-disclosure.test.ts b/packages/platform-apple/src/runner/__tests__/runner-dispatch-disclosure.test.ts index 012476536c..1580de20e7 100644 --- a/packages/platform-apple/src/runner/__tests__/runner-dispatch-disclosure.test.ts +++ b/packages/platform-apple/src/runner/__tests__/runner-dispatch-disclosure.test.ts @@ -10,14 +10,17 @@ import { } from '@agent-device/contracts/dispatch-disclosure-fixtures'; import { IOS_SIMULATOR } from './device-fixtures.ts'; import { handleRunnerTransportErrorAfterCommandSend } from '../runner-command-recovery.ts'; +import { isReadOnlyRunnerCommand } from '../runner-command-traits.ts'; import type { RunnerCommand } from '../runner-contract.ts'; import { isRetryableRunnerError, isStructuredRunnerFailure, + RUNNER_REPLY_LOST_REASON, } from '../runner-error-classification.ts'; import { runApplePressSeries } from '../runner-sequence.ts'; import { executeRunnerCommandWithSession, type RunnerSession } from '../runner-session.ts'; import { RunnerCommandAccounting } from '../runner-session-types.ts'; +import { appleRunnerTestHost } from '../test-host.ts'; import { startFakeRunnerServer, type FakeRunnerCommandScript, @@ -66,19 +69,33 @@ async function replyFailure(code: string): Promise { ); } -/** The runner hangs up on the command, then answers the status probe with `status`. */ +/** + * The runner hangs up on the command, then answers the status probe with `status`. A read goes + * through the connect loop, which posts again on each attempt, so the runner hangs up on every + * attempt until the loop gives up, and the loop's simctl curl fallback times out after its POST. + * The read's short timeout only bounds how long the loop runs. + */ async function lostResponse( status: FakeRunnerResponse[], command: RunnerCommand = TAP, ): Promise { - server = await startFakeRunnerServer({ tap: [{ kind: 'hangUp' }], status }); + const readOnly = isReadOnlyRunnerCommand(command); + server = await startFakeRunnerServer({ + [command.command]: [{ kind: readOnly ? 'hangUpAlways' : 'hangUp' }], + status, + }); + if (readOnly) { + appleRunnerTestHost.update({ + runXcrun: vi.fn(async () => ({ exitCode: 28, stdout: '', stderr: 'curl exited 28' })), + }); + } const session = runnerSession(server.port); const transportError = await executeRunnerCommandWithSession( IOS_SIMULATOR, session, command, undefined, - 5_000, + readOnly ? 400 : 5_000, ).then( () => assert.fail('the fake runner hangs up on the command'), (error: unknown) => asAppError(error, 'COMMAND_FAILED'), @@ -175,6 +192,11 @@ const DRIVERS: Record Promise> = { lostResponse(statusReply({ lifecycleState: 'failed', lifecycleErrorCode: 'RUNNER_BUSY' })), 'ios-runner.status.completed-without-retained-reply': () => lostResponse(statusReply({ lifecycleState: 'completed' })), + 'ios-runner.status.read-only-completed-without-retained-reply': () => + lostResponse(statusReply({ lifecycleState: 'completed' }), { + command: 'snapshot', + commandId: 'cmd-1', + }), 'ios-runner.status.accepted': () => lostResponse(statusReply({ lifecycleState: 'accepted' })), 'ios-runner.status.started': () => lostResponse(statusReply({ lifecycleState: 'started' })), 'ios-runner.status.notAccepted': () => @@ -185,6 +207,16 @@ const DRIVERS: Record Promise> = { lostResponse([], { command: 'tap', x: 10, y: 10 } as RunnerCommand), }; +/** The rows whose mutation's reply stayed lost: each fails as runner_reply_lost and is not resent. */ +const REPLY_LOST_ROWS: ReadonlySet = new Set([ + 'ios-runner.status.completed-without-retained-reply', + 'ios-runner.status.accepted', + 'ios-runner.status.started', + 'ios-runner.status.notAccepted', + 'ios-runner.status.probe-failed', + 'ios-runner.status.unavailable', +]); + const ROWS = dispatchDisclosureRowsOwnedBy( import.meta.url, fs.readFileSync(DISPATCH_DISCLOSURE_TABLE_PATH, 'utf8'), @@ -201,7 +233,27 @@ for (const row of ROWS) { await assert.rejects(drive(), (error: unknown) => { assert.ok(error instanceof AppError); assert.equal(error.details?.dispatched, row.dispatched); + assert.equal(error.details?.reason === RUNNER_REPLY_LOST_REASON, REPLY_LOST_ROWS.has(row.id)); return true; }); }); } + +const SNAPSHOT: RunnerCommand = { command: 'snapshot', commandId: 'cmd-1' }; + +test.each([ + ['the status probe fails', [{ kind: 'runnerError', code: 'COMMAND_FAILED', message: 'down' }]], + ['status answers notAccepted', statusReply({ lifecycleState: 'notAccepted' })], + ['status answers started', statusReply({ lifecycleState: 'started' })], + ['status answers completed with no retained reply', statusReply({ lifecycleState: 'completed' })], +] as const)( + 'a read whose reply is lost when %s keeps the transport error it is resent on', + async (_, status) => { + await assert.rejects(lostResponse([...status], SNAPSHOT), (error: unknown) => { + assert.ok(error instanceof AppError); + assert.notEqual(error.details?.reason, RUNNER_REPLY_LOST_REASON); + assert.equal(isRetryableRunnerError(error), true); + return true; + }); + }, +); diff --git a/packages/platform-apple/src/runner/__tests__/runner-error-classification.test.ts b/packages/platform-apple/src/runner/__tests__/runner-error-classification.test.ts index d5431f1322..f5870a8f21 100644 --- a/packages/platform-apple/src/runner/__tests__/runner-error-classification.test.ts +++ b/packages/platform-apple/src/runner/__tests__/runner-error-classification.test.ts @@ -7,6 +7,7 @@ import { } from '../runner-contract.ts'; import { RUNNER_ERROR_RULES, + RUNNER_REPLY_LOST_REASON, isRetryableRunnerError, isRunnerBusyError, resolveRunnerFatalErrorReason, @@ -15,7 +16,10 @@ import { shouldRestartRunnerBeforeCommandSend, shouldRetryRunnerConnectError, } from '../runner-error-classification.ts'; -import { runnerConnectFailure } from './runner-session-fixtures.ts'; +import { runnerConnectFailure, unwrittenConnectRefusal } from './runner-session-fixtures.ts'; + +const TAP = { command: 'tap' } as const; +const SNAPSHOT = { command: 'snapshot' } as const; function commandFailed(message: string, details?: Record): AppError { return new AppError('COMMAND_FAILED', message, details); @@ -43,6 +47,15 @@ test('transport-shaped failures are retryable', () => { } }); +test('a lost reply keys on its typed reason, never on the transport text it carries', () => { + for (const message of ['fetch failed', 'connect ECONNREFUSED 127.0.0.1:8100', 'socket hang up']) { + const lostReply = commandFailed(message, { reason: RUNNER_REPLY_LOST_REASON }); + assert.equal(isRetryableRunnerError(lostReply), false, message); + assert.equal(shouldRetryRunnerConnectError(lostReply), false, message); + assert.equal(shouldRestartRunnerBeforeCommandSend(lostReply, TAP), false, message); + } +}); + test('boot-shaped failures are not retryable', () => { assert.equal( isRetryableRunnerError( @@ -165,7 +178,7 @@ test('a deadline on its own earns no recovery verdict', () => { timeoutMs: 45_000, }); assert.equal(isRetryableRunnerError(deadline), false); - assert.equal(shouldRestartRunnerBeforeCommandSend(deadline), false); + assert.equal(shouldRestartRunnerBeforeCommandSend(deadline, SNAPSHOT), false); assert.equal(shouldRestartRunnerAfterReadinessPreflight(deadline), false); assert.equal(shouldRebuildCachedRunnerArtifact(deadline), false); assert.equal(shouldRetryRunnerConnectError(deadline), true); @@ -244,12 +257,16 @@ test('ordinary errors are never session-fatal', () => { // --- restart-before-send axis (shouldRestartRunnerBeforeCommandSend) --- test('a refused connection before send restarts the session', () => { - assert.equal( - shouldRestartRunnerBeforeCommandSend( - runnerConnectFailure('runner_connect_refused', 'Runner did not accept connection'), - ), - true, - ); + assert.equal(shouldRestartRunnerBeforeCommandSend(unwrittenConnectRefusal(), TAP), true); +}); + +test('a connect failure whose POST may have been written restarts only a read', () => { + // simctl curl exit 28: the POST left, then the request timed out. + const writtenThenTimedOut = runnerConnectFailure('runner_connect_refused', undefined, { + dispatched: 'unknown', + }); + assert.equal(shouldRestartRunnerBeforeCommandSend(writtenThenTimedOut, TAP), false); + assert.equal(shouldRestartRunnerBeforeCommandSend(writtenThenTimedOut, SNAPSHOT), true); }); test('an early exit or a foreign transport failure earns no restart before send', () => { @@ -257,8 +274,11 @@ test('an early exit or a foreign transport failure earns no restart before send' 'xcodebuild_exited_early', 'xcodebuild exited early: runner did not accept connection', ); - assert.equal(shouldRestartRunnerBeforeCommandSend(earlyExit), false); - assert.equal(shouldRestartRunnerBeforeCommandSend(commandFailed('socket hang up')), false); + assert.equal(shouldRestartRunnerBeforeCommandSend(earlyExit, SNAPSHOT), false); + assert.equal( + shouldRestartRunnerBeforeCommandSend(commandFailed('socket hang up'), SNAPSHOT), + false, + ); }); // --- typed connect-failure reasons (agent-device's own connect path) --- @@ -269,7 +289,7 @@ test('xcodebuild_exited_early is decided by the typed reason, not the message', assert.equal(isRetryableRunnerError(error), false, message); assert.equal(shouldRetryRunnerConnectError(error), false, message); assert.equal(shouldRebuildCachedRunnerArtifact(error), false, message); - assert.equal(shouldRestartRunnerBeforeCommandSend(error), false, message); + assert.equal(shouldRestartRunnerBeforeCommandSend(error, SNAPSHOT), false, message); } // The same words without the reason earn no terminal verdict. const untyped = commandFailed('Runner did not accept connection (xcodebuild exited early)'); @@ -282,12 +302,12 @@ test('runner_connect_refused is decided by the typed reason, not the message', ( assert.equal(isRetryableRunnerError(error), true, message); assert.equal(shouldRetryRunnerConnectError(error), true, message); assert.equal(shouldRebuildCachedRunnerArtifact(error), true, message); - assert.equal(shouldRestartRunnerBeforeCommandSend(error), true, message); + assert.equal(shouldRestartRunnerBeforeCommandSend(error, SNAPSHOT), true, message); } const untyped = commandFailed('Runner did not accept connection'); assert.equal(isRetryableRunnerError(untyped), false); assert.equal(shouldRebuildCachedRunnerArtifact(untyped), false); - assert.equal(shouldRestartRunnerBeforeCommandSend(untyped), false); + assert.equal(shouldRestartRunnerBeforeCommandSend(untyped, SNAPSHOT), false); }); test('runner_endpoint_probe_exhausted is decided by the typed reason, not the message', () => { @@ -295,7 +315,7 @@ test('runner_endpoint_probe_exhausted is decided by the typed reason, not the me const error = runnerConnectFailure('runner_endpoint_probe_exhausted', message); assert.equal(shouldRebuildCachedRunnerArtifact(error), true, message); assert.equal(isRetryableRunnerError(error), false, message); - assert.equal(shouldRestartRunnerBeforeCommandSend(error), false, message); + assert.equal(shouldRestartRunnerBeforeCommandSend(error, SNAPSHOT), false, message); assert.equal(shouldRetryRunnerConnectError(error), true, message); } assert.equal( diff --git a/packages/platform-apple/src/runner/__tests__/runner-lifecycle-dispatch-disclosure.test.ts b/packages/platform-apple/src/runner/__tests__/runner-lifecycle-dispatch-disclosure.test.ts index 6380c6295e..ac2f49b1a2 100644 --- a/packages/platform-apple/src/runner/__tests__/runner-lifecycle-dispatch-disclosure.test.ts +++ b/packages/platform-apple/src/runner/__tests__/runner-lifecycle-dispatch-disclosure.test.ts @@ -10,6 +10,7 @@ import { import { IOS_SIMULATOR } from './device-fixtures.ts'; import { createTestRequestCancellation, makeRunnerSession } from './runner-session-fixtures.ts'; import { appleRunnerTestHost } from '../test-host.ts'; +import { startFakeRunnerServer, type FakeRunnerServer } from './fake-runner-server.ts'; // contracts/fixtures/dispatch-disclosure.json, ios-runner pre-send and transport rows: each row // drives runAppleRunnerCommand through the real lifecycle and restart path with the session start @@ -39,6 +40,11 @@ vi.mock('../runner-session.ts', async () => { }); import { runAppleRunnerCommand } from '../runner-client.ts'; +import { + isRetryableRunnerError, + RUNNER_REPLY_LOST_REASON, +} from '../runner-error-classification.ts'; +import type { RunnerCommand } from '../runner-contract.ts'; import { resetRunnerRecycleLedgerForTests } from '../runner-recycle-ledger.ts'; import { waitForRunner } from '../runner-startup-transport.ts'; @@ -57,8 +63,12 @@ beforeEach(() => { }); }); -afterEach(() => { +let fakeRunner: FakeRunnerServer | undefined; + +afterEach(async () => { vi.unstubAllGlobals(); + await fakeRunner?.close(); + fakeRunner = undefined; }); async function tap(): Promise { @@ -72,12 +82,9 @@ const readinessPreflightFailure = (): AppError => /** * The first attempt runs the real connect loop against a simulator whose every fetch fails the - * same way and whose simctl curl fallback exits with `curlExitCode`; the restart it earns fails. + * same way and whose simctl curl fallback exits with `curlExitCode`. */ -async function connectLoopThenFailedRestart(transport: { - fetchFailure: () => Error; - curlExitCode: number; -}): Promise { +function stubConnectLoopFailure(transport: { fetchFailure: () => Error; curlExitCode: number }) { vi.stubGlobal( 'fetch', vi.fn(async () => { @@ -91,13 +98,21 @@ async function connectLoopThenFailedRestart(transport: { stderr: `curl exited ${transport.curlExitCode}`, })), }); - mockEnsureRunnerSession - .mockResolvedValueOnce(makeRunnerSession()) - .mockRejectedValueOnce(new AppError('COMMAND_FAILED', 'runner restart failed')); mockExecuteRunnerCommandWithSession.mockImplementationOnce( async (device, session, command) => await waitForRunner(device, session.port, command, undefined, 400), ); +} + +/** A connect loop that fails as `transport` says, then a restart that fails. */ +async function connectLoopThenFailedRestart(transport: { + fetchFailure: () => Error; + curlExitCode: number; +}): Promise { + stubConnectLoopFailure(transport); + mockEnsureRunnerSession + .mockResolvedValueOnce(makeRunnerSession()) + .mockRejectedValueOnce(new AppError('COMMAND_FAILED', 'runner restart failed')); try { return await tap(); } catch (error) { @@ -107,6 +122,38 @@ async function connectLoopThenFailedRestart(transport: { } } +/** A fetch deadline, then a simctl curl that timed out after its POST: the command may have run. */ +const writtenThenLost = { + fetchFailure: () => new AppError('COMMAND_FAILED', 'Runner command deadline exceeded'), + curlExitCode: 28, +}; + +/** + * `command` is written and its reply lost on the first runner, the runner restarts, and the + * restarted runner answers every command with `restartedAnswer`. + */ +async function writtenThenLostThenRestarted( + command: RunnerCommand, + restartedAnswer: () => Promise>, +): Promise> { + stubConnectLoopFailure(writtenThenLost); + mockEnsureRunnerSession + .mockResolvedValueOnce(makeRunnerSession()) + .mockResolvedValueOnce(makeRunnerSession({ port: 8101 })); + mockExecuteRunnerCommandWithSession.mockImplementation(restartedAnswer); + try { + return await runAppleRunnerCommand(IOS_SIMULATOR, command); + } finally { + assert.equal(mockEnsureRunnerSession.mock.calls.length, 2, 'the runner is restarted'); + } +} + +function sendsOnRestartedRunner(): number { + return mockExecuteRunnerCommandWithSession.mock.calls.filter( + ([, session]) => session.port === 8101, + ).length; +} + const DRIVERS: Record Promise> = { 'ios-runner.pre-send.session-start-failed': async () => { mockEnsureRunnerSession.mockRejectedValueOnce( @@ -124,11 +171,47 @@ const DRIVERS: Record Promise> = { }), curlExitCode: 7, }), - 'ios-runner.transport.written-then-lost': () => - connectLoopThenFailedRestart({ - fetchFailure: () => new AppError('COMMAND_FAILED', 'Runner command deadline exceeded'), - curlExitCode: 28, - }), + 'ios-runner.transport.written-then-lost': async () => { + const runnerSession = + await vi.importActual('../runner-session.ts'); + const runner = await startFakeRunnerServer({ tap: [{ kind: 'exit' }] }); + fakeRunner = runner; + // The status probe finds no listener, over fetch or the simctl curl fallback. + const { retryWithPolicy } = appleRunnerTestHost.defaults(); + appleRunnerTestHost.update({ + runXcrun: vi.fn(async () => ({ exitCode: 7, stdout: '', stderr: 'curl exited 7' })), + retryWithPolicy: (task, policy, options) => + retryWithPolicy(task, { ...policy, baseDelayMs: 1, maxDelayMs: 1, jitter: 0 }, options), + }); + mockEnsureRunnerSession.mockResolvedValueOnce(makeRunnerSession({ port: runner.port })); + mockExecuteRunnerCommandWithSession.mockImplementation( + runnerSession.executeRunnerCommandWithSession, + ); + try { + return await tap(); + } catch (error) { + assert.ok(error instanceof AppError); + assert.equal(error.details?.reason, RUNNER_REPLY_LOST_REASON); + assert.equal(error.details?.recovery, 'status_probe_failed'); + assert.equal(error.details?.runnerRestarted, undefined, 'the runner is not restarted'); + throw error; + } finally { + assert.equal(runner.requests.filter((entry) => entry.command === 'tap').length, 1); + assert.deepEqual( + mockInvalidateRunnerSession.mock.calls[0]?.[1], + 'transport_error_after_command_send', + ); + } + }, + 'ios-runner.transport.read-only-written-then-lost': async () => { + try { + return await writtenThenLostThenRestarted({ command: 'snapshot' }, async () => { + throw new AppError('COMMAND_FAILED', 'runner resend failed', { dispatched: 'unknown' }); + }); + } finally { + assert.equal(sendsOnRestartedRunner(), 1, 'the read is sent again'); + } + }, 'ios-runner.pre-send.readiness-preflight-after-restart': async () => { mockEnsureRunnerSession .mockResolvedValueOnce(makeRunnerSession()) @@ -174,6 +257,61 @@ for (const row of ROWS) { }); } +test('a mutation whose connect-loop POST timed out after writing is not restarted or resent', async () => { + stubConnectLoopFailure(writtenThenLost); + mockEnsureRunnerSession.mockResolvedValueOnce(makeRunnerSession()); + mockExecuteRunnerCommandWithSession.mockRejectedValue( + new AppError('COMMAND_FAILED', 'status probe failed'), + ); + await assert.rejects(tap(), (error: unknown) => { + assert.ok(error instanceof AppError); + assert.equal(error.details?.runnerRestarted, undefined); + assert.equal(error.details?.dispatched, 'unknown'); + assert.equal(error.details?.reason, RUNNER_REPLY_LOST_REASON); + assert.equal(error.details?.recovery, 'status_probe_failed'); + return true; + }); + assert.equal(mockEnsureRunnerSession.mock.calls.length, 1, 'the runner is not restarted'); + const taps = mockExecuteRunnerCommandWithSession.mock.calls.filter( + ([, , command]) => command.command === 'tap', + ); + assert.equal(taps.length, 1, 'the tap is sent once'); +}); + +test('a mutation whose reply and status probe both fail on transport text is not resent', async () => { + mockEnsureRunnerSession.mockResolvedValueOnce(makeRunnerSession()); + mockExecuteRunnerCommandWithSession + .mockRejectedValueOnce(new AppError('COMMAND_FAILED', 'fetch failed')) + .mockRejectedValue(new AppError('COMMAND_FAILED', 'connect ECONNREFUSED 127.0.0.1:8100')); + await assert.rejects(tap(), (error: unknown) => { + assert.ok(error instanceof AppError); + assert.equal(error.details?.reason, RUNNER_REPLY_LOST_REASON); + assert.equal(error.details?.transportError, 'fetch failed'); + assert.equal(isRetryableRunnerError(error), false); + return true; + }); + const taps = mockExecuteRunnerCommandWithSession.mock.calls.filter( + ([, , command]) => command.command === 'tap', + ); + assert.equal(taps.length, 1, 'the tap is sent once'); +}); + +test('a read whose first POST may have been written and whose restart fails discloses unknown', async () => { + stubConnectLoopFailure(writtenThenLost); + mockEnsureRunnerSession + .mockResolvedValueOnce(makeRunnerSession()) + .mockRejectedValueOnce(new AppError('COMMAND_FAILED', 'runner restart failed')); + await assert.rejects( + runAppleRunnerCommand(IOS_SIMULATOR, { command: 'snapshot' }), + (error: unknown) => { + assert.ok(error instanceof AppError); + assert.equal(error.details?.runnerRestartReason, 'runner_connect_failed_before_command_send'); + assert.equal(error.details?.dispatched, 'unknown'); + return true; + }, + ); +}); + test('a plain Error before the exchange is normalized and discloses no', async () => { mockEnsureRunnerSession.mockRejectedValueOnce(new Error('spawn EACCES')); await assert.rejects(tap(), (error: unknown) => { diff --git a/packages/platform-apple/src/runner/__tests__/runner-recovery-wiring.test.ts b/packages/platform-apple/src/runner/__tests__/runner-recovery-wiring.test.ts index 5e1c7a4294..9f862ab941 100644 --- a/packages/platform-apple/src/runner/__tests__/runner-recovery-wiring.test.ts +++ b/packages/platform-apple/src/runner/__tests__/runner-recovery-wiring.test.ts @@ -10,13 +10,18 @@ import type { RunnerSession } from '../runner-session.ts'; import { appleRunnerTestHost } from '../test-host.ts'; import { withAppleRunnerProvider } from '../runner-provider.ts'; import { classifyRunnerReportedError, type RunnerCommand } from '../runner-contract.ts'; +import { RUNNER_REPLY_LOST_REASON } from '../runner-error-classification.ts'; import { createRunnerPhaseBudget, requireRunnerPhaseRemainingMs, resolveExpectedRunnerCacheMetadata, } from '../runner-cache-metadata.ts'; import { captureDiagnostics } from './runner-session-fixtures.ts'; -import { startFakeRunnerServer, type FakeRunnerServer } from './fake-runner-server.ts'; +import { + startFakeRunnerServer, + type FakeRunnerResponse, + type FakeRunnerServer, +} from './fake-runner-server.ts'; import { resolveRunnerDetachDecision, RunnerCommandAccounting } from '../runner-session-types.ts'; import { requireLifecycleSettlementRows } from './runner-swift-settlement-fixtures.ts'; @@ -40,6 +45,7 @@ import { requireLifecycleSettlementRows } from './runner-swift-settlement-fixtur */ let server: FakeRunnerServer | undefined; +let restartedServer: FakeRunnerServer | undefined; const { ensureRunnerSessionMock, invalidateRunnerSessionMock } = vi.hoisted(() => ({ ensureRunnerSessionMock: vi.fn(), @@ -92,6 +98,8 @@ const LOST_RESPONSE_MUTATION_ROWS = { afterEach(async () => { await server?.close(); server = undefined; + await restartedServer?.close(); + restartedServer = undefined; ensureRunnerSessionMock.mockReset(); invalidateRunnerSessionMock.mockReset(); }); @@ -160,6 +168,120 @@ test.each(Object.values(LOST_RESPONSE_MUTATION_ROWS))( }, ); +// #3074: `status` answers `notAccepted`, as a runner whose journal did not survive a restart between +// the send and the probe would. That is no proof the first send did not run. +test.each(Object.values(LOST_RESPONSE_MUTATION_ROWS))( + 'a $acceptanceCommand whose status answers notAccepted fails as runner_reply_lost, sent once', + async ({ runnerCommand, request }) => { + server = await startFakeRunnerServer({ + [runnerCommand]: [{ kind: 'hangUp' }], + status: [{ kind: 'ok', data: { lifecycleState: 'notAccepted' } }], + snapshot: [{ kind: 'ok', data: { nodes: [] } }], + }); + const session = seedSession(server.port); + + await assert.rejects(runAppleRunnerCommand(IOS_SIMULATOR, { ...request }), (error: unknown) => { + assert.ok(error instanceof AppError); + assert.equal(error.details?.dispatched, 'unknown'); + assert.equal(error.details?.reason, RUNNER_REPLY_LOST_REASON); + return true; + }); + assert.equal( + server.requests.filter((entry) => entry.command === runnerCommand).length, + 1, + `${runnerCommand} is dispatched once`, + ); + assert.deepEqual(invalidateRunnerSessionMock.mock.calls, [ + [session, 'transport_error_after_command_send'], + ]); + + assert.deepEqual(await runAppleRunnerCommand(IOS_SIMULATOR, { command: 'snapshot' }), { + nodes: [], + }); + assert.equal(ensureRunnerSessionMock.mock.calls.length, 2, 'the next command gets a runner'); + }, +); + +test('a read whose reply is lost is resent and succeeds', async () => { + server = await startFakeRunnerServer({ + snapshot: [{ kind: 'hangUp' }, { kind: 'ok', data: { nodes: [] } }], + status: [{ kind: 'ok', data: { lifecycleState: 'notAccepted' } }], + }); + seedSession(server.port); + // The simctl curl route of a ready simulator could not connect, so it sent nothing. + appleRunnerTestHost.update({ + runXcrun: vi.fn(async () => ({ exitCode: 7, stdout: '', stderr: 'curl exited 7' })), + }); + + assert.deepEqual(await runAppleRunnerCommand(IOS_SIMULATOR, { command: 'snapshot' }), { + nodes: [], + }); + assert.equal(server.requests.filter((entry) => entry.command === 'snapshot').length, 2); +}); + +// #3074: the runner process dies mid-command (the fake hangs up and stops listening), and the next +// session the daemon gets is a new runner on a second server. +async function runnerDiesOnCommand( + runnerCommand: string, + restartedScript: Record, +): Promise { + server = await startFakeRunnerServer({ [runnerCommand]: [{ kind: 'exit' }] }); + restartedServer = await startFakeRunnerServer(restartedScript); + ensureRunnerSessionMock + .mockResolvedValueOnce(makeRunnerSession(server.port)) + .mockResolvedValueOnce(makeRunnerSession(restartedServer.port)); + // The simctl curl fallback of the connect loop timed out after its POST. + appleRunnerTestHost.update({ + runXcrun: vi.fn(async () => ({ exitCode: 28, stdout: '', stderr: 'curl exited 28' })), + }); + return restartedServer; +} + +function sendsOf(target: FakeRunnerServer, runnerCommand: string): number { + return target.requests.filter((entry) => entry.command === runnerCommand).length; +} + +test.each(Object.values(LOST_RESPONSE_MUTATION_ROWS))( + 'a $acceptanceCommand whose runner dies mid-command fails as runner_reply_lost, not resent', + async ({ runnerCommand, request }) => { + const restarted = await runnerDiesOnCommand(runnerCommand, { + readText: [{ kind: 'ok', data: { text: 'after' } }], + }); + + await assert.rejects(runAppleRunnerCommand(IOS_SIMULATOR, { ...request }), (error: unknown) => { + assert.ok(error instanceof AppError); + assert.equal(error.details?.dispatched, 'unknown'); + assert.equal(error.details?.reason, RUNNER_REPLY_LOST_REASON); + return true; + }); + assert.equal(invalidateRunnerSessionMock.mock.calls.length, 1, 'the dead runner is dropped'); + assert.deepEqual( + await runAppleRunnerCommand(IOS_SIMULATOR, { command: 'readText', x: 5, y: 5 }), + { text: 'after' }, + ); + assert.equal(sendsOf(server!, runnerCommand), 1, `${runnerCommand} is dispatched once`); + assert.equal(sendsOf(restarted, runnerCommand), 0, `${runnerCommand} is not resent`); + }, +); + +test('a get whose runner dies mid-read is resent once on the restarted runner', async () => { + const restarted = await runnerDiesOnCommand('readText', { + readText: [{ kind: 'ok', data: { text: 'hello' } }], + }); + + assert.deepEqual( + await runAppleRunnerCommand(IOS_SIMULATOR, { command: 'readText', x: 5, y: 5 }), + { text: 'hello' }, + ); + expect(invalidateRunnerSessionMock).toHaveBeenCalledTimes(1); + expect(invalidateRunnerSessionMock).toHaveBeenCalledWith( + expect.anything(), + 'runner_connect_failed_before_command_send', + ); + assert.equal(sendsOf(server!, 'readText'), 1); + assert.equal(sendsOf(restarted, 'readText'), 1, 'the read is resent once'); +}); + // #2965: an inline `status` probe answers while the command it probes may still be executing, so its // own reply must not clear the mutation's outstanding charge. The handoff verdict is asserted through // `resolveRunnerDetachDecision` — the exact gate `detachRunnerSessionForShutdown` consults. Rows come diff --git a/packages/platform-apple/src/runner/__tests__/runner-session-fixtures.ts b/packages/platform-apple/src/runner/__tests__/runner-session-fixtures.ts index d569b32090..ff209e4e55 100644 --- a/packages/platform-apple/src/runner/__tests__/runner-session-fixtures.ts +++ b/packages/platform-apple/src/runner/__tests__/runner-session-fixtures.ts @@ -92,6 +92,10 @@ export function runnerConnectFailure( }); } +/** Every connect attempt was refused before a byte was written, so a restart may resend the command. */ +export const unwrittenConnectRefusal = (): AppError => + runnerConnectFailure('runner_connect_refused', undefined, { dispatched: 'no' }); + // Records everything the runner package emits through host.emitDiagnostic / // host.withDiagnosticTimer during `callback` and renders it back as the same // newline-delimited-JSON shape a flushed diagnostics session file holds, so diff --git a/packages/platform-apple/src/runner/runner-command-recovery.ts b/packages/platform-apple/src/runner/runner-command-recovery.ts index 645196db27..837391355b 100644 --- a/packages/platform-apple/src/runner/runner-command-recovery.ts +++ b/packages/platform-apple/src/runner/runner-command-recovery.ts @@ -11,30 +11,43 @@ import { type RunnerResponsePayload, } from './runner-contract.ts'; import { isReadOnlyRunnerCommand } from './runner-command-traits.ts'; +import { RUNNER_REPLY_LOST_REASON } from './runner-error-classification.ts'; import type { AppleRunnerCommandOptions } from './runner-provider.ts'; import { executeRunnerCommandWithSession, type RunnerSession } from './runner-session.ts'; type RunnerTransportRecovery = | { type: 'recovered'; data: Record; reason: string; lifecycleState?: string } - | { - type: 'skipInvalidation'; - error: AppError; - dispatched: DispatchDisclosure; + | ({ + type: 'skipInvalidation' | 'retainInvalidation'; reason: string; lifecycleState?: string; - } - | { - type: 'retainInvalidation'; - error?: AppError; - dispatched: DispatchDisclosure; - reason: string; - lifecycleState?: string; - }; + } & RunnerRecoveryFailure); + +/** + * What a verdict that recovered no result fails with: the runner's own answer read back from its + * journal, or a reply that stayed lost. + */ +type RunnerRecoveryFailure = + | { runnerAnswer: AppError; dispatched: DispatchDisclosure } + | { lostReply: LostReply }; + +/** What status recovery learned about a command whose reply stayed lost. */ +type LostReply = Readonly<{ + recovery: string; + lifecycleState?: string; + /** + * Never the transport error's message: classification rows match foreign transport text, and a + * lost reply must not read as the retryable transport failure it wraps. + */ + message: string; + hint: string; +}>; type RunnerTransportRecoveryContext = { command: RunnerCommand; session: RunnerSession; transportError: AppError; + options: AppleRunnerCommandOptions; invalidationReason: string; invalidateSession: (session: RunnerSession, reason: string) => Promise; }; @@ -70,26 +83,64 @@ export async function handleRunnerTransportErrorAfterCommandSend(params: { command, session, transportError, + options, invalidationReason, invalidateSession: params.invalidateSession, }); } async function applyRunnerTransportRecovery( - recovery: RunnerTransportRecovery | undefined, + recovery: RunnerTransportRecovery, context: RunnerTransportRecoveryContext, ): Promise> { - if (!recovery) { - return await retainRunnerInvalidation(context, 'status_recovery_unavailable', 'unknown'); - } if (recovery.type === 'recovered') return recoverRunnerResponse(recovery, context); - if (recovery.type === 'skipInvalidation') throw skipRunnerInvalidation(recovery, context); - return await retainRunnerInvalidation( - context, - recovery.reason, - recovery.dispatched, - recovery.lifecycleState, - recovery.error, + const failure = resolveRunnerRecoveryFailure(recovery, context); + if (recovery.type === 'skipInvalidation') { + throw skipRunnerInvalidation(recovery, context, failure); + } + return await retainRunnerInvalidation(recovery, context, failure); +} + +/** + * A lost reply fails a mutation as {@link RUNNER_REPLY_LOST_REASON}: it is not resent. A read keeps + * the transport error, because `runAppleRunnerCommand` resends a read on exactly that error. + */ +function resolveRunnerRecoveryFailure( + failure: RunnerRecoveryFailure, + context: RunnerTransportRecoveryContext, +): AppError { + if ('runnerAnswer' in failure) return discloseDispatch(failure.runnerAnswer, failure.dispatched); + if (isReadOnlyRunnerCommand(context.command)) { + return discloseDispatch(context.transportError, 'unknown'); + } + return discloseDispatch(buildLostReplyError(context, failure.lostReply), 'unknown'); +} + +/** The one shape of a mutation's lost-reply failure; the transport's own reason stays readable. */ +function buildLostReplyError( + context: RunnerTransportRecoveryContext, + lostReply: LostReply, +): AppError { + const { command, transportError, options } = context; + const transportReason = transportError.details?.reason; + return new AppError( + 'COMMAND_FAILED', + lostReply.message, + { + command: command.command, + commandId: command.commandId, + ...(lostReply.lifecycleState === undefined + ? {} + : { lifecycleState: lostReply.lifecycleState }), + reason: RUNNER_REPLY_LOST_REASON, + ...(transportReason === undefined ? {} : { transportReason }), + recovery: lostReply.recovery, + ...readReadinessPreflightRecoveryDetails(transportError), + hint: lostReply.hint, + logPath: options.logPath ?? transportError.details?.logPath, + transportError: transportError.message, + }, + transportError, ); } @@ -109,8 +160,9 @@ function recoverRunnerResponse( } function skipRunnerInvalidation( - recovery: Extract, + recovery: Exclude, context: RunnerTransportRecoveryContext, + failure: AppError, ): AppError { emitRunnerInvalidationDecision({ command: context.command, @@ -120,26 +172,24 @@ function skipRunnerInvalidation( reason: recovery.reason, lifecycleState: recovery.lifecycleState, }); - return discloseDispatch(recovery.error, recovery.dispatched); + return failure; } async function retainRunnerInvalidation( + recovery: Exclude, context: RunnerTransportRecoveryContext, - reason: string, - dispatched: DispatchDisclosure, - lifecycleState?: string, - error?: AppError, + failure: AppError, ): Promise { emitRunnerInvalidationDecision({ command: context.command, session: context.session, transportError: context.transportError, decision: 'retained', - reason, - lifecycleState, + reason: recovery.reason, + lifecycleState: recovery.lifecycleState, }); await context.invalidateSession(context.session, context.invalidationReason); - throw discloseDispatch(error ?? context.transportError, dispatched); + throw failure; } async function tryRecoverRunnerCommandAfterTransportError( @@ -149,8 +199,18 @@ async function tryRecoverRunnerCommandAfterTransportError( transportError: AppError, options: AppleRunnerCommandOptions, signal?: AbortSignal, -): Promise { - if (command.command === 'status' || !command.commandId?.trim()) return undefined; +): Promise { + if (command.command === 'status' || !command.commandId?.trim()) { + return { + type: 'retainInvalidation', + reason: 'status_recovery_unavailable', + lostReply: { + recovery: 'status_recovery_unavailable', + message: lostReplyWithoutStatusMessage(command.command, 'status recovery was unavailable'), + hint: unknownLifecycleStateHint(command.command), + }, + }; + } const readinessPreflight = readReadinessPreflightRecoveryDetails(transportError); let status: Record; try { @@ -173,7 +233,15 @@ async function tryRecoverRunnerCommandAfterTransportError( ...readinessPreflight, }, }); - return { type: 'retainInvalidation', reason: 'status_probe_failed', dispatched: 'unknown' }; + return { + type: 'retainInvalidation', + reason: 'status_probe_failed', + lostReply: { + recovery: 'status_probe_failed', + message: lostReplyWithoutStatusMessage(command.command, 'the status probe failed'), + hint: unknownLifecycleStateHint(command.command), + }, + }; } const lifecycleState = typeof status.lifecycleState === 'string' ? status.lifecycleState : ''; @@ -241,9 +309,9 @@ function handleRunnerCommandStatusRecovery( command: RunnerCommand, transportError: AppError, options: AppleRunnerCommandOptions, -): RunnerTransportRecovery | undefined { +): RunnerTransportRecovery { if (lifecycleState === 'completed') { - return handleCompletedRunnerStatus(status, command, transportError, options); + return handleCompletedRunnerStatus(status, command, transportError); } if (lifecycleState === 'failed') { @@ -259,7 +327,13 @@ function handleRunnerCommandStatusRecovery( reason: 'runner_reported_failure', lifecycleState, dispatched: classification.details.dispatched, - error: runnerStatusFailureError(status, classification, command, transportError, options), + runnerAnswer: runnerStatusFailureError( + status, + classification, + command, + transportError, + options, + ), }; } @@ -268,8 +342,16 @@ function handleRunnerCommandStatusRecovery( type: 'skipInvalidation', reason: 'command_still_in_flight', lifecycleState, - dispatched: 'unknown', - error: runnerStatusInFlightError(lifecycleState, command, transportError, options), + lostReply: { + recovery: 'command_still_in_flight', + lifecycleState, + message: `Runner command "${command.command}" is still ${lifecycleState} after the transport response was lost.`, + hint: inFlightAfterLostResponseHint( + command.command, + lifecycleState, + readReadinessPreflightRecoveryDetails(transportError), + ), + }, }; } @@ -277,21 +359,12 @@ function handleRunnerCommandStatusRecovery( type: 'retainInvalidation', reason: lifecycleState ? 'unknown_lifecycle_state' : 'missing_lifecycle_state', lifecycleState, - dispatched: 'unknown', - error: new AppError( - 'COMMAND_FAILED', - `Runner command "${command.command}" lost its transport response and lifecycle status was ${lifecycleState ? `"${lifecycleState}"` : 'missing'}, so agent-device invalidated the runner session instead of replaying the command.`, - { - command: command.command, - commandId: command.commandId, - lifecycleState, - recovery: 'lifecycle_state_not_recoverable', - hint: unknownLifecycleStateHint(command.command), - logPath: options.logPath, - transportError: transportError.message, - }, - transportError, - ), + lostReply: { + recovery: 'lifecycle_state_not_recoverable', + lifecycleState, + message: `Runner command "${command.command}" lost its transport response and lifecycle status was ${lifecycleState ? `"${lifecycleState}"` : 'missing'}, so agent-device invalidated the runner session instead of replaying the command.`, + hint: unknownLifecycleStateHint(command.command), + }, }; } @@ -299,7 +372,6 @@ function handleCompletedRunnerStatus( status: Record, command: RunnerCommand, transportError: AppError, - options: AppleRunnerCommandOptions, ): RunnerTransportRecovery { const recovered = parseLifecycleResponseJson(status.lifecycleResponseJson); if (recovered) { @@ -310,36 +382,21 @@ function handleCompletedRunnerStatus( lifecycleState: 'completed', }; } - if (isReadOnlyRunnerCommand(command)) { - return { - type: 'skipInvalidation', - error: transportError, - dispatched: 'unknown', - reason: 'read_only_completed_without_retained_response', - lifecycleState: 'completed', - }; - } - const readinessPreflight = readReadinessPreflightRecoveryDetails(transportError); return { type: 'skipInvalidation', - reason: 'completed_without_retained_response', + reason: isReadOnlyRunnerCommand(command) + ? 'read_only_completed_without_retained_response' + : 'completed_without_retained_response', lifecycleState: 'completed', - dispatched: 'unknown', - error: new AppError( - 'COMMAND_FAILED', - `Runner command "${command.command}" completed after the transport response was lost, but no recoverable response was retained.`, - { - command: command.command, - commandId: command.commandId, - lifecycleState: 'completed', - recovery: 'completed_without_retained_response', - ...readinessPreflight, - hint: completedWithoutRetainedResponseHint(command.command, readinessPreflight), - logPath: options.logPath, - transportError: transportError.message, - }, - transportError, - ), + lostReply: { + recovery: 'completed_without_retained_response', + lifecycleState: 'completed', + message: `Runner command "${command.command}" completed after the transport response was lost, but no recoverable response was retained.`, + hint: completedWithoutRetainedResponseHint( + command.command, + readReadinessPreflightRecoveryDetails(transportError), + ), + }, }; } @@ -375,33 +432,6 @@ function runnerStatusFailureError( ); } -function runnerStatusInFlightError( - lifecycleState: string, - command: RunnerCommand, - transportError: AppError, - options: AppleRunnerCommandOptions, -): AppError { - if (isReadOnlyRunnerCommand(command)) { - return transportError; - } - const readinessPreflight = readReadinessPreflightRecoveryDetails(transportError); - return new AppError( - 'COMMAND_FAILED', - `Runner command "${command.command}" is still ${lifecycleState} after the transport response was lost.`, - { - command: command.command, - commandId: command.commandId, - lifecycleState, - recovery: 'command_still_in_flight', - ...readinessPreflight, - hint: inFlightAfterLostResponseHint(command.command, lifecycleState, readinessPreflight), - logPath: options.logPath, - transportError: transportError.message, - }, - transportError, - ); -} - function parseLifecycleResponseJson(value: unknown): Record | undefined { if (typeof value !== 'string' || value.trim().length === 0) return undefined; let payload: RunnerResponsePayload; @@ -473,6 +503,10 @@ function readReadinessPreflightRecoveryDetails( return details; } +function lostReplyWithoutStatusMessage(command: string, statusOutcome: string): string { + return `Runner command "${command}" lost its transport response and ${statusOutcome}, so agent-device invalidated the runner session instead of replaying the command.`; +} + function unknownLifecycleStateHint(command: string): string { return `The runner did not confirm that "${command}" reached a safe terminal state, so agent-device kept the conservative invalidation path. Run snapshot -i before retrying if the UI may have changed.`; } diff --git a/packages/platform-apple/src/runner/runner-error-classification.ts b/packages/platform-apple/src/runner/runner-error-classification.ts index 772932e398..40431444d5 100644 --- a/packages/platform-apple/src/runner/runner-error-classification.ts +++ b/packages/platform-apple/src/runner/runner-error-classification.ts @@ -3,6 +3,7 @@ import { isRequestCanceledDetails, type AppErrorCode, type AppErrorDetails, + type DispatchDisclosure, } from '@agent-device/kernel/errors'; import { isCommandTimeoutError, @@ -13,7 +14,9 @@ import { MAIN_THREAD_TIMEOUT_RUNNER_CODE, RUNNER_BUSY_RUNNER_CODE, RUNNER_WEDGED_RUNNER_CODE, + type RunnerCommand, } from './runner-contract.ts'; +import { isReadOnlyRunnerCommand } from './runner-command-traits.ts'; export const RUNNER_CACHE_RECOVERY_HINT = 'If runner build products look stale or corrupted, run `pnpm clean:xcuitest` in a local checkout, or remove ~/.agent-device/apple-runner/derived, then retry.'; @@ -42,6 +45,13 @@ export function runnerConnectFailureDetails(reason: RunnerConnectFailureReason): return { runnerConnectFailureReason: reason }; } +/** + * `details.reason` of a mutation whose reply stayed lost: status recovery found no result and no + * runner answer, so nothing proves the command did not run. The command is not resent; the caller + * observes the screen before acting again. A read never carries it, because it is resent. + */ +export const RUNNER_REPLY_LOST_REASON = 'runner_reply_lost'; + type RunnerErrorMatch = { /** Required `AppError.code`; absent = any AppError. */ code?: AppErrorCode; @@ -69,6 +79,8 @@ type RunnerErrorMatch = { details?: RunnerErrorDetailsMatch; }; +const hasRunnerReplyLostReason: RunnerErrorDetailsMatch = (details) => + details.reason === RUNNER_REPLY_LOST_REASON; /** * The runner refused the command before running it while abandoned main-thread work drains (#1105). * A resend keys on this code, never on `details.retriable`: that flag tells a caller's poll to try @@ -221,6 +233,18 @@ const PROFILE_UNUSABLE: RunnerErrorRule['buildFailure'] = { * typed verdict carries the recovery hint a generic connect failure would replace). */ export const RUNNER_ERROR_RULES: readonly RunnerErrorRule[] = [ + { + // A mutation that may have run: no axis may resend or restart it, whatever text it carries. + reason: RUNNER_REPLY_LOST_REASON, + match: { code: 'COMMAND_FAILED', details: hasRunnerReplyLostReason }, + verdicts: { + retryable: false, + drainResend: false, + connectRetry: false, + restartBeforeSend: false, + restartAfterReadinessPreflight: false, + }, + }, { reason: 'usbmux_device_unattached', match: { code: 'DEVICE_NOT_FOUND', details: hasUsbmuxDeviceUnattached }, @@ -610,11 +634,21 @@ export function resolveRunnerFatalErrorReason(error: unknown): string | undefine } /** - * A connect-shaped failure that surfaced before the command was sent: restart - * the runner session and replay the command, rather than probing a runner - * that never accepted the connection. + * A connect-shaped failure that lets the session restart and resend `command`, rather than probing a + * runner that never accepted the connection. The connect loop posts the command on every attempt, so + * its failure restarts only when no attempt could have written the command, or when the command is + * read-only: a POST that timed out after it was written (simctl curl exit 28) is no proof the + * command did not run, and a mutation is never resent on it. */ -export function shouldRestartRunnerBeforeCommandSend(error: unknown): boolean { +export function shouldRestartRunnerBeforeCommandSend( + error: unknown, + command: RunnerCommand, +): boolean { + if (!isRunnerConnectRefusal(error)) return false; + return resolveFirstAttemptDispatch(error) === 'no' || isReadOnlyRunnerCommand(command); +} + +function isRunnerConnectRefusal(error: unknown): boolean { return runnerErrorVerdict(error, 'restartBeforeSend') ?? false; } @@ -625,7 +659,7 @@ export function shouldRestartRunnerBeforeCommandSend(error: unknown): boolean { */ export function isRunnerPreSendRefusal(error: unknown): boolean { return ( - (shouldRestartRunnerBeforeCommandSend(error) && isRunnerCommandProvablyUnwritten(error)) || + (isRunnerConnectRefusal(error) && isRunnerCommandProvablyUnwritten(error)) || shouldRestartRunnerAfterReadinessPreflight(error) || isRunnerBusyError(error) ); @@ -641,6 +675,11 @@ export function isRunnerCommandProvablyUnwritten(error: unknown): boolean { return isConnectionRefused(error, 0); } +/** What a failed connect attempt proves about writing the command: `no` only with that proof. */ +export function resolveFirstAttemptDispatch(error: unknown): DispatchDisclosure { + return isRunnerCommandProvablyUnwritten(error) ? 'no' : 'unknown'; +} + function isConnectionRefused(error: unknown, depth: number): boolean { if (depth > 4 || typeof error !== 'object' || error === null) return false; if ((error as { code?: unknown }).code === 'ECONNREFUSED') return true; diff --git a/packages/platform-apple/src/runner/runner-lifecycle.ts b/packages/platform-apple/src/runner/runner-lifecycle.ts index 86cbbae03f..4f5e51b5a0 100644 --- a/packages/platform-apple/src/runner/runner-lifecycle.ts +++ b/packages/platform-apple/src/runner/runner-lifecycle.ts @@ -5,6 +5,7 @@ import { discloseDispatch, discloseUnclassifiedDispatch, isRequestCanceledError, + type DispatchDisclosure, } from '@agent-device/kernel/errors'; import type { DeviceInfo } from '@agent-device/kernel/device'; import type { ReadinessPhase } from '@agent-device/contracts/wait'; @@ -31,6 +32,7 @@ import { isRetryableRunnerError, isRunnerPreSendRefusal, isStructuredRunnerFailure, + resolveFirstAttemptDispatch, shouldRebuildCachedRunnerArtifact, shouldRestartRunnerAfterReadinessPreflight, shouldRestartRunnerBeforeCommandSend, @@ -351,7 +353,7 @@ async function executeRunnerCommandAttempt( appErr, ); } - if (shouldRestartRunnerBeforeCommandSend(appErr) && session) { + if (shouldRestartRunnerBeforeCommandSend(appErr, command) && session) { assertRunnerRequestActive(options.requestId); return await restartSessionAndRunCommand({ device, @@ -360,7 +362,7 @@ async function executeRunnerCommandAttempt( options, signal, restartReason: 'runner_connect_failed_before_command_send', - firstAttemptUnwritten: isRunnerPreSendRefusal(appErr), + firstAttemptDispatched: resolveFirstAttemptDispatch(appErr), }); } if (session && shouldRestartRunnerAfterReadinessPreflight(appErr)) { @@ -373,7 +375,7 @@ async function executeRunnerCommandAttempt( signal, restartReason: 'runner_readiness_preflight_failed_before_command_send', recoveredDiagnosticPhase: 'ios_runner_readiness_preflight_recovered', - firstAttemptUnwritten: true, + firstAttemptDispatched: 'no', }); } // Status recovery answers "did the command I lost the response to run?". A structured reply @@ -406,10 +408,10 @@ async function restartSessionAndRunCommand(params: { | 'runner_readiness_preflight_failed_before_command_send'; recoveredDiagnosticPhase?: string; /** - * The failed first attempt provably never wrote the command. When it may have, the replay can - * double-send, and no failure of this restart may claim `no`. + * What the failed first attempt disclosed about writing the command. After `unknown`, the replay + * can double-send, and no failure of this restart may claim `no`. */ - firstAttemptUnwritten: boolean; + firstAttemptDispatched: DispatchDisclosure; }): Promise> { const { device, command, options, signal, restartReason } = params; // At most one recycle per request: when the budget is spent, fail fast and KEEP the current @@ -419,7 +421,7 @@ async function restartSessionAndRunCommand(params: { if (!tryBeginRunnerRecycle(recycleKey)) { throw discloseDispatch( buildRunnerRecycleBudgetExhaustedError(command, options), - params.firstAttemptUnwritten ? 'no' : 'unknown', + params.firstAttemptDispatched, ); } await invalidateRunnerSession(params.session, restartReason); @@ -478,7 +480,7 @@ function markRunnerRestartError( error: unknown, params: Pick< Parameters[0], - 'session' | 'command' | 'options' | 'restartReason' | 'firstAttemptUnwritten' + 'session' | 'command' | 'options' | 'restartReason' | 'firstAttemptDispatched' >, restartedSession?: RunnerSession, ): unknown { @@ -500,7 +502,7 @@ function markRunnerRestartError( }, error.cause ?? error, ); - return discloseRestartDispatch(marked, params.firstAttemptUnwritten, restartedSession); + return discloseRestartDispatch(marked, params.firstAttemptDispatched, restartedSession); } /** @@ -510,11 +512,12 @@ function markRunnerRestartError( */ function discloseRestartDispatch( error: AppError, - firstAttemptUnwritten: boolean, + firstAttemptDispatched: DispatchDisclosure, restartedSession: RunnerSession | undefined, ): AppError { - if (!restartedSession) return discloseDispatch(error, firstAttemptUnwritten ? 'no' : 'unknown'); - if (!firstAttemptUnwritten) return discloseDispatch(error, 'unknown'); + if (!restartedSession || firstAttemptDispatched === 'unknown') { + return discloseDispatch(error, firstAttemptDispatched); + } return discloseUnclassifiedDispatch(error, isRunnerPreSendRefusal(error) ? 'no' : 'unknown'); } diff --git a/src/__tests__/runner-read-only-registry-parity.test.ts b/src/__tests__/runner-read-only-registry-parity.test.ts new file mode 100644 index 0000000000..49767e74e9 --- /dev/null +++ b/src/__tests__/runner-read-only-registry-parity.test.ts @@ -0,0 +1,120 @@ +import assert from 'node:assert/strict'; +import { test } from 'vitest'; +import { resolveCommandRecordingEffect } from '@agent-device/command-registry/registry'; +import { fileURLToPath } from 'node:url'; + +type RunnerName = string; +type RunnerCommand = { command: string; action?: string }; +type RunnerTraits = { + isReadOnlyRunnerCommand(command: RunnerCommand): boolean; + RUNNER_COMMAND_TRAITS: Record; +}; + +// The traits module is package-private and exports no subpath; loading it by computed file URL keeps +// this one cross-package parity check from adding a public subpath just for a test. +const { isReadOnlyRunnerCommand, RUNNER_COMMAND_TRAITS } = (await import( + fileURLToPath( + new URL('../../packages/platform-apple/src/runner/runner-command-traits.ts', import.meta.url), + ) +)) as RunnerTraits; + +type RegistryRequest = Readonly<{ command: string; positionals?: readonly string[] }>; + +// The runner owns no mapping to the registry, so this is the one declared place that pairs each +// runner wire command with the registry request that issues it. A runner command with no counterpart +// is runner-internal plumbing: it must be listed in RUNNER_INTERNAL and is checked on its own. +const REGISTRY_COUNTERPART: Record = { + tap: { command: 'press' }, + mouseClick: { command: 'click' }, + longPress: { command: 'longpress' }, + drag: { command: 'swipe' }, + remotePress: { command: 'tv-remote' }, + type: { command: 'type' }, + swipe: { command: 'swipe' }, + scroll: { command: 'scroll' }, + desktopScroll: { command: 'scroll' }, + findText: { command: 'find', positionals: ['text', 'x', 'exists'] }, + querySelector: { command: 'is' }, + readText: { command: 'get' }, + snapshot: { command: 'snapshot' }, + screenshot: { command: 'screenshot' }, + backInApp: { command: 'back' }, + backSystem: { command: 'back' }, + home: { command: 'home' }, + rotate: { command: 'orientation' }, + gesture: { command: 'gesture' }, + appSwitcher: { command: 'app-switcher' }, + actionButton: { command: 'action-button' }, + keyboardDismiss: { command: 'keyboard', positionals: ['dismiss'] }, + keyboardReturn: { command: 'keyboard', positionals: ['enter'] }, + pasteboardWrite: { command: 'clipboard', positionals: ['write', 'x'] }, +}; + +// Runner commands that drive runner or app lifecycle, not a user-visible command, with whether the +// resend gate may treat them as reads. +const RUNNER_INTERNAL: Readonly> = { + status: true, + uptime: true, + appState: true, + gestureViewport: true, + sequence: false, + shutdown: false, + activate: false, + terminate: false, + targetReset: false, +}; + +// `record` observes the app, but starting or stopping the recorder changes runner state, so a +// restart must not resend either; the runner is stricter than the registry here on purpose. +const RUNNER_STRICTER_THAN_REGISTRY: ReadonlySet = new Set([ + 'recordStart', + 'recordStop', +]); + +const RUNNER_COMMANDS = Object.keys(RUNNER_COMMAND_TRAITS); + +test('every runner command is paired with a registry request, internal, declared stricter, or the alert case', () => { + const declared = [ + ...Object.keys(REGISTRY_COUNTERPART), + ...Object.keys(RUNNER_INTERNAL), + ...RUNNER_STRICTER_THAN_REGISTRY, + 'alert', + ].sort(); + assert.deepEqual(declared, [...RUNNER_COMMANDS].sort()); +}); + +test('the runner read-only set equals the runner commands whose registry request observes the app', () => { + for (const [name, request] of Object.entries(REGISTRY_COUNTERPART)) { + const effect = resolveCommandRecordingEffect({ ...request, flags: {} } as never); + assert.equal( + isReadOnlyRunnerCommand({ command: name }), + effect === 'observes-app', + `${name} -> ${request.command}`, + ); + } + for (const [name, readOnly] of Object.entries(RUNNER_INTERNAL)) { + assert.equal(isReadOnlyRunnerCommand({ command: name }), readOnly, name); + } + for (const name of RUNNER_STRICTER_THAN_REGISTRY) { + assert.equal(isReadOnlyRunnerCommand({ command: name }), false, name); + assert.equal( + resolveCommandRecordingEffect({ command: 'record', flags: {} } as never), + 'observes-app', + ); + } +}); + +test('alert actions agree with the registry: only get observes', () => { + for (const action of [undefined, 'get', 'accept', 'dismiss']) { + const effect = resolveCommandRecordingEffect({ + command: 'alert', + positionals: action ? [action] : [], + flags: {}, + }); + assert.equal( + isReadOnlyRunnerCommand({ command: 'alert', action }), + effect === 'observes-app', + String(action), + ); + } +});