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
@@ -1,4 +1,5 @@
import assert from 'node:assert/strict';
import { RunnerCommandAccounting } from '../runner-session-types.ts';
import { afterEach, test, vi } from 'vitest';
import { AppError } from '@agent-device/kernel/errors';
import { IOS_SIMULATOR } from './device-fixtures.ts';
Expand Down Expand Up @@ -35,8 +36,7 @@ function makeRunnerSession(port: number): RunnerSession {
testPromise: new Promise<ExecResult>(() => {}),
child: { pid: process.pid, exitCode: null },
state: 'ready',
inFlightCommands: 0,
hasAbandonedCommands: false,
commandCharges: new RunnerCommandAccounting(),
};
}

Expand Down
Original file line number Diff line number Diff line change
@@ -1,11 +1,14 @@
import { beforeEach, test, vi } from 'vitest';
import assert from 'node:assert/strict';
import { IOS_SIMULATOR } from './device-fixtures.ts';
import { createTestRequestCancellation, runnerConnectFailure } from './runner-session-fixtures.ts';
import {
createTestRequestCancellation,
makeRunnerSession,
runnerConnectFailure,
} from './runner-session-fixtures.ts';
import { AppError } from '@agent-device/kernel/errors';
import { Deadline } from '../host.ts';
import { appleRunnerTestHost } from '../test-host.ts';
import type { RunnerSession } from '../runner-session-types.ts';

