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
4 changes: 4 additions & 0 deletions packages/platform-apple/src/core/app-launch.ts
Original file line number Diff line number Diff line change
Expand Up @@ -291,6 +291,10 @@ function buildIosSimulatorLaunchArgs(
options?: { launchConsole?: string; launchArgs?: string[]; terminateRunningApp?: boolean },
): string[] {
const args = ['launch'];
// `--console-pty` is the console mode this path needs: simctl writes the app's bytes to its own
// stdout through a PTY, which is what the caller redirects. `--stdout=<path>`/`--stderr=<path>`
// cannot substitute, because they resolve the given path inside the device's data container
// rather than on the host. Verified on Xcode 27.1.
if (options?.launchConsole) args.push('--console-pty');
if (options?.terminateRunningApp) args.push('--terminate-running-process');
args.push(deviceId, bundleId);
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,130 @@
import assert from 'node:assert/strict';
import path from 'node:path';
import { test } from 'vitest';
import {
buildRunnerSessionXctestrunDeviceCleanupPattern,
buildRunnerSessionXctestrunPathCleanupPattern,
buildRunnerSessionXctestrunSuffix,
} from '../runner-artifact-env.ts';

// Released daemons and clients pkill runner launches by these exact bytes, and the timeout client
// pins its own copy rather than deriving it, so it survives a rename here. Each expectation below
// is a literal on purpose: deriving it from this same module would survive any rename, which is
// precisely the change these lines exist to catch.

const OWNED_ARTIFACT_PATH =
'/derived/Build/Products/AgentDeviceRunner.env.session-SIM-001-owner-4242-ab12cd34-8123.xctestrun';

test('the session suffix keeps the bytes and field order the matchers rely on', () => {
assert.equal(
buildRunnerSessionXctestrunSuffix({
deviceId: 'SIM-001',
ownerToken: 'owner-4242-ab12cd34',
port: 8123,
}),
'session-SIM-001-owner-4242-ab12cd34-8123',
);
});

test('the lease cleanup pattern keeps the bytes released daemons pkilled', () => {
const owned = buildRunnerSessionXctestrunPathCleanupPattern(OWNED_ARTIFACT_PATH);

assert.equal(
owned,
String.raw`AgentDeviceRunner\.env\.session-SIM-001-owner-4242-ab12cd34-8123\.xctestrun`,
);
assert.equal(new RegExp(owned ?? '').test(argvFor(path.basename(OWNED_ARTIFACT_PATH))), true);
assert.equal(
new RegExp(owned ?? '').test(argvFor(sessionFileName('SIM-002', 'owner-4242-ab12cd34'))),
false,
'another device must not match',
);
assert.equal(
new RegExp(owned ?? '').test(argvFor(sessionFileName('SIM-001', 'owner-9999-ffee00'))),
false,
'another owner must not match',
);
assert.equal(
new RegExp(owned ?? '').test(
argvFor('AgentDeviceRunner.env.session-SIM-001-owner-4242-ab12cd34-8124.xctestrun'),
),
false,
'the same session on another port must not match',
);
});

test('the device sweep pattern keeps selecting the pre-owner-token name and only that', () => {
// A reclaiming daemon with no lease to read knows the device but not the artifact, so it sweeps by
// device. The released bytes require a digit right after the device, which is the pre-owner-token
// spelling; an owner-token name needs its own path to be selected.
const sweep = buildRunnerSessionXctestrunDeviceCleanupPattern('SIM-002');

assert.equal(sweep, String.raw`AgentDeviceRunner\.env\.session-SIM-002-[0-9]`);
assert.equal(
new RegExp(sweep).test(argvFor('AgentDeviceRunner.env.session-SIM-002-8123.xctestrun')),
true,
);
assert.equal(
new RegExp(sweep).test(argvFor(sessionFileName('SIM-002', 'owner-1-ff'))),
false,
'a launch a lease still names must not fall to the device sweep',
);
});

test('a flattened name field stays selectable', () => {
// A device id is caller-supplied and the writer flattens it onto disk, so both matchers have to
// survive that flattening: the recorded path already holds the bytes the launch carries, and the
// device sweep has to flatten the id it spells.
const suffix = buildRunnerSessionXctestrunSuffix({
deviceId: 'SIM 01/x',
ownerToken: 'owner 7',
port: 80,
});

assert.equal(suffix, 'session-SIM_01_x-owner_7-80');
const flattenedPath = `/derived/Build/Products/AgentDeviceRunner.env.${suffix}.xctestrun`;
assert.equal(
new RegExp(buildRunnerSessionXctestrunPathCleanupPattern(flattenedPath) ?? '').test(
argvFor(`AgentDeviceRunner.env.${suffix}.xctestrun`),
),
true,
);
// The pre-owner-token spelling of the same flattened device, which is the only name a sweep selects.
assert.equal(
new RegExp(buildRunnerSessionXctestrunDeviceCleanupPattern('SIM 01/x')).test(
argvFor('AgentDeviceRunner.env.session-SIM_01_x-8123.xctestrun'),
),
true,
);
});

test('a detached lease still selects the launch it started', () => {
// Detaching rewrites `ownerToken` to `detached-<token>` while the launch keeps running under the
// name the writer gave it. The bytes a token-derived pattern would have carried are spelled here
// literally: they select a file that never existed, which is the gap following the recorded path
// closes.
const detachedTokenPattern = String.raw`AgentDeviceRunner\.env\.session-SIM-001-detached-owner-4242-ab12cd34-`;
const argv = argvFor(path.basename(OWNED_ARTIFACT_PATH));

assert.equal(new RegExp(detachedTokenPattern).test(argv), false);
assert.equal(
new RegExp(buildRunnerSessionXctestrunPathCleanupPattern(OWNED_ARTIFACT_PATH) ?? '').test(argv),
true,
);
});

test('a lease artifact path outside the runner session name declines to a pattern', () => {
// A basename like `runner.xctestrun` would escape into a pattern loose enough to signal unrelated
// xcodebuilds, so the caller has to fall back to the device sweep.
assert.equal(buildRunnerSessionXctestrunPathCleanupPattern('/tmp/runner.xctestrun'), undefined);
assert.equal(buildRunnerSessionXctestrunPathCleanupPattern(''), undefined);
assert.equal(buildRunnerSessionXctestrunPathCleanupPattern(undefined), undefined);
});

function sessionFileName(deviceId: string, ownerToken: string): string {
return `AgentDeviceRunner.env.${buildRunnerSessionXctestrunSuffix({ deviceId, ownerToken, port: 8123 })}.xctestrun`;
}

function argvFor(fileName: string): string {
return `xcodebuild test-without-building -xctestrun /derived/Build/Products/${fileName}`;
}
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,13 @@ vi.mock('../runner-io.ts', async (importOriginal) => {
return { ...actual, cleanupTempFile: mockCleanupTempFile };
});

import { abortRunnerSessionsAndPrepProcesses, disposeRunnerSession } from '../runner-disposal.ts';
import {
abortRunnerSessionsAndPrepProcesses,
disposeRunnerSession,
runnerLeaseCleanupAdapter,
} from '../runner-disposal.ts';
import {
buildDetachedRunnerLease,
currentRunnerLeaseOwnerToken,
releaseRunnerLease,
withRunnerLeaseLock,
Expand Down Expand Up @@ -182,6 +187,57 @@ test('disposal serializes behind a successor reclaim window and never terminates
expect(currentRunnerLeaseOwnerToken(IOS_SIMULATOR.id)).toBe('owner-toctou-successor');
});

test('a leased cleanup selects the launch named by the artifact the lease recorded', async () => {
// The launch is found by matching its argv, and a lease knows the artifact that launch was
// started with. Detaching is why following the path rather than the token matters: it rewrites
// `ownerToken` to `detached-<token>` while the handed-over xcodebuild keeps the name the writer
// gave it, so a pattern rebuilt from the token names a file that never existed.
const detached = buildDetachedRunnerLease(
makeRunnerLease({ deviceId: IOS_SIMULATOR.id, ownerToken: 'owner-4242-ab12cd34' }),
);
expect(detached.ownerToken).toBe('detached-owner-4242-ab12cd34');

await runnerLeaseCleanupAdapter.cleanupRunnerXcodebuildProcesses({
deviceId: detached.deviceId,
xctestrunPath: detached.xctestrunPath,
});

const pattern = runnerXcodebuildPkillPatterns()[0];
expect(pattern).toBeDefined();
const selects = (xctestrunPath: string): boolean =>
new RegExp(pattern ?? '').test(runnerLaunchArgv(xctestrunPath));
expect(selects(detached.xctestrunPath)).toBe(true);
// A launch this lease does not name is somebody else's, and must stay untouched.
expect(selects('/tmp/other/AgentDeviceRunner.xctestrun')).toBe(false);
});

test('a cleanup with no recorded artifact sweeps that device launches only', async () => {
// A reclaim with no lease to read knows the device and nothing else, so it keeps the released
// pre-owner-token bytes and stays scoped to that device instead of every xcodebuild on the host.
await runnerLeaseCleanupAdapter.cleanupRunnerXcodebuildProcesses({ deviceId: 'SIM-OTHER' });

const pattern = runnerXcodebuildPkillPatterns()[0];
expect(pattern).toBeDefined();
const selects = (fileName: string): boolean =>
new RegExp(pattern ?? '').test(runnerLaunchArgv(`/tmp/${fileName}`));
expect(selects('AgentDeviceRunner.env.session-SIM-OTHER-8123.xctestrun')).toBe(true);
expect(selects('AgentDeviceRunner.env.session-SIM-VICTIM-8123.xctestrun')).toBe(false);
// A launch a lease still names is not this sweep's to take: it is reached by its own lease.
expect(
selects('AgentDeviceRunner.env.session-SIM-OTHER-owner-4242-ab12cd34-8123.xctestrun'),
).toBe(false);
});

function runnerXcodebuildPkillPatterns(): string[] {
return mockRunAppleToolCommand.mock.calls
.filter(([tool, args]) => tool === 'pkill' && (args as string[]).includes('-f'))
.map(([, args]) => String((args as string[])[2]));
}

function runnerLaunchArgv(xctestrunPath: string): string {
return `xcodebuild test-without-building -xctestrun ${xctestrunPath}`;
}

function simulatorTerminateCalls(): unknown[] {
return mockRunXcrun.mock.calls.filter(([args]) => (args as string[]).includes('terminate'));
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import assert from 'node:assert/strict';
import fs from 'node:fs';
import path from 'node:path';
import { afterEach, beforeEach, test } from 'vitest';
import type { DeviceInfo } from '@agent-device/kernel/device';
import { appleRunnerTestHost } from '../test-host.ts';
Expand Down Expand Up @@ -55,8 +56,11 @@ function recordingCleanupAdapter(): RunnerLeaseCleanupAdapter & { calls: string[
cleanupRunnerProcessTree: async (_pid, signal) => {
calls.push(`process-tree:${signal}`);
},
cleanupRunnerXcodebuildProcesses: async (_deviceId, ownerToken) => {
calls.push(`xcodebuild:${ownerToken ?? 'any'}`);
cleanupRunnerXcodebuildProcesses: async (target) => {
const basename = target.xctestrunPath
? `:${path.basename(target.xctestrunPath)}`
: ':device-sweep';
calls.push(`xcodebuild:${target.deviceId}${basename}`);
},
cleanupTempFile: (filePath) => {
calls.push(`temp:${filePath}`);
Expand All @@ -78,7 +82,13 @@ test('device-claim authority reclaims a live foreign claim-aware lease', async (

await prepareRunnerLeaseForStartup(device, cleanup);

assert.ok(cleanup.calls.includes('xcodebuild:owner-foreign-live'));
// The takeover kills the launch the lease named, so it must carry the recorded artifact path
// rather than anything derived from the owner token it is preempting.
assert.ok(
cleanup.calls.includes(
'xcodebuild:claim-takeover-sim:AgentDeviceRunner.env.session-claim-takeover-sim-owner-foreign-live-8123.xctestrun',
),
);
// The probe must receive the full device identity, never a bare id: claim
// ownership is canonical family/OS/id, and a same-id claim from another
// platform family grants nothing.
Expand All @@ -89,7 +99,7 @@ test('device-claim authority reclaims a live foreign claim-aware lease', async (
// sees an empty store instead of the foreign owner.
const emptyCleanup = recordingCleanupAdapter();
await prepareRunnerLeaseForStartup(device, emptyCleanup);
assert.ok(emptyCleanup.calls.includes('xcodebuild:any'));
assert.ok(emptyCleanup.calls.includes(`xcodebuild:${device.id}:device-sweep`));
});

test('a claim-aware lease still refuses without device-claim authority', async () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -132,6 +132,7 @@ import {
writeRunnerLease,
type RunnerLease,
type RunnerLeaseCleanupAdapter,
type RunnerXcodebuildCleanupTarget,
} from '../runner-lease.ts';

// Test-only stand-in for the daemon's own runtime lease-owner-state-dir
Expand Down Expand Up @@ -741,13 +742,13 @@ test('runner session startup reclaims dead foreign runner lease before launching
// separate adapter call and must keep running either way.
function makeRecordingCleanupAdapter() {
const treeKills: Array<{ pid: number | undefined; signal: string }> = [];
const xcodebuildCleanups: Array<{ deviceId: string; ownerToken: string | undefined }> = [];
const xcodebuildCleanups: RunnerXcodebuildCleanupTarget[] = [];
const adapter: RunnerLeaseCleanupAdapter = {
async cleanupRunnerProcessTree(pid, signal) {
treeKills.push({ pid, signal });
},
async cleanupRunnerXcodebuildProcesses(deviceId, ownerToken) {
xcodebuildCleanups.push({ deviceId, ownerToken });
async cleanupRunnerXcodebuildProcesses(target) {
xcodebuildCleanups.push(target);
},
cleanupTempFile() {},
};
Expand Down Expand Up @@ -783,9 +784,8 @@ test('stale-lease cleanup does not signal a recycled runner pid (start time mism
{ pid: undefined, signal: 'SIGTERM' },
{ pid: undefined, signal: 'SIGKILL' },
]);
assert.deepEqual(xcodebuildCleanups, [
{ deviceId: device.id, ownerToken: 'owner-dead-recycled' },
]);
const sweptDeviceIds = xcodebuildCleanups.map((target) => target.deviceId);
assert.deepEqual(sweptDeviceIds, [device.id]);
});

test('stale-lease cleanup signals the runner pid when its start time still matches', async () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,10 @@ import os from 'node:os';
import path from 'node:path';
import { pathToFileURL } from 'node:url';
import { mkdtempForTestSync } from './tmp-dir.ts';
import {
buildRunnerSessionXctestrunPathCleanupPattern,
buildRunnerSessionXctestrunSuffix,
} from '../runner-artifact-env.ts';
import { appleRunnerTestHost } from '../test-host.ts';
import type { ExecOptions, ExecResult } from '@agent-device/host-kit/command';

Expand Down Expand Up @@ -358,6 +362,46 @@ test('prepareXctestrunWithEnv writes env overlays into configured env dir', asyn
});
});

test('the session xctestrun the writer builds is found by the cleanup matcher', async () => {
// The launch is killed by `pkill -f` on a pattern the writer module itself builds, and the
// daemon-client sweep pins its own looser copy of these bytes. Binding writer to pattern and to
// those pinned bytes here is what makes a rename that keeps each side self-consistent fail.
await withTempDir('runner-xctestrun-identity-', async (root) => {
const xctestrunPath = path.join(root, 'AgentDeviceRunner.xctestrun');
fs.writeFileSync(
xctestrunPath,
JSON.stringify({
TestConfigurations: [{ TestTargets: [{ TestBundlePath: 'AgentDeviceRunnerUITests' }] }],
}),
);
appleRunnerTestHost.update({ runAppleToolCommand: fakeXctestrunPlutilToolCommand() });
const suffix = buildRunnerSessionXctestrunSuffix({
deviceId: 'SIM-001',
ownerToken: 'owner-4242-ab12cd34',
port: 8123,
});

const prepared = await prepareXctestrunWithEnv(xctestrunPath, runnerPortEnv, suffix);
const argv = `xcodebuild test-without-building -xctestrun ${prepared.xctestrunPath}`;

// The bytes the timeout sweep pins literally, so the sweep cannot drift off the writer's name.
assert.match(argv, new RegExp(String.raw`xcodebuild .*AgentDeviceRunner\.env\.session-`));
// The lease-backed pattern is spelled from the path the writer returned, so pin those bytes
// literally rather than comparing the pattern to the path it was built from: a rename that keeps
// both sides self-consistent must still fail here.
assert.equal(
buildRunnerSessionXctestrunPathCleanupPattern(prepared.xctestrunPath),
String.raw`AgentDeviceRunner\.env\.session-SIM-001-owner-4242-ab12cd34-8123\.xctestrun`,
);
assert.match(
argv,
new RegExp(
String.raw`xcodebuild.*test-without-building.*AgentDeviceRunner\.env\.session-SIM-001-owner-4242-ab12cd34-8123\.xctestrun`,
),
);
});
});

test('prepareXctestrunWithEnv leaves unrelated targets without capture policy', async () => {
await withTempDir('runner-xctestrun-policy-', async (root) => {
const xctestrunPath = path.join(root, 'AgentDeviceRunner.xctestrun');
Expand Down
Loading
Loading