From 81d6408f6e8738096b0272fbc27496389123d7d7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Pierzcha=C5=82a?= Date: Fri, 18 Sep 2026 10:29:55 +0200 Subject: [PATCH 1/2] refactor(ios-runner): decode every runner response body in one module MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The runner answers one HTTP envelope, but three places decoded it: the command path in runner-session.ts, the `status` recovery read in runner-command-recovery.ts, and the adoption `uptime` probe in runner-adoption.ts. Each had its own rule for what an unreadable body means, so a body one path refused could be read as an answer by another. `decodeRunnerResponseBody` in runner-response.ts is now the only place that turns a runner response body into a payload, and `isRunnerResponseOk` is the one acceptance rule: the runner's `ok` is a Swift `Bool`, so only the literal true answers. The command path and the recovery read accepted any truthy `ok`, so a retained body reading `ok: "true"` came back as a completed command's own result; both now refuse it, as the adoption probe already did. Each reader keeps its own failure handling — a truncated retained body is still an unrecoverable command, a truncated probe answer is still a failed probe — but none can disagree with the command path about what is readable. --- .../runner/__tests__/runner-adoption.test.ts | 21 ++++ .../__tests__/runner-command-recovery.test.ts | 52 +++++++++ .../runner/__tests__/runner-response.test.ts | 100 ++++++++++++++++++ .../runner/__tests__/runner-session.test.ts | 58 ++++++++++ .../src/runner/runner-adoption.ts | 4 +- .../src/runner/runner-command-recovery.ts | 31 +++--- .../src/runner/runner-response.ts | 64 +++++++++++ .../src/runner/runner-session.ts | 57 ++-------- 8 files changed, 320 insertions(+), 67 deletions(-) create mode 100644 packages/platform-apple/src/runner/__tests__/runner-response.test.ts create mode 100644 packages/platform-apple/src/runner/runner-response.ts diff --git a/packages/platform-apple/src/runner/__tests__/runner-adoption.test.ts b/packages/platform-apple/src/runner/__tests__/runner-adoption.test.ts index b9bb459d9f..9e34383806 100644 --- a/packages/platform-apple/src/runner/__tests__/runner-adoption.test.ts +++ b/packages/platform-apple/src/runner/__tests__/runner-adoption.test.ts @@ -289,6 +289,27 @@ test('adoption is skipped when the probe fails', async () => { expect(await tryAdoptRunnerSessionFromLease(simulator, {})).toBeNull(); }); +// #2662: the probe decodes through the command path's decoder, so a body that +// path would refuse cannot be read as an answer here either. Adoption must not +// re-stamp the lease for a runner it cannot read. +test('adoption is skipped when the uptime probe answers a truncated body', async () => { + const lease = writeStaleLease(); + mockIsProcessAlive.mockReturnValue(true); + mockSendRunnerCommandOnce.mockResolvedValue(new Response('{"ok":true,"data":{"uptime":')); + + expect(await tryAdoptRunnerSessionFromLease(simulator, {})).toBeNull(); + expect(readStaleRunnerLease(simulator.id)?.ownerToken).toBe(lease.ownerToken); +}); + +test('adoption is skipped when the uptime probe answers an ok that is not the boolean true', async () => { + const lease = writeStaleLease(); + mockIsProcessAlive.mockReturnValue(true); + mockSendRunnerCommandOnce.mockResolvedValue(new Response('{"ok":"true"}')); + + expect(await tryAdoptRunnerSessionFromLease(simulator, {})).toBeNull(); + expect(readStaleRunnerLease(simulator.id)?.ownerToken).toBe(lease.ownerToken); +}); + test('adoption is disabled by the kill switch', async () => { writeStaleLease(); mockIsProcessAlive.mockReturnValue(true); 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 7deacf9811..fbbf235cb0 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 @@ -204,3 +204,55 @@ test('a journaled RUNNER_WEDGED keeps its fatal code and stays unretriable', asy return true; }); }); + +/** + * #2662: the `status` read has no decoder of its own, so it accepts what the one + * decoder accepts. A stringly-typed `ok` used to be the seam: a private truthiness + * rule read this retained body as the command's own result. + */ +test('a retained response whose ok is not the boolean true is not recovered', async () => { + const { result, invalidate } = await runRecovery({ + script: [ + { + kind: 'ok', + data: { + lifecycleState: 'completed', + lifecycleResponseJson: '{"ok":"true","data":{"tapped":true}}', + }, + }, + ], + }); + + await assert.rejects(result, (error: unknown) => { + assert.ok(error instanceof AppError); + assert.equal(error.details?.recovery, 'completed_without_retained_response'); + return true; + }); + assert.equal(invalidate.mock.calls.length, 0); +}); + +/** + * A retained body cut off mid-write answers nothing. The session is kept — the + * runner is reachable, it proved that by serving `status` — but the truncated + * command result is not handed back. + */ +test('a truncated retained response is not recovered', async () => { + const { result, invalidate } = await runRecovery({ + script: [ + { + kind: 'ok', + data: { + lifecycleState: 'completed', + lifecycleResponseJson: '{"ok":true,"data":{"nodes":[{"label":"Sign In"', + }, + }, + ], + }); + + await assert.rejects(result, (error: unknown) => { + assert.ok(error instanceof AppError); + assert.equal(error.details?.recovery, 'completed_without_retained_response'); + return true; + }); + assert.equal(invalidate.mock.calls.length, 0); +}); diff --git a/packages/platform-apple/src/runner/__tests__/runner-response.test.ts b/packages/platform-apple/src/runner/__tests__/runner-response.test.ts new file mode 100644 index 0000000000..09f62e8ef5 --- /dev/null +++ b/packages/platform-apple/src/runner/__tests__/runner-response.test.ts @@ -0,0 +1,100 @@ +import assert from 'node:assert/strict'; +import { describe, test } from 'vitest'; +import { AppError } from '@agent-device/kernel/errors'; +import { + buildRunnerResponseError, + decodeRunnerResponseBody, + isRunnerResponseOk, + readRunnerResponseData, +} from '../runner-response.ts'; + +// A body cut off mid-write, the shape a runner that died while answering leaves behind. +const TRUNCATED_BODY = '{"ok":true,"data":{"nodes":[{"label":"Sign In"'; + +describe('decodeRunnerResponseBody', () => { + test('decodes the runner envelope', () => { + assert.deepEqual(decodeRunnerResponseBody('{"ok":true,"data":{"tapped":true}}'), { + ok: true, + data: { tapped: true }, + }); + }); + + test('rejects a truncated body instead of reading a reply out of it', () => { + assert.throws( + () => decodeRunnerResponseBody(TRUNCATED_BODY), + (error: unknown) => + error instanceof AppError && + error.code === 'COMMAND_FAILED' && + error.message === 'Invalid runner response' && + error.details?.text === TRUNCATED_BODY, + ); + }); + + test('never reads a JSON body that is not an envelope as an answer', () => { + for (const body of ['null', '42', '"ok"']) { + assert.equal(isRunnerResponseOk(decodeRunnerResponseBody(body)), false, body); + } + }); + + test('carries a bare-array body through unread rather than inventing an envelope', () => { + const payload = decodeRunnerResponseBody('[]'); + assert.equal(isRunnerResponseOk(payload), false); + assert.deepEqual(readRunnerResponseData(payload), {}); + }); +}); + +describe('isRunnerResponseOk', () => { + test('accepts only the boolean true the runner sends', () => { + assert.equal(isRunnerResponseOk({ ok: true }), true); + assert.equal(isRunnerResponseOk({ ok: false }), false); + assert.equal(isRunnerResponseOk({}), false); + // Stringly-typed truthiness would let a proxy or a wedged socket answer for the runner. + assert.equal(isRunnerResponseOk({ ok: 'true' }), false); + assert.equal(isRunnerResponseOk({ ok: 1 }), false); + }); +}); + +describe('readRunnerResponseData', () => { + test('reads only an object data payload', () => { + assert.deepEqual(readRunnerResponseData({ ok: true, data: { tapped: true } }), { + tapped: true, + }); + assert.deepEqual(readRunnerResponseData({ ok: true }), {}); + assert.deepEqual(readRunnerResponseData({ ok: true, data: null }), {}); + assert.deepEqual(readRunnerResponseData({ ok: true, data: [] }), {}); + assert.deepEqual(readRunnerResponseData({ ok: true, data: 'tapped' }), {}); + }); +}); + +describe('buildRunnerResponseError', () => { + test('carries the payload, the runner hint, and the classified code', () => { + const error = buildRunnerResponseError( + { + ok: false, + error: { + code: 'RUNNER_BUSY', + message: 'Main thread is busy', + hint: 'Retry after it drains', + }, + }, + '/tmp/runner.log', + ); + + assert.ok(error instanceof AppError); + assert.equal(error.code, 'COMMAND_FAILED'); + assert.equal(error.message, 'Main thread is busy'); + assert.equal(error.details?.hint, 'Retry after it drains'); + assert.equal(error.details?.logPath, '/tmp/runner.log'); + assert.equal(error.details?.retriable, true); + assert.deepEqual((error.details as { runner?: unknown }).runner, { + ok: false, + error: { code: 'RUNNER_BUSY', message: 'Main thread is busy', hint: 'Retry after it drains' }, + }); + }); + + test('keeps a missing runner message as a generic runner error', () => { + const error = buildRunnerResponseError({ ok: false, error: {} }); + assert.equal(error.message, 'Runner error'); + assert.equal(error.details?.hint, undefined); + }); +}); diff --git a/packages/platform-apple/src/runner/__tests__/runner-session.test.ts b/packages/platform-apple/src/runner/__tests__/runner-session.test.ts index ca8c8aafbc..52db1e44c7 100644 --- a/packages/platform-apple/src/runner/__tests__/runner-session.test.ts +++ b/packages/platform-apple/src/runner/__tests__/runner-session.test.ts @@ -340,6 +340,64 @@ test('runner session probes readiness before mutating commands', async () => { }); }); +test('runner session refuses a command response whose ok is not the boolean true', async () => { + // The runner's `ok` is a Swift `Bool`, so a stringly-typed one is not an answer + // the session may mark itself ready on (#2662). + const session = makeRunnerSession({ ready: false }); + mockWaitForRunner.mockResolvedValueOnce(runnerResponse({ uptimeMs: 42 })); + mockSendRunnerCommandOnce.mockResolvedValueOnce( + new Response('{"ok":"true","data":{"tapped":true}}'), + ); + + await assert.rejects( + () => + executeRunnerCommandWithSession( + IOS_SIMULATOR, + session, + { command: 'tap', x: 120, y: 240, appBundleId: 'com.example.demo' }, + '/tmp/runner.log', + 30_000, + ), + (error: unknown) => { + assert.ok(error instanceof AppError); + assert.equal(error.code, 'COMMAND_FAILED'); + assert.deepEqual(error.details?.runner, { ok: 'true', data: { tapped: true } }); + return true; + }, + ); + + // Refused is not the same as served: a body that is not an answer cannot be + // recorded as the healthy mutation that lets the next command skip preflight. + assert.equal(session.lastHealthyMutation, undefined); +}); + +test('runner session refuses a command response body that is not readable JSON', async () => { + const truncatedBody = '{"ok":true,"data":{"nodes":[{"label":"Sign In"'; + const session = makeRunnerSession({ ready: false }); + mockWaitForRunner.mockResolvedValueOnce(runnerResponse({ uptimeMs: 42 })); + mockSendRunnerCommandOnce.mockResolvedValueOnce(new Response(truncatedBody)); + + await assert.rejects( + () => + executeRunnerCommandWithSession( + IOS_SIMULATOR, + session, + { command: 'tap', x: 120, y: 240, appBundleId: 'com.example.demo' }, + '/tmp/runner.log', + 30_000, + ), + (error: unknown) => { + assert.ok(error instanceof AppError); + assert.equal(error.code, 'COMMAND_FAILED'); + assert.equal(error.details?.text, truncatedBody); + assert.equal(error.details?.runner, undefined); + return true; + }, + ); + + assert.equal(session.lastHealthyMutation, undefined); +}); + test('runner session emits reason diagnostics when readiness preflight is used', async () => { const session = makeRunnerSession({ ready: false }); mockWaitForRunner.mockResolvedValueOnce(runnerResponse({ uptimeMs: 42 })); diff --git a/packages/platform-apple/src/runner/runner-adoption.ts b/packages/platform-apple/src/runner/runner-adoption.ts index 3c2819def1..9d0d6bfb01 100644 --- a/packages/platform-apple/src/runner/runner-adoption.ts +++ b/packages/platform-apple/src/runner/runner-adoption.ts @@ -29,6 +29,7 @@ import { type RunnerProcessHandle, type RunnerSession, } from './runner-session-types.ts'; +import { decodeRunnerResponseBody, isRunnerResponseOk } from './runner-response.ts'; // A healthy localhost runner answers uptime in tens of milliseconds and a dead // port refuses immediately; the timeout only bounds the wedged-runner case, @@ -137,8 +138,7 @@ async function probeRunnerAnswersUptime(device: DeviceInfo, port: number): Promi withRunnerCommandId({ command: 'uptime' }), RUNNER_ADOPTION_PROBE_TIMEOUT_MS, ); - const payload = JSON.parse(await response.text()) as { ok?: unknown }; - return payload?.ok === true; + return isRunnerResponseOk(decodeRunnerResponseBody(await response.text())); } catch { return false; } diff --git a/packages/platform-apple/src/runner/runner-command-recovery.ts b/packages/platform-apple/src/runner/runner-command-recovery.ts index dd25b720ba..035044bd64 100644 --- a/packages/platform-apple/src/runner/runner-command-recovery.ts +++ b/packages/platform-apple/src/runner/runner-command-recovery.ts @@ -3,14 +3,14 @@ import type { DeviceInfo } from '@agent-device/kernel/device'; import { emitDiagnostic } from './host.ts'; import { classifyRunnerReportedError, type RunnerCommand } from './runner-contract.ts'; import { isReadOnlyRunnerCommand } from './runner-command-traits.ts'; +import { + decodeRunnerResponseBody, + isRunnerResponseOk, + readRunnerResponseData, +} from './runner-response.ts'; import type { AppleRunnerCommandOptions } from './runner-provider.ts'; import { executeRunnerCommandWithSession, type RunnerSession } from './runner-session.ts'; -type LifecycleResponsePayload = { - ok?: unknown; - data?: unknown; -}; - type RunnerTransportRecovery = | { type: 'recovered'; data: Record; reason: string; lifecycleState?: string } | { type: 'skipInvalidation'; error: AppError; reason: string; lifecycleState?: string } @@ -340,20 +340,15 @@ function runnerStatusInFlightError( function parseLifecycleResponseJson(value: unknown): Record | undefined { if (typeof value !== 'string' || value.trim().length === 0) return undefined; - const parsed = parseLifecycleResponsePayload(value); - if (!parsed.ok) return undefined; - if (parsed.data && typeof parsed.data === 'object' && !Array.isArray(parsed.data)) { - return parsed.data as Record; - } - return {}; -} - -function parseLifecycleResponsePayload(value: string): LifecycleResponsePayload { try { - const raw: unknown = JSON.parse(value); - if (raw && typeof raw === 'object') return raw as LifecycleResponsePayload; - } catch {} - return {}; + const payload = decodeRunnerResponseBody(value); + return isRunnerResponseOk(payload) ? readRunnerResponseData(payload) : undefined; + } catch { + // A retained body the one decoder refuses is not a recoverable result; the + // caller keeps the session and reports the retained response as unreadable + // instead of returning a truncated command result (#2662). + return undefined; + } } function completedWithoutRetainedResponseHint( diff --git a/packages/platform-apple/src/runner/runner-response.ts b/packages/platform-apple/src/runner/runner-response.ts new file mode 100644 index 0000000000..aa0c6fe9b2 --- /dev/null +++ b/packages/platform-apple/src/runner/runner-response.ts @@ -0,0 +1,64 @@ +/** + * The one decoding of an Apple runner response body. Every reader of the + * runner's HTTP envelope — command responses, the lifecycle `status` read, and + * the adoption `uptime` probe — decodes here, so no caller can keep a private + * rule about what is readable and what is an answer (#2662). + */ +import { AppError } from '@agent-device/kernel/errors'; +import { classifyRunnerReportedError } from './runner-contract.ts'; + +export type RunnerResponsePayload = { + ok?: unknown; + error?: { code?: unknown; message?: unknown; hint?: unknown }; + data?: unknown; +}; + +/** + * A body that is not JSON at all is transport-shaped failure, not an empty + * response: a runner that died mid-write must not be read as having answered. + */ +export function decodeRunnerResponseBody(text: string): RunnerResponsePayload { + let parsed: unknown; + try { + parsed = JSON.parse(text); + } catch { + throw new AppError('COMMAND_FAILED', 'Invalid runner response', { text }); + } + return parsed && typeof parsed === 'object' ? (parsed as RunnerResponsePayload) : {}; +} + +/** The runner's `ok` is a Swift `Bool`, so only the literal `true` is an answer. */ +export function isRunnerResponseOk(payload: RunnerResponsePayload): boolean { + return payload.ok === true; +} + +export function readRunnerResponseData(payload: RunnerResponsePayload): Record { + if (!payload.data || typeof payload.data !== 'object' || Array.isArray(payload.data)) return {}; + return payload.data as Record; +} + +export function buildRunnerResponseError( + payload: RunnerResponsePayload, + logPath?: string, +): AppError { + const runnerErrorCode = readRunnerErrorCode(payload.error?.code); + const errorMessage = + typeof payload.error?.message === 'string' ? payload.error.message : undefined; + const hint = typeof payload.error?.hint === 'string' ? payload.error.hint : undefined; + const classification = classifyRunnerReportedError(runnerErrorCode); + return new AppError(classification.code, errorMessage ?? 'Runner error', { + runner: payload, + ...classification.details, + xcodebuild: { + exitCode: 1, + stdout: '', + stderr: '', + }, + hint, + logPath, + }); +} + +function readRunnerErrorCode(rawCode: unknown): string | undefined { + return typeof rawCode === 'string' && rawCode.trim().length > 0 ? rawCode.trim() : undefined; +} diff --git a/packages/platform-apple/src/runner/runner-session.ts b/packages/platform-apple/src/runner/runner-session.ts index 6c77ca4a8a..bdafdd0ee6 100644 --- a/packages/platform-apple/src/runner/runner-session.ts +++ b/packages/platform-apple/src/runner/runner-session.ts @@ -28,7 +28,6 @@ import { type RunnerPhaseBudget, } from './runner-xctestrun.ts'; import { - classifyRunnerReportedError, resolveRunnerRequestSignal, withRunnerCommandId, type RunnerCommand, @@ -61,6 +60,12 @@ import { type RunnerDisposalOptions, } from './runner-disposal.ts'; import { enrichRunnerFailureFromLog } from './runner-failure-diagnostics.ts'; +import { + buildRunnerResponseError, + decodeRunnerResponseBody, + isRunnerResponseOk, + readRunnerResponseData, +} from './runner-response.ts'; import { buildRunnerSessionId, normalizeRunnerStartupTimeoutMs, @@ -946,62 +951,20 @@ function emitRunnerReadinessPreflightSkipped( }); } -type RunnerResponsePayload = { - ok?: unknown; - error?: { code?: unknown; message?: unknown; hint?: unknown }; - data?: unknown; -}; - export async function parseRunnerResponse( response: Response, session: Pick, logPath?: string, ): Promise> { - const json = parseRunnerResponsePayload(await response.text()); - if (!json.ok) { + const payload = decodeRunnerResponseBody(await response.text()); + if (!isRunnerResponseOk(payload)) { throw await enrichRunnerFailureFromLog({ - error: buildRunnerResponseError(json, logPath), + error: buildRunnerResponseError(payload, logPath), logPath, }); } session.ready = true; - return readRunnerResponseData(json); -} - -function parseRunnerResponsePayload(text: string): RunnerResponsePayload { - try { - const parsed: unknown = JSON.parse(text); - return parsed && typeof parsed === 'object' ? (parsed as RunnerResponsePayload) : {}; - } catch { - throw new AppError('COMMAND_FAILED', 'Invalid runner response', { text }); - } -} - -function buildRunnerResponseError(json: RunnerResponsePayload, logPath?: string): AppError { - const runnerErrorCode = readRunnerErrorCode(json.error?.code); - const errorMessage = typeof json.error?.message === 'string' ? json.error.message : undefined; - const hint = typeof json.error?.hint === 'string' ? json.error.hint : undefined; - const classification = classifyRunnerReportedError(runnerErrorCode); - return new AppError(classification.code, errorMessage ?? 'Runner error', { - runner: json, - ...classification.details, - xcodebuild: { - exitCode: 1, - stdout: '', - stderr: '', - }, - hint, - logPath, - }); -} - -function readRunnerErrorCode(rawCode: unknown): string | undefined { - return typeof rawCode === 'string' && rawCode.trim().length > 0 ? rawCode.trim() : undefined; -} - -function readRunnerResponseData(json: RunnerResponsePayload): Record { - if (!json.data || typeof json.data !== 'object' || Array.isArray(json.data)) return {}; - const data = json.data as Record; + const data = readRunnerResponseData(payload); emitRunnerResponseDiagnostics(data); return data; } From 0c1d01f6d3bd0c641e0a67de2357825d185c51e3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Pierzcha=C5=82a?= Date: Fri, 18 Sep 2026 13:47:06 +0200 Subject: [PATCH 2/2] refactor(ios-runner): give the response decoder a home the runner closure already loads The Coverage job caught the new module: runner-session.ts sits in the eager closure of the app-lifecycle, doctor and runner-operations facades, so runner-response.ts made those closures evaluate one module more than the merge-base did. The envelope belongs beside the rest of the runner wire contract, which every one of those entries already evaluates and every reader already imports, so the decode helpers move into runner-contract.ts and the separate file goes away. No behavior moves. The two command-path refusal tests leave runner-session.test.ts, which is past the test-file tripwire and may not grow, and call parseRunnerResponse from the response suite instead. --- .../runner/__tests__/runner-response.test.ts | 43 ++++++++++++- .../runner/__tests__/runner-session.test.ts | 58 ----------------- .../src/runner/runner-adoption.ts | 7 +- .../src/runner/runner-command-recovery.ts | 13 ++-- .../src/runner/runner-contract.ts | 60 +++++++++++++++++ .../src/runner/runner-response.ts | 64 ------------------- .../src/runner/runner-session.ts | 10 ++- 7 files changed, 118 insertions(+), 137 deletions(-) delete mode 100644 packages/platform-apple/src/runner/runner-response.ts diff --git a/packages/platform-apple/src/runner/__tests__/runner-response.test.ts b/packages/platform-apple/src/runner/__tests__/runner-response.test.ts index 09f62e8ef5..b0e4358a31 100644 --- a/packages/platform-apple/src/runner/__tests__/runner-response.test.ts +++ b/packages/platform-apple/src/runner/__tests__/runner-response.test.ts @@ -6,10 +6,13 @@ import { decodeRunnerResponseBody, isRunnerResponseOk, readRunnerResponseData, -} from '../runner-response.ts'; +} from '../runner-contract.ts'; +import { parseRunnerResponse } from '../runner-session.ts'; -// A body cut off mid-write, the shape a runner that died while answering leaves behind. +// A body cut off mid-write: the shape a runner that died while answering leaves behind. const TRUNCATED_BODY = '{"ok":true,"data":{"nodes":[{"label":"Sign In"'; +// `ok` is a Swift `Bool`, so this is what a private truthiness rule used to accept as an answer. +const STRINGLY_TYPED_BODY = '{"ok":"true","data":{"tapped":true}}'; describe('decodeRunnerResponseBody', () => { test('decodes the runner envelope', () => { @@ -98,3 +101,39 @@ describe('buildRunnerResponseError', () => { assert.equal(error.details?.hint, undefined); }); }); + +describe('parseRunnerResponse', () => { + test('refuses a body whose ok is not the boolean true', async () => { + const session = { ready: false }; + + await assert.rejects( + () => parseRunnerResponse(new Response(STRINGLY_TYPED_BODY), session), + (error: unknown) => { + assert.ok(error instanceof AppError); + assert.equal(error.code, 'COMMAND_FAILED'); + assert.deepEqual(error.details?.runner, { ok: 'true', data: { tapped: true } }); + return true; + }, + ); + + assert.equal(session.ready, false); + }); + + test('refuses a body that is not readable JSON', async () => { + const session = { ready: false }; + + await assert.rejects( + () => parseRunnerResponse(new Response(TRUNCATED_BODY), session), + (error: unknown) => { + assert.ok(error instanceof AppError); + assert.equal(error.code, 'COMMAND_FAILED'); + assert.equal(error.details?.text, TRUNCATED_BODY); + // Transport-shaped: no `runner` detail, so the session keeps its recency bets (#2552). + assert.equal(error.details?.runner, undefined); + return true; + }, + ); + + assert.equal(session.ready, false); + }); +}); diff --git a/packages/platform-apple/src/runner/__tests__/runner-session.test.ts b/packages/platform-apple/src/runner/__tests__/runner-session.test.ts index 52db1e44c7..ca8c8aafbc 100644 --- a/packages/platform-apple/src/runner/__tests__/runner-session.test.ts +++ b/packages/platform-apple/src/runner/__tests__/runner-session.test.ts @@ -340,64 +340,6 @@ test('runner session probes readiness before mutating commands', async () => { }); }); -test('runner session refuses a command response whose ok is not the boolean true', async () => { - // The runner's `ok` is a Swift `Bool`, so a stringly-typed one is not an answer - // the session may mark itself ready on (#2662). - const session = makeRunnerSession({ ready: false }); - mockWaitForRunner.mockResolvedValueOnce(runnerResponse({ uptimeMs: 42 })); - mockSendRunnerCommandOnce.mockResolvedValueOnce( - new Response('{"ok":"true","data":{"tapped":true}}'), - ); - - await assert.rejects( - () => - executeRunnerCommandWithSession( - IOS_SIMULATOR, - session, - { command: 'tap', x: 120, y: 240, appBundleId: 'com.example.demo' }, - '/tmp/runner.log', - 30_000, - ), - (error: unknown) => { - assert.ok(error instanceof AppError); - assert.equal(error.code, 'COMMAND_FAILED'); - assert.deepEqual(error.details?.runner, { ok: 'true', data: { tapped: true } }); - return true; - }, - ); - - // Refused is not the same as served: a body that is not an answer cannot be - // recorded as the healthy mutation that lets the next command skip preflight. - assert.equal(session.lastHealthyMutation, undefined); -}); - -test('runner session refuses a command response body that is not readable JSON', async () => { - const truncatedBody = '{"ok":true,"data":{"nodes":[{"label":"Sign In"'; - const session = makeRunnerSession({ ready: false }); - mockWaitForRunner.mockResolvedValueOnce(runnerResponse({ uptimeMs: 42 })); - mockSendRunnerCommandOnce.mockResolvedValueOnce(new Response(truncatedBody)); - - await assert.rejects( - () => - executeRunnerCommandWithSession( - IOS_SIMULATOR, - session, - { command: 'tap', x: 120, y: 240, appBundleId: 'com.example.demo' }, - '/tmp/runner.log', - 30_000, - ), - (error: unknown) => { - assert.ok(error instanceof AppError); - assert.equal(error.code, 'COMMAND_FAILED'); - assert.equal(error.details?.text, truncatedBody); - assert.equal(error.details?.runner, undefined); - return true; - }, - ); - - assert.equal(session.lastHealthyMutation, undefined); -}); - test('runner session emits reason diagnostics when readiness preflight is used', async () => { const session = makeRunnerSession({ ready: false }); mockWaitForRunner.mockResolvedValueOnce(runnerResponse({ uptimeMs: 42 })); diff --git a/packages/platform-apple/src/runner/runner-adoption.ts b/packages/platform-apple/src/runner/runner-adoption.ts index 9d0d6bfb01..0b1cf03a83 100644 --- a/packages/platform-apple/src/runner/runner-adoption.ts +++ b/packages/platform-apple/src/runner/runner-adoption.ts @@ -9,7 +9,11 @@ import { import type { DeviceInfo } from '@agent-device/kernel/device'; import { isRequestCanceledError } from '@agent-device/kernel/errors'; import { sendRunnerCommandOnce } from './runner-transport.ts'; -import { withRunnerCommandId } from './runner-contract.ts'; +import { + decodeRunnerResponseBody, + isRunnerResponseOk, + withRunnerCommandId, +} from './runner-contract.ts'; import { buildRunnerLease, readStaleRunnerLease, @@ -29,7 +33,6 @@ import { type RunnerProcessHandle, type RunnerSession, } from './runner-session-types.ts'; -import { decodeRunnerResponseBody, isRunnerResponseOk } from './runner-response.ts'; // A healthy localhost runner answers uptime in tens of milliseconds and a dead // port refuses immediately; the timeout only bounds the wedged-runner case, diff --git a/packages/platform-apple/src/runner/runner-command-recovery.ts b/packages/platform-apple/src/runner/runner-command-recovery.ts index 035044bd64..6345e0fd68 100644 --- a/packages/platform-apple/src/runner/runner-command-recovery.ts +++ b/packages/platform-apple/src/runner/runner-command-recovery.ts @@ -1,13 +1,15 @@ import { AppError } from '@agent-device/kernel/errors'; import type { DeviceInfo } from '@agent-device/kernel/device'; import { emitDiagnostic } from './host.ts'; -import { classifyRunnerReportedError, type RunnerCommand } from './runner-contract.ts'; -import { isReadOnlyRunnerCommand } from './runner-command-traits.ts'; import { + classifyRunnerReportedError, decodeRunnerResponseBody, isRunnerResponseOk, readRunnerResponseData, -} from './runner-response.ts'; + type RunnerCommand, + type RunnerResponsePayload, +} from './runner-contract.ts'; +import { isReadOnlyRunnerCommand } from './runner-command-traits.ts'; import type { AppleRunnerCommandOptions } from './runner-provider.ts'; import { executeRunnerCommandWithSession, type RunnerSession } from './runner-session.ts'; @@ -340,15 +342,16 @@ function runnerStatusInFlightError( function parseLifecycleResponseJson(value: unknown): Record | undefined { if (typeof value !== 'string' || value.trim().length === 0) return undefined; + let payload: RunnerResponsePayload; try { - const payload = decodeRunnerResponseBody(value); - return isRunnerResponseOk(payload) ? readRunnerResponseData(payload) : undefined; + payload = decodeRunnerResponseBody(value); } catch { // A retained body the one decoder refuses is not a recoverable result; the // caller keeps the session and reports the retained response as unreadable // instead of returning a truncated command result (#2662). return undefined; } + return isRunnerResponseOk(payload) ? readRunnerResponseData(payload) : undefined; } function completedWithoutRetainedResponseHint( diff --git a/packages/platform-apple/src/runner/runner-contract.ts b/packages/platform-apple/src/runner/runner-contract.ts index d670b9854f..7835c38662 100644 --- a/packages/platform-apple/src/runner/runner-contract.ts +++ b/packages/platform-apple/src/runner/runner-contract.ts @@ -468,6 +468,66 @@ export function classifyRunnerReportedError( }); } +export type RunnerResponsePayload = { + ok?: unknown; + error?: { code?: unknown; message?: unknown; hint?: unknown }; + data?: unknown; +}; + +/** + * The one decoding of a runner response body (#2662). The envelope arrives at three readers — a + * command's own response, the lifecycle journal a status probe reads back after the transport + * response was lost, and the adoption `uptime` probe — and all three must agree on what is + * readable, or a body one of them refuses becomes an answer for another. A body that is not JSON + * at all is transport-shaped failure: a runner that died mid-write must not be read as having + * answered. + */ +export function decodeRunnerResponseBody(text: string): RunnerResponsePayload { + let parsed: unknown; + try { + parsed = JSON.parse(text); + } catch { + throw new AppError('COMMAND_FAILED', 'Invalid runner response', { text }); + } + return parsed && typeof parsed === 'object' ? (parsed as RunnerResponsePayload) : {}; +} + +/** The runner's `ok` is a Swift `Bool`, so only the literal `true` is an answer. */ +export function isRunnerResponseOk(payload: RunnerResponsePayload): boolean { + return payload.ok === true; +} + +export function readRunnerResponseData(payload: RunnerResponsePayload): Record { + if (!payload.data || typeof payload.data !== 'object' || Array.isArray(payload.data)) return {}; + return payload.data as Record; +} + +export function buildRunnerResponseError( + payload: RunnerResponsePayload, + logPath?: string, +): AppError { + const runnerErrorCode = readRunnerErrorCode(payload.error?.code); + const errorMessage = + typeof payload.error?.message === 'string' ? payload.error.message : undefined; + const hint = typeof payload.error?.hint === 'string' ? payload.error.hint : undefined; + const classification = classifyRunnerReportedError(runnerErrorCode); + return new AppError(classification.code, errorMessage ?? 'Runner error', { + runner: payload, + ...classification.details, + xcodebuild: { + exitCode: 1, + stdout: '', + stderr: '', + }, + hint, + logPath, + }); +} + +function readRunnerErrorCode(rawCode: unknown): string | undefined { + return typeof rawCode === 'string' && rawCode.trim().length > 0 ? rawCode.trim() : undefined; +} + export function resolveRunnerEarlyExitHint( message: string, stdout: string, diff --git a/packages/platform-apple/src/runner/runner-response.ts b/packages/platform-apple/src/runner/runner-response.ts deleted file mode 100644 index aa0c6fe9b2..0000000000 --- a/packages/platform-apple/src/runner/runner-response.ts +++ /dev/null @@ -1,64 +0,0 @@ -/** - * The one decoding of an Apple runner response body. Every reader of the - * runner's HTTP envelope — command responses, the lifecycle `status` read, and - * the adoption `uptime` probe — decodes here, so no caller can keep a private - * rule about what is readable and what is an answer (#2662). - */ -import { AppError } from '@agent-device/kernel/errors'; -import { classifyRunnerReportedError } from './runner-contract.ts'; - -export type RunnerResponsePayload = { - ok?: unknown; - error?: { code?: unknown; message?: unknown; hint?: unknown }; - data?: unknown; -}; - -/** - * A body that is not JSON at all is transport-shaped failure, not an empty - * response: a runner that died mid-write must not be read as having answered. - */ -export function decodeRunnerResponseBody(text: string): RunnerResponsePayload { - let parsed: unknown; - try { - parsed = JSON.parse(text); - } catch { - throw new AppError('COMMAND_FAILED', 'Invalid runner response', { text }); - } - return parsed && typeof parsed === 'object' ? (parsed as RunnerResponsePayload) : {}; -} - -/** The runner's `ok` is a Swift `Bool`, so only the literal `true` is an answer. */ -export function isRunnerResponseOk(payload: RunnerResponsePayload): boolean { - return payload.ok === true; -} - -export function readRunnerResponseData(payload: RunnerResponsePayload): Record { - if (!payload.data || typeof payload.data !== 'object' || Array.isArray(payload.data)) return {}; - return payload.data as Record; -} - -export function buildRunnerResponseError( - payload: RunnerResponsePayload, - logPath?: string, -): AppError { - const runnerErrorCode = readRunnerErrorCode(payload.error?.code); - const errorMessage = - typeof payload.error?.message === 'string' ? payload.error.message : undefined; - const hint = typeof payload.error?.hint === 'string' ? payload.error.hint : undefined; - const classification = classifyRunnerReportedError(runnerErrorCode); - return new AppError(classification.code, errorMessage ?? 'Runner error', { - runner: payload, - ...classification.details, - xcodebuild: { - exitCode: 1, - stdout: '', - stderr: '', - }, - hint, - logPath, - }); -} - -function readRunnerErrorCode(rawCode: unknown): string | undefined { - return typeof rawCode === 'string' && rawCode.trim().length > 0 ? rawCode.trim() : undefined; -} diff --git a/packages/platform-apple/src/runner/runner-session.ts b/packages/platform-apple/src/runner/runner-session.ts index bdafdd0ee6..8b47a5bfb4 100644 --- a/packages/platform-apple/src/runner/runner-session.ts +++ b/packages/platform-apple/src/runner/runner-session.ts @@ -28,6 +28,10 @@ import { type RunnerPhaseBudget, } from './runner-xctestrun.ts'; import { + buildRunnerResponseError, + decodeRunnerResponseBody, + isRunnerResponseOk, + readRunnerResponseData, resolveRunnerRequestSignal, withRunnerCommandId, type RunnerCommand, @@ -60,12 +64,6 @@ import { type RunnerDisposalOptions, } from './runner-disposal.ts'; import { enrichRunnerFailureFromLog } from './runner-failure-diagnostics.ts'; -import { - buildRunnerResponseError, - decodeRunnerResponseBody, - isRunnerResponseOk, - readRunnerResponseData, -} from './runner-response.ts'; import { buildRunnerSessionId, normalizeRunnerStartupTimeoutMs,