const {
mockEnsureRunnerSession,
Expand Down Expand Up @@ -1090,21 +1093,6 @@ function assertDiagnosticDecision(expected: {
);
}

function makeRunnerSession(overrides: Partial<RunnerSession> = {}): RunnerSession {
return {
sessionId: `session-${overrides.port ?? 8100}`,
device: IOS_SIMULATOR,
deviceId: IOS_SIMULATOR.id,
port: 8100,
xctestrunPath: '/tmp/runner.xctestrun',
jsonPath: '/tmp/runner.json',
testPromise: Promise.resolve({ exitCode: 0, stdout: '', stderr: '' }),
child: { pid: 1234, exitCode: null },
state: 'ready',
...overrides,
} as RunnerSession;
}

function makeRunnerArtifact(
overrides: Partial<RunnerXctestrunArtifact> = {},
): RunnerXctestrunArtifact {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import { afterEach, beforeEach, expect, test, vi } from 'vitest';
import { IOS_SIMULATOR, MACOS_DEVICE, TVOS_SIMULATOR } from './device-fixtures.ts';
import type { ExecResult } from '@agent-device/host-kit/command';
import type { RunnerSession } from '../runner-session-types.ts';
import { RunnerCommandAccounting, type RunnerSession } from '../runner-session-types.ts';
import { appleRunnerTestHost } from '../test-host.ts';
import { makeRunnerLease } from './runner-session-fixtures.ts';
import { mkdtempForTestSync } from './tmp-dir.ts';
Expand Down Expand Up @@ -257,8 +257,7 @@ function makeRunnerSession(
testPromise,
child: { pid: 42, exitCode: null },
state: 'ready',
inFlightCommands: 0,
hasAbandonedCommands: false,
commandCharges: new RunnerCommandAccounting(),
...overrides,
};
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import { resetAllProcessMemosForTests } from '@agent-device/kernel/ttl-memo';
import type { ExecBackgroundResult } from '@agent-device/host-kit/command';
import { buildRunnerEarlyExitError } from '../runner-startup-transport.ts';
import { readRunnerLogTail } from '../runner-io.ts';
import type { RunnerSession } from '../runner-session-types.ts';
import { RunnerCommandAccounting, type RunnerSession } from '../runner-session-types.ts';
import { mkdtempForTestSync } from './tmp-dir.ts';
import { STUBBED_APPLE_TOOLCHAIN, stubAppleToolchainProbes } from './apple-toolchain-fixtures.ts';
import {
Expand Down Expand Up @@ -52,8 +52,7 @@ function sessionFailingWith(
child: { pid: 4242, exitCode: 1 } as ExecBackgroundResult['child'],
state: 'starting',
startupDeviceStates,
inFlightCommands: 0,
hasAbandonedCommands: false,
commandCharges: new RunnerCommandAccounting(),
};
}

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
import { test } from 'vitest';
import type { RunnerCommand } from '../runner-contract.ts';
import { isRunnerReadinessProbeCommand, RUNNER_COMMAND_TRAITS } from '../runner-command-traits.ts';
import { readSwiftInlineCommands } from './runner-swift-settlement-fixtures.ts';

/**
* The runner serves `status` and `uptime` inline — outside its journal and off the serial command
* queue — which is why the daemon charges no exchange for them: an inline reply proves the runner is
* reachable and nothing about queued work, so a charge for one could only ever be paid by the wrong
* exchange (#2965). The `readinessProbe` trait is how the daemon names that set, and this tie is what
* keeps the two sides one claim: add an inline arm in Swift that the trait missed and that reply would
* discharge an unrelated command's charge; drop one and a genuinely queued command would go uncharged,
* so a shutdown could hand off a runner with work on its queue.
*
* The Swift half is read from the runner's own switch rather than restated here, so the check cannot
* agree with a stale copy of the answer.
*/

function readinessProbeCommands(): string[] {
return (Object.keys(RUNNER_COMMAND_TRAITS) as RunnerCommand['command'][])
.filter((command) => isRunnerReadinessProbeCommand({ command }))
.sort();
}

test('the readinessProbe trait names exactly the commands the runner serves inline', () => {
const inline = readSwiftInlineCommands();

if (inline.length === 0) {
throw new Error(
'the runner serves no command inline, so the daemon must stop routing any reply as an inline answer',
);
}
for (const command of inline) {
if (!readinessProbeCommands().includes(command)) {
throw new Error(`the runner answers "${command}" inline but the trait does not name it`);
}
}
for (const command of readinessProbeCommands()) {
if (!inline.includes(command)) {
throw new Error(`the trait calls "${command}" a probe but the runner queues it`);
}
}
});
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,10 @@ import {
requireRunnerPhaseRemainingMs,
resolveExpectedRunnerCacheMetadata,
} from '../runner-cache-metadata.ts';
import { captureDiagnostics } from './runner-session-fixtures.ts';
import { startFakeRunnerServer, type FakeRunnerServer } from './fake-runner-server.ts';
import { resolveRunnerDetachDecision, RunnerCommandAccounting } from '../runner-session-types.ts';
import { requireLifecycleSettlementRows } from './runner-swift-settlement-fixtures.ts';

/**
* The wiring regression the recovery suite cannot provide (#1644 review P1):
Expand Down Expand Up @@ -104,8 +107,7 @@ function makeRunnerSession(port: number, sessionId = `wiring:${port}`): RunnerSe
testPromise: new Promise<ExecResult>(() => {}),
child: { pid: process.pid, exitCode: null },
state: 'ready',
inFlightCommands: 0,
hasAbandonedCommands: false,
commandCharges: new RunnerCommandAccounting(),
};
return session;
}
Expand Down Expand Up @@ -158,6 +160,186 @@ test.each(Object.values(LOST_RESPONSE_MUTATION_ROWS))(
},
);

// #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
// from the runner journal's own state list, so a state the runner gains is a missing row rather than an
// unruled verdict.
const LOST_RESPONSE_HANDOFF_ROWS = requireLifecycleSettlementRows({
completed: true,
failed: true,
accepted: false,
started: false,
notAccepted: false,
});

test.each(LOST_RESPONSE_HANDOFF_ROWS)(
'a lost mutation and status $lifecycleState leaves handoff $settlesCharge',
async ({ lifecycleState, settlesCharge }) => {
server = await startFakeRunnerServer({
tap: [{ kind: 'hangUp' }],
status: [{ kind: 'ok', data: { lifecycleState } }],
});
const session = seedSession(server.port);

// The mutation is refused, never replayed, whatever the terminal verdict — recovery keeps the
// session and reports the command's state to the caller.
await expect(
runAppleRunnerCommand(IOS_SIMULATOR, { command: 'tap', x: 5, y: 5 }),
).rejects.toThrow(AppError);
assert.equal(
server.requests.filter((request) => request.command === 'tap').length,
1,
'the mutation is sent once',
);
// `notAccepted` is the journal's "never seen this id", which this daemon cannot read as a safe
// terminal state: it keeps the invalidation path and its charge, while every state the journal
// actually reports for the command keeps the session.
assert.equal(
invalidateRunnerSessionMock.mock.calls.length,
lifecycleState === 'notAccepted' ? 1 : 0,
`${lifecycleState} must ${lifecycleState === 'notAccepted' ? '' : 'not '}invalidate the session`,
);
assert.equal(resolveRunnerDetachDecision(session).detach, settlesCharge);
},
);

test('a lost mutation answered by an inline status probe keeps handoff refused', async () => {
// `started` with the runner reporting itself not busy is the legitimate state this issue is about:
// the probe is served off the XCTest channel while the abandoned mutation keeps running.
server = await startFakeRunnerServer({
tap: [{ kind: 'hangUp' }],
status: [{ kind: 'ok', data: { lifecycleState: 'started', runnerMainThreadBusy: false } }],
});
const session = seedSession(server.port);

await expect(
runAppleRunnerCommand(IOS_SIMULATOR, { command: 'tap', x: 5, y: 5 }),
).rejects.toThrow(AppError);
assert.equal(session.runnerMainThreadBusy, false);
assert.deepEqual(resolveRunnerDetachDecision(session), {
detach: false,
reason: 'command_in_flight',
});
});

test('an answered uptime health probe keeps handoff refused over an abandoned mutation', async () => {
// The other inline probe, and the realistic #2965 producer: `prepareIosRunner` and prewarm send
// `uptime` as background health traffic while a mutation is still owed. `status` only ever comes
// from recovery, so pinning it alone would leave `uptime` free to start carrying a charge again —
// whose answer would then forgive the mutation's residue on the serial-queue premise and hand a
// runner off mid-mutation.
server = await startFakeRunnerServer({
tap: [{ kind: 'hangUp' }],
status: [{ kind: 'ok', data: { lifecycleState: 'started' } }],
uptime: [
{ kind: 'ok', data: { uptime: 12 } },
{ kind: 'ok', data: { uptime: 13 } },
],
});
const session = seedSession(server.port);

await expect(
runAppleRunnerCommand(IOS_SIMULATOR, { command: 'tap', x: 5, y: 5 }),
).rejects.toThrow(AppError);
// One charge: the mutation. The readiness preflight's probe is inline and carries no charge, so it
// cannot arrive here as a second debt that some later answer could pay off (#2965).
assert.equal(session.commandCharges.outstandingChargeCount, 1);

await runAppleRunnerCommand(IOS_SIMULATOR, { command: 'uptime' });
assert.equal(
server.requests.filter((request) => request.command === 'uptime').length > 1,
true,
'this command is a second probe, answered by the same session',
);
// The probe answered and owed nothing, so the mutation is still the only thing owed and the runner
// stays on the kill path rather than being handed off mid-mutation.
assert.equal(session.commandCharges.outstandingChargeCount, 1);
assert.equal(session.commandCharges.hasAbandonedCharges, true);
assert.deepEqual(resolveRunnerDetachDecision(session), {
detach: false,
reason: 'command_in_flight',
});
});

test('terminal status for a lost mutation charges the next lost mutation afresh', async () => {
server = await startFakeRunnerServer({
tap: [{ kind: 'hangUp' }, { kind: 'hangUp' }],
status: [
{ kind: 'ok', data: { lifecycleState: 'completed' } },
{ kind: 'ok', data: { lifecycleState: 'started' } },
],
});
const session = seedSession(server.port);

const first = await captureDiagnostics(async () => {
await expect(
runAppleRunnerCommand(IOS_SIMULATOR, { command: 'tap', x: 5, y: 5 }),
).rejects.toThrow(AppError);
});
// The daemon log has to say whether terminal evidence actually paid the debt, because a refused
// settlement and a settled one both leave the runner answering (#2965).
assert.match(first, /"abandonedChargeSettled":true/);
assert.equal(resolveRunnerDetachDecision(session).detach, true);

// The settlement is this command's, not a latch: a second mutation that loses its response is
// charged again and only its own terminal verdict frees the runner.
await expect(
runAppleRunnerCommand(IOS_SIMULATOR, { command: 'tap', x: 5, y: 5 }),
).rejects.toThrow(AppError);
assert.equal(resolveRunnerDetachDecision(session).detach, false);
});

test('terminal status with the runner reporting busy still refuses handoff', async () => {
server = await startFakeRunnerServer({
tap: [{ kind: 'hangUp' }],
status: [{ kind: 'ok', data: { lifecycleState: 'completed', runnerMainThreadBusy: true } }],
});
const session = seedSession(server.port);

await expect(
runAppleRunnerCommand(IOS_SIMULATOR, { command: 'tap', x: 5, y: 5 }),
).rejects.toThrow(AppError);

// The journal verdict discharged the charge, yet the runner's own occupancy report keeps it on the
// kill path: a terminal state never bypasses the busy gate (#2552).
assert.equal(session.runnerMainThreadBusy, true);
assert.deepEqual(resolveRunnerDetachDecision(session), {
detach: false,
reason: 'main_thread_occupied',
});
});

test('a lost mutation whose status probe fails keeps the runner on the kill path', async () => {
server = await startFakeRunnerServer({
tap: [{ kind: 'hangUp' }],
status: [{ kind: 'hangUp' }],
});
const session = seedSession(server.port);

await expect(
runAppleRunnerCommand(IOS_SIMULATOR, { command: 'tap', x: 5, y: 5 }),
).rejects.toThrow(AppError);
assert.equal(resolveRunnerDetachDecision(session).detach, false);
});

test('a healthy command after a settled abandoned charge returns the runner to handoff-eligible', async () => {
server = await startFakeRunnerServer({
tap: [{ kind: 'hangUp' }],
status: [{ kind: 'ok', data: { lifecycleState: 'completed' } }],
snapshot: [{ kind: 'ok', data: { nodes: [] } }],
});
const session = seedSession(server.port);

await expect(
runAppleRunnerCommand(IOS_SIMULATOR, { command: 'tap', x: 5, y: 5 }),
).rejects.toThrow(AppError);
assert.equal(resolveRunnerDetachDecision(session).detach, true);

await runAppleRunnerCommand(IOS_SIMULATOR, { command: 'snapshot' });
assert.equal(resolveRunnerDetachDecision(session).detach, true);
});

test('a runner that reports the command failed surfaces that failure, not the transport error', async () => {
server = await startFakeRunnerServer({
tap: [{ kind: 'hangUp' }],
Expand Down
Loading
Loading