From b5a510dd9a24d722c51446e0e1bf6017742acb24 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Pierzcha=C5=82a?= Date: Thu, 24 Sep 2026 07:53:28 +0200 Subject: [PATCH] fix(ios): refuse every runner read over a not-running session app The refusal now keys on the runner's own read trait, not on a host-stamped per-request mark: isReadOnlyCommand covers findText, readText, snapshot, gestureViewport, and alert get, so a mutation's leading read -- the viewport read a gesture starts with, the capture that resolves a selector click/fill -- refuses with APP_NOT_RUNNING exactly like a user-level read. A bare activate over a launch SpringBoard holds behind its "Open in ...?" confirmation is a launch without the URL, whatever command asked for the read. The daemon-side request-app-intent plumbing and the observationOnly wire field are removed: the runner's command trait table is the single source of the rule. The refusal stays retriable for a wait poll while the transport reads it as a definite answer; open, activate, and interactions that mutate without a leading read keep the foreground repair. This adopts the rule picked in the #2852 review (option 1) and says so in the CHANGELOG, the foreground-repairs docs section, and the physical-device help topic's lifecycle facts. --- .github/workflows/ios.yml | 2 + CHANGELOG.md | 11 ++++++ .../RunnerTests+CommandDispatch.swift | 5 +++ .../RunnerTests+Lifecycle.swift | 25 ++++++++++++ .../RunnerTests.swift | 2 + .../RunnerTests+LifecycleTests.swift | 36 +++++++++++++++++ .../__tests__/runner-recovery-wiring.test.ts | 27 +++++++++++++ .../src/runner/runner-contract.ts | 39 ++++++++++++------- .../src/runner/runner-error-classification.ts | 16 +++++++- src/commands/schema/cli-help.ts | 1 + website/docs/docs/commands.md | 9 +++++ 11 files changed, 158 insertions(+), 15 deletions(-) diff --git a/.github/workflows/ios.yml b/.github/workflows/ios.yml index 4bc494a504..759c20b554 100644 --- a/.github/workflows/ios.yml +++ b/.github/workflows/ios.yml @@ -157,6 +157,8 @@ jobs: -only-testing:AgentDeviceRunnerUITests/RunnerTests/testRecordStartThrowsTheCaptureRefusalItReceived \ -only-testing:AgentDeviceRunnerUITests/RunnerTests/testAlertDispatchResolvesItsOwnModalWithoutCoordinateTapRoutingProbe \ -only-testing:AgentDeviceRunnerUITests/RunnerTests/testAlertResolutionCannotBypassRequestedDeadline \ + -only-testing:AgentDeviceRunnerUITests/RunnerTests/testReadRefusesToLaunchANotRunningSessionApp \ + -only-testing:AgentDeviceRunnerUITests/RunnerTests/testNonReadCommandStillLaunchesANotRunningSessionApp \ -only-testing:AgentDeviceRunnerUITests/RunnerTests/testTypeWithoutResolvedInputReturnsTypedFailureBeforeDispatchingText \ -only-testing:AgentDeviceRunnerUITests/RunnerTests/testBareTypeUsesTappedInputWhenSoftwareKeyboardIsHidden \ -only-testing:AgentDeviceRunnerUITests/RunnerTests/testHardwareKeyboardResponderConfirmsItsOwnKeyboardFocus \ diff --git a/CHANGELOG.md b/CHANGELOG.md index 187b1f3762..f4ee6cdf56 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -33,6 +33,17 @@ config that sets `screenshotFullscreen` for one of those surfaces now fails instead of succeeding with the same image it always produced; drop the flag there. macOS app sessions and every other platform keep accepting `--fullscreen` unchanged. (#2799) +- Fixed (ios): no runner read launches a session app that is not running anymore. The XCTest runner + repaired foreground loss by calling `activate()`, which launches an app that is not running. After + `open --relaunch --launch-url ` on a Simulator, iOS can hold the launch behind an + "Open in …?" confirmation, and the first read of the waiting session launched the app without the + URL. Reads — `snapshot`, `wait`, `is`, `get`, a reading `find`, and an interaction's leading reads + (the viewport read a `gesture` starts with, the capture that resolves a selector `click`/`fill`) — + now refuse with `COMMAND_FAILED`, `details.runnerErrorCode: "APP_NOT_RUNNING"`, `retriable: true` + and a hint to answer the prompt with `alert accept` or relaunch with `open`. A `wait` polls + through the refusal; the runner transport never resends it. Interactions that mutate without a + leading read (`press`, a coordinate `fill`, `swipe`, `scroll`, hardware keys) keep the foreground + repair and still bring a stopped app up. - Fixed (ios): a local Simulator snapshot taken through the host AX bridge once again publishes the geometric `hittable` fact, so `is hittable` and a `hittable:` selector resolve the same controls on the bridge and the XCTest runner. The snapshot capability table has declared `hittable = diff --git a/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/RunnerTests+CommandDispatch.swift b/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/RunnerTests+CommandDispatch.swift index a791984dc1..2d4e3ffb6e 100644 --- a/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/RunnerTests+CommandDispatch.swift +++ b/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/RunnerTests+CommandDispatch.swift @@ -441,6 +441,11 @@ extension RunnerTests { let normalizedBundleId = command.appBundleId? .trimmingCharacters(in: .whitespacesAndNewlines) let requestedBundleId = (normalizedBundleId?.isEmpty == true) ? nil : normalizedBundleId + if let bundleId = requestedBundleId, + let notRunning = notRunningReadResponse(command: command, bundleId: bundleId) + { + return .response(notRunning) + } if let bundleId = requestedBundleId { if currentBundleId != bundleId || currentApp == nil { _ = activateTarget(bundleId: bundleId, reason: "bundle_changed") diff --git a/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/RunnerTests+Lifecycle.swift b/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/RunnerTests+Lifecycle.swift index 0314cbab9b..18302d7892 100644 --- a/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/RunnerTests+Lifecycle.swift +++ b/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/RunnerTests+Lifecycle.swift @@ -316,6 +316,31 @@ extension RunnerTests { return foreign.count == 1 ? foreign.first : nil } + /// `activate()` on a not-running app is a bare launch, which would drop the URL of a launch + /// SpringBoard still holds behind its "Open in …?" confirmation; see `APP_NOT_RUNNING_RUNNER_CODE`. + func notRunningReadResponse(command: Command, bundleId: String) -> Response? { +#if os(iOS) + guard isReadOnlyCommand(command), + XCUIApplication(bundleIdentifier: bundleId).state == .notRunning + else { return nil } + NSLog( + "AGENT_DEVICE_RUNNER_READ_TARGET_NOT_RUNNING bundle=%@ command=%@", + bundleId, + command.command.rawValue + ) + return Response( + ok: false, + error: ErrorPayload( + code: RunnerWireErrorCode.appNotRunning, + message: "app '\(bundleId)' is not running", + hint: "Reads do not launch the app. Relaunch it with open; if a system prompt such as a deep-link confirmation holds its launch, answer it with alert accept." + ) + ) +#else + return nil +#endif + } + func activateTarget(bundleId: String, reason: String) -> XCUIApplication { let target = XCUIApplication(bundleIdentifier: bundleId) let initialState = target.state diff --git a/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/RunnerTests.swift b/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/RunnerTests.swift index 0785df2b96..49296b93cb 100644 --- a/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/RunnerTests.swift +++ b/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/RunnerTests.swift @@ -31,8 +31,10 @@ final class RunnerTests: XCTestCase { /// String codes the daemon keys behavior on. `RUNNER_BUSY` and `RUNNER_WEDGED` come from the busy /// gate; `MAIN_THREAD_TIMEOUT` is emitted by the transport when a command trips the execution /// watchdog, so the daemon can tell "the main thread is now occupied" from a generic failure. + /// `APP_NOT_RUNNING` refuses a read whose session app is not running. enum RunnerWireErrorCode { static let mainThreadTimeout = "MAIN_THREAD_TIMEOUT" + static let appNotRunning = "APP_NOT_RUNNING" } static let springboardBundleId = "com.apple.springboard" diff --git a/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/UnitTests/RunnerTests+LifecycleTests.swift b/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/UnitTests/RunnerTests+LifecycleTests.swift index 559fa9021b..a4e75fe198 100644 --- a/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/UnitTests/RunnerTests+LifecycleTests.swift +++ b/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/UnitTests/RunnerTests+LifecycleTests.swift @@ -134,3 +134,39 @@ extension RunnerTests { } } #endif + +#if AGENT_DEVICE_RUNNER_UNIT_TESTS && os(iOS) +extension RunnerTests { + /// Installed on every Simulator runtime and cheap to leave terminated. + private static let notRunningTargetBundleId = "com.apple.Preferences" + + private func executeOnTerminatedTarget(_ json: String) throws -> (Response, XCUIApplication) { + let target = XCUIApplication(bundleIdentifier: Self.notRunningTargetBundleId) + target.terminate() + invalidateCachedTarget(reason: "unit_test_setup") + defer { invalidateCachedTarget(reason: "unit_test_cleanup") } + return (try execute(command: try runnerCommandFixture(json)), target) + } + + /// Covers a user-level read and a mutation's leading read (a gesture's `gestureViewport`). + func testReadRefusesToLaunchANotRunningSessionApp() throws { + for command in ["snapshot", "gestureViewport"] { + let (response, target) = try executeOnTerminatedTarget( + #"{"command":"\#(command)","commandId":"read","appBundleId":"\#(Self.notRunningTargetBundleId)"}"# + ) + XCTAssertEqual(response.error?.code, RunnerWireErrorCode.appNotRunning, command) + XCTAssertEqual(target.state, .notRunning, "\(command) must not launch the session app") + target.terminate() + } + } + + func testNonReadCommandStillLaunchesANotRunningSessionApp() throws { + let (response, target) = try executeOnTerminatedTarget( + #"{"command":"activate","commandId":"repair","appBundleId":"\#(Self.notRunningTargetBundleId)"}"# + ) + defer { target.terminate() } + XCTAssertTrue(response.ok) + XCTAssertNotEqual(target.state, .notRunning) + } +} +#endif diff --git a/packages/platform-apple/src/runner/__tests__/runner-recovery-wiring.test.ts b/packages/platform-apple/src/runner/__tests__/runner-recovery-wiring.test.ts index 662d1d503a..5ca6489722 100644 --- a/packages/platform-apple/src/runner/__tests__/runner-recovery-wiring.test.ts +++ b/packages/platform-apple/src/runner/__tests__/runner-recovery-wiring.test.ts @@ -319,3 +319,30 @@ test.each([undefined, 'get', 'accept', 'dismiss'] as const)( } }, ); + +// The refusal is retriable for the caller's own poll, but to the transport it is a definite runner +// answer: no resend, no lost-response status probe, and no invalidation of a healthy session. +test('a read refused over a not-running app is one definite answer, not a transport retry', async () => { + server = await startFakeRunnerServer({ + snapshot: [ + { kind: 'runnerError', code: 'APP_NOT_RUNNING', message: "app 'com.example' is not running" }, + ], + }); + seedSession(server.port); + + await assert.rejects( + runAppleRunnerCommand(IOS_SIMULATOR, { command: 'snapshot' }), + (error: unknown) => { + assert.ok(error instanceof AppError); + assert.equal(error.code, 'COMMAND_FAILED'); + assert.equal(error.details?.runnerErrorCode, 'APP_NOT_RUNNING'); + assert.equal(error.details?.retriable, true); + return true; + }, + ); + assert.deepEqual( + server.requests.map((request) => request.command).filter((command) => command !== 'uptime'), + ['snapshot'], + ); + assert.equal(invalidateRunnerSessionMock.mock.calls.length, 0); +}); diff --git a/packages/platform-apple/src/runner/runner-contract.ts b/packages/platform-apple/src/runner/runner-contract.ts index b7ecaafd14..3ffb929e19 100644 --- a/packages/platform-apple/src/runner/runner-contract.ts +++ b/packages/platform-apple/src/runner/runner-contract.ts @@ -27,6 +27,18 @@ export const RUNNER_BUSY_RUNNER_CODE = 'RUNNER_BUSY'; */ export const MAIN_THREAD_TIMEOUT_RUNNER_CODE = 'MAIN_THREAD_TIMEOUT'; +/** + * The runner's own code for a read whose session app is not running. No runner read launches the + * app — a bare launch would drop the payload of a launch still pending, such as a deep link held + * behind SpringBoard's confirmation — so the runner refuses any command carrying its read-only + * trait, including a mutation's leading read (a gesture's viewport read, a selector's resolving + * capture). Only the iOS runner refuses (`#if os(iOS)`); the macOS, tvOS and visionOS runners keep the + * activate repair. The refusal describes one poll: the launch that confirmation releases may still be + * starting when the next read arrives, so it is retriable for a `wait`, while the transport reads + * it as a definite answer and never resends it. + */ +export const APP_NOT_RUNNING_RUNNER_CODE = 'APP_NOT_RUNNING'; + export type RunnerCommand = { command: | 'tap' @@ -175,15 +187,17 @@ export const RUNNER_SCREEN_CAPTURE_REFUSAL_RUNNER_CODES: ReadonlySet = n /** * Runner codes that classify a failure for the host without renaming it on the wire. They stay * `COMMAND_FAILED` and survive as `details.runnerErrorCode`, which is what family policy reads: - * `RUNNER_BUSY` for retriable contention, `ALERT_NOT_FOUND` for an alert that is not there yet, and - * the scroll keyboard refusal for a surface the runner declined to swipe under the keys. + * `RUNNER_BUSY` for retriable contention, `ALERT_NOT_FOUND` for an alert that is not there yet, + * the scroll keyboard refusal for a surface the runner declined to swipe under the keys, and the + * retriable `APP_NOT_RUNNING` for a read the runner refused rather than launch the session app. */ -const DIAGNOSTIC_ONLY_RUNNER_ERROR_CODES: ReadonlySet = new Set([ - RUNNER_BUSY_RUNNER_CODE, - MAIN_THREAD_TIMEOUT_RUNNER_CODE, - ALERT_NOT_FOUND_RUNNER_CODE, - SCROLL_KEYBOARD_OCCLUDES_SURFACE_RUNNER_CODE, - ...RUNNER_SCREEN_CAPTURE_REFUSAL_RUNNER_CODES, +const DIAGNOSTIC_ONLY_RUNNER_ERROR_CODES: ReadonlyMap = new Map([ + [RUNNER_BUSY_RUNNER_CODE, { retriable: true }], + [MAIN_THREAD_TIMEOUT_RUNNER_CODE, {}], + [APP_NOT_RUNNING_RUNNER_CODE, { retriable: true }], + [ALERT_NOT_FOUND_RUNNER_CODE, {}], + [SCROLL_KEYBOARD_OCCLUDES_SURFACE_RUNNER_CODE, {}], + ...[...RUNNER_SCREEN_CAPTURE_REFUSAL_RUNNER_CODES].map((code) => [code, {}] as const), ]); /** Wire code plus the details every path must publish for one runner-reported error code. */ @@ -202,13 +216,12 @@ export function classifyRunnerReportedError( runnerErrorCode: string | undefined, ): RunnerReportedErrorClass { const diagnosticOnly = - runnerErrorCode !== undefined && DIAGNOSTIC_ONLY_RUNNER_ERROR_CODES.has(runnerErrorCode); + runnerErrorCode === undefined + ? undefined + : DIAGNOSTIC_ONLY_RUNNER_ERROR_CODES.get(runnerErrorCode); return Object.freeze({ code: diagnosticOnly ? 'COMMAND_FAILED' : toAppErrorCode(runnerErrorCode), - details: Object.freeze({ - runnerErrorCode, - ...(runnerErrorCode === RUNNER_BUSY_RUNNER_CODE ? { retriable: true as const } : {}), - }), + details: Object.freeze({ runnerErrorCode, ...diagnosticOnly }), }); } diff --git a/packages/platform-apple/src/runner/runner-error-classification.ts b/packages/platform-apple/src/runner/runner-error-classification.ts index 32514622aa..9d5fca5d2c 100644 --- a/packages/platform-apple/src/runner/runner-error-classification.ts +++ b/packages/platform-apple/src/runner/runner-error-classification.ts @@ -9,7 +9,11 @@ import { type IosDeveloperDiskImageState, type IosDeveloperModeState, } from './host.ts'; -import { MAIN_THREAD_TIMEOUT_RUNNER_CODE, RUNNER_BUSY_RUNNER_CODE } from './runner-contract.ts'; +import { + APP_NOT_RUNNING_RUNNER_CODE, + MAIN_THREAD_TIMEOUT_RUNNER_CODE, + RUNNER_BUSY_RUNNER_CODE, +} from './runner-contract.ts'; export const RUNNER_CACHE_RECOVERY_HINT = 'If runner build products look stale or corrupted, run `pnpm clean:xcuitest` in a local checkout, or remove ~/.agent-device/apple-runner/derived, then retry.'; @@ -66,6 +70,8 @@ type RunnerErrorMatch = { }; const hasRetriableFlag: RunnerErrorDetailsMatch = (details) => details.retriable === true; +const hasAppNotRunningRunnerCode: RunnerErrorDetailsMatch = (details) => + details.runnerErrorCode === APP_NOT_RUNNING_RUNNER_CODE; /** * The host's own `DevToolsSecurity -status` read, published as typed details by the probe that * takes it. The build-failure rule below keys on this field and never on the probe's message, so @@ -199,7 +205,8 @@ const PROFILE_UNUSABLE: RunnerErrorRule['buildFailure'] = { * `flagged_retriable` precedes the denials (an explicitly retriable error * stays retriable whatever its message says), and `usbmux_device_unattached` * sits first (retrying cannot attach a cable, and its typed verdict carries - * the recovery hint a generic connect failure would replace). + * the recovery hint a generic connect failure would replace). `app_not_running` + * precedes it too: its retriable flag is for the caller's poll, not a resend. */ export const RUNNER_ERROR_RULES: readonly RunnerErrorRule[] = [ { @@ -207,6 +214,11 @@ export const RUNNER_ERROR_RULES: readonly RunnerErrorRule[] = [ match: { code: 'DEVICE_NOT_FOUND', details: hasUsbmuxDeviceUnattached }, verdicts: { connectRetry: false }, }, + { + reason: 'app_not_running', + match: { code: 'COMMAND_FAILED', details: hasAppNotRunningRunnerCode }, + verdicts: { retryable: false, connectRetry: false }, + }, { reason: 'flagged_retriable', match: { code: 'COMMAND_FAILED', details: hasRetriableFlag }, diff --git a/src/commands/schema/cli-help.ts b/src/commands/schema/cli-help.ts index 9afafa45dd..d72fcd1264 100644 --- a/src/commands/schema/cli-help.ts +++ b/src/commands/schema/cli-help.ts @@ -699,6 +699,7 @@ Android physical-device prerequisites: Runner and daemon lifecycle (applies to simulators too): open without --relaunch is idempotent-foreground for an already-running app (it brings the process forward; it does not restart it). open --relaunch restarts the app; on iOS simulators this collapses to one simctl launch --terminate-running-process call instead of a separate terminate-then-launch. + No runner read launches a session app that is not running: snapshot, wait, is, get, a reading find, and an interaction's leading reads (a gesture's viewport read, the capture that resolves a selector click/fill) answer the retriable APP_NOT_RUNNING instead of bare-launching over a launch SpringBoard still holds behind its deep-link confirmation. Only open, activate, and a command that mutates without a leading read bring a stopped app up. close keeps a healthy iOS simulator XCTest runner warm by default so the next open on that device skips the runner build, unless --shutdown was requested, the session was recording, the session held a device lease, or the device used a scoped (non-default) simulator set. A retained runner auto-stops after an idle window (default 5 minutes); set AGENT_DEVICE_IOS_RUNNER_IDLE_STOP_MS to override, or 0 to disable idle stop and retain until daemon exit. Each AGENT_DEVICE_STATE_DIR runs its own daemon. It self-exits after an idle window (default 5 minutes, matching the runner idle-stop default) once it has no open sessions, no in-flight requests, and no active recording; set AGENT_DEVICE_DAEMON_IDLE_TIMEOUT_MS to override, or 0 to disable idle reap. A stale iOS runner lease — its owner process dead, or its AGENT_DEVICE_STATE_DIR deleted — is reclaimed automatically instead of failing with "is already owned by another agent-device daemon". A live owner's runner is also reclaimed when the requesting daemon holds the host-global device claim for that device: claims are exclusive, so holding one proves the runner's owner released the device and merely kept the runner warm. The error remains only for owners outside claim arbitration (a pre-claims build, or daemons pointed at different claim stores). diff --git a/website/docs/docs/commands.md b/website/docs/docs/commands.md index ad3175a063..516cc31350 100644 --- a/website/docs/docs/commands.md +++ b/website/docs/docs/commands.md @@ -835,6 +835,15 @@ state the session app was found in and why the runner activated it: distinction matters, spend a `snapshot -i` and read its disclosure. - The warning is appended; staleness, snapshot-quality, and occluding-system-surface warnings that came before it are never replaced. +- **No read launches a stopped app.** A command that only observes the app (`snapshot`, `wait`, + `is`, `get`, a reading `find`) repairs a backgrounded session app, but never launches one that is + not running — and neither does an interaction's leading read: the viewport read a `gesture` starts + with, or the capture that resolves a selector `click`/`fill`. A bare launch would start the app + without the URL of a launch that SpringBoard is still holding behind an "Open in …?" confirmation. + A refused command answers `COMMAND_FAILED`, `details.runnerErrorCode: "APP_NOT_RUNNING"` and + `retriable: true`, and `wait` keeps polling through it. Answer the prompt with `alert accept`, or + relaunch with `open`. Only an interaction that mutates without a leading read — `press`, a + coordinate `fill`, `swipe`, `scroll`, a hardware key — still brings a stopped app up. ## Clipboard