Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);
});
139 changes: 139 additions & 0 deletions packages/platform-apple/src/runner/__tests__/runner-response.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,139 @@
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-contract.ts';
import { parseRunnerResponse } from '../runner-session.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"';
// `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', () => {
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);
});
});

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);
});
});
9 changes: 6 additions & 3 deletions packages/platform-apple/src/runner/runner-adoption.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -137,8 +141,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;
}
Expand Down
36 changes: 17 additions & 19 deletions packages/platform-apple/src/runner/runner-command-recovery.ts
Original file line number Diff line number Diff line change
@@ -1,16 +1,18 @@
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 {
classifyRunnerReportedError,
decodeRunnerResponseBody,
isRunnerResponseOk,
readRunnerResponseData,
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';

type LifecycleResponsePayload = {
ok?: unknown;
data?: unknown;
};

type RunnerTransportRecovery =
| { type: 'recovered'; data: Record<string, unknown>; reason: string; lifecycleState?: string }
| { type: 'skipInvalidation'; error: AppError; reason: string; lifecycleState?: string }
Expand Down Expand Up @@ -340,20 +342,16 @@ function runnerStatusInFlightError(

function parseLifecycleResponseJson(value: unknown): Record<string, unknown> | 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<string, unknown>;
}
return {};
}

function parseLifecycleResponsePayload(value: string): LifecycleResponsePayload {
let payload: RunnerResponsePayload;
try {
const raw: unknown = JSON.parse(value);
if (raw && typeof raw === 'object') return raw as LifecycleResponsePayload;
} catch {}
return {};
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(
Expand Down
60 changes: 60 additions & 0 deletions packages/platform-apple/src/runner/runner-contract.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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<string, unknown> {
if (!payload.data || typeof payload.data !== 'object' || Array.isArray(payload.data)) return {};
return payload.data as Record<string, unknown>;
}

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,
Expand Down
Loading
Loading