From e6a411df5eefadb6c5e5a4bad8385b3e05c14eeb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Pierzcha=C5=82a?= Date: Sun, 1 Nov 2026 10:47:39 +0100 Subject: [PATCH 01/15] fix(ios): serialize status bar normalization around simulator captures The screenshot status bar transaction read the simulator's current overrides as the state to restore to, applied its deterministic normalization, captured, and restored. Two captures that overlapped on one simulator broke that: the second read the first capture's normalization as its "previous" state, so its restore leg re-applied the normalized network and Wi-Fi overrides and left them on. The daemon serializes ordinary requests for a device, but the capture interface itself did not, so any caller outside that path hit it. `withNormalizedSimulatorStatusBar` now owns the whole normalize -> capture -> restore window and holds the simulator's keyed lock across it, so a capture can only ever snapshot the caller's own overrides, and a capture that throws still restores before the queued one starts. --- .../core/__tests__/fake-status-bar-simctl.ts | 168 +++++++++++++++ .../screenshot-status-bar-concurrency.test.ts | 192 ++++++++++++++++++ .../__tests__/screenshot-status-bar.test.ts | 111 +++++++--- .../src/core/__tests__/screenshot.test.ts | 158 +++++--------- .../src/core/screenshot-status-bar.ts | 93 ++++++--- .../platform-apple/src/core/screenshot.ts | 32 ++- 6 files changed, 573 insertions(+), 181 deletions(-) create mode 100644 packages/platform-apple/src/core/__tests__/fake-status-bar-simctl.ts create mode 100644 packages/platform-apple/src/core/__tests__/screenshot-status-bar-concurrency.test.ts diff --git a/packages/platform-apple/src/core/__tests__/fake-status-bar-simctl.ts b/packages/platform-apple/src/core/__tests__/fake-status-bar-simctl.ts new file mode 100644 index 0000000000..5d5e68bef0 --- /dev/null +++ b/packages/platform-apple/src/core/__tests__/fake-status-bar-simctl.ts @@ -0,0 +1,168 @@ +import type { FakeAppleToolResponse } from '../../__tests__/fake-apple-tool.ts'; + +/** + * `simctl status_bar` with the simulator's own override state behind it. The production reader asks + * the tool what the status bar currently shows, so a test can only prove what a capture would + * restore to when the tool answers from state rather than from one fixed listing. + */ + +export type StatusBarOverrideState = { + time?: string; + dataNetwork?: string; + wifiMode?: string; + wifiBars?: string; + cellularMode?: string; + cellularBars?: string; + operatorName?: string; + batteryState?: string; + batteryLevel?: string; +}; + +export type FakeStatusBarSimctl = { + /** Acts as the `withFakeAppleTool` script. */ + script(args: string[]): FakeAppleToolResponse; + /** The override values the simulator holds now. */ + overrides(): StatusBarOverrideState; + /** The state each `list` served, in order: one entry per snapshot a capture took. */ + servedSnapshots(): StatusBarOverrideState[]; +}; + +const DATA_NETWORK_CODE: Record = { + hide: 0, + wifi: 1, + '3g': 6, + '4g': 7, + lte: 8, + 'lte-a': 9, + 'lte+': 10, + '5g': 11, + '5g+': 12, + '5g-uwb': 13, + '5g-uc': 14, +}; + +const WIFI_MODE_CODE: Record = { + searching: 1, + failed: 2, + active: 3, +}; + +const CELLULAR_MODE_CODE: Record = { + notSupported: 0, + searching: 1, + failed: 2, + active: 3, +}; + +const OVERRIDE_OPTION_FIELDS = { + '--time': 'time', + '--dataNetwork': 'dataNetwork', + '--wifiMode': 'wifiMode', + '--wifiBars': 'wifiBars', + '--cellularMode': 'cellularMode', + '--cellularBars': 'cellularBars', + '--operatorName': 'operatorName', + '--batteryState': 'batteryState', + '--batteryLevel': 'batteryLevel', +} as const satisfies Record; + +function parseOverrideOptions(args: string[]): Record { + const options: Record = {}; + for (let index = 0; index < args.length; index += 2) { + const option = args[index]; + if (option === undefined) break; + options[option] = args[index + 1] ?? ''; + } + return options; +} + +/** One `list` line, rendered from the state fields it reads, or nothing when the state omits them. */ +type StatusBarOverrideLine = { + reads: (keyof StatusBarOverrideState)[]; + render: (state: StatusBarOverrideState) => string; +}; + +function codeOf(codes: Record, value: string | undefined): number { + return value === undefined ? 0 : (codes[value] ?? 0); +} + +const STATUS_BAR_OVERRIDES_LISTING: StatusBarOverrideLine[] = [ + { + reads: ['time'], + render: (state) => `Time: ${state.time}`, + }, + { + reads: ['dataNetwork'], + render: (state) => `DataNetworkType: ${codeOf(DATA_NETWORK_CODE, state.dataNetwork)}`, + }, + { + reads: ['wifiMode', 'wifiBars'], + render: (state) => + `WiFi Mode: ${codeOf(WIFI_MODE_CODE, state.wifiMode)}, WiFi Bars: ${state.wifiBars ?? 0}`, + }, + { + reads: ['cellularMode', 'cellularBars'], + render: (state) => + `Cell Mode: ${codeOf(CELLULAR_MODE_CODE, state.cellularMode)}, Cell Bars: ${state.cellularBars ?? 0}`, + }, + { + reads: ['operatorName'], + render: (state) => `Operator Name: ${state.operatorName}`, + }, + { + reads: ['batteryState', 'batteryLevel'], + render: (state) => + `Battery State: ${state.batteryState ? Number(state.batteryState === 'unplugged') : 0}, Battery Level: ${state.batteryLevel ?? 100}, Not Charging: 0`, + }, +]; + +/** Renders state the way `simctl status_bar list` does, codes and all. */ +function renderStatusBarOverrides(state: StatusBarOverrideState): string { + const lines = ['Current Status Bar Overrides:', '=============================']; + for (const entry of STATUS_BAR_OVERRIDES_LISTING) { + if (entry.reads.every((field) => state[field] === undefined)) continue; + lines.push(entry.render(state)); + } + return lines.join('\n'); +} + +export function createFakeStatusBarSimctl( + udid: string, + initialOverrides: StatusBarOverrideState, +): FakeStatusBarSimctl { + let state: StatusBarOverrideState = { ...initialOverrides }; + const servedSnapshots: StatusBarOverrideState[] = []; + + return { + script: (args): FakeAppleToolResponse => { + const unexpected = (detail: string): FakeAppleToolResponse => ({ + stderr: `unexpected status bar call: ${detail}`, + exitCode: 1, + }); + if (args[0] !== 'simctl' || args[1] !== 'status_bar' || args[2] !== udid) { + return unexpected(args.join(' ')); + } + const verb = args[3]; + if (verb === 'list') { + servedSnapshots.push({ ...state }); + return renderStatusBarOverrides(state); + } + if (verb === 'clear') { + state = {}; + return ''; + } + if (verb !== 'override') return unexpected(args.join(' ')); + + const applied: StatusBarOverrideState = {}; + for (const [option, value] of Object.entries(parseOverrideOptions(args.slice(4)))) { + const field = OVERRIDE_OPTION_FIELDS[option as keyof typeof OVERRIDE_OPTION_FIELDS]; + if (field === undefined) return unexpected(`unknown option ${option}`); + applied[field] = value; + } + state = { ...state, ...applied }; + return ''; + }, + overrides: () => ({ ...state }), + servedSnapshots: () => servedSnapshots.map((snapshot) => ({ ...snapshot })), + }; +} diff --git a/packages/platform-apple/src/core/__tests__/screenshot-status-bar-concurrency.test.ts b/packages/platform-apple/src/core/__tests__/screenshot-status-bar-concurrency.test.ts new file mode 100644 index 0000000000..43dd00667f --- /dev/null +++ b/packages/platform-apple/src/core/__tests__/screenshot-status-bar-concurrency.test.ts @@ -0,0 +1,192 @@ +import { beforeEach, test } from 'vitest'; +import assert from 'node:assert/strict'; +import type { DeviceInfo } from '@agent-device/kernel/device'; +import { AppError } from '@agent-device/kernel/errors'; +import { withFakeAppleTool } from '../../__tests__/fake-apple-tool.ts'; +import { captureSimulatorScreenshotWithFallback } from '../screenshot.ts'; +import { invalidateSimulatorStatusBarOverrideCache } from '../screenshot-status-bar.ts'; +import { + createFakeStatusBarSimctl, + type FakeStatusBarSimctl, + type StatusBarOverrideState, +} from './fake-status-bar-simctl.ts'; +import { IOS_TEST_SIMULATOR } from './apple-core-stub-helpers.ts'; + +/** + * Two captures that overlap on one simulator. The normalization a capture applies is temporary, so + * a capture must never read it back as the state to restore to, and it must never leave it behind. + * + * These run through the capture route with the status bar owner's real implementation: the only + * seam they replace is the capture itself, so a gate can hold one capture open while another starts. + */ + +/** The caller's own override, the state both captures must leave behind. */ +const CALLER_OVERRIDES: StatusBarOverrideState = { + dataNetwork: 'hide', + wifiMode: 'failed', + wifiBars: '0', + cellularMode: 'failed', + cellularBars: '0', + operatorName: 'No Service', +}; + +/** The normalization, which is what the tool reports while a capture is in flight. */ +const NORMALIZED_OVERRIDES: StatusBarOverrideState = { + time: '9:41', + dataNetwork: 'wifi', + wifiMode: 'active', + wifiBars: '3', + batteryState: 'charged', + batteryLevel: '100', +}; + +/** Holds one capture open until the test releases it, so the overlap is the test's, not the host's. */ +function createCaptureGate() { + let release!: () => void; + let markEntered!: () => void; + const entered = new Promise((resolve) => { + markEntered = resolve; + }); + const released = new Promise((resolve) => { + release = resolve; + }); + return { + entered, + release, + capture: async (): Promise => { + markEntered(); + await released; + }, + }; +} + +/** + * Everything a simulator capture needs except the status bar, which stays the real owner so these + * cases exercise the same status bar commands the route sends on a device. + */ +function captureDeps(captureWithRetry: (device: DeviceInfo) => Promise) { + return { + resolveCaptureTarget: async () => ({}) as const, + captureWithRetry, + normalizeDensity: async () => {}, + captureWithRunner: async () => undefined, + shouldFallbackToRunner: () => false, + }; +} + +function screenshotFailure(): AppError { + return new AppError('COMMAND_FAILED', 'simctl screenshot failed', { + stderr: 'Unable to capture screenshot', + exitCode: 60, + }); +} + +/** Lets the second capture reach the point where it would read the first capture's state. */ +async function yieldToQueuedWork(): Promise { + await new Promise((resolve) => setImmediate(resolve)); +} + +beforeEach(() => { + invalidateSimulatorStatusBarOverrideCache(IOS_TEST_SIMULATOR); +}); + +test('overlapping simulator captures both restore the caller status bar overrides', async () => { + const statusBar: FakeStatusBarSimctl = createFakeStatusBarSimctl( + IOS_TEST_SIMULATOR.id, + CALLER_OVERRIDES, + ); + const firstGate = createCaptureGate(); + const secondGate = createCaptureGate(); + + await withFakeAppleTool(statusBar.script, async () => { + const first = captureSimulatorScreenshotWithFallback(IOS_TEST_SIMULATOR, '/tmp/first.png', { + env: process.env, + normalizeStatusBar: true, + skipIosSimulatorBootCheck: true, + deps: captureDeps(() => firstGate.capture()), + }); + await firstGate.entered; + + const second = captureSimulatorScreenshotWithFallback(IOS_TEST_SIMULATOR, '/tmp/second.png', { + env: process.env, + normalizeStatusBar: true, + skipIosSimulatorBootCheck: true, + deps: captureDeps(() => secondGate.capture()), + }); + await yieldToQueuedWork(); + + firstGate.release(); + await first; + secondGate.release(); + await second; + + assert.deepEqual( + statusBar.overrides(), + CALLER_OVERRIDES, + 'the second capture must restore the caller overrides, not the normalization it snapshotted', + ); + assert.deepEqual( + statusBar.servedSnapshots(), + [CALLER_OVERRIDES, CALLER_OVERRIDES], + "the second capture must snapshot the caller overrides, not the first capture's normalization", + ); + }); +}); + +test('a failed simulator capture restores the caller overrides and lets the queued capture finish', async () => { + const statusBar = createFakeStatusBarSimctl(IOS_TEST_SIMULATOR.id, CALLER_OVERRIDES); + const firstGate = createCaptureGate(); + const secondGate = createCaptureGate(); + + await withFakeAppleTool(statusBar.script, async () => { + const first = captureSimulatorScreenshotWithFallback(IOS_TEST_SIMULATOR, '/tmp/first.png', { + env: process.env, + normalizeStatusBar: true, + skipIosSimulatorBootCheck: true, + deps: captureDeps(async () => { + await firstGate.capture(); + throw screenshotFailure(); + }), + }); + await firstGate.entered; + + const second = captureSimulatorScreenshotWithFallback(IOS_TEST_SIMULATOR, '/tmp/second.png', { + env: process.env, + normalizeStatusBar: true, + skipIosSimulatorBootCheck: true, + deps: captureDeps(() => secondGate.capture()), + }); + await yieldToQueuedWork(); + + firstGate.release(); + await assert.rejects(() => first); + secondGate.release(); + await second; + + assert.deepEqual(statusBar.overrides(), CALLER_OVERRIDES); + assert.deepEqual( + statusBar.servedSnapshots(), + [CALLER_OVERRIDES, CALLER_OVERRIDES], + 'the capture queued behind a failing capture snapshots the caller overrides as well', + ); + }); +}); + +test('a single simulator capture normalizes while it captures and restores afterwards', async () => { + const statusBar = createFakeStatusBarSimctl(IOS_TEST_SIMULATOR.id, CALLER_OVERRIDES); + const seenDuringCapture: StatusBarOverrideState[] = []; + + await withFakeAppleTool(statusBar.script, async () => { + await captureSimulatorScreenshotWithFallback(IOS_TEST_SIMULATOR, '/tmp/out.png', { + env: process.env, + normalizeStatusBar: true, + skipIosSimulatorBootCheck: true, + deps: captureDeps(async () => { + seenDuringCapture.push(statusBar.overrides()); + }), + }); + + assert.deepEqual(seenDuringCapture, [NORMALIZED_OVERRIDES]); + assert.deepEqual(statusBar.overrides(), CALLER_OVERRIDES); + }); +}); diff --git a/packages/platform-apple/src/core/__tests__/screenshot-status-bar.test.ts b/packages/platform-apple/src/core/__tests__/screenshot-status-bar.test.ts index 2af570634c..9f728efe4f 100644 --- a/packages/platform-apple/src/core/__tests__/screenshot-status-bar.test.ts +++ b/packages/platform-apple/src/core/__tests__/screenshot-status-bar.test.ts @@ -3,9 +3,9 @@ import assert from 'node:assert/strict'; import { AppError } from '@agent-device/kernel/errors'; import { invalidateSimulatorStatusBarOverrideCache, - prepareSimulatorStatusBarForScreenshot, + withNormalizedSimulatorStatusBar, } from '../screenshot-status-bar.ts'; -import { withFakeAppleTool } from '../../__tests__/fake-apple-tool.ts'; +import { withFakeAppleTool, type FakeAppleToolResponse } from '../../__tests__/fake-apple-tool.ts'; import { IOS_TEST_SIMULATOR } from './apple-core-stub-helpers.ts'; import { statusBarDiagnosticEvents } from './screenshot-diagnostic-log.ts'; @@ -21,8 +21,14 @@ Cell Mode: 2, Cell Bars: 0 Operator Name: No Service Battery State: 1, Battery Level: 42, Not Charging: 0`; +const NORMALIZE_ARGS = + '--time 9:41 --dataNetwork wifi --wifiMode active --wifiBars 3 --batteryState charged --batteryLevel 100'; + +const RESTORE_ARGS = + '--dataNetwork hide --wifiMode failed --wifiBars 0 --cellularMode failed --cellularBars 0 --operatorName No Service'; + function statusBarScript(listResponse: () => string | { stderr: string; exitCode: number }) { - return (args: string[]) => { + return (args: string[]): FakeAppleToolResponse | undefined => { if (args[0] !== 'simctl' || args[1] !== 'status_bar') { return { stderr: `unexpected xcrun args: ${args.join(' ')}`, exitCode: 1 }; } @@ -32,47 +38,48 @@ function statusBarScript(listResponse: () => string | { stderr: string; exitCode }; } +/** The caller's step that the status bar surrounds; the status bar tests do not exercise it. */ +async function capture(): Promise { + return 'captured'; +} + beforeEach(() => { invalidateSimulatorStatusBarOverrideCache(IOS_TEST_SIMULATOR); }); -test('prepareSimulatorStatusBarForScreenshot restores prior visible overrides', async () => { +test('withNormalizedSimulatorStatusBar restores prior visible overrides', async () => { await withFakeAppleTool( statusBarScript(() => STATUS_BAR_OVERRIDES_LISTING), async ({ calls }) => { - const restore = await prepareSimulatorStatusBarForScreenshot(IOS_TEST_SIMULATOR); - await restore(); + const captured = await withNormalizedSimulatorStatusBar(IOS_TEST_SIMULATOR, capture); + assert.equal(captured, 'captured'); assert.deepEqual( calls.map((args) => args.join(' ')), [ 'simctl status_bar sim-1 list', 'simctl status_bar sim-1 clear', - 'simctl status_bar sim-1 override --time 9:41 --dataNetwork wifi --wifiMode active --wifiBars 3 --batteryState charged --batteryLevel 100', + `simctl status_bar sim-1 override ${NORMALIZE_ARGS}`, 'simctl status_bar sim-1 clear', - 'simctl status_bar sim-1 override --dataNetwork hide --wifiMode failed --wifiBars 0 --cellularMode failed --cellularBars 0 --operatorName No Service', + `simctl status_bar sim-1 override ${RESTORE_ARGS}`, ], ); }, ); }); -test('prepareSimulatorStatusBarForScreenshot skips known redundant status bar commands', async () => { +test('withNormalizedSimulatorStatusBar still normalizes when snapshotting current overrides fails', async () => { await withFakeAppleTool( - statusBarScript(() => 'Current Status Bar Overrides:\n============================='), + statusBarScript(() => ({ stderr: 'list failed', exitCode: 1 })), async ({ calls }) => { - const restoreFirst = await prepareSimulatorStatusBarForScreenshot(IOS_TEST_SIMULATOR); - await restoreFirst(); - const restoreSecond = await prepareSimulatorStatusBarForScreenshot(IOS_TEST_SIMULATOR); - await restoreSecond(); + await withNormalizedSimulatorStatusBar(IOS_TEST_SIMULATOR, capture); assert.deepEqual( calls.map((args) => args.join(' ')), [ 'simctl status_bar sim-1 list', - 'simctl status_bar sim-1 override --time 9:41 --dataNetwork wifi --wifiMode active --wifiBars 3 --batteryState charged --batteryLevel 100', 'simctl status_bar sim-1 clear', - 'simctl status_bar sim-1 override --time 9:41 --dataNetwork wifi --wifiMode active --wifiBars 3 --batteryState charged --batteryLevel 100', + `simctl status_bar sim-1 override ${NORMALIZE_ARGS}`, 'simctl status_bar sim-1 clear', ], ); @@ -80,19 +87,20 @@ test('prepareSimulatorStatusBarForScreenshot skips known redundant status bar co ); }); -test('prepareSimulatorStatusBarForScreenshot still normalizes when snapshotting current overrides fails', async () => { +test('withNormalizedSimulatorStatusBar restores once for back-to-back captures of a cleared status bar', async () => { await withFakeAppleTool( - statusBarScript(() => ({ stderr: 'list failed', exitCode: 1 })), + statusBarScript(() => 'Current Status Bar Overrides:\n============================='), async ({ calls }) => { - const restore = await prepareSimulatorStatusBarForScreenshot(IOS_TEST_SIMULATOR); - await restore(); + await withNormalizedSimulatorStatusBar(IOS_TEST_SIMULATOR, capture); + await withNormalizedSimulatorStatusBar(IOS_TEST_SIMULATOR, capture); assert.deepEqual( calls.map((args) => args.join(' ')), [ 'simctl status_bar sim-1 list', + `simctl status_bar sim-1 override ${NORMALIZE_ARGS}`, 'simctl status_bar sim-1 clear', - 'simctl status_bar sim-1 override --time 9:41 --dataNetwork wifi --wifiMode active --wifiBars 3 --batteryState charged --batteryLevel 100', + `simctl status_bar sim-1 override ${NORMALIZE_ARGS}`, 'simctl status_bar sim-1 clear', ], ); @@ -105,8 +113,7 @@ test('a failed status bar snapshot logs snapshot_failed with the tool failure', await withFakeAppleTool( statusBarScript(() => ({ stderr: 'list failed', exitCode: 1 })), async () => { - const restore = await prepareSimulatorStatusBarForScreenshot(IOS_TEST_SIMULATOR); - await restore(); + await withNormalizedSimulatorStatusBar(IOS_TEST_SIMULATOR, capture); }, ); }); @@ -128,7 +135,59 @@ test('a failed status bar snapshot logs snapshot_failed with the tool failure', ]); }); -test('a failed status bar override logs prepare_failed and the step stays best-effort', async () => { +test('a failed status bar restore logs restore_failed and keeps the capture result', async () => { + const events = await statusBarDiagnosticEvents(async () => { + await withFakeAppleTool( + (args) => { + if (args[3] === 'list') return STATUS_BAR_OVERRIDES_LISTING; + if (args[3] === 'clear') { + return new AppError('COMMAND_FAILED', 'clear rejected', { + exitCode: 8, + stderr: 'clear rejected', + }); + } + return ''; + }, + async () => { + assert.equal( + await withNormalizedSimulatorStatusBar(IOS_TEST_SIMULATOR, capture), + 'captured', + ); + }, + ); + }); + + assert.deepEqual(events, [ + { + level: 'warn', + phase: 'ios_screenshot_status_bar_prepare_failed', + data: { + platform: 'apple', + deviceKind: 'simulator', + deviceId: 'sim-1', + errorCode: 'COMMAND_FAILED', + reason: 'clear rejected', + exitCode: 8, + stderr: 'clear rejected', + }, + }, + { + level: 'warn', + phase: 'ios_screenshot_status_bar_restore_failed', + data: { + platform: 'apple', + deviceKind: 'simulator', + deviceId: 'sim-1', + errorCode: 'COMMAND_FAILED', + reason: 'clear rejected', + exitCode: 8, + stderr: 'clear rejected', + }, + }, + ]); +}); + +test('a failed status bar override logs prepare_failed and the capture still runs', async () => { const events = await statusBarDiagnosticEvents(async () => { await withFakeAppleTool( (args) => { @@ -143,8 +202,8 @@ test('a failed status bar override logs prepare_failed and the step stays best-e return ''; }, async ({ calls }) => { - const restore = await prepareSimulatorStatusBarForScreenshot(IOS_TEST_SIMULATOR); - await restore(); + const captured = await withNormalizedSimulatorStatusBar(IOS_TEST_SIMULATOR, capture); + assert.equal(captured, 'captured'); assert.equal(calls.at(-1)?.join(' '), 'simctl status_bar sim-1 clear'); }, ); diff --git a/packages/platform-apple/src/core/__tests__/screenshot.test.ts b/packages/platform-apple/src/core/__tests__/screenshot.test.ts index 0cb8c35b88..3e352360bb 100644 --- a/packages/platform-apple/src/core/__tests__/screenshot.test.ts +++ b/packages/platform-apple/src/core/__tests__/screenshot.test.ts @@ -32,7 +32,7 @@ vi.mock('../screenshot-status-bar.ts', async (importOriginal) => { const actual = await importOriginal(); return { ...actual, - prepareSimulatorStatusBarForScreenshot: vi.fn(actual.prepareSimulatorStatusBarForScreenshot), + withNormalizedSimulatorStatusBar: vi.fn(actual.withNormalizedSimulatorStatusBar), }; }); @@ -45,10 +45,6 @@ const retryActual = await vi.importActual('../runner-client.ts'); const simulatorActual = await vi.importActual('../simulator.ts'); -const screenshotStatusBarActual = await vi.importActual< - typeof import('../screenshot-status-bar.ts') ->('../screenshot-status-bar.ts'); - import { captureSimulatorScreenshotWithFallback, captureSimulatorScreenshotWithRetry, @@ -57,7 +53,7 @@ import { shouldRetryIosSimulatorScreenshot, } from '../screenshot.ts'; import { ensureBootedSimulator, openIosSimulatorApp } from '../simulator.ts'; -import { prepareSimulatorStatusBarForScreenshot } from '../screenshot-status-bar.ts'; +import { withNormalizedSimulatorStatusBar } from '../screenshot-status-bar.ts'; import { runAppleRunnerCommand } from '../runner-client.ts'; import { runCmd } from '@agent-device/host-kit/command'; import { withDiagnosticsScope } from '@agent-device/host-kit/diagnostics'; @@ -65,14 +61,13 @@ import { retryWithPolicy } from '@agent-device/host-kit/retry'; import { AppError } from '@agent-device/kernel/errors'; import { IOS_TEST_SIMULATOR, MACOS_TEST_DEVICE } from './apple-core-stub-helpers.ts'; -import { statusBarDiagnosticEvents } from './screenshot-diagnostic-log.ts'; const mockRunCmd = vi.mocked(runCmd); const mockRetryWithPolicy = vi.mocked(retryWithPolicy); const mockRunAppleRunnerCommand = vi.mocked(runAppleRunnerCommand); const mockEnsureBootedSimulator = vi.mocked(ensureBootedSimulator); const mockOpenIosSimulatorApp = vi.mocked(openIosSimulatorApp); -const mockPrepareStatusBarForScreenshot = vi.mocked(prepareSimulatorStatusBarForScreenshot); +const mockWithNormalizedStatusBar = vi.mocked(withNormalizedSimulatorStatusBar); // The wire key and a real measured capture come from the cross-language golden table, so this // consumer suite cannot spell a shape the Swift encoder disagrees with (#2728). @@ -92,9 +87,7 @@ beforeEach(() => { mockRunAppleRunnerCommand.mockImplementation(runnerActual.runAppleRunnerCommand); mockEnsureBootedSimulator.mockImplementation(simulatorActual.ensureBootedSimulator); mockOpenIosSimulatorApp.mockImplementation(simulatorActual.openIosSimulatorApp); - mockPrepareStatusBarForScreenshot.mockImplementation( - screenshotStatusBarActual.prepareSimulatorStatusBarForScreenshot, - ); + mockWithNormalizedStatusBar.mockImplementation(async (_device, capture) => await capture()); }); async function waitForFileText(filePath: string, attempts = 20): Promise { @@ -145,7 +138,6 @@ test('captureSimulatorScreenshotWithFallback falls back to runner after retry ex ensureBootedCalls += 1; }); mockOpenIosSimulatorApp.mockResolvedValue(undefined); - mockPrepareStatusBarForScreenshot.mockResolvedValue(async () => {}); mockRetryWithPolicy.mockRejectedValue( new AppError('COMMAND_FAILED', 'Detected file type from extension: PNG', { stderr: 'Timeout waiting for screen surfaces', @@ -168,7 +160,7 @@ test('captureSimulatorScreenshotWithFallback falls back to runner after retry ex appBundleId: 'com.example.app', deps: { ensureBooted: ensureBootedSimulator, - prepareStatusBarForScreenshot: prepareSimulatorStatusBarForScreenshot, + withNormalizedStatusBar: withNormalizedSimulatorStatusBar, captureWithRetry: captureSimulatorScreenshotWithRetry, normalizeDensity: async () => {}, captureWithRunner: captureScreenshotViaRunner, @@ -193,7 +185,6 @@ test('captureSimulatorScreenshotWithFallback falls back to runner after simctl s await fs.writeFile(runnerImage, 'runner-timeout', 'utf8'); mockEnsureBootedSimulator.mockResolvedValue(undefined); mockOpenIosSimulatorApp.mockResolvedValue(undefined); - mockPrepareStatusBarForScreenshot.mockResolvedValue(async () => {}); mockRetryWithPolicy.mockRejectedValue( new AppError('COMMAND_FAILED', 'xcrun timed out after 20000ms', { args: ['simctl', 'io', 'sim-1', 'screenshot', '/tmp/out.png'], @@ -216,7 +207,7 @@ test('captureSimulatorScreenshotWithFallback falls back to runner after simctl s appBundleId: 'com.example.app', deps: { ensureBooted: ensureBootedSimulator, - prepareStatusBarForScreenshot: prepareSimulatorStatusBarForScreenshot, + withNormalizedStatusBar: withNormalizedSimulatorStatusBar, captureWithRetry: captureSimulatorScreenshotWithRetry, normalizeDensity: async () => {}, captureWithRunner: captureScreenshotViaRunner, @@ -231,13 +222,19 @@ test('captureSimulatorScreenshotWithFallback falls back to runner after simctl s } }); -test('captureSimulatorScreenshotWithFallback continues when status bar preparation fails', async () => { - mockPrepareStatusBarForScreenshot.mockRejectedValue( - new AppError('COMMAND_FAILED', 'status_bar override failed'), - ); +test('captureSimulatorScreenshotWithFallback runs the capture inside status bar normalization', async () => { + const steps: string[] = []; + mockWithNormalizedStatusBar.mockImplementation(async (_device, capture) => { + steps.push('normalize'); + const captured = await capture(); + steps.push('restore'); + return captured; + }); mockEnsureBootedSimulator.mockResolvedValue(undefined); mockOpenIosSimulatorApp.mockResolvedValue(undefined); - mockRetryWithPolicy.mockResolvedValue(undefined); + mockRetryWithPolicy.mockImplementation(async () => { + steps.push('capture'); + }); await captureSimulatorScreenshotWithFallback(IOS_TEST_SIMULATOR, '/tmp/out.png', { env: process.env, @@ -245,14 +242,12 @@ test('captureSimulatorScreenshotWithFallback continues when status bar preparati normalizeStatusBar: true, deps: { normalizeDensity: async () => {} }, }); - assert.equal(mockPrepareStatusBarForScreenshot.mock.calls.length, 1); - assert.equal(mockRetryWithPolicy.mock.calls.length > 0, true); + assert.deepEqual(steps, ['normalize', 'capture', 'restore']); assert.equal(mockRunAppleRunnerCommand.mock.calls.length, 0); }); test('captureSimulatorScreenshotWithFallback can skip session-backed simulator boot probe', async () => { mockEnsureBootedSimulator.mockRejectedValue(new Error('should not probe boot state')); - mockPrepareStatusBarForScreenshot.mockResolvedValue(async () => {}); mockRetryWithPolicy.mockResolvedValue(undefined); await captureSimulatorScreenshotWithFallback(IOS_TEST_SIMULATOR, '/tmp/out.png', { @@ -270,7 +265,6 @@ test('captureSimulatorScreenshotWithFallback can skip session-backed simulator b test('captureSimulatorScreenshotWithFallback boots skipped-check simulator after shutdown screenshot failure', async () => { const ensureBooted = vi.fn(async () => {}); - const prepareStatusBarForScreenshot = vi.fn(async () => async () => {}); let captureAttempts = 0; const captureWithRetry = vi.fn(async () => { captureAttempts += 1; @@ -290,7 +284,6 @@ test('captureSimulatorScreenshotWithFallback boots skipped-check simulator after skipIosSimulatorBootCheck: true, deps: { ensureBooted, - prepareStatusBarForScreenshot, captureWithRetry, normalizeDensity: async () => {}, captureWithRunner, @@ -306,7 +299,6 @@ test('captureSimulatorScreenshotWithFallback boots skipped-check simulator after test('captureSimulatorScreenshotWithFallback keeps runner fallback after skipped-check boot recovery', async () => { const ensureBooted = vi.fn(async () => {}); - const prepareStatusBarForScreenshot = vi.fn(async () => async () => {}); let captureAttempts = 0; const captureWithRetry = vi.fn(async () => { captureAttempts += 1; @@ -330,7 +322,6 @@ test('captureSimulatorScreenshotWithFallback keeps runner fallback after skipped skipIosSimulatorBootCheck: true, deps: { ensureBooted, - prepareStatusBarForScreenshot, captureWithRetry, normalizeDensity: async () => {}, captureWithRunner, @@ -344,108 +335,56 @@ test('captureSimulatorScreenshotWithFallback keeps runner fallback after skipped assert.equal(captureWithRunner.mock.calls.length, 1); }); -test('captureSimulatorScreenshotWithFallback ignores status bar restore failures', async () => { - mockPrepareStatusBarForScreenshot.mockResolvedValue(async () => { - throw new AppError('COMMAND_FAILED', 'status_bar clear failed'); - }); +test('captureSimulatorScreenshotWithFallback skips status bar normalization by default', async () => { mockEnsureBootedSimulator.mockResolvedValue(undefined); mockOpenIosSimulatorApp.mockResolvedValue(undefined); mockRetryWithPolicy.mockResolvedValue(undefined); + await captureSimulatorScreenshotWithFallback(IOS_TEST_SIMULATOR, '/tmp/out.png', { env: process.env, appBundleId: 'com.example.app', - normalizeStatusBar: true, deps: { normalizeDensity: async () => {} }, }); - assert.equal(mockPrepareStatusBarForScreenshot.mock.calls.length, 1); + + assert.equal(mockWithNormalizedStatusBar.mock.calls.length, 0); assert.equal(mockRetryWithPolicy.mock.calls.length > 0, true); assert.equal(mockRunAppleRunnerCommand.mock.calls.length, 0); }); -function statusBarItemSummary(event: { - level: string; - phase: string; - data?: Record; -}) { - return { - level: event.level, - phase: event.phase, - deviceId: event.data?.deviceId, - exitCode: event.data?.exitCode, - }; -} - -test('captureSimulatorScreenshotWithFallback logs a status bar preparation failure', async () => { - mockEnsureBootedSimulator.mockResolvedValue(undefined); - mockOpenIosSimulatorApp.mockResolvedValue(undefined); - mockRetryWithPolicy.mockResolvedValue(undefined); - mockPrepareStatusBarForScreenshot.mockRejectedValue( - new AppError('COMMAND_FAILED', 'status_bar override failed', { exitCode: 69 }), - ); - - const events = await statusBarDiagnosticEvents(async () => { - await captureSimulatorScreenshotWithFallback(IOS_TEST_SIMULATOR, '/tmp/out.png', { - env: process.env, - appBundleId: 'com.example.app', - normalizeStatusBar: true, - deps: { normalizeDensity: async () => {}, resolveCaptureTarget: async () => ({}) }, - }); - }); - - assert.deepEqual(events.map(statusBarItemSummary), [ - { - level: 'warn', - phase: 'ios_screenshot_status_bar_prepare_failed', - deviceId: 'sim-1', - exitCode: 69, - }, - ]); -}); - -test('captureSimulatorScreenshotWithFallback logs a status bar restoration failure', async () => { - mockEnsureBootedSimulator.mockResolvedValue(undefined); - mockOpenIosSimulatorApp.mockResolvedValue(undefined); - mockRetryWithPolicy.mockResolvedValue(undefined); - mockPrepareStatusBarForScreenshot.mockResolvedValue(async () => { - throw new AppError('COMMAND_FAILED', 'status_bar clear failed', { exitCode: 70 }); - }); - - const events = await statusBarDiagnosticEvents(async () => { - await captureSimulatorScreenshotWithFallback(IOS_TEST_SIMULATOR, '/tmp/out.png', { - env: process.env, - appBundleId: 'com.example.app', - normalizeStatusBar: true, - deps: { normalizeDensity: async () => {}, resolveCaptureTarget: async () => ({}) }, - }); +test('captureSimulatorScreenshotWithFallback keeps the runner fallback inside the normalization window', async () => { + const steps: string[] = []; + mockWithNormalizedStatusBar.mockImplementation(async (_device, capture) => { + steps.push('normalize'); + const captured = await capture(); + steps.push('restore'); + return captured; }); - - assert.deepEqual(events.map(statusBarItemSummary), [ - { - level: 'warn', - phase: 'ios_screenshot_status_bar_restore_failed', - deviceId: 'sim-1', - exitCode: 70, - }, - ]); -}); - -test('captureSimulatorScreenshotWithFallback skips status bar normalization by default', async () => { - mockPrepareStatusBarForScreenshot.mockResolvedValue(async () => {}); mockEnsureBootedSimulator.mockResolvedValue(undefined); mockOpenIosSimulatorApp.mockResolvedValue(undefined); - mockRetryWithPolicy.mockResolvedValue(undefined); + mockRetryWithPolicy.mockRejectedValue( + new AppError('COMMAND_FAILED', 'xcrun timed out after 20000ms', { + args: ['simctl', 'io', 'sim-1', 'screenshot', '/tmp/out.png'], + timeoutMs: 20_000, + }), + ); await captureSimulatorScreenshotWithFallback(IOS_TEST_SIMULATOR, '/tmp/out.png', { env: process.env, - appBundleId: 'com.example.app', - deps: { normalizeDensity: async () => {} }, + normalizeStatusBar: true, + deps: { + normalizeDensity: async () => {}, + resolveCaptureTarget: async () => ({}), + shouldFallbackToRunner: shouldRetryIosSimulatorScreenshot, + captureWithRunner: async () => { + steps.push('runner fallback capture'); + return undefined; + }, + }, }); - assert.equal(mockPrepareStatusBarForScreenshot.mock.calls.length, 0); - assert.equal(mockRetryWithPolicy.mock.calls.length > 0, true); - assert.equal(mockRunAppleRunnerCommand.mock.calls.length, 0); + assert.deepEqual(steps, ['normalize', 'runner fallback capture', 'restore']); }); test('captureSimulatorScreenshotWithFallback emits fallback diagnostic before using runner', async () => { @@ -467,7 +406,7 @@ test('captureSimulatorScreenshotWithFallback emits fallback diagnostic before us await fs.writeFile(runnerImage, 'diag-fallback', 'utf8'); mockEnsureBootedSimulator.mockResolvedValue(undefined); mockOpenIosSimulatorApp.mockResolvedValue(undefined); - mockPrepareStatusBarForScreenshot.mockResolvedValue(async () => {}); + mockWithNormalizedStatusBar.mockImplementation(async (_device, capture) => await capture()); mockRetryWithPolicy.mockRejectedValue( new AppError('COMMAND_FAILED', 'xcrun timed out after 20000ms', { args: ['simctl', 'io', 'sim-1', 'screenshot', '/tmp/out.png'], @@ -487,7 +426,7 @@ test('captureSimulatorScreenshotWithFallback emits fallback diagnostic before us appBundleId: 'com.example.app', deps: { ensureBooted: ensureBootedSimulator, - prepareStatusBarForScreenshot: prepareSimulatorStatusBarForScreenshot, + withNormalizedStatusBar: withNormalizedSimulatorStatusBar, captureWithRetry: captureSimulatorScreenshotWithRetry, normalizeDensity: async () => {}, captureWithRunner: captureScreenshotViaRunner, @@ -518,7 +457,6 @@ test('captureSimulatorScreenshotWithFallback uses simulator runner fallback by d await fs.writeFile(runnerImage, 'default-fallback', 'utf8'); mockEnsureBootedSimulator.mockResolvedValue(undefined); mockOpenIosSimulatorApp.mockResolvedValue(undefined); - mockPrepareStatusBarForScreenshot.mockResolvedValue(async () => {}); mockRetryWithPolicy.mockRejectedValue( new AppError('COMMAND_FAILED', 'xcrun timed out after 20000ms', { args: ['simctl', 'io', 'sim-1', 'screenshot', '/tmp/out.png'], diff --git a/packages/platform-apple/src/core/screenshot-status-bar.ts b/packages/platform-apple/src/core/screenshot-status-bar.ts index 03fd273c0e..16da49f343 100644 --- a/packages/platform-apple/src/core/screenshot-status-bar.ts +++ b/packages/platform-apple/src/core/screenshot-status-bar.ts @@ -2,6 +2,7 @@ import type { DeviceInfo } from '@agent-device/kernel/device'; import { requireExecSuccess } from '@agent-device/host-kit/command'; import { emitDiagnostic } from '@agent-device/host-kit/diagnostics'; import { AppError } from '@agent-device/kernel/errors'; +import { withKeyedLock } from '@agent-device/kernel/keyed-lock'; import { runSimctlForDevice } from './simctl.ts'; import { extractAppleToolErrorMeta } from './tool-diagnostics.ts'; @@ -57,37 +58,71 @@ const CELLULAR_MODE_BY_CODE: Record = { const devicesKnownWithoutStatusBarOverrides = new Set(); -export async function prepareSimulatorStatusBarForScreenshot( +/** One entry per simulator whose status bar a capture is normalizing right now. */ +const simulatorStatusBarTransactions = new Map>(); + +/** What the simulator's status bar held before a capture normalized it. */ +type StatusBarOverrideSnapshot = { + /** False when the tool refused to answer, so nothing here is safe to restore to. */ + canRestore: boolean; + overrides: RestorableStatusBarOverrides | null; +}; + +/** + * Runs `capture` with the simulator status bar normalized, then restores the overrides the + * simulator held before this call. One simulator normalizes one capture at a time: the + * normalization is temporary state, so a capture that overlapped another would snapshot it and + * later hand it back as though the caller had set it. A status bar problem never decides whether + * `capture` runs or what it returns. + */ +export async function withNormalizedSimulatorStatusBar( + device: DeviceInfo, + capture: () => Promise, +): Promise { + return await withKeyedLock( + simulatorStatusBarTransactions, + statusBarSimulatorKey(device), + async () => { + const snapshot = await snapshotSimulatorStatusBarOverrides(device); + await normalizeSimulatorStatusBar(device, snapshot); + try { + return await capture(); + } finally { + await restoreSimulatorStatusBarOverrides( + device, + snapshot.canRestore ? snapshot.overrides : null, + ).catch((error) => emitSimulatorStatusBarDiagnostic(device, 'restore_failed', error)); + } + }, + ); +} + +async function snapshotSimulatorStatusBarOverrides( device: DeviceInfo, -): Promise<() => Promise> { - let previousOverrides: RestorableStatusBarOverrides | null = null; - let canRestorePreviousOverrides = false; +): Promise { if (hasKnownClearedStatusBarOverrides(device)) { - canRestorePreviousOverrides = true; - } else { - try { - previousOverrides = await readSimulatorStatusBarOverrides(device); - canRestorePreviousOverrides = true; - } catch (error) { - emitSimulatorStatusBarDiagnostic(device, 'snapshot_failed', error); - } + return { canRestore: true, overrides: null }; + } + try { + return { canRestore: true, overrides: await readSimulatorStatusBarOverrides(device) }; + } catch (error) { + emitSimulatorStatusBarDiagnostic(device, 'snapshot_failed', error); + return { canRestore: false, overrides: null }; } +} +async function normalizeSimulatorStatusBar( + device: DeviceInfo, + snapshot: StatusBarOverrideSnapshot, +): Promise { try { - if (!canRestorePreviousOverrides || previousOverrides) { + if (!snapshot.canRestore || snapshot.overrides) { await clearSimulatorStatusBarOverride(device); } await applySimulatorStatusBarOverrideArgs(device, DETERMINISTIC_SCREENSHOT_STATUS_BAR_ARGS); } catch (error) { emitSimulatorStatusBarDiagnostic(device, 'prepare_failed', error); } - - return async () => { - await restoreSimulatorStatusBarOverrides( - device, - canRestorePreviousOverrides ? previousOverrides : null, - ); - }; } async function restoreSimulatorStatusBarOverrides( @@ -107,18 +142,23 @@ async function restoreSimulatorStatusBarOverrides( } export function invalidateSimulatorStatusBarOverrideCache(device: DeviceInfo): void { - devicesKnownWithoutStatusBarOverrides.delete(statusBarOverrideCacheKey(device)); + devicesKnownWithoutStatusBarOverrides.delete(statusBarSimulatorKey(device)); } export function rememberClearedStatusBarOverrides(device: DeviceInfo): void { - devicesKnownWithoutStatusBarOverrides.add(statusBarOverrideCacheKey(device)); + devicesKnownWithoutStatusBarOverrides.add(statusBarSimulatorKey(device)); } function hasKnownClearedStatusBarOverrides(device: DeviceInfo): boolean { - return devicesKnownWithoutStatusBarOverrides.has(statusBarOverrideCacheKey(device)); + return devicesKnownWithoutStatusBarOverrides.has(statusBarSimulatorKey(device)); } -function statusBarOverrideCacheKey(device: DeviceInfo): string { +/** + * Key for both the known-cleared cache and the transaction lock. It names the simulator without its + * device set, so two simulators that could ever share a udid are serialized rather than normalized + * at once. + */ +function statusBarSimulatorKey(device: DeviceInfo): string { return `${device.platform}:${device.kind}:${device.id}`; } @@ -236,9 +276,10 @@ function isCellularDataNetworkType(value: string | undefined): boolean { /** * Records one best-effort status bar step that failed, under `ios_screenshot_status_bar_`. - * A status bar problem never decides a screenshot's outcome, so callers swallow the error. + * A status bar problem never decides a screenshot's outcome, so every caller of this swallows the + * error and the transaction still runs the capture. */ -export function emitSimulatorStatusBarDiagnostic( +function emitSimulatorStatusBarDiagnostic( device: DeviceInfo, phase: 'snapshot_failed' | 'prepare_failed' | 'restore_failed', error: unknown, diff --git a/packages/platform-apple/src/core/screenshot.ts b/packages/platform-apple/src/core/screenshot.ts index 7d1353b484..2568666bad 100644 --- a/packages/platform-apple/src/core/screenshot.ts +++ b/packages/platform-apple/src/core/screenshot.ts @@ -27,10 +27,7 @@ import { resolveRunnerContainerBundleIds, type AppleRunnerCommandOptions, } from '@agent-device/apple-runner'; -import { - emitSimulatorStatusBarDiagnostic, - prepareSimulatorStatusBarForScreenshot, -} from './screenshot-status-bar.ts'; +import { withNormalizedSimulatorStatusBar } from './screenshot-status-bar.ts'; import { appleSimulatorDisplayArgvFragment, resolveAppleCaptureTarget, @@ -44,7 +41,11 @@ import { resolveIosPhysicalDeviceControl } from './physical-device-control.ts'; type SimulatorScreenshotFlowDeps = { ensureBooted: (device: DeviceInfo) => Promise; - prepareStatusBarForScreenshot: (device: DeviceInfo) => Promise<() => Promise>; + /** + * Runs `capture` with the simulator status bar normalized and restores the caller's overrides + * afterwards, one capture per simulator at a time. + */ + withNormalizedStatusBar: (device: DeviceInfo, capture: () => Promise) => Promise; resolveCaptureTarget: (device: DeviceInfo) => Promise; captureWithRetry: ( device: DeviceInfo, @@ -89,7 +90,7 @@ type SimulatorScreenshotFlowOptions = { const defaultSimulatorScreenshotFlowDeps: SimulatorScreenshotFlowDeps = { ensureBooted: ensureBootedSimulator, - prepareStatusBarForScreenshot: prepareSimulatorStatusBarForScreenshot, + withNormalizedStatusBar: withNormalizedSimulatorStatusBar, resolveCaptureTarget: resolveAppleCaptureTarget, captureWithRetry: captureSimulatorScreenshotWithRetry, normalizeDensity: normalizeIosSimulatorScreenshotDensity, @@ -154,15 +155,7 @@ export async function captureSimulatorScreenshotWithFallback( await deps.captureWithRetry(device, outPath, display); await deps.normalizeDensity(device, outPath, options.pixelDensity, display?.pointScale); }; - let restoreStatusBar = async () => {}; - if (options.normalizeStatusBar === true) { - try { - restoreStatusBar = await deps.prepareStatusBarForScreenshot(device); - } catch (error) { - emitSimulatorStatusBarDiagnostic(device, 'prepare_failed', error); - } - } - try { + const captureWithFallback = async (): Promise => { try { await captureAndNormalize(); return captureFacts; @@ -199,11 +192,12 @@ export async function captureSimulatorScreenshotWithFallback( // than borrowing a scale from a panel nobody captured (#2728). await deps.normalizeDensity(device, outPath, options.pixelDensity, captured?.pixelsPerPoint); return captureFacts; - } finally { - await restoreStatusBar().catch((error) => - emitSimulatorStatusBarDiagnostic(device, 'restore_failed', error), - ); + }; + + if (options.normalizeStatusBar !== true) { + return await captureWithFallback(); } + return await deps.withNormalizedStatusBar(device, captureWithFallback); } export async function captureSimulatorScreenshotWithRetry( From e6a97ebcaf76c016e07dc60ab957a15a231e02c1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Pierzcha=C5=82a?= Date: Sun, 1 Nov 2026 11:25:54 +0100 Subject: [PATCH 02/15] fix(ios): keep plain captures out of the status bar window Route a capture without --normalize-status-bar through the same per-simulator lock so it cannot take its screenshot of the temporary normalized bar another capture installed. Pass the request's abort signal to the lock and bound every simctl status_bar call so a wedged tool cannot hold a simulator's captures. Restore stays uncancellable: the normalization is shared device state. Release both capture gates in a finally, pin the propagated capture error, throw on an override value the fake cannot list, and rename the cleared-status bar case to what it asserts. --- .../core/__tests__/fake-status-bar-simctl.ts | 17 ++-- .../screenshot-status-bar-concurrency.test.ts | 86 ++++++++++++++++--- .../__tests__/screenshot-status-bar.test.ts | 2 +- .../src/core/screenshot-status-bar.ts | 46 ++++++++-- .../platform-apple/src/core/screenshot.ts | 32 +++++-- 5 files changed, 152 insertions(+), 31 deletions(-) diff --git a/packages/platform-apple/src/core/__tests__/fake-status-bar-simctl.ts b/packages/platform-apple/src/core/__tests__/fake-status-bar-simctl.ts index 5d5e68bef0..a9df742b08 100644 --- a/packages/platform-apple/src/core/__tests__/fake-status-bar-simctl.ts +++ b/packages/platform-apple/src/core/__tests__/fake-status-bar-simctl.ts @@ -82,8 +82,14 @@ type StatusBarOverrideLine = { render: (state: StatusBarOverrideState) => string; }; -function codeOf(codes: Record, value: string | undefined): number { - return value === undefined ? 0 : (codes[value] ?? 0); +/** The listing code for one override value; an unmapped value is the fake's bug, not a silent `hide`. */ +function codeOf(codes: Record, value: string | undefined, field: string): number { + if (value === undefined) return 0; + const code = codes[value]; + if (code === undefined) { + throw new Error(`fake simctl status_bar cannot list ${field}: ${value}`); + } + return code; } const STATUS_BAR_OVERRIDES_LISTING: StatusBarOverrideLine[] = [ @@ -93,17 +99,18 @@ const STATUS_BAR_OVERRIDES_LISTING: StatusBarOverrideLine[] = [ }, { reads: ['dataNetwork'], - render: (state) => `DataNetworkType: ${codeOf(DATA_NETWORK_CODE, state.dataNetwork)}`, + render: (state) => + `DataNetworkType: ${codeOf(DATA_NETWORK_CODE, state.dataNetwork, 'dataNetwork')}`, }, { reads: ['wifiMode', 'wifiBars'], render: (state) => - `WiFi Mode: ${codeOf(WIFI_MODE_CODE, state.wifiMode)}, WiFi Bars: ${state.wifiBars ?? 0}`, + `WiFi Mode: ${codeOf(WIFI_MODE_CODE, state.wifiMode, 'wifiMode')}, WiFi Bars: ${state.wifiBars ?? 0}`, }, { reads: ['cellularMode', 'cellularBars'], render: (state) => - `Cell Mode: ${codeOf(CELLULAR_MODE_CODE, state.cellularMode)}, Cell Bars: ${state.cellularBars ?? 0}`, + `Cell Mode: ${codeOf(CELLULAR_MODE_CODE, state.cellularMode, 'cellularMode')}, Cell Bars: ${state.cellularBars ?? 0}`, }, { reads: ['operatorName'], diff --git a/packages/platform-apple/src/core/__tests__/screenshot-status-bar-concurrency.test.ts b/packages/platform-apple/src/core/__tests__/screenshot-status-bar-concurrency.test.ts index 43dd00667f..81c02548d4 100644 --- a/packages/platform-apple/src/core/__tests__/screenshot-status-bar-concurrency.test.ts +++ b/packages/platform-apple/src/core/__tests__/screenshot-status-bar-concurrency.test.ts @@ -113,12 +113,19 @@ test('overlapping simulator captures both restore the caller status bar override skipIosSimulatorBootCheck: true, deps: captureDeps(() => secondGate.capture()), }); - await yieldToQueuedWork(); + try { + await yieldToQueuedWork(); - firstGate.release(); - await first; - secondGate.release(); - await second; + firstGate.release(); + await first; + secondGate.release(); + await second; + } finally { + // A capture that never reached its gate would keep the simulator's lock and hang every + // later case in this file, hiding the failure the test is here to report. + firstGate.release(); + secondGate.release(); + } assert.deepEqual( statusBar.overrides(), @@ -156,12 +163,28 @@ test('a failed simulator capture restores the caller overrides and lets the queu skipIosSimulatorBootCheck: true, deps: captureDeps(() => secondGate.capture()), }); - await yieldToQueuedWork(); + let firstError: unknown; + try { + await yieldToQueuedWork(); - firstGate.release(); - await assert.rejects(() => first); - secondGate.release(); - await second; + firstGate.release(); + firstError = await first.then( + () => undefined, + (error: unknown) => error, + ); + secondGate.release(); + await second; + } finally { + firstGate.release(); + secondGate.release(); + } + + assert.ok( + firstError instanceof AppError, + `expected an AppError, received: ${String(firstError)}`, + ); + assert.equal((firstError as AppError).code, 'COMMAND_FAILED'); + assert.equal((firstError as AppError).message, 'simctl screenshot failed'); assert.deepEqual(statusBar.overrides(), CALLER_OVERRIDES); assert.deepEqual( @@ -190,3 +213,46 @@ test('a single simulator capture normalizes while it captures and restores after assert.deepEqual(statusBar.overrides(), CALLER_OVERRIDES); }); }); + +test('a plain simulator capture waits for the normalization window instead of capturing it', async () => { + const statusBar = createFakeStatusBarSimctl(IOS_TEST_SIMULATOR.id, CALLER_OVERRIDES); + const normalizingGate = createCaptureGate(); + const seenDuringPlainCapture: StatusBarOverrideState[] = []; + + await withFakeAppleTool(statusBar.script, async () => { + const normalizing = captureSimulatorScreenshotWithFallback( + IOS_TEST_SIMULATOR, + '/tmp/normalized.png', + { + env: process.env, + normalizeStatusBar: true, + skipIosSimulatorBootCheck: true, + deps: captureDeps(() => normalizingGate.capture()), + }, + ); + await normalizingGate.entered; + + const plain = captureSimulatorScreenshotWithFallback(IOS_TEST_SIMULATOR, '/tmp/plain.png', { + env: process.env, + skipIosSimulatorBootCheck: true, + deps: captureDeps(async () => { + seenDuringPlainCapture.push(statusBar.overrides()); + }), + }); + try { + await yieldToQueuedWork(); + + normalizingGate.release(); + await normalizing; + await plain; + } finally { + normalizingGate.release(); + } + + assert.deepEqual( + seenDuringPlainCapture, + [CALLER_OVERRIDES], + 'a capture without --normalize-status-bar must not take its screenshot of the temporary normalized bar', + ); + }); +}); diff --git a/packages/platform-apple/src/core/__tests__/screenshot-status-bar.test.ts b/packages/platform-apple/src/core/__tests__/screenshot-status-bar.test.ts index 9f728efe4f..975728e00e 100644 --- a/packages/platform-apple/src/core/__tests__/screenshot-status-bar.test.ts +++ b/packages/platform-apple/src/core/__tests__/screenshot-status-bar.test.ts @@ -87,7 +87,7 @@ test('withNormalizedSimulatorStatusBar still normalizes when snapshotting curren ); }); -test('withNormalizedSimulatorStatusBar restores once for back-to-back captures of a cleared status bar', async () => { +test('withNormalizedSimulatorStatusBar skips the snapshot read for back-to-back captures of a cleared status bar', async () => { await withFakeAppleTool( statusBarScript(() => 'Current Status Bar Overrides:\n============================='), async ({ calls }) => { diff --git a/packages/platform-apple/src/core/screenshot-status-bar.ts b/packages/platform-apple/src/core/screenshot-status-bar.ts index 16da49f343..4dc421ed4a 100644 --- a/packages/platform-apple/src/core/screenshot-status-bar.ts +++ b/packages/platform-apple/src/core/screenshot-status-bar.ts @@ -58,9 +58,12 @@ const CELLULAR_MODE_BY_CODE: Record = { const devicesKnownWithoutStatusBarOverrides = new Set(); -/** One entry per simulator whose status bar a capture is normalizing right now. */ +/** One entry per simulator whose status bar a capture is holding right now. */ const simulatorStatusBarTransactions = new Map>(); +/** Bounds one `simctl status_bar` step so a wedged tool cannot hold a simulator's captures. */ +const STATUS_BAR_COMMAND_TIMEOUT_MS = 10_000; + /** What the simulator's status bar held before a capture normalized it. */ type StatusBarOverrideSnapshot = { /** False when the tool refused to answer, so nothing here is safe to restore to. */ @@ -70,14 +73,19 @@ type StatusBarOverrideSnapshot = { /** * Runs `capture` with the simulator status bar normalized, then restores the overrides the - * simulator held before this call. One simulator normalizes one capture at a time: the - * normalization is temporary state, so a capture that overlapped another would snapshot it and - * later hand it back as though the caller had set it. A status bar problem never decides whether - * `capture` runs or what it returns. + * simulator held before this call. One simulator handles one of these at a time: the normalization + * is temporary state, so a capture that overlapped another would snapshot it and later hand it back + * as though the caller had set it. A status bar problem never decides whether `capture` runs or + * what it returns. + * + * `signal` cancels the wait for the simulator, never the restore: the normalization is shared device + * state, so it has to be undone even once the caller has given up. Every status bar command is + * bounded by {@link STATUS_BAR_COMMAND_TIMEOUT_MS}, so a wedged `simctl` cannot hold the simulator. */ export async function withNormalizedSimulatorStatusBar( device: DeviceInfo, capture: () => Promise, + signal?: AbortSignal, ): Promise { return await withKeyedLock( simulatorStatusBarTransactions, @@ -94,6 +102,25 @@ export async function withNormalizedSimulatorStatusBar( ).catch((error) => emitSimulatorStatusBarDiagnostic(device, 'restore_failed', error)); } }, + signal, + ); +} + +/** + * Reserves one simulator's status bar while `capture` runs, without changing the bar. A capture that + * does not normalize still has to wait: while another capture normalizes the bar, this one would + * otherwise take its screenshot of the temporary 9:41 overrides. + */ +export async function withSimulatorStatusBarHeld( + device: DeviceInfo, + capture: () => Promise, + signal?: AbortSignal, +): Promise { + return await withKeyedLock( + simulatorStatusBarTransactions, + statusBarSimulatorKey(device), + capture, + signal, ); } @@ -168,6 +195,7 @@ async function readSimulatorStatusBarOverrides( const result = requireExecSuccess( await runSimctlForDevice(device, ['status_bar', device.id, 'list'], { allowFailure: true, + timeoutMs: STATUS_BAR_COMMAND_TIMEOUT_MS, }), 'Failed to read simulator status bar overrides', ); @@ -175,7 +203,9 @@ async function readSimulatorStatusBarOverrides( } async function clearSimulatorStatusBarOverride(device: DeviceInfo): Promise { - await runSimctlForDevice(device, ['status_bar', device.id, 'clear']); + await runSimctlForDevice(device, ['status_bar', device.id, 'clear'], { + timeoutMs: STATUS_BAR_COMMAND_TIMEOUT_MS, + }); } async function applySimulatorStatusBarOverrideArgs( @@ -183,7 +213,9 @@ async function applySimulatorStatusBarOverrideArgs( args: string[], ): Promise { if (args.length === 0) return; - await runSimctlForDevice(device, ['status_bar', device.id, 'override', ...args]); + await runSimctlForDevice(device, ['status_bar', device.id, 'override', ...args], { + timeoutMs: STATUS_BAR_COMMAND_TIMEOUT_MS, + }); } function parseSimulatorStatusBarOverrides(output: string): RestorableStatusBarOverrides | null { diff --git a/packages/platform-apple/src/core/screenshot.ts b/packages/platform-apple/src/core/screenshot.ts index 2568666bad..f6cef48038 100644 --- a/packages/platform-apple/src/core/screenshot.ts +++ b/packages/platform-apple/src/core/screenshot.ts @@ -27,7 +27,10 @@ import { resolveRunnerContainerBundleIds, type AppleRunnerCommandOptions, } from '@agent-device/apple-runner'; -import { withNormalizedSimulatorStatusBar } from './screenshot-status-bar.ts'; +import { + withNormalizedSimulatorStatusBar, + withSimulatorStatusBarHeld, +} from './screenshot-status-bar.ts'; import { appleSimulatorDisplayArgvFragment, resolveAppleCaptureTarget, @@ -42,10 +45,23 @@ import { resolveIosPhysicalDeviceControl } from './physical-device-control.ts'; type SimulatorScreenshotFlowDeps = { ensureBooted: (device: DeviceInfo) => Promise; /** - * Runs `capture` with the simulator status bar normalized and restores the caller's overrides - * afterwards, one capture per simulator at a time. + * Runs `capture` while it owns the simulator's status bar: normalized to the deterministic state, + * with the caller's overrides restored afterwards. One capture per simulator at a time. + */ + withNormalizedStatusBar: ( + device: DeviceInfo, + capture: () => Promise, + signal?: AbortSignal, + ) => Promise; + /** + * Runs `capture` while it owns the simulator's status bar, without changing the bar. It still + * waits: a capture that normalizes shows the deterministic state while it is in flight. */ - withNormalizedStatusBar: (device: DeviceInfo, capture: () => Promise) => Promise; + withStatusBarHeld: ( + device: DeviceInfo, + capture: () => Promise, + signal?: AbortSignal, + ) => Promise; resolveCaptureTarget: (device: DeviceInfo) => Promise; captureWithRetry: ( device: DeviceInfo, @@ -91,6 +107,7 @@ type SimulatorScreenshotFlowOptions = { const defaultSimulatorScreenshotFlowDeps: SimulatorScreenshotFlowDeps = { ensureBooted: ensureBootedSimulator, withNormalizedStatusBar: withNormalizedSimulatorStatusBar, + withStatusBarHeld: withSimulatorStatusBarHeld, resolveCaptureTarget: resolveAppleCaptureTarget, captureWithRetry: captureSimulatorScreenshotWithRetry, normalizeDensity: normalizeIosSimulatorScreenshotDensity, @@ -194,10 +211,9 @@ export async function captureSimulatorScreenshotWithFallback( return captureFacts; }; - if (options.normalizeStatusBar !== true) { - return await captureWithFallback(); - } - return await deps.withNormalizedStatusBar(device, captureWithFallback); + const holdStatusBar = + options.normalizeStatusBar === true ? deps.withNormalizedStatusBar : deps.withStatusBarHeld; + return await holdStatusBar(device, captureWithFallback, options.runnerOptions?.signal); } export async function captureSimulatorScreenshotWithRetry( From b5fa69de00d88a5696b5b7f6e8a80b184e86e0f6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Pierzcha=C5=82a?= Date: Sun, 1 Nov 2026 11:57:47 +0100 Subject: [PATCH 03/15] fix(ios): keep runner captures out of the status bar window An iOS simulator's runner capture renders the same screen a simctl capture does, but the interactor's runner backend called it directly, so it could copy an image of the temporary normalized overrides another capture had installed. It now takes the same per-simulator reservation without normalizing the bar, and re-enters the one the simctl fallback already holds. Take both capture reads at the top of the capture step, where the status bar owner still holds the state the PNG is taken with, and settle both captures through one helper so a rejection is never left unobserved. --- .../screenshot-status-bar-concurrency.test.ts | 274 ++++++++++-------- .../src/core/__tests__/screenshot.test.ts | 77 ++++- .../src/core/screenshot-status-bar.ts | 6 +- .../platform-apple/src/core/screenshot.ts | 32 ++ 4 files changed, 263 insertions(+), 126 deletions(-) diff --git a/packages/platform-apple/src/core/__tests__/screenshot-status-bar-concurrency.test.ts b/packages/platform-apple/src/core/__tests__/screenshot-status-bar-concurrency.test.ts index 81c02548d4..13b5e11bb6 100644 --- a/packages/platform-apple/src/core/__tests__/screenshot-status-bar-concurrency.test.ts +++ b/packages/platform-apple/src/core/__tests__/screenshot-status-bar-concurrency.test.ts @@ -60,10 +60,10 @@ function createCaptureGate() { }; } -/** - * Everything a simulator capture needs except the status bar, which stays the real owner so these - * cases exercise the same status bar commands the route sends on a device. - */ +type CaptureGate = ReturnType; + +/** Everything a simulator capture needs except the status bar, which stays the real owner so these + * cases exercise the same status bar commands the route sends on a device. */ function captureDeps(captureWithRetry: (device: DeviceInfo) => Promise) { return { resolveCaptureTarget: async () => ({}) as const, @@ -86,6 +86,66 @@ async function yieldToQueuedWork(): Promise { await new Promise((resolve) => setImmediate(resolve)); } +function screenshotOptions( + gate: CaptureGate, + normalizeStatusBar: boolean, + options: { readDuringCapture?: () => void; failsAfterGate?: boolean } = {}, +) { + return { + env: process.env, + normalizeStatusBar, + skipIosSimulatorBootCheck: true, + deps: captureDeps(async () => { + // The read happens at the top of the capture step: that is the state this capture's image + // is taken with, and it is the only moment the status bar owner has it. + options.readDuringCapture?.(); + await gate.capture(); + if (options.failsAfterGate === true) { + throw screenshotFailure(); + } + }), + } as const; +} + +/** + * Runs a body that starts two captures on one simulator and hands back both settled. Gates are + * released and both captures are observed whatever the body threw: a capture left holding the + * simulator's lock would hang the later cases in this file, and a rejection nobody awaited would + * fail one of them instead of the case that caused it. + */ +async function withOverlappingCaptures( + start: () => Promise<{ captures: Array>; gates: CaptureGate[] }>, + run: (settled: PromiseSettledResult[]) => void, +): Promise { + let captures: Array> = []; + let gates: CaptureGate[] = []; + let settled: PromiseSettledResult[] = []; + try { + const started = await start(); + captures = started.captures; + gates = started.gates; + await yieldToQueuedWork(); + for (const gate of gates) gate.release(); + settled = await Promise.allSettled(captures); + } finally { + for (const gate of gates) gate.release(); + settled = await Promise.allSettled(captures); + } + run(settled); +} + +function fulfilledCount(settled: PromiseSettledResult[]): number { + return settled.filter((result) => result.status === 'fulfilled').length; +} + +function rejectionReason(settled: PromiseSettledResult[], index: number): unknown { + const result = settled[index]; + if (result?.status !== 'rejected') { + throw new assert.AssertionError({ message: `expected capture ${index} to reject` }); + } + return result.reason; +} + beforeEach(() => { invalidateSimulatorStatusBarOverrideCache(IOS_TEST_SIMULATOR); }); @@ -95,102 +155,77 @@ test('overlapping simulator captures both restore the caller status bar override IOS_TEST_SIMULATOR.id, CALLER_OVERRIDES, ); - const firstGate = createCaptureGate(); - const secondGate = createCaptureGate(); await withFakeAppleTool(statusBar.script, async () => { - const first = captureSimulatorScreenshotWithFallback(IOS_TEST_SIMULATOR, '/tmp/first.png', { - env: process.env, - normalizeStatusBar: true, - skipIosSimulatorBootCheck: true, - deps: captureDeps(() => firstGate.capture()), - }); - await firstGate.entered; - - const second = captureSimulatorScreenshotWithFallback(IOS_TEST_SIMULATOR, '/tmp/second.png', { - env: process.env, - normalizeStatusBar: true, - skipIosSimulatorBootCheck: true, - deps: captureDeps(() => secondGate.capture()), - }); - try { - await yieldToQueuedWork(); - - firstGate.release(); - await first; - secondGate.release(); - await second; - } finally { - // A capture that never reached its gate would keep the simulator's lock and hang every - // later case in this file, hiding the failure the test is here to report. - firstGate.release(); - secondGate.release(); - } - - assert.deepEqual( - statusBar.overrides(), - CALLER_OVERRIDES, - 'the second capture must restore the caller overrides, not the normalization it snapshotted', - ); - assert.deepEqual( - statusBar.servedSnapshots(), - [CALLER_OVERRIDES, CALLER_OVERRIDES], - "the second capture must snapshot the caller overrides, not the first capture's normalization", + await withOverlappingCaptures( + async () => { + const firstGate = createCaptureGate(); + const secondGate = createCaptureGate(); + const first = captureSimulatorScreenshotWithFallback( + IOS_TEST_SIMULATOR, + '/tmp/first.png', + screenshotOptions(firstGate, true), + ); + await firstGate.entered; + const second = captureSimulatorScreenshotWithFallback( + IOS_TEST_SIMULATOR, + '/tmp/second.png', + screenshotOptions(secondGate, true), + ); + return { captures: [first, second], gates: [firstGate, secondGate] }; + }, + (settled) => { + assert.equal(fulfilledCount(settled), 2, 'both captures must finish'); + assert.deepEqual( + statusBar.overrides(), + CALLER_OVERRIDES, + 'the second capture must restore the caller overrides, not the normalization it snapshotted', + ); + assert.deepEqual( + statusBar.servedSnapshots(), + [CALLER_OVERRIDES, CALLER_OVERRIDES], + "the second capture must snapshot the caller overrides, not the first capture's normalization", + ); + }, ); }); }); test('a failed simulator capture restores the caller overrides and lets the queued capture finish', async () => { const statusBar = createFakeStatusBarSimctl(IOS_TEST_SIMULATOR.id, CALLER_OVERRIDES); - const firstGate = createCaptureGate(); - const secondGate = createCaptureGate(); await withFakeAppleTool(statusBar.script, async () => { - const first = captureSimulatorScreenshotWithFallback(IOS_TEST_SIMULATOR, '/tmp/first.png', { - env: process.env, - normalizeStatusBar: true, - skipIosSimulatorBootCheck: true, - deps: captureDeps(async () => { - await firstGate.capture(); - throw screenshotFailure(); - }), - }); - await firstGate.entered; - - const second = captureSimulatorScreenshotWithFallback(IOS_TEST_SIMULATOR, '/tmp/second.png', { - env: process.env, - normalizeStatusBar: true, - skipIosSimulatorBootCheck: true, - deps: captureDeps(() => secondGate.capture()), - }); - let firstError: unknown; - try { - await yieldToQueuedWork(); - - firstGate.release(); - firstError = await first.then( - () => undefined, - (error: unknown) => error, - ); - secondGate.release(); - await second; - } finally { - firstGate.release(); - secondGate.release(); - } - - assert.ok( - firstError instanceof AppError, - `expected an AppError, received: ${String(firstError)}`, - ); - assert.equal((firstError as AppError).code, 'COMMAND_FAILED'); - assert.equal((firstError as AppError).message, 'simctl screenshot failed'); + await withOverlappingCaptures( + async () => { + const firstGate = createCaptureGate(); + const secondGate = createCaptureGate(); + const first = captureSimulatorScreenshotWithFallback( + IOS_TEST_SIMULATOR, + '/tmp/first.png', + screenshotOptions(firstGate, true, { failsAfterGate: true }), + ); + await firstGate.entered; + const second = captureSimulatorScreenshotWithFallback( + IOS_TEST_SIMULATOR, + '/tmp/second.png', + screenshotOptions(secondGate, true), + ); + return { captures: [first, second], gates: [firstGate, secondGate] }; + }, + (settled) => { + const error = rejectionReason(settled, 0); + assert.ok(error instanceof AppError, `expected an AppError, received: ${String(error)}`); + assert.equal((error as AppError).code, 'COMMAND_FAILED'); + assert.equal((error as AppError).message, 'simctl screenshot failed'); + assert.equal(settled[1]?.status, 'fulfilled', 'the queued capture must still finish'); - assert.deepEqual(statusBar.overrides(), CALLER_OVERRIDES); - assert.deepEqual( - statusBar.servedSnapshots(), - [CALLER_OVERRIDES, CALLER_OVERRIDES], - 'the capture queued behind a failing capture snapshots the caller overrides as well', + assert.deepEqual(statusBar.overrides(), CALLER_OVERRIDES); + assert.deepEqual( + statusBar.servedSnapshots(), + [CALLER_OVERRIDES, CALLER_OVERRIDES], + 'the capture queued behind a failing capture snapshots the caller overrides as well', + ); + }, ); }); }); @@ -216,43 +251,38 @@ test('a single simulator capture normalizes while it captures and restores after test('a plain simulator capture waits for the normalization window instead of capturing it', async () => { const statusBar = createFakeStatusBarSimctl(IOS_TEST_SIMULATOR.id, CALLER_OVERRIDES); - const normalizingGate = createCaptureGate(); const seenDuringPlainCapture: StatusBarOverrideState[] = []; await withFakeAppleTool(statusBar.script, async () => { - const normalizing = captureSimulatorScreenshotWithFallback( - IOS_TEST_SIMULATOR, - '/tmp/normalized.png', - { - env: process.env, - normalizeStatusBar: true, - skipIosSimulatorBootCheck: true, - deps: captureDeps(() => normalizingGate.capture()), + await withOverlappingCaptures( + async () => { + const normalizingGate = createCaptureGate(); + const plainGate = createCaptureGate(); + const normalizing = captureSimulatorScreenshotWithFallback( + IOS_TEST_SIMULATOR, + '/tmp/normalized.png', + screenshotOptions(normalizingGate, true), + ); + await normalizingGate.entered; + const plain = captureSimulatorScreenshotWithFallback( + IOS_TEST_SIMULATOR, + '/tmp/plain.png', + screenshotOptions(plainGate, false, { + readDuringCapture: () => { + seenDuringPlainCapture.push(statusBar.overrides()); + }, + }), + ); + return { captures: [normalizing, plain], gates: [normalizingGate, plainGate] }; + }, + (settled) => { + assert.equal(fulfilledCount(settled), 2, 'both captures must finish'); + assert.deepEqual( + seenDuringPlainCapture, + [CALLER_OVERRIDES], + 'a capture without --normalize-status-bar must not take its screenshot of the temporary normalized bar', + ); }, - ); - await normalizingGate.entered; - - const plain = captureSimulatorScreenshotWithFallback(IOS_TEST_SIMULATOR, '/tmp/plain.png', { - env: process.env, - skipIosSimulatorBootCheck: true, - deps: captureDeps(async () => { - seenDuringPlainCapture.push(statusBar.overrides()); - }), - }); - try { - await yieldToQueuedWork(); - - normalizingGate.release(); - await normalizing; - await plain; - } finally { - normalizingGate.release(); - } - - assert.deepEqual( - seenDuringPlainCapture, - [CALLER_OVERRIDES], - 'a capture without --normalize-status-bar must not take its screenshot of the temporary normalized bar', ); }); }); diff --git a/packages/platform-apple/src/core/__tests__/screenshot.test.ts b/packages/platform-apple/src/core/__tests__/screenshot.test.ts index 3e352360bb..f6ea5ccc05 100644 --- a/packages/platform-apple/src/core/__tests__/screenshot.test.ts +++ b/packages/platform-apple/src/core/__tests__/screenshot.test.ts @@ -53,7 +53,10 @@ import { shouldRetryIosSimulatorScreenshot, } from '../screenshot.ts'; import { ensureBootedSimulator, openIosSimulatorApp } from '../simulator.ts'; -import { withNormalizedSimulatorStatusBar } from '../screenshot-status-bar.ts'; +import { + withNormalizedSimulatorStatusBar, + withSimulatorStatusBarHeld, +} from '../screenshot-status-bar.ts'; import { runAppleRunnerCommand } from '../runner-client.ts'; import { runCmd } from '@agent-device/host-kit/command'; import { withDiagnosticsScope } from '@agent-device/host-kit/diagnostics'; @@ -618,6 +621,78 @@ test('captureScreenshotViaRunner reports the display facts a runner capture meas } }); +// A runner capture renders the same screen a simctl capture does, so it must not be taken while a +// simctl capture holds the simulator's status bar normalized. The holder here is the real status bar +// owner's reservation (this file's mock keeps every other export real). +test('captureScreenshotViaRunner waits for a held simulator status bar', async () => { + const tmpDir = await mkdtempForTest('agent-device-runner-bar-window-'); + const containerPath = path.join(tmpDir, 'container'); + const runnerImage = path.join(containerPath, 'tmp', 'held.png'); + const device = { ...IOS_TEST_SIMULATOR, id: 'sim-runner-held' }; + await fs.mkdir(path.dirname(runnerImage), { recursive: true }); + await fs.writeFile(runnerImage, 'runner-image', 'utf8'); + + let releaseHolder!: () => void; + const held = new Promise((resolve) => { + releaseHolder = resolve; + }); + let releaseRunnerCommand!: () => void; + const runnerCommandReleased = new Promise((resolve) => { + releaseRunnerCommand = resolve; + }); + let releaseHolderGate!: () => void; + const holderEntered = new Promise((resolve) => { + releaseHolderGate = resolve; + }); + + mockRunAppleRunnerCommand.mockImplementation(async () => { + await runnerCommandReleased; + return { message: 'tmp/held.png' }; + }); + mockRunCmd.mockImplementation(async (_cmd, args) => { + if (args.includes('get_app_container')) { + return { exitCode: 0, stdout: `${containerPath}\n`, stderr: '' }; + } + throw new Error(`Unexpected xcrun args: ${args.join(' ')}`); + }); + + const holder = withSimulatorStatusBarHeld(device, async () => { + releaseHolderGate(); + await held; + }); + + try { + await holderEntered; + const outPath = path.join(tmpDir, 'out.png'); + const runnerCapture = captureScreenshotViaRunner( + device, + outPath, + undefined, + undefined, + undefined, + process.env, + ); + await new Promise((resolve) => setImmediate(resolve)); + assert.equal( + mockRunAppleRunnerCommand.mock.calls.length, + 0, + 'a runner capture must wait while another capture holds the status bar', + ); + + releaseHolder(); + await holder; + await new Promise((resolve) => setImmediate(resolve)); + releaseRunnerCommand(); + await runnerCapture; + assert.equal(mockRunAppleRunnerCommand.mock.calls.length, 1); + assert.equal(await fs.readFile(outPath, 'utf8'), 'runner-image'); + } finally { + releaseHolder(); + releaseRunnerCommand(); + await fs.rm(tmpDir, { recursive: true, force: true }); + } +}); + test('captureScreenshotViaRunner reports no display facts for a pre-panel runner', async () => { const tmpDir = await mkdtempForTest('agent-device-runner-no-metadata-'); const containerPath = path.join(tmpDir, 'container'); diff --git a/packages/platform-apple/src/core/screenshot-status-bar.ts b/packages/platform-apple/src/core/screenshot-status-bar.ts index 4dc421ed4a..8034044e0f 100644 --- a/packages/platform-apple/src/core/screenshot-status-bar.ts +++ b/packages/platform-apple/src/core/screenshot-status-bar.ts @@ -181,9 +181,9 @@ function hasKnownClearedStatusBarOverrides(device: DeviceInfo): boolean { } /** - * Key for both the known-cleared cache and the transaction lock. It names the simulator without its - * device set, so two simulators that could ever share a udid are serialized rather than normalized - * at once. + * Key for both the known-cleared cache and the transaction lock. It names a simulator by platform, + * kind and id, and leaves out the device set, so two records of one simulator share a key and one + * transaction instead of normalizing the same status bar at once. */ function statusBarSimulatorKey(device: DeviceInfo): string { return `${device.platform}:${device.kind}:${device.id}`; diff --git a/packages/platform-apple/src/core/screenshot.ts b/packages/platform-apple/src/core/screenshot.ts index f6cef48038..c19fccc650 100644 --- a/packages/platform-apple/src/core/screenshot.ts +++ b/packages/platform-apple/src/core/screenshot.ts @@ -249,6 +249,12 @@ export async function captureSimulatorScreenshotWithRetry( ); } +/** + * Captures through the runner backend. An iOS simulator's runner capture renders the same screen a + * simctl capture does, so it takes the same status bar reservation: while another capture + * normalizes the bar, this one would otherwise copy an image of the temporary 9:41 overrides. The + * reservation re-enters when the caller already holds it, as the simctl fallback does. + */ export async function captureScreenshotViaRunner( device: DeviceInfo, outPath: string, @@ -256,6 +262,32 @@ export async function captureScreenshotViaRunner( fullscreen: boolean | undefined, runnerOptions: AppleRunnerCommandOptions | undefined, env: Readonly, +): Promise { + if (device.kind !== 'simulator' || isMacOs(device)) { + return await captureRunnerScreenshot( + device, + outPath, + appBundleId, + fullscreen, + runnerOptions, + env, + ); + } + return await withSimulatorStatusBarHeld( + device, + async () => + await captureRunnerScreenshot(device, outPath, appBundleId, fullscreen, runnerOptions, env), + runnerOptions?.signal, + ); +} + +async function captureRunnerScreenshot( + device: DeviceInfo, + outPath: string, + appBundleId: string | undefined, + fullscreen: boolean | undefined, + runnerOptions: AppleRunnerCommandOptions | undefined, + env: Readonly, ): Promise { if (device.kind === 'device' && !isMacOs(device)) { await resolveIosPhysicalDeviceControl(device).captureScreenshot(device, outPath, { From 6a8579c811a9ee1c490710a473557c41f81f2c18 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Pierzcha=C5=82a?= Date: Sun, 1 Nov 2026 12:31:09 +0100 Subject: [PATCH 04/15] test(ios): cover the runner fallback inside the status bar window One request's two capture legs both run inside the status bar window, and the runner leg takes that same window. Drive the real normalization around the real runner fallback with only the tools faked, and assert the order: snapshot, normalize, runner capture, restore. The runner leg re-entering that window is what keeps the nested reservation from opening a second one. Give the restore a longer deadline than setup: leaving the normalization on the simulator is the failure this owner exists to prevent, and an untimed wedged tool would hold the simulator forever. State in the doc which commands the request's abort signal does and does not reach. --- .../src/core/__tests__/screenshot.test.ts | 78 ++++++++++++++++++- .../src/core/screenshot-status-bar.ts | 32 +++++--- 2 files changed, 96 insertions(+), 14 deletions(-) diff --git a/packages/platform-apple/src/core/__tests__/screenshot.test.ts b/packages/platform-apple/src/core/__tests__/screenshot.test.ts index f6ea5ccc05..bc559ef93c 100644 --- a/packages/platform-apple/src/core/__tests__/screenshot.test.ts +++ b/packages/platform-apple/src/core/__tests__/screenshot.test.ts @@ -54,6 +54,7 @@ import { } from '../screenshot.ts'; import { ensureBootedSimulator, openIosSimulatorApp } from '../simulator.ts'; import { + invalidateSimulatorStatusBarOverrideCache, withNormalizedSimulatorStatusBar, withSimulatorStatusBarHeld, } from '../screenshot-status-bar.ts'; @@ -621,9 +622,80 @@ test('captureScreenshotViaRunner reports the display facts a runner capture meas } }); -// A runner capture renders the same screen a simctl capture does, so it must not be taken while a -// simctl capture holds the simulator's status bar normalized. The holder here is the real status bar -// owner's reservation (this file's mock keeps every other export real). +// Both capture legs of one request run inside the status bar window, and the runner leg takes that +// same window for itself. This drives the real normalization around the real runner fallback, with +// only the tools faked, so the nesting is exercised rather than assumed. +test('captureSimulatorScreenshotWithFallback keeps the real runner fallback inside its own status bar window', async () => { + const tmpDir = await mkdtempForTest('agent-device-normalized-runner-fallback-'); + const device = { ...IOS_TEST_SIMULATOR, id: 'sim-1' }; + const containerPath = path.join(tmpDir, 'container'); + const runnerImage = path.join(containerPath, 'tmp', 'fallback-window.png'); + await fs.mkdir(path.dirname(runnerImage), { recursive: true }); + await fs.writeFile(runnerImage, 'runner-window-image', 'utf8'); + const timeline: string[] = []; + const steps: string[] = []; + + // The file's mock of the status bar owner stands in for the plain pass-through by default; this + // case needs the real transaction, so it restores the implementation `vi.fn` was created with. + mockWithNormalizedStatusBar.mockReset(); + invalidateSimulatorStatusBarOverrideCache(device); + mockRetryWithPolicy.mockRejectedValue( + new AppError('COMMAND_FAILED', 'Detected file type from extension: PNG', { + stderr: 'Timeout waiting for screen surfaces', + exitCode: 60, + }), + ); + mockRunAppleRunnerCommand.mockImplementation(async () => { + steps.push('runner'); + timeline.push('runner'); + return { message: 'tmp/fallback-window.png' }; + }); + mockRunCmd.mockImplementation(async (cmd, args) => { + const argv = [cmd, ...(args ?? [])].join(' '); + if (argv.includes('status_bar')) { + timeline.push(`status_bar:${/status_bar\s+\S+\s+(\S+)/.exec(argv)?.[1]}`); + return { + exitCode: 0, + stdout: 'Current Status Bar Overrides:\n=============================\n', + stderr: '', + }; + } + if (argv.includes('get_app_container')) { + return { exitCode: 0, stdout: `${containerPath}\n`, stderr: '' }; + } + throw new Error(`Unexpected command: ${argv}`); + }); + + try { + const outPath = path.join(tmpDir, 'out.png'); + await captureSimulatorScreenshotWithFallback(device, outPath, { + env: process.env, + normalizeStatusBar: true, + skipIosSimulatorBootCheck: true, + deps: { + captureWithRetry: captureSimulatorScreenshotWithRetry, + normalizeDensity: async () => {}, + captureWithRunner: captureScreenshotViaRunner, + shouldFallbackToRunner: shouldRetryIosSimulatorScreenshot, + resolveCaptureTarget: async () => ({}), + }, + }); + + assert.deepEqual(steps, ['runner']); + assert.equal(await fs.readFile(outPath, 'utf8'), 'runner-window-image'); + // The window belongs to the request: it opens before either capture and closes after the + // runner capture, and the runner leg re-enters that window instead of opening its own. + assert.deepEqual(timeline, [ + 'status_bar:list', + 'status_bar:override', + 'runner', + 'status_bar:clear', + ]); + } finally { + await fs.rm(tmpDir, { recursive: true, force: true }); + } +}); + test('captureScreenshotViaRunner waits for a held simulator status bar', async () => { const tmpDir = await mkdtempForTest('agent-device-runner-bar-window-'); const containerPath = path.join(tmpDir, 'container'); diff --git a/packages/platform-apple/src/core/screenshot-status-bar.ts b/packages/platform-apple/src/core/screenshot-status-bar.ts index 8034044e0f..6281a53edc 100644 --- a/packages/platform-apple/src/core/screenshot-status-bar.ts +++ b/packages/platform-apple/src/core/screenshot-status-bar.ts @@ -64,6 +64,13 @@ const simulatorStatusBarTransactions = new Map>(); /** Bounds one `simctl status_bar` step so a wedged tool cannot hold a simulator's captures. */ const STATUS_BAR_COMMAND_TIMEOUT_MS = 10_000; +/** + * Budget for the restore specifically. Leaving the normalization on the simulator is the failure + * this owner exists to prevent, so the restore is given the longer deadline; it is still bounded, + * because an untimed wedged tool would hold the simulator's captures forever. + */ +const STATUS_BAR_RESTORE_TIMEOUT_MS = 30_000; + /** What the simulator's status bar held before a capture normalized it. */ type StatusBarOverrideSnapshot = { /** False when the tool refused to answer, so nothing here is safe to restore to. */ @@ -78,9 +85,11 @@ type StatusBarOverrideSnapshot = { * as though the caller had set it. A status bar problem never decides whether `capture` runs or * what it returns. * - * `signal` cancels the wait for the simulator, never the restore: the normalization is shared device - * state, so it has to be undone even once the caller has given up. Every status bar command is - * bounded by {@link STATUS_BAR_COMMAND_TIMEOUT_MS}, so a wedged `simctl` cannot hold the simulator. + * `signal` cancels the wait for the simulator only. It is not handed to the status bar commands: a + * request that gives up must not leave the normalization behind, so the restore runs to its budget + * instead — {@link STATUS_BAR_COMMAND_TIMEOUT_MS} per setup command and the longer + * {@link STATUS_BAR_RESTORE_TIMEOUT_MS} per restore command, so even a wedged `simctl` cannot hold + * the simulator forever. */ export async function withNormalizedSimulatorStatusBar( device: DeviceInfo, @@ -156,7 +165,7 @@ async function restoreSimulatorStatusBarOverrides( device: DeviceInfo, overrides: RestorableStatusBarOverrides | null, ): Promise { - await clearSimulatorStatusBarOverride(device); + await clearSimulatorStatusBarOverride(device, STATUS_BAR_RESTORE_TIMEOUT_MS); if (!overrides) { rememberClearedStatusBarOverrides(device); return; @@ -164,6 +173,7 @@ async function restoreSimulatorStatusBarOverrides( await applySimulatorStatusBarOverrideArgs( device, buildRestorableStatusBarOverrideArgs(overrides), + STATUS_BAR_RESTORE_TIMEOUT_MS, ); invalidateSimulatorStatusBarOverrideCache(device); } @@ -202,20 +212,20 @@ async function readSimulatorStatusBarOverrides( return parseSimulatorStatusBarOverrides(result.stdout); } -async function clearSimulatorStatusBarOverride(device: DeviceInfo): Promise { - await runSimctlForDevice(device, ['status_bar', device.id, 'clear'], { - timeoutMs: STATUS_BAR_COMMAND_TIMEOUT_MS, - }); +async function clearSimulatorStatusBarOverride( + device: DeviceInfo, + timeoutMs = STATUS_BAR_COMMAND_TIMEOUT_MS, +): Promise { + await runSimctlForDevice(device, ['status_bar', device.id, 'clear'], { timeoutMs }); } async function applySimulatorStatusBarOverrideArgs( device: DeviceInfo, args: string[], + timeoutMs = STATUS_BAR_COMMAND_TIMEOUT_MS, ): Promise { if (args.length === 0) return; - await runSimctlForDevice(device, ['status_bar', device.id, 'override', ...args], { - timeoutMs: STATUS_BAR_COMMAND_TIMEOUT_MS, - }); + await runSimctlForDevice(device, ['status_bar', device.id, 'override', ...args], { timeoutMs }); } function parseSimulatorStatusBarOverrides(output: string): RestorableStatusBarOverrides | null { From c69f2313556887e4cde907b5a14f7219f72cf1f8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Pierzcha=C5=82a?= Date: Sun, 1 Nov 2026 13:13:19 +0100 Subject: [PATCH 05/15] fix(ios): let an aborted request finish a capture leg inside its window A runner capture nested in the status bar window its own request held was cancelling on that request's abort signal, so a normalized capture that fell back to the runner after an abort never took the image while the window still restored a bar it had normalized for nothing. The reservation now drops the signal once the caller already holds the simulator's window, and the restore comment states that its budget applies per command. --- .../__tests__/screenshot-status-bar.test.ts | 37 +++++++++++++++++++ .../src/core/screenshot-status-bar.ts | 17 +++++++-- .../platform-apple/src/core/screenshot.ts | 3 +- 3 files changed, 52 insertions(+), 5 deletions(-) diff --git a/packages/platform-apple/src/core/__tests__/screenshot-status-bar.test.ts b/packages/platform-apple/src/core/__tests__/screenshot-status-bar.test.ts index 975728e00e..be972047e7 100644 --- a/packages/platform-apple/src/core/__tests__/screenshot-status-bar.test.ts +++ b/packages/platform-apple/src/core/__tests__/screenshot-status-bar.test.ts @@ -4,6 +4,7 @@ import { AppError } from '@agent-device/kernel/errors'; import { invalidateSimulatorStatusBarOverrideCache, withNormalizedSimulatorStatusBar, + withSimulatorStatusBarHeld, } from '../screenshot-status-bar.ts'; import { withFakeAppleTool, type FakeAppleToolResponse } from '../../__tests__/fake-apple-tool.ts'; import { IOS_TEST_SIMULATOR } from './apple-core-stub-helpers.ts'; @@ -68,6 +69,42 @@ test('withNormalizedSimulatorStatusBar restores prior visible overrides', async ); }); +test('a capture leg nested in its own status bar window keeps it after the request aborts', async () => { + // The sequence this pins: a normalized capture whose simctl leg fails falls back to the runner + // leg while the window is still held. The request may abort in between; the leg the caller is + // already inside must still run, and must not queue behind, or be rejected by, the window that + // caller holds. + const abortController = new AbortController(); + await withFakeAppleTool( + statusBarScript(() => 'Current Status Bar Overrides:\n============================='), + async ({ calls }) => { + const captured = await withNormalizedSimulatorStatusBar( + IOS_TEST_SIMULATOR, + async () => { + abortController.abort(new Error('caller gave up')); + return await withSimulatorStatusBarHeld( + IOS_TEST_SIMULATOR, + capture, + abortController.signal, + ); + }, + abortController.signal, + ); + + assert.equal(captured, 'captured', 'the leg inside the window must run'); + assert.deepEqual( + calls.map((args) => args.join(' ')), + [ + 'simctl status_bar sim-1 list', + `simctl status_bar sim-1 override ${NORMALIZE_ARGS}`, + 'simctl status_bar sim-1 clear', + ], + 'the window must still restore once the aborted leg finishes', + ); + }, + ); +}); + test('withNormalizedSimulatorStatusBar still normalizes when snapshotting current overrides fails', async () => { await withFakeAppleTool( statusBarScript(() => ({ stderr: 'list failed', exitCode: 1 })), diff --git a/packages/platform-apple/src/core/screenshot-status-bar.ts b/packages/platform-apple/src/core/screenshot-status-bar.ts index 6281a53edc..9ca3519a1f 100644 --- a/packages/platform-apple/src/core/screenshot-status-bar.ts +++ b/packages/platform-apple/src/core/screenshot-status-bar.ts @@ -2,7 +2,7 @@ import type { DeviceInfo } from '@agent-device/kernel/device'; import { requireExecSuccess } from '@agent-device/host-kit/command'; import { emitDiagnostic } from '@agent-device/host-kit/diagnostics'; import { AppError } from '@agent-device/kernel/errors'; -import { withKeyedLock } from '@agent-device/kernel/keyed-lock'; +import { heldKeyedLockKeys, withKeyedLock } from '@agent-device/kernel/keyed-lock'; import { runSimctlForDevice } from './simctl.ts'; import { extractAppleToolErrorMeta } from './tool-diagnostics.ts'; @@ -67,7 +67,9 @@ const STATUS_BAR_COMMAND_TIMEOUT_MS = 10_000; /** * Budget for the restore specifically. Leaving the normalization on the simulator is the failure * this owner exists to prevent, so the restore is given the longer deadline; it is still bounded, - * because an untimed wedged tool would hold the simulator's captures forever. + * because an untimed wedged tool would hold the simulator's captures forever. It applies to each + * restore command, so one stuck `clear` followed by one stuck `override` can hold this simulator's + * captures for twice this budget. */ const STATUS_BAR_RESTORE_TIMEOUT_MS = 30_000; @@ -119,17 +121,24 @@ export async function withNormalizedSimulatorStatusBar( * Reserves one simulator's status bar while `capture` runs, without changing the bar. A capture that * does not normalize still has to wait: while another capture normalizes the bar, this one would * otherwise take its screenshot of the temporary 9:41 overrides. + * + * `signal` cancels waiting for another capture. It is dropped when the caller already holds this + * simulator's window: that caller is the one who gave up, not an obstacle, so a leg of its own work + * — a normalized capture falling back to the runner — still runs, and its abort cannot leave the + * window it holds without that leg. */ export async function withSimulatorStatusBarHeld( device: DeviceInfo, capture: () => Promise, signal?: AbortSignal, ): Promise { + const key = statusBarSimulatorKey(device); + const holdsOwnWindow = heldKeyedLockKeys(simulatorStatusBarTransactions).includes(key); return await withKeyedLock( simulatorStatusBarTransactions, - statusBarSimulatorKey(device), + key, capture, - signal, + holdsOwnWindow ? undefined : signal, ); } diff --git a/packages/platform-apple/src/core/screenshot.ts b/packages/platform-apple/src/core/screenshot.ts index c19fccc650..4af589bd24 100644 --- a/packages/platform-apple/src/core/screenshot.ts +++ b/packages/platform-apple/src/core/screenshot.ts @@ -253,7 +253,8 @@ export async function captureSimulatorScreenshotWithRetry( * Captures through the runner backend. An iOS simulator's runner capture renders the same screen a * simctl capture does, so it takes the same status bar reservation: while another capture * normalizes the bar, this one would otherwise copy an image of the temporary 9:41 overrides. The - * reservation re-enters when the caller already holds it, as the simctl fallback does. + * reservation re-enters when the caller already holds it, as the simctl fallback does, and an + * already-aborted request does not cancel that re-entry — see `withSimulatorStatusBarHeld`. */ export async function captureScreenshotViaRunner( device: DeviceInfo, From e84c775eb352dd33111a26e1c0f326727b5e704d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Pierzcha=C5=82a?= Date: Sun, 1 Nov 2026 13:23:25 +0100 Subject: [PATCH 06/15] docs(ios): say the status bar hold keeps the request signal on the leg --- packages/platform-apple/src/core/screenshot-status-bar.ts | 4 ++-- packages/platform-apple/src/core/screenshot.ts | 3 ++- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/packages/platform-apple/src/core/screenshot-status-bar.ts b/packages/platform-apple/src/core/screenshot-status-bar.ts index 9ca3519a1f..94d548e16a 100644 --- a/packages/platform-apple/src/core/screenshot-status-bar.ts +++ b/packages/platform-apple/src/core/screenshot-status-bar.ts @@ -124,8 +124,8 @@ export async function withNormalizedSimulatorStatusBar( * * `signal` cancels waiting for another capture. It is dropped when the caller already holds this * simulator's window: that caller is the one who gave up, not an obstacle, so a leg of its own work - * — a normalized capture falling back to the runner — still runs, and its abort cannot leave the - * window it holds without that leg. + * is not rejected at the lock boundary and the window still restores what it normalized. The leg + * keeps the request's signal, so a request that has given up still stops as early as it can. */ export async function withSimulatorStatusBarHeld( device: DeviceInfo, diff --git a/packages/platform-apple/src/core/screenshot.ts b/packages/platform-apple/src/core/screenshot.ts index 4af589bd24..d1c8a36d08 100644 --- a/packages/platform-apple/src/core/screenshot.ts +++ b/packages/platform-apple/src/core/screenshot.ts @@ -254,7 +254,8 @@ export async function captureSimulatorScreenshotWithRetry( * simctl capture does, so it takes the same status bar reservation: while another capture * normalizes the bar, this one would otherwise copy an image of the temporary 9:41 overrides. The * reservation re-enters when the caller already holds it, as the simctl fallback does, and an - * already-aborted request does not cancel that re-entry — see `withSimulatorStatusBarHeld`. + * already-aborted request does not reject that re-entry at the lock boundary — see + * `withSimulatorStatusBarHeld`. The runner command keeps the request's signal. */ export async function captureScreenshotViaRunner( device: DeviceInfo, From 61a4a6ad7d2f8f992446233f5040e75b018ff4d0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Pierzcha=C5=82a?= Date: Sun, 1 Nov 2026 13:51:24 +0100 Subject: [PATCH 07/15] fix(ios): keep status bar writers and capture restores in one order A `settings wifi` or `settings airplane` write that landed while a normalized capture held the simulator's status bar was reverted by that capture's restore, which wrote back the bar it had snapshotted. Both writers now take the same per-simulator hold, so a setting lands after the restore instead of under it. A restore whose clear command fails now drops the known-cleared answer, because the simulator holds whatever the failed command left, and the next capture has to read the bar again. The runner paths read one named predicate each, and the kernel pins that heldKeyedLockKeys reports exactly the keys withKeyedLock would re-enter. --- packages/kernel/src/keyed-lock.test.ts | 34 +++++ .../screenshot-status-bar-concurrency.test.ts | 131 +++++++++++++++++- .../__tests__/screenshot-status-bar.test.ts | 33 +++++ .../platform-apple/src/core/app-settings.ts | 55 ++++---- .../src/core/screenshot-status-bar.ts | 24 +++- .../platform-apple/src/core/screenshot.ts | 22 ++- packages/platform-apple/src/interactor.ts | 4 +- 7 files changed, 263 insertions(+), 40 deletions(-) diff --git a/packages/kernel/src/keyed-lock.test.ts b/packages/kernel/src/keyed-lock.test.ts index 9b3a5cad29..75a30d05a7 100644 --- a/packages/kernel/src/keyed-lock.test.ts +++ b/packages/kernel/src/keyed-lock.test.ts @@ -312,6 +312,40 @@ test('heldKeyedLockKeys lists only the locks of the map held by the current cont }); }); +test('heldKeyedLockKeys reports exactly the keys withKeyedLock would re-enter for that context', async () => { + // A caller may decide behaviour from `heldKeyedLockKeys` before calling `withKeyedLock`, as the + // iOS status bar owner does. Both must answer from the same scan of the current context, or a + // caller that reads "held" could be queued behind, or reject at the boundary of, its own lock. + // Each reported key re-enters here while the frames that report it are still open, which a queued + // task could not do. + const locks = new Map>(); + const reEntered: string[] = []; + await withKeyedLock(locks, 'host', async () => { + await withKeyedLock(locks, 'host:app', async () => { + for (const key of heldKeyedLockKeys(locks)) { + await withKeyedLock(locks, key, async () => { + reEntered.push(key); + }); + } + }); + }); + assert.deepEqual(reEntered.sort(), ['host', 'host:app']); + + const holderReEntered: string[] = []; + await withKeyedLockHolder(async (holder) => { + const taking = holder.hold(locks, 'device:app'); + // A hold the holder is still taking reads as held, so a nested step may not queue behind it. + await taking; + for (const key of heldKeyedLockKeys(locks)) { + await withKeyedLock(locks, key, async () => { + holderReEntered.push(key); + }); + } + }); + assert.deepEqual(holderReEntered, ['device:app']); + assert.equal(locks.size, 0); +}); + test('a scope wanting a key its enclosing holder is still taking re-enters it once taken', async () => { const locks = new Map>(); let releaseOwner: () => void = () => {}; diff --git a/packages/platform-apple/src/core/__tests__/screenshot-status-bar-concurrency.test.ts b/packages/platform-apple/src/core/__tests__/screenshot-status-bar-concurrency.test.ts index 13b5e11bb6..b375ac7997 100644 --- a/packages/platform-apple/src/core/__tests__/screenshot-status-bar-concurrency.test.ts +++ b/packages/platform-apple/src/core/__tests__/screenshot-status-bar-concurrency.test.ts @@ -1,8 +1,17 @@ -import { beforeEach, test } from 'vitest'; +import { beforeEach, test, vi } from 'vitest'; import assert from 'node:assert/strict'; import type { DeviceInfo } from '@agent-device/kernel/device'; import { AppError } from '@agent-device/kernel/errors'; import { withFakeAppleTool } from '../../__tests__/fake-apple-tool.ts'; + +// `setIosSetting` boots through this owner; the capture route under test skips it, so the settings +// case below needs it quiet without touching the status bar owner. +vi.mock('../simulator.ts', async (importOriginal) => { + const actual = await importOriginal(); + return { ...actual, ensureBootedSimulator: async () => {} }; +}); + +import { setIosSetting } from '../app-settings.ts'; import { captureSimulatorScreenshotWithFallback } from '../screenshot.ts'; import { invalidateSimulatorStatusBarOverrideCache } from '../screenshot-status-bar.ts'; import { @@ -249,6 +258,126 @@ test('a single simulator capture normalizes while it captures and restores after }); }); +test('an already-aborted request does not start a plain simulator capture', async () => { + // Since the plain path takes the window it also honours the request's cancellation: a request the + // caller gave up on stops before it does device work nobody will read. + const statusBar = createFakeStatusBarSimctl(IOS_TEST_SIMULATOR.id, CALLER_OVERRIDES); + const abortController = new AbortController(); + abortController.abort(new Error('request canceled')); + let captureStarted = false; + + await withFakeAppleTool(statusBar.script, async () => { + const options = screenshotOptions(createCaptureGate(), false); + const settled = await captureSimulatorScreenshotWithFallback( + IOS_TEST_SIMULATOR, + '/tmp/aborted.png', + { + ...options, + deps: { + ...options.deps, + captureWithRetry: async () => { + captureStarted = true; + }, + }, + runnerOptions: { signal: abortController.signal }, + }, + ).then( + () => 'fulfilled', + (error: unknown) => (error instanceof Error ? error.message : String(error)), + ); + + assert.equal(settled, 'request canceled'); + assert.equal(captureStarted, false); + }); +}); + +test('an aborted request leaves the queue behind another request on the same simulator', async () => { + // Dropping the signal for a holder's own nested leg must stay caller-scoped: another request that + // merely shares the simulator keeps its cancellation, so it leaves the queue when its own caller + // gives up instead of waiting for the holder to finish. + const statusBar = createFakeStatusBarSimctl(IOS_TEST_SIMULATOR.id, CALLER_OVERRIDES); + const holderGate = createCaptureGate(); + const abortController = new AbortController(); + + await withFakeAppleTool(statusBar.script, async () => { + const holder = captureSimulatorScreenshotWithFallback( + IOS_TEST_SIMULATOR, + '/tmp/holder.png', + screenshotOptions(holderGate, true), + ); + await holderGate.entered; + + let queuedCaptureStarted = false; + const queued = captureSimulatorScreenshotWithFallback( + IOS_TEST_SIMULATOR, + '/tmp/aborted-queue.png', + { + env: process.env, + normalizeStatusBar: false, + skipIosSimulatorBootCheck: true, + deps: captureDeps(async () => { + queuedCaptureStarted = true; + }), + runnerOptions: { signal: abortController.signal }, + }, + ).catch((error: unknown) => error); + + await yieldToQueuedWork(); + abortController.abort(new Error('request canceled')); + const outcome = await queued; + holderGate.release(); + await holder; + + assert.ok( + outcome instanceof Error && outcome.message === 'request canceled', + `expected the queued capture to reject with the abort reason, received: ${String(outcome)}`, + ); + assert.equal(queuedCaptureStarted, false, 'the aborted capture must not reach the device'); + }); +}); + +test('a settings write to the status bar waits for the capture window', async () => { + // Its own simulator id: a lock this case leaves held must never make a later case wait for it. + const device: DeviceInfo = { ...IOS_TEST_SIMULATOR, id: 'sim-bar-writer' }; + invalidateSimulatorStatusBarOverrideCache(device); + const statusBar = createFakeStatusBarSimctl(device.id, { wifiMode: 'failed', wifiBars: '0' }); + + await withFakeAppleTool( + (args) => statusBar.script(args), + async ({ calls }) => { + const gate = createCaptureGate(); + const capturing = captureSimulatorScreenshotWithFallback( + device, + '/tmp/normalized.png', + screenshotOptions(gate, true), + ); + let settingDone = false; + let setting: Promise = Promise.resolve(); + try { + await gate.entered; + setting = setIosSetting(device, 'wifi', 'on', undefined, undefined, process.env).then( + () => { + settingDone = true; + }, + ); + await yieldToQueuedWork(); + assert.equal(settingDone, false, 'a status bar write must wait for the capture window'); + } finally { + gate.release(); + await Promise.allSettled([capturing, setting]); + } + + // The write lands after the capture restored the bar the caller had, so that restore cannot + // revert a setting the caller asked for while the capture was running. + assert.deepEqual( + calls.map((args) => args.join(' ')).at(-1), + 'simctl status_bar sim-bar-writer override --wifiMode active', + ); + assert.deepEqual(statusBar.overrides(), { wifiMode: 'active', wifiBars: '0' }); + }, + ); +}); + test('a plain simulator capture waits for the normalization window instead of capturing it', async () => { const statusBar = createFakeStatusBarSimctl(IOS_TEST_SIMULATOR.id, CALLER_OVERRIDES); const seenDuringPlainCapture: StatusBarOverrideState[] = []; diff --git a/packages/platform-apple/src/core/__tests__/screenshot-status-bar.test.ts b/packages/platform-apple/src/core/__tests__/screenshot-status-bar.test.ts index be972047e7..fdd026ea19 100644 --- a/packages/platform-apple/src/core/__tests__/screenshot-status-bar.test.ts +++ b/packages/platform-apple/src/core/__tests__/screenshot-status-bar.test.ts @@ -224,6 +224,39 @@ test('a failed status bar restore logs restore_failed and keeps the capture resu ]); }); +test('a failed status bar restore leaves no known-cleared answer for the next capture', async () => { + // "This simulator holds no overrides" is only true once a restore's `clear` actually completed. + // When it fails, the simulator holds whatever the failed command left, so the next capture must + // read the bar again instead of trusting that answer and normalizing without a clear. + const CLEARED_LISTING = 'Current Status Bar Overrides:\n============================='; + let clearAttempts = 0; + await withFakeAppleTool( + (args) => { + if (args[3] === 'list') return CLEARED_LISTING; + if (args[3] === 'clear') { + clearAttempts += 1; + if (clearAttempts === 1) return ''; + return new AppError('COMMAND_FAILED', 'clear timed out', { + exitCode: 1, + stderr: 'clear timed out', + }); + } + return ''; + }, + async ({ calls }) => { + // First capture: a clean restore earns the known-cleared answer. + await withNormalizedSimulatorStatusBar(IOS_TEST_SIMULATOR, capture); + calls.length = 0; + // Second capture: its restore fails, and that answer has to go with it. + await withNormalizedSimulatorStatusBar(IOS_TEST_SIMULATOR, capture); + calls.length = 0; + await withNormalizedSimulatorStatusBar(IOS_TEST_SIMULATOR, capture); + + assert.equal(calls[0]?.join(' '), 'simctl status_bar sim-1 list'); + }, + ); +}); + test('a failed status bar override logs prepare_failed and the capture still runs', async () => { const events = await statusBarDiagnosticEvents(async () => { await withFakeAppleTool( diff --git a/packages/platform-apple/src/core/app-settings.ts b/packages/platform-apple/src/core/app-settings.ts index 01c3f98768..35f698ab58 100644 --- a/packages/platform-apple/src/core/app-settings.ts +++ b/packages/platform-apple/src/core/app-settings.ts @@ -35,6 +35,7 @@ import { buildSimctlArgsForDevice, readSimctlContainerPath, runSimctlForDevice } import { invalidateSimulatorStatusBarOverrideCache, rememberClearedStatusBarOverrides, + withSimulatorStatusBarHeld, } from './screenshot-status-bar.ts'; import { ensureBootedSimulator, requireSimulatorDevice } from './simulator.ts'; import { runXcrun } from './tool-provider.ts'; @@ -104,35 +105,39 @@ export async function setIosSetting( case 'wifi': { const enabled = parseSettingState(state); const mode = enabled ? 'active' : 'failed'; - await runSimctlForDevice(device, ['status_bar', device.id, 'override', '--wifiMode', mode]); - invalidateSimulatorStatusBarOverrideCache(device); + await withSimulatorStatusBarHeld(device, async () => { + await runSimctlForDevice(device, ['status_bar', device.id, 'override', '--wifiMode', mode]); + invalidateSimulatorStatusBarOverrideCache(device); + }); return; } case 'airplane': { const enabled = parseSettingState(state); - if (enabled) { - await runSimctlForDevice(device, [ - 'status_bar', - device.id, - 'override', - '--dataNetwork', - 'hide', - '--wifiMode', - 'failed', - '--wifiBars', - '0', - '--cellularMode', - 'failed', - '--cellularBars', - '0', - '--operatorName', - '', - ]); - invalidateSimulatorStatusBarOverrideCache(device); - } else { - await runSimctlForDevice(device, ['status_bar', device.id, 'clear']); - rememberClearedStatusBarOverrides(device); - } + await withSimulatorStatusBarHeld(device, async () => { + if (enabled) { + await runSimctlForDevice(device, [ + 'status_bar', + device.id, + 'override', + '--dataNetwork', + 'hide', + '--wifiMode', + 'failed', + '--wifiBars', + '0', + '--cellularMode', + 'failed', + '--cellularBars', + '0', + '--operatorName', + '', + ]); + invalidateSimulatorStatusBarOverrideCache(device); + } else { + await runSimctlForDevice(device, ['status_bar', device.id, 'clear']); + rememberClearedStatusBarOverrides(device); + } + }); return; } case 'faceid': diff --git a/packages/platform-apple/src/core/screenshot-status-bar.ts b/packages/platform-apple/src/core/screenshot-status-bar.ts index 94d548e16a..8eab0457d3 100644 --- a/packages/platform-apple/src/core/screenshot-status-bar.ts +++ b/packages/platform-apple/src/core/screenshot-status-bar.ts @@ -174,17 +174,27 @@ async function restoreSimulatorStatusBarOverrides( device: DeviceInfo, overrides: RestorableStatusBarOverrides | null, ): Promise { - await clearSimulatorStatusBarOverride(device, STATUS_BAR_RESTORE_TIMEOUT_MS); + try { + await clearSimulatorStatusBarOverride(device, STATUS_BAR_RESTORE_TIMEOUT_MS); + } catch (error) { + // The simulator now holds whatever the timed-out command left, so no later capture may trust a + // cached answer about it: it has to read the bar again. + invalidateSimulatorStatusBarOverrideCache(device); + throw error; + } if (!overrides) { rememberClearedStatusBarOverrides(device); return; } - await applySimulatorStatusBarOverrideArgs( - device, - buildRestorableStatusBarOverrideArgs(overrides), - STATUS_BAR_RESTORE_TIMEOUT_MS, - ); - invalidateSimulatorStatusBarOverrideCache(device); + try { + await applySimulatorStatusBarOverrideArgs( + device, + buildRestorableStatusBarOverrideArgs(overrides), + STATUS_BAR_RESTORE_TIMEOUT_MS, + ); + } finally { + invalidateSimulatorStatusBarOverrideCache(device); + } } export function invalidateSimulatorStatusBarOverrideCache(device: DeviceInfo): void { diff --git a/packages/platform-apple/src/core/screenshot.ts b/packages/platform-apple/src/core/screenshot.ts index d1c8a36d08..ea62fd4e3a 100644 --- a/packages/platform-apple/src/core/screenshot.ts +++ b/packages/platform-apple/src/core/screenshot.ts @@ -249,13 +249,25 @@ export async function captureSimulatorScreenshotWithRetry( ); } +/** The runner renders this target's screen the way `simctl io screenshot` does, through the simulator. */ +function capturesThroughSimulatorStatusBar(device: DeviceInfo): boolean { + return device.kind === 'simulator' && !isMacOs(device); +} + +/** The runner reaches this target through the physical-device control rather than the runner app. */ +function capturesThroughPhysicalDeviceControl(device: DeviceInfo): boolean { + return device.kind === 'device' && !isMacOs(device); +} + /** * Captures through the runner backend. An iOS simulator's runner capture renders the same screen a * simctl capture does, so it takes the same status bar reservation: while another capture * normalizes the bar, this one would otherwise copy an image of the temporary 9:41 overrides. The - * reservation re-enters when the caller already holds it, as the simctl fallback does, and an - * already-aborted request does not reject that re-entry at the lock boundary — see - * `withSimulatorStatusBarHeld`. The runner command keeps the request's signal. + * reservation covers the whole capture, including the density normalization the capture flow applies + * afterwards, because that step reads the file the capture wrote. It re-enters when the caller + * already holds it, as the simctl fallback does, and an already-aborted request does not reject that + * re-entry at the lock boundary — see `withSimulatorStatusBarHeld`. The runner command keeps the + * request's signal. */ export async function captureScreenshotViaRunner( device: DeviceInfo, @@ -265,7 +277,7 @@ export async function captureScreenshotViaRunner( runnerOptions: AppleRunnerCommandOptions | undefined, env: Readonly, ): Promise { - if (device.kind !== 'simulator' || isMacOs(device)) { + if (!capturesThroughSimulatorStatusBar(device)) { return await captureRunnerScreenshot( device, outPath, @@ -291,7 +303,7 @@ async function captureRunnerScreenshot( runnerOptions: AppleRunnerCommandOptions | undefined, env: Readonly, ): Promise { - if (device.kind === 'device' && !isMacOs(device)) { + if (capturesThroughPhysicalDeviceControl(device)) { await resolveIosPhysicalDeviceControl(device).captureScreenshot(device, outPath, { appBundleId, fullscreen, diff --git a/packages/platform-apple/src/interactor.ts b/packages/platform-apple/src/interactor.ts index c35fcd70c9..d242cf6c90 100644 --- a/packages/platform-apple/src/interactor.ts +++ b/packages/platform-apple/src/interactor.ts @@ -513,8 +513,8 @@ async function runAppleScreenshot( return {}; } if (options.captureBackend === 'runner') { - // Runner capture returns the XCTest surface as-is; density and simulator - // status-bar normalization belong only to the simctl capture pipeline. + // Runner capture returns the XCTest surface as-is; density normalization belongs only to the + // simctl capture pipeline. The capture still waits for the simulator's status bar window. await captureScreenshotViaRunner( device, outPath, From b02d6724b354159beb39d77694e4524d63b8117d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Pierzcha=C5=82a?= Date: Sun, 1 Nov 2026 14:01:53 +0100 Subject: [PATCH 08/15] fix(ios): keep the cleared status bar answer when a restore clear fails A restore whose clear command fails leaves the deterministic bar on the simulator. Reading the bar again would make the next capture adopt that normalization as the caller's state and hand it back on every later capture, so the answer that says the bar is cleared now survives the failure: the next capture skips the read and its own restore clears the leftover. The kernel test checks the pending hold before it is awaited, and the runner branch comment names the targets that take a status bar window. --- packages/kernel/src/keyed-lock.test.ts | 12 ++++---- .../__tests__/screenshot-status-bar.test.ts | 24 +++++++++++---- .../src/core/screenshot-status-bar.ts | 29 ++++++++----------- packages/platform-apple/src/interactor.ts | 3 +- 4 files changed, 38 insertions(+), 30 deletions(-) diff --git a/packages/kernel/src/keyed-lock.test.ts b/packages/kernel/src/keyed-lock.test.ts index 75a30d05a7..dc40292f87 100644 --- a/packages/kernel/src/keyed-lock.test.ts +++ b/packages/kernel/src/keyed-lock.test.ts @@ -334,13 +334,13 @@ test('heldKeyedLockKeys reports exactly the keys withKeyedLock would re-enter fo const holderReEntered: string[] = []; await withKeyedLockHolder(async (holder) => { const taking = holder.hold(locks, 'device:app'); - // A hold the holder is still taking reads as held, so a nested step may not queue behind it. + // While the hold is still being taken it already reads as held, and a nested step must not queue + // behind it, so the report and the re-entry are both checked before anything is awaited. + assert.deepEqual(heldKeyedLockKeys(locks), ['device:app']); + await withKeyedLock(locks, 'device:app', async () => { + holderReEntered.push('device:app'); + }); await taking; - for (const key of heldKeyedLockKeys(locks)) { - await withKeyedLock(locks, key, async () => { - holderReEntered.push(key); - }); - } }); assert.deepEqual(holderReEntered, ['device:app']); assert.equal(locks.size, 0); diff --git a/packages/platform-apple/src/core/__tests__/screenshot-status-bar.test.ts b/packages/platform-apple/src/core/__tests__/screenshot-status-bar.test.ts index fdd026ea19..15d3880c80 100644 --- a/packages/platform-apple/src/core/__tests__/screenshot-status-bar.test.ts +++ b/packages/platform-apple/src/core/__tests__/screenshot-status-bar.test.ts @@ -224,10 +224,11 @@ test('a failed status bar restore logs restore_failed and keeps the capture resu ]); }); -test('a failed status bar restore leaves no known-cleared answer for the next capture', async () => { - // "This simulator holds no overrides" is only true once a restore's `clear` actually completed. - // When it fails, the simulator holds whatever the failed command left, so the next capture must - // read the bar again instead of trusting that answer and normalizing without a clear. +test('a failed status bar restore keeps the known-cleared answer so the next capture re-clears', async () => { + // A restore whose `clear` fails leaves the deterministic bar on the simulator. Reading the bar + // then would make the next capture adopt that normalization as the caller's state and hand it back + // forever, so the answer that says "this bar is cleared" survives: the next capture skips the read + // and its own restore clears the leftover. const CLEARED_LISTING = 'Current Status Bar Overrides:\n============================='; let clearAttempts = 0; await withFakeAppleTool( @@ -247,12 +248,23 @@ test('a failed status bar restore leaves no known-cleared answer for the next ca // First capture: a clean restore earns the known-cleared answer. await withNormalizedSimulatorStatusBar(IOS_TEST_SIMULATOR, capture); calls.length = 0; - // Second capture: its restore fails, and that answer has to go with it. + // Second capture: its restore fails and leaves the deterministic bar on the simulator. await withNormalizedSimulatorStatusBar(IOS_TEST_SIMULATOR, capture); + assert.match(calls[0]?.join(' ') ?? '', /^simctl status_bar sim-1 override/); + assert.equal( + calls.some((args) => args[3] === 'list'), + false, + 'a failed restore must not send the next capture to read the leftover back', + ); calls.length = 0; + await withNormalizedSimulatorStatusBar(IOS_TEST_SIMULATOR, capture); - assert.equal(calls[0]?.join(' '), 'simctl status_bar sim-1 list'); + assert.deepEqual( + calls.map((args) => args.join(' ')), + [`simctl status_bar sim-1 override ${NORMALIZE_ARGS}`, 'simctl status_bar sim-1 clear'], + 'the next capture must trust the cleared answer and clear the leftover, never read it back', + ); }, ); }); diff --git a/packages/platform-apple/src/core/screenshot-status-bar.ts b/packages/platform-apple/src/core/screenshot-status-bar.ts index 8eab0457d3..298c422255 100644 --- a/packages/platform-apple/src/core/screenshot-status-bar.ts +++ b/packages/platform-apple/src/core/screenshot-status-bar.ts @@ -174,27 +174,22 @@ async function restoreSimulatorStatusBarOverrides( device: DeviceInfo, overrides: RestorableStatusBarOverrides | null, ): Promise { - try { - await clearSimulatorStatusBarOverride(device, STATUS_BAR_RESTORE_TIMEOUT_MS); - } catch (error) { - // The simulator now holds whatever the timed-out command left, so no later capture may trust a - // cached answer about it: it has to read the bar again. - invalidateSimulatorStatusBarOverrideCache(device); - throw error; - } + // Only a restore that ran to the end may change the known-cleared answer. A `clear` that fails + // leaves the normalization on the bar, and the answer that says "this bar is cleared" is the one + // that fixes it: the next capture skips the read, so it cannot adopt the leftover as the caller's + // state, and its own restore clears the bar. Reading the bar after a failed clear would do the + // opposite — adopt the leftover and hand it back on every later capture. + await clearSimulatorStatusBarOverride(device, STATUS_BAR_RESTORE_TIMEOUT_MS); if (!overrides) { rememberClearedStatusBarOverrides(device); return; } - try { - await applySimulatorStatusBarOverrideArgs( - device, - buildRestorableStatusBarOverrideArgs(overrides), - STATUS_BAR_RESTORE_TIMEOUT_MS, - ); - } finally { - invalidateSimulatorStatusBarOverrideCache(device); - } + await applySimulatorStatusBarOverrideArgs( + device, + buildRestorableStatusBarOverrideArgs(overrides), + STATUS_BAR_RESTORE_TIMEOUT_MS, + ); + invalidateSimulatorStatusBarOverrideCache(device); } export function invalidateSimulatorStatusBarOverrideCache(device: DeviceInfo): void { diff --git a/packages/platform-apple/src/interactor.ts b/packages/platform-apple/src/interactor.ts index d242cf6c90..a7a1d31a7d 100644 --- a/packages/platform-apple/src/interactor.ts +++ b/packages/platform-apple/src/interactor.ts @@ -514,7 +514,8 @@ async function runAppleScreenshot( } if (options.captureBackend === 'runner') { // Runner capture returns the XCTest surface as-is; density normalization belongs only to the - // simctl capture pipeline. The capture still waits for the simulator's status bar window. + // simctl capture pipeline. Of this branch's targets, only an iOS simulator capture waits for a + // status bar window: physical-device and macOS captures take none. await captureScreenshotViaRunner( device, outPath, From af11ed6661a1e3ddf3ce8c0654bfbd0591c315eb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Pierzcha=C5=82a?= Date: Sun, 1 Nov 2026 14:16:24 +0100 Subject: [PATCH 09/15] fix(ios): name the status bar window by the whole simulator address `simctl status_bar` runs scoped to a device set, so one udid in two sets is two bars. The transaction and cache key now carry the resolved set path, so records of one simulator still share one window while simulators of different sets keep their own. The settings writes take the request's signal, so a write queued behind a capture window can still be cancelled, and the screenshot help states that an iOS simulator capture waits for the bar and reports cancellation. --- .../command-surface/src/capture/screenshot.ts | 2 +- .../screenshot-status-bar-concurrency.test.ts | 87 +++++++++++++++++++ .../platform-apple/src/core/app-settings.ts | 69 +++++++++------ .../src/core/screenshot-status-bar.ts | 12 +-- packages/platform-apple/src/interactor.ts | 2 +- 5 files changed, 139 insertions(+), 33 deletions(-) diff --git a/packages/command-surface/src/capture/screenshot.ts b/packages/command-surface/src/capture/screenshot.ts index ed6cc0c393..b01468ea30 100644 --- a/packages/command-surface/src/capture/screenshot.ts +++ b/packages/command-surface/src/capture/screenshot.ts @@ -74,7 +74,7 @@ export const screenshotCommandFacet = defineCommandFacet({ text: { summary: 'Capture a screenshot', cliDetail: - "Web defaults to the viewport; use --fullscreen, --full, or -f for the entire page. iOS simulators default to 1x logical-point output; use --pixel-density to request a different screenshot density. macOS app sessions default to the app window; use --fullscreen for full desktop, --scale to downscale, --crop-on to crop the capture to the frame the selector resolves on the same screen (currently iOS simulators, Android emulators, and CoreDevice physical iPhones), --overlay-refs to annotate current refs, --normalize-status-bar for deterministic iOS simulator or Android chrome. Physical iPhone crop verification covers portrait CoreDevice/devicectl captures only. XCTest, runner fallback, and physical iPad crops are refused; landscape iPhone crop is unverified. Android captures immediately by default; --normalize-status-bar also waits for transient UI to fade, and --no-stabilize keeps capture immediate. With the native macOS backend, the front window is the app's frontmost visible, nonempty normal-level (layer-0) window on the current Space: floating palettes and app-modal alerts at window level 8 are skipped, and a sheet is captured as its own window. On macOS, any --surface other than app (desktop, menubar, frontmost-app) always captures the main display and refuses an explicit --fullscreen. On single-display iOS simulators and Android devices, the result reports displayRotation, the rotation the display rendered the capture in, when the device returns a reading in time; otherwise the field is absent.", + "Web defaults to the viewport; use --fullscreen, --full, or -f for the entire page. iOS simulators default to 1x logical-point output; use --pixel-density to request a different screenshot density. macOS app sessions default to the app window; use --fullscreen for full desktop, --scale to downscale, --crop-on to crop the capture to the frame the selector resolves on the same screen (currently iOS simulators, Android emulators, and CoreDevice physical iPhones), --overlay-refs to annotate current refs, --normalize-status-bar for deterministic iOS simulator or Android chrome. Physical iPhone crop verification covers portrait CoreDevice/devicectl captures only. XCTest, runner fallback, and physical iPad crops are refused; landscape iPhone crop is unverified. Android captures immediately by default; --normalize-status-bar also waits for transient UI to fade, and --no-stabilize keeps capture immediate. With the native macOS backend, the front window is the app's frontmost visible, nonempty normal-level (layer-0) window on the current Space: floating palettes and app-modal alerts at window level 8 are skipped, and a sheet is captured as its own window. On macOS, any --surface other than app (desktop, menubar, frontmost-app) always captures the main display and refuses an explicit --fullscreen. On single-display iOS simulators and Android devices, the result reports displayRotation, the rotation the display rendered the capture in, when the device returns a reading in time; otherwise the field is absent. On iOS simulators a screenshot waits for that simulator's status bar to be free, and a request its caller cancelled reports the cancellation instead of capturing.", }, metadata: screenshotCommandMetadata, run: (client, input) => client.capture.screenshot(input), diff --git a/packages/platform-apple/src/core/__tests__/screenshot-status-bar-concurrency.test.ts b/packages/platform-apple/src/core/__tests__/screenshot-status-bar-concurrency.test.ts index b375ac7997..582dc8abbb 100644 --- a/packages/platform-apple/src/core/__tests__/screenshot-status-bar-concurrency.test.ts +++ b/packages/platform-apple/src/core/__tests__/screenshot-status-bar-concurrency.test.ts @@ -378,6 +378,93 @@ test('a settings write to the status bar waits for the capture window', async () ); }); +test('a settings write queued behind a capture window can still be cancelled', async () => { + const device: DeviceInfo = { ...IOS_TEST_SIMULATOR, id: 'sim-bar-cancel' }; + invalidateSimulatorStatusBarOverrideCache(device); + const statusBar = createFakeStatusBarSimctl(device.id, { wifiMode: 'failed', wifiBars: '0' }); + const abortController = new AbortController(); + + await withFakeAppleTool(statusBar.script, async () => { + const gate = createCaptureGate(); + const capturing = captureSimulatorScreenshotWithFallback( + device, + '/tmp/normalized.png', + screenshotOptions(gate, true), + ); + let outcome: unknown = 'still running'; + try { + await gate.entered; + const setting = setIosSetting( + device, + 'wifi', + 'on', + undefined, + undefined, + process.env, + abortController.signal, + ).catch((error: unknown) => error); + await yieldToQueuedWork(); + abortController.abort(new Error('request canceled')); + outcome = await setting; + assert.ok( + outcome instanceof Error && outcome.message === 'request canceled', + `expected the queued write to reject, received: ${String(outcome)}`, + ); + } finally { + gate.release(); + await Promise.allSettled([capturing]); + } + + assert.deepEqual(statusBar.overrides(), { wifiMode: 'failed', wifiBars: '0' }); + }); +}); + +test('simulators of different device sets keep their own status bar windows', async () => { + // `simctl status_bar` is scoped with `--set`, so one udid in two sets is two bars. Records that + // name one simulator — same udid and same set — share a window; different sets must not queue. + const entered: string[] = []; + const gates = { one: createCaptureGate(), two: createCaptureGate() }; + const deviceFor = (tag: keyof typeof gates): DeviceInfo => ({ + ...IOS_TEST_SIMULATOR, + simulatorSetPath: `/sets/${tag}`, + }); + + await withFakeAppleTool( + (args) => { + // The `--set` scope lands before `status_bar`, so locate the verb rather than a fixed index. + const index = args.indexOf('status_bar'); + if (index === -1) return { stderr: `unexpected args: ${args.join(' ')}`, exitCode: 1 }; + const verb = args[index + 2]; + return verb === 'list' ? 'Current Status Bar Overrides:\n=============================' : ''; + }, + async () => { + const captureFor = (tag: keyof typeof gates) => + captureSimulatorScreenshotWithFallback(deviceFor(tag), `/tmp/${tag}.png`, { + env: process.env, + normalizeStatusBar: true, + skipIosSimulatorBootCheck: true, + deps: captureDeps(async () => { + entered.push(tag); + await gates[tag].capture(); + }), + }); + + const first = captureFor('one'); + await gates.one.entered; + const second = captureFor('two'); + try { + await yieldToQueuedWork(); + // Both capture steps run at once: the second set never queued behind the first. + assert.deepEqual(entered, ['one', 'two']); + } finally { + gates.one.release(); + gates.two.release(); + await Promise.allSettled([first, second]); + } + }, + ); +}); + test('a plain simulator capture waits for the normalization window instead of capturing it', async () => { const statusBar = createFakeStatusBarSimctl(IOS_TEST_SIMULATOR.id, CALLER_OVERRIDES); const seenDuringPlainCapture: StatusBarOverrideState[] = []; diff --git a/packages/platform-apple/src/core/app-settings.ts b/packages/platform-apple/src/core/app-settings.ts index 35f698ab58..3c155d8a79 100644 --- a/packages/platform-apple/src/core/app-settings.ts +++ b/packages/platform-apple/src/core/app-settings.ts @@ -67,6 +67,7 @@ export async function setIosSetting( appBundleId: string | undefined, options: SettingOptions | undefined, env: Readonly, + signal?: AbortSignal, ): Promise | void> { if (isMacOs(device)) return await setMacOsHostSetting(setting, state, options, env); requireSimulatorDevice(device, 'settings'); @@ -105,39 +106,55 @@ export async function setIosSetting( case 'wifi': { const enabled = parseSettingState(state); const mode = enabled ? 'active' : 'failed'; - await withSimulatorStatusBarHeld(device, async () => { - await runSimctlForDevice(device, ['status_bar', device.id, 'override', '--wifiMode', mode]); - invalidateSimulatorStatusBarOverrideCache(device); - }); - return; - } - case 'airplane': { - const enabled = parseSettingState(state); - await withSimulatorStatusBarHeld(device, async () => { - if (enabled) { + // The write waits for the simulator's status bar window: a capture restores the bar it + // snapshotted, so a write that landed inside that window would be reverted by that restore. + await withSimulatorStatusBarHeld( + device, + async () => { await runSimctlForDevice(device, [ 'status_bar', device.id, 'override', - '--dataNetwork', - 'hide', '--wifiMode', - 'failed', - '--wifiBars', - '0', - '--cellularMode', - 'failed', - '--cellularBars', - '0', - '--operatorName', - '', + mode, ]); invalidateSimulatorStatusBarOverrideCache(device); - } else { - await runSimctlForDevice(device, ['status_bar', device.id, 'clear']); - rememberClearedStatusBarOverrides(device); - } - }); + }, + signal, + ); + return; + } + case 'airplane': { + const enabled = parseSettingState(state); + await withSimulatorStatusBarHeld( + device, + async () => { + if (enabled) { + await runSimctlForDevice(device, [ + 'status_bar', + device.id, + 'override', + '--dataNetwork', + 'hide', + '--wifiMode', + 'failed', + '--wifiBars', + '0', + '--cellularMode', + 'failed', + '--cellularBars', + '0', + '--operatorName', + '', + ]); + invalidateSimulatorStatusBarOverrideCache(device); + } else { + await runSimctlForDevice(device, ['status_bar', device.id, 'clear']); + rememberClearedStatusBarOverrides(device); + } + }, + signal, + ); return; } case 'faceid': diff --git a/packages/platform-apple/src/core/screenshot-status-bar.ts b/packages/platform-apple/src/core/screenshot-status-bar.ts index 298c422255..5cbc1b9add 100644 --- a/packages/platform-apple/src/core/screenshot-status-bar.ts +++ b/packages/platform-apple/src/core/screenshot-status-bar.ts @@ -4,7 +4,7 @@ import { emitDiagnostic } from '@agent-device/host-kit/diagnostics'; import { AppError } from '@agent-device/kernel/errors'; import { heldKeyedLockKeys, withKeyedLock } from '@agent-device/kernel/keyed-lock'; -import { runSimctlForDevice } from './simctl.ts'; +import { runSimctlForDevice, simulatorAddressFor } from './simctl.ts'; import { extractAppleToolErrorMeta } from './tool-diagnostics.ts'; type RestorableStatusBarOverrides = Partial< @@ -205,12 +205,14 @@ function hasKnownClearedStatusBarOverrides(device: DeviceInfo): boolean { } /** - * Key for both the known-cleared cache and the transaction lock. It names a simulator by platform, - * kind and id, and leaves out the device set, so two records of one simulator share a key and one - * transaction instead of normalizing the same status bar at once. + * Key for both the known-cleared cache and the transaction lock. It names the simulator's whole + * address, because that is the thing whose bar the commands write: `runSimctlForDevice` scopes every + * status bar command to the device set, so two records of one simulator share one key, and one + * simulator of each set gets its own transaction. */ function statusBarSimulatorKey(device: DeviceInfo): string { - return `${device.platform}:${device.kind}:${device.id}`; + const { simulatorSetPath } = simulatorAddressFor(device); + return `${device.platform}:${device.kind}:${device.id}:${simulatorSetPath ?? ''}`; } async function readSimulatorStatusBarOverrides( diff --git a/packages/platform-apple/src/interactor.ts b/packages/platform-apple/src/interactor.ts index a7a1d31a7d..87b3e40013 100644 --- a/packages/platform-apple/src/interactor.ts +++ b/packages/platform-apple/src/interactor.ts @@ -231,7 +231,7 @@ export function createAppleInteractor( readClipboard: () => readIosClipboardText(device), writeClipboard: (text) => writeIosClipboardText(device, text, runnerOpts), setSetting: (setting, state, appId, options) => - setIosSetting(device, setting, state, appId, options, env), + setIosSetting(device, setting, state, appId, options, env, runnerOpts.signal), readSetting: (setting) => readIosSetting(device, setting), readAlert: (options) => readAppleAlert(device, runnerOpts, options, env), awaitAlert: (options) => awaitAppleAlert(device, runnerOpts, options, env), From 97a10902152e4153366355b87cdc4ed39758ee66 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Pierzcha=C5=82a?= Date: Sun, 1 Nov 2026 14:39:11 +0100 Subject: [PATCH 10/15] fix(ios): bound the settings writes that hold the status bar window The wifi and airplane writes now hold the per-simulator window every iOS capture queues behind, so a wedged `simctl status_bar` inside that hold would stop those captures. Each of the three commands keeps the same per-command budget the capture's own status bar commands use, asserted by a new case in the concurrency file. The settings cancellation case releases its capture gate before awaiting the write, so a regression reports its assertion instead of hanging, and the screenshot help says cancellation covers the wait for the bar. --- .../command-surface/src/capture/screenshot.ts | 2 +- .../screenshot-status-bar-concurrency.test.ts | 80 +++++++++++++------ .../platform-apple/src/core/app-settings.ts | 57 +++++++------ .../src/core/screenshot-status-bar.ts | 2 +- 4 files changed, 91 insertions(+), 50 deletions(-) diff --git a/packages/command-surface/src/capture/screenshot.ts b/packages/command-surface/src/capture/screenshot.ts index b01468ea30..2fd871e96d 100644 --- a/packages/command-surface/src/capture/screenshot.ts +++ b/packages/command-surface/src/capture/screenshot.ts @@ -74,7 +74,7 @@ export const screenshotCommandFacet = defineCommandFacet({ text: { summary: 'Capture a screenshot', cliDetail: - "Web defaults to the viewport; use --fullscreen, --full, or -f for the entire page. iOS simulators default to 1x logical-point output; use --pixel-density to request a different screenshot density. macOS app sessions default to the app window; use --fullscreen for full desktop, --scale to downscale, --crop-on to crop the capture to the frame the selector resolves on the same screen (currently iOS simulators, Android emulators, and CoreDevice physical iPhones), --overlay-refs to annotate current refs, --normalize-status-bar for deterministic iOS simulator or Android chrome. Physical iPhone crop verification covers portrait CoreDevice/devicectl captures only. XCTest, runner fallback, and physical iPad crops are refused; landscape iPhone crop is unverified. Android captures immediately by default; --normalize-status-bar also waits for transient UI to fade, and --no-stabilize keeps capture immediate. With the native macOS backend, the front window is the app's frontmost visible, nonempty normal-level (layer-0) window on the current Space: floating palettes and app-modal alerts at window level 8 are skipped, and a sheet is captured as its own window. On macOS, any --surface other than app (desktop, menubar, frontmost-app) always captures the main display and refuses an explicit --fullscreen. On single-display iOS simulators and Android devices, the result reports displayRotation, the rotation the display rendered the capture in, when the device returns a reading in time; otherwise the field is absent. On iOS simulators a screenshot waits for that simulator's status bar to be free, and a request its caller cancelled reports the cancellation instead of capturing.", + "Web defaults to the viewport; use --fullscreen, --full, or -f for the entire page. iOS simulators default to 1x logical-point output; use --pixel-density to request a different screenshot density. macOS app sessions default to the app window; use --fullscreen for full desktop, --scale to downscale, --crop-on to crop the capture to the frame the selector resolves on the same screen (currently iOS simulators, Android emulators, and CoreDevice physical iPhones), --overlay-refs to annotate current refs, --normalize-status-bar for deterministic iOS simulator or Android chrome. Physical iPhone crop verification covers portrait CoreDevice/devicectl captures only. XCTest, runner fallback, and physical iPad crops are refused; landscape iPhone crop is unverified. Android captures immediately by default; --normalize-status-bar also waits for transient UI to fade, and --no-stabilize keeps capture immediate. With the native macOS backend, the front window is the app's frontmost visible, nonempty normal-level (layer-0) window on the current Space: floating palettes and app-modal alerts at window level 8 are skipped, and a sheet is captured as its own window. On macOS, any --surface other than app (desktop, menubar, frontmost-app) always captures the main display and refuses an explicit --fullscreen. On single-display iOS simulators and Android devices, the result reports displayRotation, the rotation the display rendered the capture in, when the device returns a reading in time; otherwise the field is absent. On iOS simulators a screenshot waits for that simulator's status bar to be free, and a request its caller cancelled while it waits reports the cancellation instead of capturing.", }, metadata: screenshotCommandMetadata, run: (client, input) => client.capture.screenshot(input), diff --git a/packages/platform-apple/src/core/__tests__/screenshot-status-bar-concurrency.test.ts b/packages/platform-apple/src/core/__tests__/screenshot-status-bar-concurrency.test.ts index 582dc8abbb..2e548502a6 100644 --- a/packages/platform-apple/src/core/__tests__/screenshot-status-bar-concurrency.test.ts +++ b/packages/platform-apple/src/core/__tests__/screenshot-status-bar-concurrency.test.ts @@ -3,6 +3,7 @@ import assert from 'node:assert/strict'; import type { DeviceInfo } from '@agent-device/kernel/device'; import { AppError } from '@agent-device/kernel/errors'; import { withFakeAppleTool } from '../../__tests__/fake-apple-tool.ts'; +import { createLocalAppleToolProvider, withAppleToolProvider } from '../tool-provider.ts'; // `setIosSetting` boots through this owner; the capture route under test skips it, so the settings // case below needs it quiet without touching the status bar owner. @@ -13,7 +14,10 @@ vi.mock('../simulator.ts', async (importOriginal) => { import { setIosSetting } from '../app-settings.ts'; import { captureSimulatorScreenshotWithFallback } from '../screenshot.ts'; -import { invalidateSimulatorStatusBarOverrideCache } from '../screenshot-status-bar.ts'; +import { + invalidateSimulatorStatusBarOverrideCache, + STATUS_BAR_COMMAND_TIMEOUT_MS, +} from '../screenshot-status-bar.ts'; import { createFakeStatusBarSimctl, type FakeStatusBarSimctl, @@ -392,33 +396,63 @@ test('a settings write queued behind a capture window can still be cancelled', a screenshotOptions(gate, true), ); let outcome: unknown = 'still running'; - try { - await gate.entered; - const setting = setIosSetting( - device, - 'wifi', - 'on', - undefined, - undefined, - process.env, - abortController.signal, - ).catch((error: unknown) => error); - await yieldToQueuedWork(); - abortController.abort(new Error('request canceled')); - outcome = await setting; - assert.ok( - outcome instanceof Error && outcome.message === 'request canceled', - `expected the queued write to reject, received: ${String(outcome)}`, - ); - } finally { - gate.release(); - await Promise.allSettled([capturing]); - } + await gate.entered; + const setting = setIosSetting( + device, + 'wifi', + 'on', + undefined, + undefined, + process.env, + abortController.signal, + ).catch((error: unknown) => error); + await yieldToQueuedWork(); + abortController.abort(new Error('request canceled')); + // Released before the wait: if the cancellation were ignored, the write finishes and the + // assertion below reports that, instead of the case hanging on the gate until its timeout. + gate.release(); + outcome = await setting; + await capturing; + assert.ok( + outcome instanceof Error && outcome.message === 'request canceled', + `expected the queued write to reject, received: ${String(outcome)}`, + ); assert.deepEqual(statusBar.overrides(), { wifiMode: 'failed', wifiBars: '0' }); }); }); +test('a settings write bounds its status bar command like a capture does', async () => { + // The write holds the simulator's window, so a wedged `simctl status_bar` inside it would hold + // every later capture of that simulator. It keeps the same per-command budget the capture's own + // status bar commands use. + const budgets: Array = []; + const device: DeviceInfo = { ...IOS_TEST_SIMULATOR, id: 'sim-bar-budget' }; + invalidateSimulatorStatusBarOverrideCache(device); + + await withAppleToolProvider( + createLocalAppleToolProvider({ + simctl: { + run: async (args, options) => { + if (args.includes('status_bar')) budgets.push(options?.timeoutMs); + return { stdout: '', stderr: '', exitCode: 0 }; + }, + }, + }), + async () => { + await setIosSetting(device, 'wifi', 'on', undefined, undefined, process.env); + await setIosSetting(device, 'airplane', 'on', undefined, undefined, process.env); + await setIosSetting(device, 'airplane', 'off', undefined, undefined, process.env); + }, + ); + + assert.deepEqual(budgets, [ + STATUS_BAR_COMMAND_TIMEOUT_MS, + STATUS_BAR_COMMAND_TIMEOUT_MS, + STATUS_BAR_COMMAND_TIMEOUT_MS, + ]); +}); + test('simulators of different device sets keep their own status bar windows', async () => { // `simctl status_bar` is scoped with `--set`, so one udid in two sets is two bars. Records that // name one simulator — same udid and same set — share a window; different sets must not queue. diff --git a/packages/platform-apple/src/core/app-settings.ts b/packages/platform-apple/src/core/app-settings.ts index 3c155d8a79..5bed13b320 100644 --- a/packages/platform-apple/src/core/app-settings.ts +++ b/packages/platform-apple/src/core/app-settings.ts @@ -35,6 +35,7 @@ import { buildSimctlArgsForDevice, readSimctlContainerPath, runSimctlForDevice } import { invalidateSimulatorStatusBarOverrideCache, rememberClearedStatusBarOverrides, + STATUS_BAR_COMMAND_TIMEOUT_MS, withSimulatorStatusBarHeld, } from './screenshot-status-bar.ts'; import { ensureBootedSimulator, requireSimulatorDevice } from './simulator.ts'; @@ -111,13 +112,11 @@ export async function setIosSetting( await withSimulatorStatusBarHeld( device, async () => { - await runSimctlForDevice(device, [ - 'status_bar', - device.id, - 'override', - '--wifiMode', - mode, - ]); + await runSimctlForDevice( + device, + ['status_bar', device.id, 'override', '--wifiMode', mode], + { timeoutMs: STATUS_BAR_COMMAND_TIMEOUT_MS }, + ); invalidateSimulatorStatusBarOverrideCache(device); }, signal, @@ -126,30 +125,38 @@ export async function setIosSetting( } case 'airplane': { const enabled = parseSettingState(state); + // Same reason as `wifi`: the write waits for the capture window so a restore cannot undo it, + // and the command is bounded so a wedged `simctl` cannot hold the simulator's captures. await withSimulatorStatusBarHeld( device, async () => { if (enabled) { - await runSimctlForDevice(device, [ - 'status_bar', - device.id, - 'override', - '--dataNetwork', - 'hide', - '--wifiMode', - 'failed', - '--wifiBars', - '0', - '--cellularMode', - 'failed', - '--cellularBars', - '0', - '--operatorName', - '', - ]); + await runSimctlForDevice( + device, + [ + 'status_bar', + device.id, + 'override', + '--dataNetwork', + 'hide', + '--wifiMode', + 'failed', + '--wifiBars', + '0', + '--cellularMode', + 'failed', + '--cellularBars', + '0', + '--operatorName', + '', + ], + { timeoutMs: STATUS_BAR_COMMAND_TIMEOUT_MS }, + ); invalidateSimulatorStatusBarOverrideCache(device); } else { - await runSimctlForDevice(device, ['status_bar', device.id, 'clear']); + await runSimctlForDevice(device, ['status_bar', device.id, 'clear'], { + timeoutMs: STATUS_BAR_COMMAND_TIMEOUT_MS, + }); rememberClearedStatusBarOverrides(device); } }, diff --git a/packages/platform-apple/src/core/screenshot-status-bar.ts b/packages/platform-apple/src/core/screenshot-status-bar.ts index 5cbc1b9add..e04ff9ea54 100644 --- a/packages/platform-apple/src/core/screenshot-status-bar.ts +++ b/packages/platform-apple/src/core/screenshot-status-bar.ts @@ -62,7 +62,7 @@ const devicesKnownWithoutStatusBarOverrides = new Set(); const simulatorStatusBarTransactions = new Map>(); /** Bounds one `simctl status_bar` step so a wedged tool cannot hold a simulator's captures. */ -const STATUS_BAR_COMMAND_TIMEOUT_MS = 10_000; +export const STATUS_BAR_COMMAND_TIMEOUT_MS = 10_000; /** * Budget for the restore specifically. Leaving the normalization on the simulator is the failure From 6d59c2b3e5680fd7ea84ce09891b610c3fd1a923 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Pierzcha=C5=82a?= Date: Sun, 1 Nov 2026 14:43:44 +0100 Subject: [PATCH 11/15] test(ios): require the settings status bar command to carry its budget --- .../core/__tests__/screenshot-status-bar-concurrency.test.ts | 3 +++ 1 file changed, 3 insertions(+) diff --git a/packages/platform-apple/src/core/__tests__/screenshot-status-bar-concurrency.test.ts b/packages/platform-apple/src/core/__tests__/screenshot-status-bar-concurrency.test.ts index 2e548502a6..a49b3f5d0b 100644 --- a/packages/platform-apple/src/core/__tests__/screenshot-status-bar-concurrency.test.ts +++ b/packages/platform-apple/src/core/__tests__/screenshot-status-bar-concurrency.test.ts @@ -446,6 +446,9 @@ test('a settings write bounds its status bar command like a capture does', async }, ); + for (const budget of budgets) { + assert.equal(typeof budget, 'number', 'a status bar write must bound its own command'); + } assert.deepEqual(budgets, [ STATUS_BAR_COMMAND_TIMEOUT_MS, STATUS_BAR_COMMAND_TIMEOUT_MS, From 6f7b9ead18328ac4dd3c5a2ed0bba8d5a9b878eb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Pierzcha=C5=82a?= Date: Sun, 1 Nov 2026 15:39:51 +0100 Subject: [PATCH 12/15] fix(ios): give one simulator one status bar identity across set-path aliases Resolve the simulator device set to one canonical path where the address is minted, and use that value for both the status bar lock key and the `--set` argument, so aliased or non-normalized spellings of one set cannot open two windows over one status bar. Naming the default device root now answers as the default set. Platform code reaches the canonical path through the host-kit file port, which is where host authority belongs. Drop the cached known-cleared answer before a `wifi` or `airplane` status bar command instead of after it: a write that hits its deadline may still have reached `simctl`, so no cached answer survives the attempt and the next capture reads the bar. Document that `settings wifi` and `settings airplane` wait for the simulator's screenshot window and bound their own write to 10 seconds, in `settings --help` and the command reference. --- .../command-surface/src/capture/settings.ts | 1 + packages/host-kit/src/host-file.ts | 1 + packages/host-kit/src/internal/host-file.ts | 8 + .../screenshot-status-bar-concurrency.test.ts | 174 ++++++++++++++++++ .../platform-apple/src/core/app-settings.ts | 8 +- .../src/core/screenshot-status-bar.ts | 12 +- packages/platform-apple/src/core/simctl.ts | 36 +++- website/docs/docs/commands.md | 2 + 8 files changed, 233 insertions(+), 9 deletions(-) diff --git a/packages/command-surface/src/capture/settings.ts b/packages/command-surface/src/capture/settings.ts index 83d258f352..b86c1813ea 100644 --- a/packages/command-surface/src/capture/settings.ts +++ b/packages/command-surface/src/capture/settings.ts @@ -102,6 +102,7 @@ export const settingsCommandFacet = defineCommandFacet({ summary: 'Change OS settings and app permissions', cliDetail: [ 'A bare readable setting reads the value the target holds. A target that does not serve a setting refuses it with UNSUPPORTED_OPERATION before anything changes.', + 'On an iOS simulator, settings wifi and settings airplane write the status bar, which screenshots share: each waits until no screenshot of that simulator is in flight, then bounds its own status bar command to 10 seconds. That 10 seconds covers the write after the wait, never the wait. A request you cancel while it waits reports the cancellation and changes nothing.', '', 'Settings:', ...settingsHelpLines().map((line) => ` ${line}`), diff --git a/packages/host-kit/src/host-file.ts b/packages/host-kit/src/host-file.ts index 7d1ded3f22..14da8c1f66 100644 --- a/packages/host-kit/src/host-file.ts +++ b/packages/host-kit/src/host-file.ts @@ -1,6 +1,7 @@ export { createHostDirectoryLinkSync, accessHostFile, + canonicalHostPathSync, chmodHostFile, copyHostFile, copyHostPath, diff --git a/packages/host-kit/src/internal/host-file.ts b/packages/host-kit/src/internal/host-file.ts index dc278bc8d7..2b9915775f 100644 --- a/packages/host-kit/src/internal/host-file.ts +++ b/packages/host-kit/src/internal/host-file.ts @@ -135,6 +135,14 @@ export function readHostSymbolicLinkSync(filePath: string): string { return fs.readlinkSync(filePath); } +/** + * The canonical form of a path on this host: every symlink and `.`/`..` segment resolved, so two + * spellings of one directory answer with one value. Throws when the host cannot resolve the path. + */ +export function canonicalHostPathSync(candidatePath: string): string { + return fs.realpathSync(candidatePath); +} + export function removeHostFileSync(filePath: string): void { fs.unlinkSync(filePath); } diff --git a/packages/platform-apple/src/core/__tests__/screenshot-status-bar-concurrency.test.ts b/packages/platform-apple/src/core/__tests__/screenshot-status-bar-concurrency.test.ts index a49b3f5d0b..3a287e8a1b 100644 --- a/packages/platform-apple/src/core/__tests__/screenshot-status-bar-concurrency.test.ts +++ b/packages/platform-apple/src/core/__tests__/screenshot-status-bar-concurrency.test.ts @@ -1,5 +1,8 @@ import { beforeEach, test, vi } from 'vitest'; import assert from 'node:assert/strict'; +import fs from 'node:fs/promises'; +import path from 'node:path'; +import { mkdtempForTest } from '../../__tests__/tmp-dir.ts'; import type { DeviceInfo } from '@agent-device/kernel/device'; import { AppError } from '@agent-device/kernel/errors'; import { withFakeAppleTool } from '../../__tests__/fake-apple-tool.ts'; @@ -99,6 +102,9 @@ async function yieldToQueuedWork(): Promise { await new Promise((resolve) => setImmediate(resolve)); } +/** What `simctl status_bar list` answers when the simulator holds no overrides. */ +const CLEARED_STATUS_BAR_LISTING = 'Current Status Bar Overrides:\n============================='; + function screenshotOptions( gate: CaptureGate, normalizeStatusBar: boolean, @@ -456,6 +462,174 @@ test('a settings write bounds its status bar command like a capture does', async ]); }); +test('one simulator reached through two spellings of its device set keeps one window', async () => { + // `simctl --set` follows aliases, so a symlinked or otherwise non-normalized spelling of one set + // reaches the same status bar. Two windows over it would let a capture snapshot another capture's + // normalization, which is the leak this owner exists to prevent. + const setRoot = await mkdtempForTest('sm11-set-'); + const canonicalSet = await fs.realpath(setRoot); + const aliasSet = path.join(canonicalSet, 'link-to-set'); + await fs.symlink(canonicalSet, aliasSet); + + const entered: string[] = []; + const gates = { canonical: createCaptureGate(), alias: createCaptureGate() }; + const deviceFor = (tag: keyof typeof gates): DeviceInfo => ({ + ...IOS_TEST_SIMULATOR, + simulatorSetPath: tag === 'canonical' ? canonicalSet : aliasSet, + }); + + await withFakeAppleTool( + (args) => { + const index = args.indexOf('status_bar'); + if (index === -1) return { stderr: `unexpected args: ${args.join(' ')}`, exitCode: 1 }; + return args[index + 2] === 'list' ? CLEARED_STATUS_BAR_LISTING : ''; + }, + async () => { + const captureFor = (tag: keyof typeof gates) => + captureSimulatorScreenshotWithFallback(deviceFor(tag), `/tmp/${tag}.png`, { + env: process.env, + normalizeStatusBar: true, + skipIosSimulatorBootCheck: true, + deps: captureDeps(async () => { + entered.push(tag); + await gates[tag].capture(); + }), + }); + + const canonical = captureFor('canonical'); + await gates.canonical.entered; + const alias = captureFor('alias'); + try { + await yieldToQueuedWork(); + // The aliased capture is still queued: both records address one simulator, so they share + // one window and the second cannot snapshot the first capture's normalization. + assert.deepEqual(entered, ['canonical']); + } finally { + gates.canonical.release(); + gates.alias.release(); + await Promise.allSettled([canonical, alias]); + } + + assert.deepEqual(entered, ['canonical', 'alias']); + }, + ); +}); + +test('naming the default device set explicitly shares the default window', async () => { + // `simctl --set ` selects the same simulators as naming no set, so the two spellings + // must not open two windows over one status bar. + const entered: string[] = []; + const gates = { implicit: createCaptureGate(), explicit: createCaptureGate() }; + // `os.homedir()` answers with `$HOME` on POSIX, which is how the owner derives this root. + const home = process.env.HOME; + assert.ok(home, 'the default device root needs a home directory'); + const defaultSet = path.join(home, 'Library', 'Developer', 'CoreSimulator', 'Devices'); + const deviceFor = (tag: keyof typeof gates): DeviceInfo => ({ + ...IOS_TEST_SIMULATOR, + simulatorSetPath: tag === 'explicit' ? defaultSet : undefined, + }); + + await withFakeAppleTool( + (args) => { + const index = args.indexOf('status_bar'); + if (index === -1) return { stderr: `unexpected args: ${args.join(' ')}`, exitCode: 1 }; + return args[index + 2] === 'list' ? CLEARED_STATUS_BAR_LISTING : ''; + }, + async () => { + const captureFor = (tag: keyof typeof gates) => + captureSimulatorScreenshotWithFallback(deviceFor(tag), `/tmp/${tag}.png`, { + env: process.env, + normalizeStatusBar: true, + skipIosSimulatorBootCheck: true, + deps: captureDeps(async () => { + entered.push(tag); + await gates[tag].capture(); + }), + }); + + const implicit = captureFor('implicit'); + await gates.implicit.entered; + const explicit = captureFor('explicit'); + try { + await yieldToQueuedWork(); + assert.deepEqual(entered, ['implicit']); + } finally { + gates.implicit.release(); + gates.explicit.release(); + await Promise.allSettled([implicit, explicit]); + } + + assert.deepEqual(entered, ['implicit', 'explicit']); + }, + ); +}); + +test('a timed-out settings write forces the next capture to read the bar again', async () => { + // A command that hits its deadline may still have reached `simctl`, so a write that times out + // cannot leave a cached answer about the bar behind: the next capture has to read what the write + // applied, or it would normalize over a setting the caller asked for and restore without it. + const device: DeviceInfo = { ...IOS_TEST_SIMULATOR, id: 'sim-bar-timeout' }; + invalidateSimulatorStatusBarOverrideCache(device); + const statusBar = createFakeStatusBarSimctl(device.id, {}); + let timeoutApplied = false; + + await withFakeAppleTool( + (args) => { + const response = statusBar.script(args); + // The settings write is the override that names only `--wifiMode`: a capture's normalization + // and its restore both name the other fields. + const lastOption = args.at(-2); + if (args.includes('--wifiMode') && lastOption === '--wifiMode' && !timeoutApplied) { + timeoutApplied = true; + return new AppError('COMMAND_FAILED', 'xcrun timed out after 10000ms', { exitCode: 1 }); + } + return response; + }, + async ({ calls }) => { + // A capture with nothing on the bar earns the answer that the bar is cleared. + await captureSimulatorScreenshotWithFallback(device, '/tmp/cleared.png', { + env: process.env, + normalizeStatusBar: true, + skipIosSimulatorBootCheck: true, + deps: captureDeps(async () => {}), + }); + calls.length = 0; + + await setIosSetting(device, 'wifi', 'on', undefined, undefined, process.env).then( + () => assert.fail('expected the write to time out'), + (error: unknown) => + assert.match(String((error as Error).message), /timed out/, 'the timeout must surface'), + ); + assert.deepEqual( + statusBar.overrides(), + { wifiMode: 'active' }, + 'the write reached the simulator before its deadline', + ); + calls.length = 0; + + await captureSimulatorScreenshotWithFallback(device, '/tmp/after-timeout.png', { + env: process.env, + normalizeStatusBar: true, + skipIosSimulatorBootCheck: true, + deps: captureDeps(async () => {}), + }); + + assert.equal( + calls[0]?.join(' '), + `simctl status_bar ${device.id} list`, + 'a capture after a timed-out write must read the bar, not trust a cached answer', + ); + assert.deepEqual( + statusBar.overrides(), + // `simctl` reports a Wi-Fi mode together with a bars count, so the snapshot the capture + // restores from names both once the timed-out write has applied the mode. + { wifiMode: 'active', wifiBars: '0' }, + 'the capture must restore the setting the timed-out write applied', + ); + }, + ); +}); + test('simulators of different device sets keep their own status bar windows', async () => { // `simctl status_bar` is scoped with `--set`, so one udid in two sets is two bars. Records that // name one simulator — same udid and same set — share a window; different sets must not queue. diff --git a/packages/platform-apple/src/core/app-settings.ts b/packages/platform-apple/src/core/app-settings.ts index 5bed13b320..7e97f5ed80 100644 --- a/packages/platform-apple/src/core/app-settings.ts +++ b/packages/platform-apple/src/core/app-settings.ts @@ -112,12 +112,14 @@ export async function setIosSetting( await withSimulatorStatusBarHeld( device, async () => { + // Dropped before the command, not after it: a write that hits its deadline may still have + // reached `simctl`, so no answer about the bar survives the attempt either way. + invalidateSimulatorStatusBarOverrideCache(device); await runSimctlForDevice( device, ['status_bar', device.id, 'override', '--wifiMode', mode], { timeoutMs: STATUS_BAR_COMMAND_TIMEOUT_MS }, ); - invalidateSimulatorStatusBarOverrideCache(device); }, signal, ); @@ -130,6 +132,9 @@ export async function setIosSetting( await withSimulatorStatusBarHeld( device, async () => { + // Dropped before the command, as for `wifi`: a write that hits its deadline may still have + // reached `simctl`, so no answer about the bar survives the attempt. + invalidateSimulatorStatusBarOverrideCache(device); if (enabled) { await runSimctlForDevice( device, @@ -152,7 +157,6 @@ export async function setIosSetting( ], { timeoutMs: STATUS_BAR_COMMAND_TIMEOUT_MS }, ); - invalidateSimulatorStatusBarOverrideCache(device); } else { await runSimctlForDevice(device, ['status_bar', device.id, 'clear'], { timeoutMs: STATUS_BAR_COMMAND_TIMEOUT_MS, diff --git a/packages/platform-apple/src/core/screenshot-status-bar.ts b/packages/platform-apple/src/core/screenshot-status-bar.ts index e04ff9ea54..e683c1783b 100644 --- a/packages/platform-apple/src/core/screenshot-status-bar.ts +++ b/packages/platform-apple/src/core/screenshot-status-bar.ts @@ -174,11 +174,13 @@ async function restoreSimulatorStatusBarOverrides( device: DeviceInfo, overrides: RestorableStatusBarOverrides | null, ): Promise { - // Only a restore that ran to the end may change the known-cleared answer. A `clear` that fails - // leaves the normalization on the bar, and the answer that says "this bar is cleared" is the one - // that fixes it: the next capture skips the read, so it cannot adopt the leftover as the caller's - // state, and its own restore clears the bar. Reading the bar after a failed clear would do the - // opposite — adopt the leftover and hand it back on every later capture. + // Only a restore that ran to the end may change the known-cleared answer, and this covers the + // simulator the answer already says is cleared. A `clear` that fails leaves the normalization on + // the bar, and that answer is the one that fixes it: the next capture skips the read, so it cannot + // adopt the leftover as the caller's state, and its own restore clears the bar. Reading the bar + // after a failed clear would do the opposite — adopt the leftover and hand it back on every later + // capture. When the snapshot held the caller's own overrides, a failed `clear` cannot return them: + // that recovery needs the snapshot itself and is tracked separately. await clearSimulatorStatusBarOverride(device, STATUS_BAR_RESTORE_TIMEOUT_MS); if (!overrides) { rememberClearedStatusBarOverrides(device); diff --git a/packages/platform-apple/src/core/simctl.ts b/packages/platform-apple/src/core/simctl.ts index db5e65485f..0465f85049 100644 --- a/packages/platform-apple/src/core/simctl.ts +++ b/packages/platform-apple/src/core/simctl.ts @@ -1,7 +1,9 @@ import { isIosFamily, type DeviceInfo } from '@agent-device/kernel/device'; import { type ExecOptions, type ExecResult } from '@agent-device/host-kit/command'; +import { canonicalHostPathSync, hostHomeDirectory } from '@agent-device/host-kit/host-file'; import { resolveIosSimulatorDeviceSetPath } from '@agent-device/kernel/device-isolation'; import type { ScopedSimctlArgs } from '@agent-device/contracts/platform-runtime-host'; +import path from 'node:path'; import { runXcrun, simctlCommand, type ScopedSimctlCommand } from './tool-provider.ts'; declare const simulatorAddress: unique symbol; @@ -13,10 +15,40 @@ export type SimulatorAddress = Readonly<{ udid: string; simulatorSetPath: string readonly [simulatorAddress]: true; }; +/** + * The device root `simctl` uses when a command names no set. Naming this directory explicitly with + * `--set` selects the same simulators, so the flag value and the default share one identity. + */ +function defaultSimulatorSetPath(): string { + return path.join(hostHomeDirectory(), 'Library', 'Developer', 'CoreSimulator', 'Devices'); +} + +/** + * Canonical identity of one simulator set: `simctl --set` follows aliases and trailing separators, + * so every spelling of one directory must answer as one value, and naming the default root + * explicitly is the default set. Unreadable paths (a set that does not exist on this host) keep + * their resolved spelling. + */ +function canonicalSimulatorSetPath(simulatorSetPath: string | undefined): string | undefined { + const spelled = resolveIosSimulatorDeviceSetPath(simulatorSetPath); + if (spelled === undefined) return undefined; + const canonical = canonicalExistingPath(spelled); + return canonical === canonicalExistingPath(defaultSimulatorSetPath()) ? undefined : canonical; +} + +function canonicalExistingPath(candidate: string): string { + const resolved = path.resolve(candidate); + try { + return canonicalHostPathSync(resolved); + } catch { + return path.normalize(resolved); + } +} + export function simulatorAddressFor(device: DeviceInfo): SimulatorAddress { const simulatorSetPath = isIosFamily(device) && device.kind === 'simulator' - ? resolveIosSimulatorDeviceSetPath(device.simulatorSetPath) + ? canonicalSimulatorSetPath(device.simulatorSetPath) : undefined; return Object.freeze({ udid: device.id, simulatorSetPath }) as SimulatorAddress; } @@ -25,7 +57,7 @@ function scopeSimctlArgs( simulatorSetPath: string | undefined, args: readonly string[], ): ScopedSimctlArgs { - const resolvedSetPath = resolveIosSimulatorDeviceSetPath(simulatorSetPath); + const resolvedSetPath = canonicalSimulatorSetPath(simulatorSetPath); const scoped = resolvedSetPath ? ['--set', resolvedSetPath, ...args] : [...args]; return Object.freeze(scoped) as ScopedSimctlArgs; } diff --git a/website/docs/docs/commands.md b/website/docs/docs/commands.md index 2bf8d26ec2..219215a06e 100644 --- a/website/docs/docs/commands.md +++ b/website/docs/docs/commands.md @@ -758,6 +758,7 @@ Platform support: - `settings appearance` sets macOS appearance, iOS simulator appearance, or Android night mode. - `settings location set ` sets coordinates on iOS simulators and Android emulators. - A target that does not serve a setting refuses it with `UNSUPPORTED_OPERATION` and `error.details.reason` (for example `unsupported-platform-leaf`) before anything changes. A request that names an app the change cannot take is refused first, with `INVALID_ARGS` and `setting_app_not_consumed`. `settings --help` describes where each setting runs. +- On an iOS simulator, `settings wifi` and `settings airplane` write the status bar, which screenshots share. Each waits until no screenshot of that simulator is in flight, then allows 10 seconds for its own write; the 10 seconds covers the write, never the wait, so a request issued during a screenshot can take as long as that screenshot. Cancel it while it waits and nothing changes. Device settings: @@ -1042,6 +1043,7 @@ agent-device record stop # Stop active recording - `--crop-on ` crops the capture to the frame the selector resolves on the same screen. It works only on iOS simulators, Android emulators, and physical iPhones. A physical iPhone crop is verified only in portrait using CoreDevice/devicectl; the XCTest backend and runner fallback are not accepted. Physical iPads remain rejected for crop, and landscape iPhone crop remains unverified. It needs exactly one framed node, cannot be combined with `--overlay-refs` or `--fullscreen`, and warns when the frame is clipped. The crop is re-encoded, so do not byte-compare it with an older crop. - `--fullscreen` on macOS applies only to app sessions. Other surfaces (`desktop`, `menubar`, `frontmost-app`) always capture the main display and refuse `--fullscreen` with `INVALID_ARGS`. - `--normalize-status-bar` temporarily normalizes iOS simulator status-bar chrome for reusable baselines. +- On an iOS simulator, a screenshot waits until no other screenshot or `settings wifi` / `settings airplane` write is using that simulator's status bar, so overlapping captures cannot see each other's temporary chrome. A request you cancel while it is waiting fails with the cancellation instead of capturing. - On single-display iOS simulators and Android devices, the result may report `displayRotation` (`portrait`, `portrait-upside-down`, `landscape-left`, or `landscape-right`). It is absent when the device returns no reading in time, on other targets, and on multi-display devices such as foldables. The saved image is always upright. **Screenshot diffs** From 5ffe9ecf54ffc8c51fc954c972351762ac1cff09 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Pierzcha=C5=82a?= Date: Sun, 1 Nov 2026 16:24:59 +0100 Subject: [PATCH 13/15] fix(ios): take the simulator default set from the host that runs simctl Derive the default device root from the bound command host's environment instead of this process's home, so an explicit `--set` is only ever recognised as the default when the host that will execute `simctl` owns that root. When no host is bound, or that host names no absolute home, keep the caller's explicit set rather than guess from another home. The host-environment home lookup moves to the kernel owner of the environment type, where every caller already reaches it. Describe the density step as covered by the fallback flow's own hold, and tell settings callers that a timed-out write may already have changed the bar: only cancelling while the request is queued guarantees no write. --- .../command-surface/src/capture/settings.ts | 2 +- packages/host-kit/src/command.ts | 1 + packages/host-kit/src/host-file.ts | 1 - packages/host-kit/src/internal/exec.test.ts | 33 +++++++ packages/host-kit/src/internal/exec.ts | 20 +++- packages/host-kit/src/internal/host-file.ts | 12 --- packages/kernel/src/source-value.ts | 10 ++ .../screenshot-status-bar-concurrency.test.ts | 70 +++++++------- .../src/core/__tests__/simctl.test.ts | 95 +++++++++++++++++++ .../platform-apple/src/core/screenshot.ts | 12 +-- packages/platform-apple/src/core/simctl.ts | 32 +++++-- packages/platform-vega/src/vega-executable.ts | 3 +- src/platform-runtime-host.ts | 3 +- website/docs/docs/commands.md | 2 +- 14 files changed, 228 insertions(+), 68 deletions(-) diff --git a/packages/command-surface/src/capture/settings.ts b/packages/command-surface/src/capture/settings.ts index b86c1813ea..d79aaa1f29 100644 --- a/packages/command-surface/src/capture/settings.ts +++ b/packages/command-surface/src/capture/settings.ts @@ -102,7 +102,7 @@ export const settingsCommandFacet = defineCommandFacet({ summary: 'Change OS settings and app permissions', cliDetail: [ 'A bare readable setting reads the value the target holds. A target that does not serve a setting refuses it with UNSUPPORTED_OPERATION before anything changes.', - 'On an iOS simulator, settings wifi and settings airplane write the status bar, which screenshots share: each waits until no screenshot of that simulator is in flight, then bounds its own status bar command to 10 seconds. That 10 seconds covers the write after the wait, never the wait. A request you cancel while it waits reports the cancellation and changes nothing.', + 'On an iOS simulator, settings wifi and settings airplane write the status bar, which screenshots share: each waits until no screenshot of that simulator is in flight, then bounds its own status bar command to 10 seconds. That 10 seconds covers the write after the wait, never the wait. A write that hits the deadline may already have changed the bar. Only cancelling while the request is still waiting guarantees nothing was written.', '', 'Settings:', ...settingsHelpLines().map((line) => ` ${line}`), diff --git a/packages/host-kit/src/command.ts b/packages/host-kit/src/command.ts index c57e0157bd..eeeac1b82c 100644 --- a/packages/host-kit/src/command.ts +++ b/packages/host-kit/src/command.ts @@ -1,6 +1,7 @@ export { coerceExecResult, commandHostEnvironment, + commandHostHomeDirectory, commandDeveloperDir, type CommandExecutorOverride, type ExecBackgroundOptions, diff --git a/packages/host-kit/src/host-file.ts b/packages/host-kit/src/host-file.ts index 14da8c1f66..e8e0c25866 100644 --- a/packages/host-kit/src/host-file.ts +++ b/packages/host-kit/src/host-file.ts @@ -12,7 +12,6 @@ export { hostFileStat, hostFileStatSync, hostHomeDirectory, - hostHomeDirectoryFromEnv, hostTemporaryDirectory, makeHostTemporaryDirectory, readHostBinaryFile, diff --git a/packages/host-kit/src/internal/exec.test.ts b/packages/host-kit/src/internal/exec.test.ts index 6009a3f6a9..9d61724f15 100644 --- a/packages/host-kit/src/internal/exec.test.ts +++ b/packages/host-kit/src/internal/exec.test.ts @@ -6,6 +6,7 @@ import { coerceExecResult, commandDeveloperDir, commandHostEnvironment, + commandHostHomeDirectory, isCommandTimeoutError, requireExecSuccess, runCmd, @@ -484,6 +485,38 @@ test('commandHostEnvironment names the enclosing host and refuses outside one', }); }); +test('commandHostHomeDirectory answers with the home the spawned commands inherit', async () => { + // Outside a bound host there is no host home to name: callers keep the caller's own value. + assert.equal(commandHostHomeDirectory(), undefined); + + await withCommandHost( + { host: hostWith({ HOME: '/host/home' }), requestEnv: undefined }, + async () => { + assert.equal(commandHostHomeDirectory(), '/host/home'); + }, + ); + + // A request value wins over the host value, exactly as it does for the spawned child. + await withCommandHost( + { host: hostWith({ HOME: '/host/home' }), requestEnv: { HOME: '/request/home' } }, + async () => { + assert.equal(commandHostHomeDirectory(), '/request/home'); + }, + ); + + // A host that names no home, or names one the child cannot resolve, answers undefined rather than + // this process's home. + await withCommandHost({ host: hostWith({ HOME: '' }), requestEnv: undefined }, async () => { + assert.equal(commandHostHomeDirectory(), undefined); + }); + await withCommandHost( + { host: hostWith({ HOME: 'relative/home' }), requestEnv: undefined }, + async () => { + assert.equal(commandHostHomeDirectory(), undefined); + }, + ); +}); + // Windows reports no signal for a terminated process. test.skipIf(process.platform === 'win32')( 'every foreground executor reports the signal that ended the process', diff --git a/packages/host-kit/src/internal/exec.ts b/packages/host-kit/src/internal/exec.ts index e9611933a6..ee3d64d676 100644 --- a/packages/host-kit/src/internal/exec.ts +++ b/packages/host-kit/src/internal/exec.ts @@ -7,7 +7,11 @@ import { Readable } from 'node:stream'; import { pipeline } from 'node:stream/promises'; import { AppError, createRequestCanceledError } from '@agent-device/kernel/errors'; import { createExecTraceContext, type ExecSettlementReason } from './exec-trace.ts'; -import type { EnvMap, HostEnvironment } from '@agent-device/kernel/source-value'; +import { + type EnvMap, + hostHomeDirectoryFromEnv, + type HostEnvironment, +} from '@agent-device/kernel/source-value'; export type ExecResult = { stdout: string; @@ -154,6 +158,20 @@ export function commandDeveloperDir(): string | undefined { return scope?.host.env.DEVELOPER_DIR || undefined; } +/** + * The home directory of the host that owns the commands this scope spawns, which is the home its + * children inherit and the directory tools such as `simctl` resolve `~` from. Answers undefined + * outside a bound host, or when that host names no home, or names one that is not absolute: code + * that needs a default must then keep what the caller asked for rather than guess this process's + * home, which can belong to a different host. + */ +export function commandHostHomeDirectory(): string | undefined { + const scope = commandHostScope.getStore(); + if (!scope) return undefined; + const home = hostHomeDirectoryFromEnv({ ...scope.host.env, ...scope.requestEnv })?.trim(); + return home && path.isAbsolute(home) ? home : undefined; +} + function resolveSpawnEnv(env: NodeJS.ProcessEnv | undefined): NodeJS.ProcessEnv | undefined { const scope = commandHostScope.getStore(); if (!scope) return env; diff --git a/packages/host-kit/src/internal/host-file.ts b/packages/host-kit/src/internal/host-file.ts index 2b9915775f..4bc2c8315c 100644 --- a/packages/host-kit/src/internal/host-file.ts +++ b/packages/host-kit/src/internal/host-file.ts @@ -7,18 +7,6 @@ export function hostHomeDirectory(): string { return os.homedir(); } -/** - * The home directory a named host names for itself: its own HOME (or USERPROFILE on Windows). - * Returns undefined when the host env names no home; callers must fail closed or skip the - * home-bound lookup, never fall back to `os.homedir()`, because the process home is an ambient - * read that #3753 forbids. Derive per-runtime home config through this (#3794). - */ -export function hostHomeDirectoryFromEnv( - env: Record, -): string | undefined { - return env['HOME'] ?? env['USERPROFILE']; -} - export function hostTemporaryDirectory(): string { return os.tmpdir(); } diff --git a/packages/kernel/src/source-value.ts b/packages/kernel/src/source-value.ts index d6a0f87d10..773844fc3e 100644 --- a/packages/kernel/src/source-value.ts +++ b/packages/kernel/src/source-value.ts @@ -140,6 +140,16 @@ function parseEnumSetValue( export type EnvMap = Record; +/** + * The home directory a host environment names for itself: its own `HOME`, or `USERPROFILE` on + * Windows. Returns undefined when the environment names no home; callers must fail closed or skip + * the home-bound lookup, never fall back to `os.homedir()`, because the process home is an ambient + * read that #3753 forbids. Derive per-runtime home config through this (#3794). + */ +export function hostHomeDirectoryFromEnv(env: EnvMap): string | undefined { + return env['HOME'] ?? env['USERPROFILE']; +} + /** * The process-level inputs a host hands to the code it runs: the environment and working * directory. An entry point captures it once; everything below receives it. diff --git a/packages/platform-apple/src/core/__tests__/screenshot-status-bar-concurrency.test.ts b/packages/platform-apple/src/core/__tests__/screenshot-status-bar-concurrency.test.ts index 3a287e8a1b..331a7a5eb2 100644 --- a/packages/platform-apple/src/core/__tests__/screenshot-status-bar-concurrency.test.ts +++ b/packages/platform-apple/src/core/__tests__/screenshot-status-bar-concurrency.test.ts @@ -3,6 +3,7 @@ import assert from 'node:assert/strict'; import fs from 'node:fs/promises'; import path from 'node:path'; import { mkdtempForTest } from '../../__tests__/tmp-dir.ts'; +import { withCommandHost } from '@agent-device/host-kit/command'; import type { DeviceInfo } from '@agent-device/kernel/device'; import { AppError } from '@agent-device/kernel/errors'; import { withFakeAppleTool } from '../../__tests__/fake-apple-tool.ts'; @@ -517,10 +518,10 @@ test('one simulator reached through two spellings of its device set keeps one wi test('naming the default device set explicitly shares the default window', async () => { // `simctl --set ` selects the same simulators as naming no set, so the two spellings - // must not open two windows over one status bar. + // must not open two windows over one status bar. The host that runs the command owns that root, + // so the test binds one whose HOME is the home the owner derives it from. const entered: string[] = []; const gates = { implicit: createCaptureGate(), explicit: createCaptureGate() }; - // `os.homedir()` answers with `$HOME` on POSIX, which is how the owner derives this root. const home = process.env.HOME; assert.ok(home, 'the default device root needs a home directory'); const defaultSet = path.join(home, 'Library', 'Developer', 'CoreSimulator', 'Devices'); @@ -529,37 +530,42 @@ test('naming the default device set explicitly shares the default window', async simulatorSetPath: tag === 'explicit' ? defaultSet : undefined, }); - await withFakeAppleTool( - (args) => { - const index = args.indexOf('status_bar'); - if (index === -1) return { stderr: `unexpected args: ${args.join(' ')}`, exitCode: 1 }; - return args[index + 2] === 'list' ? CLEARED_STATUS_BAR_LISTING : ''; - }, + await withCommandHost( + { host: { env: { HOME: home }, cwd: '/' }, requestEnv: undefined }, async () => { - const captureFor = (tag: keyof typeof gates) => - captureSimulatorScreenshotWithFallback(deviceFor(tag), `/tmp/${tag}.png`, { - env: process.env, - normalizeStatusBar: true, - skipIosSimulatorBootCheck: true, - deps: captureDeps(async () => { - entered.push(tag); - await gates[tag].capture(); - }), - }); - - const implicit = captureFor('implicit'); - await gates.implicit.entered; - const explicit = captureFor('explicit'); - try { - await yieldToQueuedWork(); - assert.deepEqual(entered, ['implicit']); - } finally { - gates.implicit.release(); - gates.explicit.release(); - await Promise.allSettled([implicit, explicit]); - } - - assert.deepEqual(entered, ['implicit', 'explicit']); + await withFakeAppleTool( + (args) => { + const index = args.indexOf('status_bar'); + if (index === -1) return { stderr: `unexpected args: ${args.join(' ')}`, exitCode: 1 }; + return args[index + 2] === 'list' ? CLEARED_STATUS_BAR_LISTING : ''; + }, + async () => { + const captureFor = (tag: keyof typeof gates) => + captureSimulatorScreenshotWithFallback(deviceFor(tag), `/tmp/${tag}.png`, { + env: process.env, + normalizeStatusBar: true, + skipIosSimulatorBootCheck: true, + deps: captureDeps(async () => { + entered.push(tag); + await gates[tag].capture(); + }), + }); + + const implicit = captureFor('implicit'); + await gates.implicit.entered; + const explicit = captureFor('explicit'); + try { + await yieldToQueuedWork(); + assert.deepEqual(entered, ['implicit']); + } finally { + gates.implicit.release(); + gates.explicit.release(); + await Promise.allSettled([implicit, explicit]); + } + + assert.deepEqual(entered, ['implicit', 'explicit']); + }, + ); }, ); }); diff --git a/packages/platform-apple/src/core/__tests__/simctl.test.ts b/packages/platform-apple/src/core/__tests__/simctl.test.ts index 16192b32f3..10c401a3ef 100644 --- a/packages/platform-apple/src/core/__tests__/simctl.test.ts +++ b/packages/platform-apple/src/core/__tests__/simctl.test.ts @@ -1,5 +1,9 @@ import { test } from 'vitest'; import assert from 'node:assert/strict'; +import fs from 'node:fs/promises'; +import path from 'node:path'; +import { withCommandHost } from '@agent-device/host-kit/command'; +import { mkdtempForTest } from '../../__tests__/tmp-dir.ts'; import { buildSimctlArgsForAddress, buildSimctlArgsForDevice, @@ -67,6 +71,97 @@ test('simctlListInventoryArgs prefixes a trimmed simulator set and omits a blank assert.deepEqual(simctlListInventoryArgs(undefined), ['list', '-j']); }); +/** + * The default device root is where `simctl` looks when argv names no set, and `simctl` resolves it + * from the environment it is spawned with. A daemon request runs on a bound command host whose HOME + * can differ from this process's, so the identity of "the default set" belongs to that host. + */ +async function withHostHome(home: string | undefined, run: () => Promise): Promise { + return await withCommandHost( + { host: { env: home === undefined ? {} : { HOME: home }, cwd: '/' }, requestEnv: undefined }, + run, + ); +} + +test('an explicit set is kept when the executing host names a different home', async () => { + // The regression: this process's home is the real default root, the host that will run `simctl` + // names another one. Erasing the caller's explicit set here would run the command against the + // process user's simulators instead of the ones the caller asked for. + const requested = await mkdtempForTest('sm11-host-home-set-'); + const canonicalRequested = await fs.realpath(requested); + const processHome = await fs.realpath(process.env.HOME ?? ''); + const processHomeDefault = path.join( + processHome, + 'Library', + 'Developer', + 'CoreSimulator', + 'Devices', + ); + + await withHostHome('/tmp/another-host-home', async () => { + assert.deepEqual(simctlListInventoryArgs(canonicalRequested), [ + '--set', + canonicalRequested, + 'list', + '-j', + ]); + assert.deepEqual( + simulatorAddressFor({ ...IOS_SIMULATOR, simulatorSetPath: canonicalRequested }), + { udid: 'sim-1', simulatorSetPath: canonicalRequested }, + ); + }); + + // Naming the process user's own default root is not the host's default either. + await withHostHome('/tmp/another-host-home', async () => { + assert.deepEqual(simctlListInventoryArgs(processHomeDefault), [ + '--set', + processHomeDefault, + 'list', + '-j', + ]); + }); +}); + +test('naming the executing host default root answers as the default set', async () => { + // The host that runs the command is the authority: when its own HOME names the default root, + // spelling it explicitly is the same set as naming none, so both share one lock/cache identity. + const hostHome = await fs.realpath(process.env.HOME ?? ''); + const defaultSet = path.join(hostHome, 'Library', 'Developer', 'CoreSimulator', 'Devices'); + + await withHostHome(hostHome, async () => { + assert.deepEqual(simctlListInventoryArgs(defaultSet), ['list', '-j']); + assert.deepEqual(simulatorAddressFor({ ...IOS_SIMULATOR, simulatorSetPath: defaultSet }), { + udid: 'sim-1', + simulatorSetPath: undefined, + }); + }); +}); + +test('an explicit set survives a host that names no home', async () => { + // No authority means no default to compare against: the caller's scope is kept, never guessed. + const requested = await mkdtempForTest('sm11-unbound-set-'); + const canonicalRequested = await fs.realpath(requested); + await withHostHome(undefined, async () => { + assert.deepEqual(simctlListInventoryArgs(canonicalRequested), [ + '--set', + canonicalRequested, + 'list', + '-j', + ]); + }); + await withCommandHost( + { host: { env: { HOME: 'relative-home' }, cwd: '/' }, requestEnv: undefined }, + async () => { + assert.deepEqual(simctlListInventoryArgs(canonicalRequested), [ + '--set', + canonicalRequested, + 'list', + '-j', + ]); + }, + ); +}); + test('simctlAvailabilityProbeArgs names no set', () => { assert.deepEqual(simctlAvailabilityProbeArgs(), ['help']); }); diff --git a/packages/platform-apple/src/core/screenshot.ts b/packages/platform-apple/src/core/screenshot.ts index ea62fd4e3a..a564ee0ecc 100644 --- a/packages/platform-apple/src/core/screenshot.ts +++ b/packages/platform-apple/src/core/screenshot.ts @@ -262,12 +262,12 @@ function capturesThroughPhysicalDeviceControl(device: DeviceInfo): boolean { /** * Captures through the runner backend. An iOS simulator's runner capture renders the same screen a * simctl capture does, so it takes the same status bar reservation: while another capture - * normalizes the bar, this one would otherwise copy an image of the temporary 9:41 overrides. The - * reservation covers the whole capture, including the density normalization the capture flow applies - * afterwards, because that step reads the file the capture wrote. It re-enters when the caller - * already holds it, as the simctl fallback does, and an already-aborted request does not reject that - * re-entry at the lock boundary — see `withSimulatorStatusBarHeld`. The runner command keeps the - * request's signal. + * normalizes the bar, this one would otherwise copy an image of the temporary 9:41 overrides. This + * reservation covers the runner capture alone; the density normalization the fallback flow applies + * afterwards is covered by that flow's own hold, and a direct call here performs no density step. + * It re-enters when the caller already holds it, as the simctl fallback does, and an already-aborted + * request does not reject that re-entry at the lock boundary — see `withSimulatorStatusBarHeld`. The + * runner command keeps the request's signal. */ export async function captureScreenshotViaRunner( device: DeviceInfo, diff --git a/packages/platform-apple/src/core/simctl.ts b/packages/platform-apple/src/core/simctl.ts index 0465f85049..26c435ee5f 100644 --- a/packages/platform-apple/src/core/simctl.ts +++ b/packages/platform-apple/src/core/simctl.ts @@ -1,6 +1,10 @@ import { isIosFamily, type DeviceInfo } from '@agent-device/kernel/device'; -import { type ExecOptions, type ExecResult } from '@agent-device/host-kit/command'; -import { canonicalHostPathSync, hostHomeDirectory } from '@agent-device/host-kit/host-file'; +import { + commandHostHomeDirectory, + type ExecOptions, + type ExecResult, +} from '@agent-device/host-kit/command'; +import { canonicalHostPathSync } from '@agent-device/host-kit/host-file'; import { resolveIosSimulatorDeviceSetPath } from '@agent-device/kernel/device-isolation'; import type { ScopedSimctlArgs } from '@agent-device/contracts/platform-runtime-host'; import path from 'node:path'; @@ -16,24 +20,32 @@ export type SimulatorAddress = Readonly<{ udid: string; simulatorSetPath: string }; /** - * The device root `simctl` uses when a command names no set. Naming this directory explicitly with - * `--set` selects the same simulators, so the flag value and the default share one identity. + * The device root `simctl` uses when a command names no set, as the host that will run the command + * sees it: `simctl` resolves `~` from the environment it is spawned with, which is the bound command + * host's, not this process's. Answers undefined when no host is bound or that host names no home. */ -function defaultSimulatorSetPath(): string { - return path.join(hostHomeDirectory(), 'Library', 'Developer', 'CoreSimulator', 'Devices'); +function defaultSimulatorSetPath(): string | undefined { + const home = commandHostHomeDirectory(); + return home === undefined + ? undefined + : path.join(home, 'Library', 'Developer', 'CoreSimulator', 'Devices'); } /** * Canonical identity of one simulator set: `simctl --set` follows aliases and trailing separators, - * so every spelling of one directory must answer as one value, and naming the default root - * explicitly is the default set. Unreadable paths (a set that does not exist on this host) keep - * their resolved spelling. + * so every spelling of one directory must answer as one value, and naming the executing host's + * default root explicitly is the default set. Unreadable paths (a set that does not exist on this + * host) keep their resolved spelling. An explicit set is only ever dropped for the default root of + * the host that runs the command: without that host's home the explicit set is kept, because a + * guessed default could erase a scope the caller asked for. */ function canonicalSimulatorSetPath(simulatorSetPath: string | undefined): string | undefined { const spelled = resolveIosSimulatorDeviceSetPath(simulatorSetPath); if (spelled === undefined) return undefined; const canonical = canonicalExistingPath(spelled); - return canonical === canonicalExistingPath(defaultSimulatorSetPath()) ? undefined : canonical; + const defaultSet = defaultSimulatorSetPath(); + if (defaultSet === undefined) return canonical; + return canonical === canonicalExistingPath(defaultSet) ? undefined : canonical; } function canonicalExistingPath(candidate: string): string { diff --git a/packages/platform-vega/src/vega-executable.ts b/packages/platform-vega/src/vega-executable.ts index 64cd9db278..cc75b6ef6b 100644 --- a/packages/platform-vega/src/vega-executable.ts +++ b/packages/platform-vega/src/vega-executable.ts @@ -1,7 +1,6 @@ import path from 'node:path'; import { isExecutablePath, whichCmd } from '@agent-device/host-kit/command'; -import { hostHomeDirectoryFromEnv } from '@agent-device/host-kit/host-file'; -import type { EnvMap } from '@agent-device/kernel/source-value'; +import { type EnvMap, hostHomeDirectoryFromEnv } from '@agent-device/kernel/source-value'; /** * The host ports one Vega executable lookup needs — the same three the device-inventory host diff --git a/src/platform-runtime-host.ts b/src/platform-runtime-host.ts index 366844f90e..197a31aa2b 100644 --- a/src/platform-runtime-host.ts +++ b/src/platform-runtime-host.ts @@ -1,4 +1,4 @@ -import type { HostEnvironment } from '@agent-device/kernel/source-value'; +import { hostHomeDirectoryFromEnv, type HostEnvironment } from '@agent-device/kernel/source-value'; import type { DeviceInventoryHost, HostOperatingSystem, @@ -12,7 +12,6 @@ import path from 'node:path'; import { createAppleToolHost } from './platform-runtime-apple-tool-host.ts'; import { createHostToolchainPreparer } from './platform-runtime-toolchain-host.ts'; import { createHostCommandRunner } from './platform-runtime-host-command-runner.ts'; -import { hostHomeDirectoryFromEnv } from '@agent-device/host-kit/host-file'; export function createDeviceInventoryHost(host: HostEnvironment): DeviceInventoryHost { const env = host.env; diff --git a/website/docs/docs/commands.md b/website/docs/docs/commands.md index 219215a06e..69bb7afc51 100644 --- a/website/docs/docs/commands.md +++ b/website/docs/docs/commands.md @@ -758,7 +758,7 @@ Platform support: - `settings appearance` sets macOS appearance, iOS simulator appearance, or Android night mode. - `settings location set ` sets coordinates on iOS simulators and Android emulators. - A target that does not serve a setting refuses it with `UNSUPPORTED_OPERATION` and `error.details.reason` (for example `unsupported-platform-leaf`) before anything changes. A request that names an app the change cannot take is refused first, with `INVALID_ARGS` and `setting_app_not_consumed`. `settings --help` describes where each setting runs. -- On an iOS simulator, `settings wifi` and `settings airplane` write the status bar, which screenshots share. Each waits until no screenshot of that simulator is in flight, then allows 10 seconds for its own write; the 10 seconds covers the write, never the wait, so a request issued during a screenshot can take as long as that screenshot. Cancel it while it waits and nothing changes. +- On an iOS simulator, `settings wifi` and `settings airplane` write the status bar, which screenshots share. Each waits until no screenshot of that simulator is in flight, then allows 10 seconds for its own write; the 10 seconds covers the write, never the wait, so a request issued during a screenshot can take as long as that screenshot. If that 10 seconds runs out, the bar may already have changed — read it back before you rely on it. Only cancelling while the request is still waiting guarantees nothing was written. Device settings: From f01442f6e58ba12d4b5a296f6cc2b200422d59c2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Pierzcha=C5=82a?= Date: Sun, 1 Nov 2026 22:12:42 +0100 Subject: [PATCH 14/15] fix(ios): key status bar transactions by simulator UUID --- packages/host-kit/src/command.ts | 1 - packages/host-kit/src/host-file.ts | 2 +- packages/host-kit/src/internal/exec.test.ts | 33 ----- packages/host-kit/src/internal/exec.ts | 20 +-- packages/host-kit/src/internal/host-file.ts | 20 +-- packages/kernel/src/source-value.ts | 10 -- .../screenshot-status-bar-concurrency.test.ts | 124 +----------------- .../src/core/__tests__/simctl.test.ts | 95 -------------- .../src/core/screenshot-status-bar.ts | 11 +- packages/platform-apple/src/core/simctl.ts | 50 +------ packages/platform-vega/src/vega-executable.ts | 3 +- src/platform-runtime-host.ts | 3 +- 12 files changed, 29 insertions(+), 343 deletions(-) diff --git a/packages/host-kit/src/command.ts b/packages/host-kit/src/command.ts index eeeac1b82c..c57e0157bd 100644 --- a/packages/host-kit/src/command.ts +++ b/packages/host-kit/src/command.ts @@ -1,7 +1,6 @@ export { coerceExecResult, commandHostEnvironment, - commandHostHomeDirectory, commandDeveloperDir, type CommandExecutorOverride, type ExecBackgroundOptions, diff --git a/packages/host-kit/src/host-file.ts b/packages/host-kit/src/host-file.ts index e8e0c25866..7d1ded3f22 100644 --- a/packages/host-kit/src/host-file.ts +++ b/packages/host-kit/src/host-file.ts @@ -1,7 +1,6 @@ export { createHostDirectoryLinkSync, accessHostFile, - canonicalHostPathSync, chmodHostFile, copyHostFile, copyHostPath, @@ -12,6 +11,7 @@ export { hostFileStat, hostFileStatSync, hostHomeDirectory, + hostHomeDirectoryFromEnv, hostTemporaryDirectory, makeHostTemporaryDirectory, readHostBinaryFile, diff --git a/packages/host-kit/src/internal/exec.test.ts b/packages/host-kit/src/internal/exec.test.ts index 9d61724f15..6009a3f6a9 100644 --- a/packages/host-kit/src/internal/exec.test.ts +++ b/packages/host-kit/src/internal/exec.test.ts @@ -6,7 +6,6 @@ import { coerceExecResult, commandDeveloperDir, commandHostEnvironment, - commandHostHomeDirectory, isCommandTimeoutError, requireExecSuccess, runCmd, @@ -485,38 +484,6 @@ test('commandHostEnvironment names the enclosing host and refuses outside one', }); }); -test('commandHostHomeDirectory answers with the home the spawned commands inherit', async () => { - // Outside a bound host there is no host home to name: callers keep the caller's own value. - assert.equal(commandHostHomeDirectory(), undefined); - - await withCommandHost( - { host: hostWith({ HOME: '/host/home' }), requestEnv: undefined }, - async () => { - assert.equal(commandHostHomeDirectory(), '/host/home'); - }, - ); - - // A request value wins over the host value, exactly as it does for the spawned child. - await withCommandHost( - { host: hostWith({ HOME: '/host/home' }), requestEnv: { HOME: '/request/home' } }, - async () => { - assert.equal(commandHostHomeDirectory(), '/request/home'); - }, - ); - - // A host that names no home, or names one the child cannot resolve, answers undefined rather than - // this process's home. - await withCommandHost({ host: hostWith({ HOME: '' }), requestEnv: undefined }, async () => { - assert.equal(commandHostHomeDirectory(), undefined); - }); - await withCommandHost( - { host: hostWith({ HOME: 'relative/home' }), requestEnv: undefined }, - async () => { - assert.equal(commandHostHomeDirectory(), undefined); - }, - ); -}); - // Windows reports no signal for a terminated process. test.skipIf(process.platform === 'win32')( 'every foreground executor reports the signal that ended the process', diff --git a/packages/host-kit/src/internal/exec.ts b/packages/host-kit/src/internal/exec.ts index ee3d64d676..e9611933a6 100644 --- a/packages/host-kit/src/internal/exec.ts +++ b/packages/host-kit/src/internal/exec.ts @@ -7,11 +7,7 @@ import { Readable } from 'node:stream'; import { pipeline } from 'node:stream/promises'; import { AppError, createRequestCanceledError } from '@agent-device/kernel/errors'; import { createExecTraceContext, type ExecSettlementReason } from './exec-trace.ts'; -import { - type EnvMap, - hostHomeDirectoryFromEnv, - type HostEnvironment, -} from '@agent-device/kernel/source-value'; +import type { EnvMap, HostEnvironment } from '@agent-device/kernel/source-value'; export type ExecResult = { stdout: string; @@ -158,20 +154,6 @@ export function commandDeveloperDir(): string | undefined { return scope?.host.env.DEVELOPER_DIR || undefined; } -/** - * The home directory of the host that owns the commands this scope spawns, which is the home its - * children inherit and the directory tools such as `simctl` resolve `~` from. Answers undefined - * outside a bound host, or when that host names no home, or names one that is not absolute: code - * that needs a default must then keep what the caller asked for rather than guess this process's - * home, which can belong to a different host. - */ -export function commandHostHomeDirectory(): string | undefined { - const scope = commandHostScope.getStore(); - if (!scope) return undefined; - const home = hostHomeDirectoryFromEnv({ ...scope.host.env, ...scope.requestEnv })?.trim(); - return home && path.isAbsolute(home) ? home : undefined; -} - function resolveSpawnEnv(env: NodeJS.ProcessEnv | undefined): NodeJS.ProcessEnv | undefined { const scope = commandHostScope.getStore(); if (!scope) return env; diff --git a/packages/host-kit/src/internal/host-file.ts b/packages/host-kit/src/internal/host-file.ts index 4bc2c8315c..dc278bc8d7 100644 --- a/packages/host-kit/src/internal/host-file.ts +++ b/packages/host-kit/src/internal/host-file.ts @@ -7,6 +7,18 @@ export function hostHomeDirectory(): string { return os.homedir(); } +/** + * The home directory a named host names for itself: its own HOME (or USERPROFILE on Windows). + * Returns undefined when the host env names no home; callers must fail closed or skip the + * home-bound lookup, never fall back to `os.homedir()`, because the process home is an ambient + * read that #3753 forbids. Derive per-runtime home config through this (#3794). + */ +export function hostHomeDirectoryFromEnv( + env: Record, +): string | undefined { + return env['HOME'] ?? env['USERPROFILE']; +} + export function hostTemporaryDirectory(): string { return os.tmpdir(); } @@ -123,14 +135,6 @@ export function readHostSymbolicLinkSync(filePath: string): string { return fs.readlinkSync(filePath); } -/** - * The canonical form of a path on this host: every symlink and `.`/`..` segment resolved, so two - * spellings of one directory answer with one value. Throws when the host cannot resolve the path. - */ -export function canonicalHostPathSync(candidatePath: string): string { - return fs.realpathSync(candidatePath); -} - export function removeHostFileSync(filePath: string): void { fs.unlinkSync(filePath); } diff --git a/packages/kernel/src/source-value.ts b/packages/kernel/src/source-value.ts index 773844fc3e..d6a0f87d10 100644 --- a/packages/kernel/src/source-value.ts +++ b/packages/kernel/src/source-value.ts @@ -140,16 +140,6 @@ function parseEnumSetValue( export type EnvMap = Record; -/** - * The home directory a host environment names for itself: its own `HOME`, or `USERPROFILE` on - * Windows. Returns undefined when the environment names no home; callers must fail closed or skip - * the home-bound lookup, never fall back to `os.homedir()`, because the process home is an ambient - * read that #3753 forbids. Derive per-runtime home config through this (#3794). - */ -export function hostHomeDirectoryFromEnv(env: EnvMap): string | undefined { - return env['HOME'] ?? env['USERPROFILE']; -} - /** * The process-level inputs a host hands to the code it runs: the environment and working * directory. An entry point captures it once; everything below receives it. diff --git a/packages/platform-apple/src/core/__tests__/screenshot-status-bar-concurrency.test.ts b/packages/platform-apple/src/core/__tests__/screenshot-status-bar-concurrency.test.ts index 331a7a5eb2..f165873c1e 100644 --- a/packages/platform-apple/src/core/__tests__/screenshot-status-bar-concurrency.test.ts +++ b/packages/platform-apple/src/core/__tests__/screenshot-status-bar-concurrency.test.ts @@ -1,9 +1,5 @@ import { beforeEach, test, vi } from 'vitest'; import assert from 'node:assert/strict'; -import fs from 'node:fs/promises'; -import path from 'node:path'; -import { mkdtempForTest } from '../../__tests__/tmp-dir.ts'; -import { withCommandHost } from '@agent-device/host-kit/command'; import type { DeviceInfo } from '@agent-device/kernel/device'; import { AppError } from '@agent-device/kernel/errors'; import { withFakeAppleTool } from '../../__tests__/fake-apple-tool.ts'; @@ -463,113 +459,6 @@ test('a settings write bounds its status bar command like a capture does', async ]); }); -test('one simulator reached through two spellings of its device set keeps one window', async () => { - // `simctl --set` follows aliases, so a symlinked or otherwise non-normalized spelling of one set - // reaches the same status bar. Two windows over it would let a capture snapshot another capture's - // normalization, which is the leak this owner exists to prevent. - const setRoot = await mkdtempForTest('sm11-set-'); - const canonicalSet = await fs.realpath(setRoot); - const aliasSet = path.join(canonicalSet, 'link-to-set'); - await fs.symlink(canonicalSet, aliasSet); - - const entered: string[] = []; - const gates = { canonical: createCaptureGate(), alias: createCaptureGate() }; - const deviceFor = (tag: keyof typeof gates): DeviceInfo => ({ - ...IOS_TEST_SIMULATOR, - simulatorSetPath: tag === 'canonical' ? canonicalSet : aliasSet, - }); - - await withFakeAppleTool( - (args) => { - const index = args.indexOf('status_bar'); - if (index === -1) return { stderr: `unexpected args: ${args.join(' ')}`, exitCode: 1 }; - return args[index + 2] === 'list' ? CLEARED_STATUS_BAR_LISTING : ''; - }, - async () => { - const captureFor = (tag: keyof typeof gates) => - captureSimulatorScreenshotWithFallback(deviceFor(tag), `/tmp/${tag}.png`, { - env: process.env, - normalizeStatusBar: true, - skipIosSimulatorBootCheck: true, - deps: captureDeps(async () => { - entered.push(tag); - await gates[tag].capture(); - }), - }); - - const canonical = captureFor('canonical'); - await gates.canonical.entered; - const alias = captureFor('alias'); - try { - await yieldToQueuedWork(); - // The aliased capture is still queued: both records address one simulator, so they share - // one window and the second cannot snapshot the first capture's normalization. - assert.deepEqual(entered, ['canonical']); - } finally { - gates.canonical.release(); - gates.alias.release(); - await Promise.allSettled([canonical, alias]); - } - - assert.deepEqual(entered, ['canonical', 'alias']); - }, - ); -}); - -test('naming the default device set explicitly shares the default window', async () => { - // `simctl --set ` selects the same simulators as naming no set, so the two spellings - // must not open two windows over one status bar. The host that runs the command owns that root, - // so the test binds one whose HOME is the home the owner derives it from. - const entered: string[] = []; - const gates = { implicit: createCaptureGate(), explicit: createCaptureGate() }; - const home = process.env.HOME; - assert.ok(home, 'the default device root needs a home directory'); - const defaultSet = path.join(home, 'Library', 'Developer', 'CoreSimulator', 'Devices'); - const deviceFor = (tag: keyof typeof gates): DeviceInfo => ({ - ...IOS_TEST_SIMULATOR, - simulatorSetPath: tag === 'explicit' ? defaultSet : undefined, - }); - - await withCommandHost( - { host: { env: { HOME: home }, cwd: '/' }, requestEnv: undefined }, - async () => { - await withFakeAppleTool( - (args) => { - const index = args.indexOf('status_bar'); - if (index === -1) return { stderr: `unexpected args: ${args.join(' ')}`, exitCode: 1 }; - return args[index + 2] === 'list' ? CLEARED_STATUS_BAR_LISTING : ''; - }, - async () => { - const captureFor = (tag: keyof typeof gates) => - captureSimulatorScreenshotWithFallback(deviceFor(tag), `/tmp/${tag}.png`, { - env: process.env, - normalizeStatusBar: true, - skipIosSimulatorBootCheck: true, - deps: captureDeps(async () => { - entered.push(tag); - await gates[tag].capture(); - }), - }); - - const implicit = captureFor('implicit'); - await gates.implicit.entered; - const explicit = captureFor('explicit'); - try { - await yieldToQueuedWork(); - assert.deepEqual(entered, ['implicit']); - } finally { - gates.implicit.release(); - gates.explicit.release(); - await Promise.allSettled([implicit, explicit]); - } - - assert.deepEqual(entered, ['implicit', 'explicit']); - }, - ); - }, - ); -}); - test('a timed-out settings write forces the next capture to read the bar again', async () => { // A command that hits its deadline may still have reached `simctl`, so a write that times out // cannot leave a cached answer about the bar behind: the next capture has to read what the write @@ -636,9 +525,7 @@ test('a timed-out settings write forces the next capture to read the bar again', ); }); -test('simulators of different device sets keep their own status bar windows', async () => { - // `simctl status_bar` is scoped with `--set`, so one udid in two sets is two bars. Records that - // name one simulator — same udid and same set — share a window; different sets must not queue. +test('two device records with the same udid and different set paths share one lock', async () => { const entered: string[] = []; const gates = { one: createCaptureGate(), two: createCaptureGate() }; const deviceFor = (tag: keyof typeof gates): DeviceInfo => ({ @@ -652,7 +539,7 @@ test('simulators of different device sets keep their own status bar windows', as const index = args.indexOf('status_bar'); if (index === -1) return { stderr: `unexpected args: ${args.join(' ')}`, exitCode: 1 }; const verb = args[index + 2]; - return verb === 'list' ? 'Current Status Bar Overrides:\n=============================' : ''; + return verb === 'list' ? CLEARED_STATUS_BAR_LISTING : ''; }, async () => { const captureFor = (tag: keyof typeof gates) => @@ -671,13 +558,14 @@ test('simulators of different device sets keep their own status bar windows', as const second = captureFor('two'); try { await yieldToQueuedWork(); - // Both capture steps run at once: the second set never queued behind the first. - assert.deepEqual(entered, ['one', 'two']); + assert.deepEqual(entered, ['one']); } finally { gates.one.release(); gates.two.release(); - await Promise.allSettled([first, second]); + const results = await Promise.allSettled([first, second]); + assert.equal(fulfilledCount(results), 2); } + assert.deepEqual(entered, ['one', 'two']); }, ); }); diff --git a/packages/platform-apple/src/core/__tests__/simctl.test.ts b/packages/platform-apple/src/core/__tests__/simctl.test.ts index 10c401a3ef..16192b32f3 100644 --- a/packages/platform-apple/src/core/__tests__/simctl.test.ts +++ b/packages/platform-apple/src/core/__tests__/simctl.test.ts @@ -1,9 +1,5 @@ import { test } from 'vitest'; import assert from 'node:assert/strict'; -import fs from 'node:fs/promises'; -import path from 'node:path'; -import { withCommandHost } from '@agent-device/host-kit/command'; -import { mkdtempForTest } from '../../__tests__/tmp-dir.ts'; import { buildSimctlArgsForAddress, buildSimctlArgsForDevice, @@ -71,97 +67,6 @@ test('simctlListInventoryArgs prefixes a trimmed simulator set and omits a blank assert.deepEqual(simctlListInventoryArgs(undefined), ['list', '-j']); }); -/** - * The default device root is where `simctl` looks when argv names no set, and `simctl` resolves it - * from the environment it is spawned with. A daemon request runs on a bound command host whose HOME - * can differ from this process's, so the identity of "the default set" belongs to that host. - */ -async function withHostHome(home: string | undefined, run: () => Promise): Promise { - return await withCommandHost( - { host: { env: home === undefined ? {} : { HOME: home }, cwd: '/' }, requestEnv: undefined }, - run, - ); -} - -test('an explicit set is kept when the executing host names a different home', async () => { - // The regression: this process's home is the real default root, the host that will run `simctl` - // names another one. Erasing the caller's explicit set here would run the command against the - // process user's simulators instead of the ones the caller asked for. - const requested = await mkdtempForTest('sm11-host-home-set-'); - const canonicalRequested = await fs.realpath(requested); - const processHome = await fs.realpath(process.env.HOME ?? ''); - const processHomeDefault = path.join( - processHome, - 'Library', - 'Developer', - 'CoreSimulator', - 'Devices', - ); - - await withHostHome('/tmp/another-host-home', async () => { - assert.deepEqual(simctlListInventoryArgs(canonicalRequested), [ - '--set', - canonicalRequested, - 'list', - '-j', - ]); - assert.deepEqual( - simulatorAddressFor({ ...IOS_SIMULATOR, simulatorSetPath: canonicalRequested }), - { udid: 'sim-1', simulatorSetPath: canonicalRequested }, - ); - }); - - // Naming the process user's own default root is not the host's default either. - await withHostHome('/tmp/another-host-home', async () => { - assert.deepEqual(simctlListInventoryArgs(processHomeDefault), [ - '--set', - processHomeDefault, - 'list', - '-j', - ]); - }); -}); - -test('naming the executing host default root answers as the default set', async () => { - // The host that runs the command is the authority: when its own HOME names the default root, - // spelling it explicitly is the same set as naming none, so both share one lock/cache identity. - const hostHome = await fs.realpath(process.env.HOME ?? ''); - const defaultSet = path.join(hostHome, 'Library', 'Developer', 'CoreSimulator', 'Devices'); - - await withHostHome(hostHome, async () => { - assert.deepEqual(simctlListInventoryArgs(defaultSet), ['list', '-j']); - assert.deepEqual(simulatorAddressFor({ ...IOS_SIMULATOR, simulatorSetPath: defaultSet }), { - udid: 'sim-1', - simulatorSetPath: undefined, - }); - }); -}); - -test('an explicit set survives a host that names no home', async () => { - // No authority means no default to compare against: the caller's scope is kept, never guessed. - const requested = await mkdtempForTest('sm11-unbound-set-'); - const canonicalRequested = await fs.realpath(requested); - await withHostHome(undefined, async () => { - assert.deepEqual(simctlListInventoryArgs(canonicalRequested), [ - '--set', - canonicalRequested, - 'list', - '-j', - ]); - }); - await withCommandHost( - { host: { env: { HOME: 'relative-home' }, cwd: '/' }, requestEnv: undefined }, - async () => { - assert.deepEqual(simctlListInventoryArgs(canonicalRequested), [ - '--set', - canonicalRequested, - 'list', - '-j', - ]); - }, - ); -}); - test('simctlAvailabilityProbeArgs names no set', () => { assert.deepEqual(simctlAvailabilityProbeArgs(), ['help']); }); diff --git a/packages/platform-apple/src/core/screenshot-status-bar.ts b/packages/platform-apple/src/core/screenshot-status-bar.ts index e683c1783b..2a1729f730 100644 --- a/packages/platform-apple/src/core/screenshot-status-bar.ts +++ b/packages/platform-apple/src/core/screenshot-status-bar.ts @@ -4,7 +4,7 @@ import { emitDiagnostic } from '@agent-device/host-kit/diagnostics'; import { AppError } from '@agent-device/kernel/errors'; import { heldKeyedLockKeys, withKeyedLock } from '@agent-device/kernel/keyed-lock'; -import { runSimctlForDevice, simulatorAddressFor } from './simctl.ts'; +import { runSimctlForDevice } from './simctl.ts'; import { extractAppleToolErrorMeta } from './tool-diagnostics.ts'; type RestorableStatusBarOverrides = Partial< @@ -206,15 +206,8 @@ function hasKnownClearedStatusBarOverrides(device: DeviceInfo): boolean { return devicesKnownWithoutStatusBarOverrides.has(statusBarSimulatorKey(device)); } -/** - * Key for both the known-cleared cache and the transaction lock. It names the simulator's whole - * address, because that is the thing whose bar the commands write: `runSimctlForDevice` scopes every - * status bar command to the device set, so two records of one simulator share one key, and one - * simulator of each set gets its own transaction. - */ function statusBarSimulatorKey(device: DeviceInfo): string { - const { simulatorSetPath } = simulatorAddressFor(device); - return `${device.platform}:${device.kind}:${device.id}:${simulatorSetPath ?? ''}`; + return `${device.platform}:${device.kind}:${device.id}`; } async function readSimulatorStatusBarOverrides( diff --git a/packages/platform-apple/src/core/simctl.ts b/packages/platform-apple/src/core/simctl.ts index 26c435ee5f..db5e65485f 100644 --- a/packages/platform-apple/src/core/simctl.ts +++ b/packages/platform-apple/src/core/simctl.ts @@ -1,13 +1,7 @@ import { isIosFamily, type DeviceInfo } from '@agent-device/kernel/device'; -import { - commandHostHomeDirectory, - type ExecOptions, - type ExecResult, -} from '@agent-device/host-kit/command'; -import { canonicalHostPathSync } from '@agent-device/host-kit/host-file'; +import { type ExecOptions, type ExecResult } from '@agent-device/host-kit/command'; import { resolveIosSimulatorDeviceSetPath } from '@agent-device/kernel/device-isolation'; import type { ScopedSimctlArgs } from '@agent-device/contracts/platform-runtime-host'; -import path from 'node:path'; import { runXcrun, simctlCommand, type ScopedSimctlCommand } from './tool-provider.ts'; declare const simulatorAddress: unique symbol; @@ -19,48 +13,10 @@ export type SimulatorAddress = Readonly<{ udid: string; simulatorSetPath: string readonly [simulatorAddress]: true; }; -/** - * The device root `simctl` uses when a command names no set, as the host that will run the command - * sees it: `simctl` resolves `~` from the environment it is spawned with, which is the bound command - * host's, not this process's. Answers undefined when no host is bound or that host names no home. - */ -function defaultSimulatorSetPath(): string | undefined { - const home = commandHostHomeDirectory(); - return home === undefined - ? undefined - : path.join(home, 'Library', 'Developer', 'CoreSimulator', 'Devices'); -} - -/** - * Canonical identity of one simulator set: `simctl --set` follows aliases and trailing separators, - * so every spelling of one directory must answer as one value, and naming the executing host's - * default root explicitly is the default set. Unreadable paths (a set that does not exist on this - * host) keep their resolved spelling. An explicit set is only ever dropped for the default root of - * the host that runs the command: without that host's home the explicit set is kept, because a - * guessed default could erase a scope the caller asked for. - */ -function canonicalSimulatorSetPath(simulatorSetPath: string | undefined): string | undefined { - const spelled = resolveIosSimulatorDeviceSetPath(simulatorSetPath); - if (spelled === undefined) return undefined; - const canonical = canonicalExistingPath(spelled); - const defaultSet = defaultSimulatorSetPath(); - if (defaultSet === undefined) return canonical; - return canonical === canonicalExistingPath(defaultSet) ? undefined : canonical; -} - -function canonicalExistingPath(candidate: string): string { - const resolved = path.resolve(candidate); - try { - return canonicalHostPathSync(resolved); - } catch { - return path.normalize(resolved); - } -} - export function simulatorAddressFor(device: DeviceInfo): SimulatorAddress { const simulatorSetPath = isIosFamily(device) && device.kind === 'simulator' - ? canonicalSimulatorSetPath(device.simulatorSetPath) + ? resolveIosSimulatorDeviceSetPath(device.simulatorSetPath) : undefined; return Object.freeze({ udid: device.id, simulatorSetPath }) as SimulatorAddress; } @@ -69,7 +25,7 @@ function scopeSimctlArgs( simulatorSetPath: string | undefined, args: readonly string[], ): ScopedSimctlArgs { - const resolvedSetPath = canonicalSimulatorSetPath(simulatorSetPath); + const resolvedSetPath = resolveIosSimulatorDeviceSetPath(simulatorSetPath); const scoped = resolvedSetPath ? ['--set', resolvedSetPath, ...args] : [...args]; return Object.freeze(scoped) as ScopedSimctlArgs; } diff --git a/packages/platform-vega/src/vega-executable.ts b/packages/platform-vega/src/vega-executable.ts index cc75b6ef6b..64cd9db278 100644 --- a/packages/platform-vega/src/vega-executable.ts +++ b/packages/platform-vega/src/vega-executable.ts @@ -1,6 +1,7 @@ import path from 'node:path'; import { isExecutablePath, whichCmd } from '@agent-device/host-kit/command'; -import { type EnvMap, hostHomeDirectoryFromEnv } from '@agent-device/kernel/source-value'; +import { hostHomeDirectoryFromEnv } from '@agent-device/host-kit/host-file'; +import type { EnvMap } from '@agent-device/kernel/source-value'; /** * The host ports one Vega executable lookup needs — the same three the device-inventory host diff --git a/src/platform-runtime-host.ts b/src/platform-runtime-host.ts index 197a31aa2b..366844f90e 100644 --- a/src/platform-runtime-host.ts +++ b/src/platform-runtime-host.ts @@ -1,4 +1,4 @@ -import { hostHomeDirectoryFromEnv, type HostEnvironment } from '@agent-device/kernel/source-value'; +import type { HostEnvironment } from '@agent-device/kernel/source-value'; import type { DeviceInventoryHost, HostOperatingSystem, @@ -12,6 +12,7 @@ import path from 'node:path'; import { createAppleToolHost } from './platform-runtime-apple-tool-host.ts'; import { createHostToolchainPreparer } from './platform-runtime-toolchain-host.ts'; import { createHostCommandRunner } from './platform-runtime-host-command-runner.ts'; +import { hostHomeDirectoryFromEnv } from '@agent-device/host-kit/host-file'; export function createDeviceInventoryHost(host: HostEnvironment): DeviceInventoryHost { const env = host.env; From be0846f81c868339e01a1faff6047ffe1450acee Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Pierzcha=C5=82a?= Date: Sun, 1 Nov 2026 22:55:55 +0100 Subject: [PATCH 15/15] docs(ios): clarify status bar request and command timeouts --- packages/command-surface/src/capture/settings.ts | 3 ++- website/docs/docs/commands.md | 4 ++-- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/packages/command-surface/src/capture/settings.ts b/packages/command-surface/src/capture/settings.ts index d79aaa1f29..b444cd9d1e 100644 --- a/packages/command-surface/src/capture/settings.ts +++ b/packages/command-surface/src/capture/settings.ts @@ -102,7 +102,8 @@ export const settingsCommandFacet = defineCommandFacet({ summary: 'Change OS settings and app permissions', cliDetail: [ 'A bare readable setting reads the value the target holds. A target that does not serve a setting refuses it with UNSUPPORTED_OPERATION before anything changes.', - 'On an iOS simulator, settings wifi and settings airplane write the status bar, which screenshots share: each waits until no screenshot of that simulator is in flight, then bounds its own status bar command to 10 seconds. That 10 seconds covers the write after the wait, never the wait. A write that hits the deadline may already have changed the bar. Only cancelling while the request is still waiting guarantees nothing was written.', + '', + 'On an iOS simulator, settings wifi and settings airplane wait for screenshots and other status bar writes. The request timeout includes this wait (90 seconds for a standalone CLI request). Once the write starts, its status bar command has a separate 10-second timeout. A request or write that times out may already have changed the bar; check the bar before relying on it. Cancelling while still queued guarantees nothing was written.', '', 'Settings:', ...settingsHelpLines().map((line) => ` ${line}`), diff --git a/website/docs/docs/commands.md b/website/docs/docs/commands.md index 69bb7afc51..369fee7c14 100644 --- a/website/docs/docs/commands.md +++ b/website/docs/docs/commands.md @@ -758,7 +758,7 @@ Platform support: - `settings appearance` sets macOS appearance, iOS simulator appearance, or Android night mode. - `settings location set ` sets coordinates on iOS simulators and Android emulators. - A target that does not serve a setting refuses it with `UNSUPPORTED_OPERATION` and `error.details.reason` (for example `unsupported-platform-leaf`) before anything changes. A request that names an app the change cannot take is refused first, with `INVALID_ARGS` and `setting_app_not_consumed`. `settings --help` describes where each setting runs. -- On an iOS simulator, `settings wifi` and `settings airplane` write the status bar, which screenshots share. Each waits until no screenshot of that simulator is in flight, then allows 10 seconds for its own write; the 10 seconds covers the write, never the wait, so a request issued during a screenshot can take as long as that screenshot. If that 10 seconds runs out, the bar may already have changed — read it back before you rely on it. Only cancelling while the request is still waiting guarantees nothing was written. +- On an iOS simulator, `settings wifi` and `settings airplane` wait for screenshots and other status bar writes. The request timeout includes this wait (90 seconds for a standalone CLI request). Once the write starts, its status bar command has a separate 10-second timeout. A request or write that times out may already have changed the bar; check the bar before relying on it. Cancelling while still queued guarantees nothing was written. Device settings: @@ -1042,7 +1042,7 @@ agent-device record stop # Stop active recording - `--overlay-refs` takes a fresh snapshot and burns visible `@eN` refs and their rectangles into the PNG. With `--scale`, refs are drawn at the final size, so avoid scales where labels become unreadable. In `--json` mode each ref also has a screenshot-space `center` for `press ` fallback. - `--crop-on ` crops the capture to the frame the selector resolves on the same screen. It works only on iOS simulators, Android emulators, and physical iPhones. A physical iPhone crop is verified only in portrait using CoreDevice/devicectl; the XCTest backend and runner fallback are not accepted. Physical iPads remain rejected for crop, and landscape iPhone crop remains unverified. It needs exactly one framed node, cannot be combined with `--overlay-refs` or `--fullscreen`, and warns when the frame is clipped. The crop is re-encoded, so do not byte-compare it with an older crop. - `--fullscreen` on macOS applies only to app sessions. Other surfaces (`desktop`, `menubar`, `frontmost-app`) always capture the main display and refuse `--fullscreen` with `INVALID_ARGS`. -- `--normalize-status-bar` temporarily normalizes iOS simulator status-bar chrome for reusable baselines. +- `--normalize-status-bar` temporarily normalizes iOS simulator status-bar chrome for reusable baselines. Each status-bar read or preparation command has a 10-second timeout. Each restore command has a 30-second timeout, so clearing and reapplying overrides can take up to 60 seconds. The request timeout includes the wait and can expire before these steps finish. - On an iOS simulator, a screenshot waits until no other screenshot or `settings wifi` / `settings airplane` write is using that simulator's status bar, so overlapping captures cannot see each other's temporary chrome. A request you cancel while it is waiting fails with the cancellation instead of capturing. - On single-display iOS simulators and Android devices, the result may report `displayRotation` (`portrait`, `portrait-upside-down`, `landscape-left`, or `landscape-right`). It is absent when the device returns no reading in time, on other targets, and on multi-display devices such as foldables. The saved image is always upright.