From a5dbdf3195c87548040d62ed0319c549eef5051c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Pierzcha=C5=82a?= Date: Thu, 24 Sep 2026 13:58:00 +0200 Subject: [PATCH 1/4] fix(ios-runner): give app launch its own policy axis MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit CommandTraits.readOnly was documented as retry eligibility and consumed as five decisions, so opting a command out of one silently opted it out of the rest. querySelector is deliberately not retried, and as a side effect it stopped being refused while its app was stopped: it bare-launched the app, which #2852 forbids for a runner read. Replace readOnly with the facts each decision actually asks for — retryOnSessionLoss, launchPolicy (noApp | existingApp | mayLaunch), convertsRecordedFailure, and clearsRememberedTextEntryTap — and replace isLifecycle, which served both the activation bypass and the recorded-failure exemption. Payload-dependent facts resolve in one exhaustive switch against Command, so no consumer re-derives the payload rule and CommandTraits.ReadOnly.conditional is gone. A command hosted by the surface that already has focus keeps the route its platform proved: the skip is iOS-only, so macOS and tvOS still activate, and only iOS answers a stopped-app read with APP_NOT_RUNNING. querySelector now refuses rather than launching, and stays non-retried. Co-Authored-By: opencode --- .../RunnerTests+CommandDispatch.swift | 35 ++- .../RunnerTests+CommandExecution.swift | 2 +- .../RunnerTests+Lifecycle.swift | 37 +--- .../RunnerTests+Models.swift | 206 +++++++++++++----- .../RunnerTests+CommandDispatchTests.swift | 75 +++++-- .../RunnerTests+LifecycleTests.swift | 27 ++- .../UnitTests/RunnerTests+ModelsTests.swift | 85 ++++++++ docs/adr/0014-session-ref-frame-lifetime.md | 5 +- 8 files changed, 337 insertions(+), 135 deletions(-) diff --git a/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/RunnerTests+CommandDispatch.swift b/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/RunnerTests+CommandDispatch.swift index f58f712686..04da86b2df 100644 --- a/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/RunnerTests+CommandDispatch.swift +++ b/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/RunnerTests+CommandDispatch.swift @@ -222,7 +222,7 @@ extension RunnerTests { invalidateCachedTarget(reason: "xctest_recorded_failure") return failureResponse } - if !hasRetried, shouldRetryCommand(command), shouldRetryResponse(response) { + if !hasRetried, command.traits.retryOnSessionLoss, shouldRetryResponse(response) { NSLog( "AGENT_DEVICE_RUNNER_RETRY command=%@ reason=response_unavailable", command.command.rawValue @@ -281,7 +281,7 @@ extension RunnerTests { } return recordedFailureResponse } - if !hasRetried, shouldRetryCommand(command), shouldRetryResponse(response) { + if !hasRetried, command.traits.retryOnSessionLoss, shouldRetryResponse(response) { NSLog( "AGENT_DEVICE_RUNNER_RETRY command=%@ reason=response_unavailable", command.command.rawValue @@ -426,7 +426,10 @@ extension RunnerTests { var systemSurface: SystemSurfaceHost? = nil if routeToSpringboard { activeApp = springboard - } else if shouldSkipAppActivationPreflight(command) { + } else if command.traits.launchPolicy == .noApp || shouldSkipAppActivationPreflight(command) { + // A command that answers from an in-place surface or from state the runner already holds, or a + // synthesized coordinate tap whose cached target is already foreground: none of them may bring + // anything forward, so the target is resolved as it stands. activeApp = resolveAppWithoutActivation(command: command) } else if let presented = presentedSystemSurfaceHost() { // Serve and drive the presented surface IN PLACE: never activate it (that cancels what it @@ -434,15 +437,17 @@ extension RunnerTests { // command resolves back to the still-bound session app (#2438). activeApp = presented.app systemSurface = presented.host - if isInteractionCommand(command.command) { + if command.traits.isInteraction { applyInteractionStabilizationIfNeeded() } - } else if !isRunnerLifecycleCommand(command.command) { + } else { + // The launch policy decides what happens to a stopped app here: `.existingApp` was refused + // above, and `.mayLaunch` brings the app up through the activation below. 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) + let notRunning = notRunningRefusal(command: command, bundleId: bundleId) { return .response(notRunning) } @@ -480,7 +485,7 @@ extension RunnerTests { } } - if isInteractionCommand(command.command) { + if command.traits.isInteraction { if let bundleId = requestedBundleId, activeApp.state != .runningForeground { activeApp = activateTarget(bundleId: bundleId, reason: "interaction_foreground_guard") } else if requestedBundleId == nil, activeApp.state != .runningForeground { @@ -533,7 +538,7 @@ extension RunnerTests { if response.data?.runnerFatal == true { return nil } - guard !isReadOnlyCommand(command), !isRunnerLifecycleCommand(command.command) else { + guard command.traits.convertsRecordedFailure else { return nil } return Response( @@ -546,19 +551,11 @@ extension RunnerTests { ) } + /// The one activation bypass that depends on the request rather than on the command: a tap that + /// needs nothing the preflight would bring forward. Commands whose own classification answers + /// without the session app's foreground state are handled by their `launchPolicy` (#2890). func shouldSkipAppActivationPreflight(_ command: Command) -> Bool { #if os(iOS) - if command.command == .alert { - return true - } - // A hardware Action Button press belongs to the system, not to the session app: the Shortcut or - // App Intent behind it is expected to run whether that app is foregrounded, backgrounded, or - // terminated, and activating first would foreground exactly what the press should leave alone. - // The press keeps its recorded-failure conversion, which `isLifecycle` would have removed - // (#2699, #2702 review). - if command.command == .actionButton { - return true - } // Coordinate-only synthesized taps can run after an AX-fatal foreground screen because they do not // need app activation, window lookup, keyboard lookup, or element resolution. Selector/text // interactions intentionally stay on the normal AX path because they need an element query. diff --git a/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/RunnerTests+CommandExecution.swift b/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/RunnerTests+CommandExecution.swift index 491b7962ad..086c61fed6 100644 --- a/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/RunnerTests+CommandExecution.swift +++ b/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/RunnerTests+CommandExecution.swift @@ -8,7 +8,7 @@ extension RunnerTests { alertDeadline: Date? = nil ) throws -> Response { var activeApp = activeApp - if command.command != .tap && command.command != .type && !isReadOnlyCommand(command) { + if command.traits.clearsRememberedTextEntryTap { clearRememberedTextEntryTap() } switch command.command { diff --git a/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/RunnerTests+Lifecycle.swift b/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/RunnerTests+Lifecycle.swift index 304a5b1829..8c1f78d028 100644 --- a/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/RunnerTests+Lifecycle.swift +++ b/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/RunnerTests+Lifecycle.swift @@ -316,11 +316,12 @@ 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? { + /// The `.existingApp` refusal: `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` (#2852). + func notRunningRefusal(command: Command, bundleId: String) -> Response? { #if os(iOS) - guard isReadOnlyCommand(command), + guard command.traits.launchPolicy == .existingApp, XCUIApplication(bundleIdentifier: bundleId).state == .notRunning else { return nil } NSLog( @@ -450,43 +451,19 @@ extension RunnerTests { } } - func shouldRetryCommand(_ command: Command) -> Bool { - isReadOnlyCommand(command) - } + // MARK: - Session-Loss Retry func shouldRetryException(_ command: Command, message: String) -> Bool { - guard shouldRetryCommand(command) else { return false } + guard command.traits.retryOnSessionLoss else { return false } // XCTest raises this AX error as an ObjC exception whose reason is the only handle on it. return message.lowercased().contains("kaxerrorservernotfound") } - // MARK: - Command Classification - - func isReadOnlyCommand(_ command: Command) -> Bool { - switch command.command.traits.readOnly { - case .always: - return true - case .never: - return false - case .conditional: - // Today only `alert` is conditional: read-only when getting, mutating otherwise. - return (command.action ?? "get").lowercased() == "get" - } - } - func shouldRetryResponse(_ response: Response) -> Bool { guard response.ok == false else { return false } return response.error?.retryableFailure != nil } - func isInteractionCommand(_ command: CommandType) -> Bool { - return command.traits.isInteraction - } - - func isRunnerLifecycleCommand(_ command: CommandType) -> Bool { - return command.traits.isLifecycle - } - // MARK: - Interaction Stabilization func applyInteractionStabilizationIfNeeded() { diff --git a/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/RunnerTests+Models.swift b/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/RunnerTests+Models.swift index d33303a79f..5e95965325 100644 --- a/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/RunnerTests+Models.swift +++ b/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/RunnerTests+Models.swift @@ -39,82 +39,174 @@ enum CommandType: String, Codable, CaseIterable { case shutdown } +/// What the runner may do about a command whose app is not running. This is the only fact that +/// decides whether a stopped app is started, so it is declared per command rather than inferred +/// from whether the command may be replayed (#2890). +enum CommandLaunchPolicy: Equatable { + /// Answers from what is already on screen — an in-place system surface, or state the runner itself + /// holds — and never touches the target app's foreground state. + case noApp + /// Refuses with `APP_NOT_RUNNING` rather than starting a stopped app, because `activate()` on a + /// not-running app is a bare launch (#2852). The refusal is about a session app, so it answers an + /// explicitly requested bundle id on the platform that can read that app's state; a request naming + /// no app has no session app to refuse. + case existingApp + /// Brings the app forward, which bare-launches it when it is not running. + case mayLaunch +} + +extension CommandLaunchPolicy { + /// The policy of a command answered by the surface that already has focus, where activating an app + /// would cancel exactly what the command is about. Only iOS proved that skip before this axis + /// existed, so macOS and tvOS keep the route they had: `mayLaunch`, which still serves a presented + /// surface first and activates only when nothing is presented. + static var hostedByFocusedSurface: CommandLaunchPolicy { +#if os(iOS) + return .noApp +#else + return .mayLaunch +#endif + } +} + /// Runner command traits — see CONTEXT.md ("Runner command traits"). /// -/// Single source of truth for how the runner classifies a command across three -/// independent axes, replacing the three hand-maintained switches that used to live -/// in RunnerTests+Lifecycle.swift (isInteractionCommand / isReadOnlyCommand / -/// isRunnerLifecycleCommand). The classification is load-bearing for ADR-0002 session -/// invalidation: `readOnly` gates the retry that nulls currentApp/currentBundleId. -struct CommandTraits { +/// Single source of truth for how the runner classifies one request. Each fact names the one +/// decision that reads it, so opting a command out of a decision is a declaration about that +/// decision alone and cannot silently move another. `Command.traits` resolves them against the +/// request, so a payload-dependent fact is settled once instead of re-read per consumer. +/// +/// The classification is load-bearing for ADR-0002 session invalidation: `retryOnSessionLoss` gates +/// the retry that nulls currentApp/currentBundleId, and `launchPolicy` — never the retry fact — +/// decides whether a stopped app is brought up. +struct CommandTraits: Equatable { /// Whether the command needs the foreground-guard + stabilization preflight before running. let isInteraction: Bool /// Whether the command is eligible for the session-invalidating retry. - /// `.conditional` is resolved against the request (alert is read-only only for its `get` action). - let readOnly: ReadOnly - /// Whether the command skips the app-activation preflight entirely. - let isLifecycle: Bool - - enum ReadOnly { - case always - case never - /// Alert-only today. Resolved in `isReadOnlyCommand` with alert's rule (read-only for the - /// `get` action, mutating otherwise). A new `.conditional` command would inherit that rule - /// until the resolver is generalized — give it explicit handling there if its semantics differ. - case conditional - } + let retryOnSessionLoss: Bool + /// What the runner may do when the command's app is not running. + let launchPolicy: CommandLaunchPolicy + /// Whether an XCTest-recorded failure during this command turns its own healthy response into a + /// failure and invalidates the session. That conversion is the only evidence a mutation with no + /// settle and no post-action observation ever landed, while a command that reports the runner's own + /// state or drives its lifecycle has no user-visible mutation to prove. + let convertsRecordedFailure: Bool + /// Whether serving this command makes a remembered text-entry tap stale. Consumed by the prepared + /// command path, so a command that answers before that path has a declaration and no consumer. + let clearsRememberedTextEntryTap: Bool } extension CommandType { - /// The classification for this command. Exhaustive by construction: a new CommandType - /// cannot compile without being classified here, so commands can no longer silently drift - /// out of classification the way the parallel switches allowed. - var traits: CommandTraits { + /// The classification of this command for one request. Exhaustive by construction: a new + /// CommandType cannot compile without declaring every fact, and the facts that depend on the + /// payload are settled here rather than re-read by each consumer. + func traits(for command: Command) -> CommandTraits { switch self { // Interaction commands: require the foreground-guard + stabilization preflight. // keyboardReturn is the sibling of keyboardDismiss (missing from the historical switch — // drift the table now prevents). .scroll is the fused frame-resolve + drag scroll; same // classification as .drag. .desktopScroll is the macOS frame-resolve + wheel event sibling. // .sequence is the fused multi-step gesture batch. - case .tap, .longPress, .drag, .remotePress, .type, .swipe, .scroll, .desktopScroll, + case .longPress, .drag, .remotePress, .swipe, .scroll, .desktopScroll, .backInApp, .backSystem, .rotate, .appSwitcher, .keyboardDismiss, .keyboardReturn, .sequence, .gesture: - return CommandTraits(isInteraction: true, readOnly: .never, isLifecycle: false) - - // Read-only reads: eligible for the session-invalidating retry. + return CommandTraits( + isInteraction: true, + retryOnSessionLoss: false, + launchPolicy: .mayLaunch, + convertsRecordedFailure: true, + clearsRememberedTextEntryTap: true + ) + + // The two interactions that own the text-entry witness themselves: `tap` records it and clears + // it where a tap demonstrably did not land, and `type` reads the one this command relies on. + case .tap, .type: + return CommandTraits( + isInteraction: true, + retryOnSessionLoss: false, + launchPolicy: .mayLaunch, + convertsRecordedFailure: true, + clearsRememberedTextEntryTap: false + ) + + // Reads of the session app: replayable after session invalidation, and refused rather than + // answered by starting the app. case .findText, .readText, .snapshot, .gestureViewport: - return CommandTraits(isInteraction: false, readOnly: .always, isLifecycle: false) - - // Screenshot is both a read and a runner-lifecycle command (skips app-activation preflight). - case .screenshot: - return CommandTraits(isInteraction: false, readOnly: .always, isLifecycle: true) - - // Alert is read-only only for its `get` action (resolved by isReadOnlyCommand). + return CommandTraits( + isInteraction: false, + retryOnSessionLoss: true, + launchPolicy: .existingApp, + convertsRecordedFailure: false, + clearsRememberedTextEntryTap: false + ) + + // Reads the runner answers from its own capture and state, so preparation never brings an app + // forward; a capture aimed at an app still observes that app while it executes. + case .screenshot, .status: + return CommandTraits( + isInteraction: false, + retryOnSessionLoss: true, + launchPolicy: .noApp, + convertsRecordedFailure: false, + clearsRememberedTextEntryTap: false + ) + + // Alert answers from the modal where it sits: activating anything would cancel the surface the + // command is about. Only its `get` action changes nothing, so only `get` may be replayed. case .alert: - return CommandTraits(isInteraction: false, readOnly: .conditional, isLifecycle: false) - - // Runner-lifecycle commands: skip the app-activation preflight. + let reads = (command.action ?? "get").lowercased() == "get" + return CommandTraits( + isInteraction: false, + retryOnSessionLoss: reads, + launchPolicy: .hostedByFocusedSurface, + convertsRecordedFailure: !reads, + clearsRememberedTextEntryTap: !reads + ) + + // The runner's own lifecycle: no session app is brought forward, and no mutation is proven. case .recordStop, .uptime, .terminate, .targetReset, .shutdown: - return CommandTraits(isInteraction: false, readOnly: .never, isLifecycle: true) - - // A hardware press mutates, is not an element interaction, and is not runner-lifecycle. It stays - // outside the lifecycle group because that flag also exempts a command from the recorded-failure - // conversion, and this command has no settle or post-action observation, so that conversion is - // the only evidence the press landed. It skips the app-activation preflight on its own terms in - // `shouldSkipAppActivationPreflight`, the way `.alert` does (#2699, #2702 review). + return CommandTraits( + isInteraction: false, + retryOnSessionLoss: false, + launchPolicy: .noApp, + convertsRecordedFailure: false, + clearsRememberedTextEntryTap: true + ) + + // A hardware press belongs to the system, not to the session app, and the press has no settle + // or post-action observation, so the recorded-failure conversion is its only evidence. case .actionButton: - return CommandTraits(isInteraction: false, readOnly: .never, isLifecycle: false) - - case .status: - return CommandTraits(isInteraction: false, readOnly: .always, isLifecycle: true) + return CommandTraits( + isInteraction: false, + retryOnSessionLoss: false, + launchPolicy: .hostedByFocusedSurface, + convertsRecordedFailure: true, + clearsRememberedTextEntryTap: true + ) + + // Selector resolution is an observation: it refuses a stopped app instead of bare-launching it, + // and the runner still must not replay it after session invalidation. Those are two facts about + // one command, which is why they are two declarations (#2890). + case .querySelector: + return CommandTraits( + isInteraction: false, + retryOnSessionLoss: false, + launchPolicy: .existingApp, + convertsRecordedFailure: true, + clearsRememberedTextEntryTap: true + ) - // Normal preflight, not retried. // NOTE: mouseClick stays non-interaction for now — it is macOS-only and the foreground // guard interacts with bespoke macOS activation, so classifying it needs a macOS smoke - // check first (tracked as a follow-up). Also preserved: querySelector is NOT read-only; - // recordStart is NOT a lifecycle command; home/alert remain non-interaction by design. - case .mouseClick, .querySelector, .home, .recordStart, .activate: - return CommandTraits(isInteraction: false, readOnly: .never, isLifecycle: false) + // check first (tracked as a follow-up). + case .mouseClick, .home, .recordStart, .activate: + return CommandTraits( + isInteraction: false, + retryOnSessionLoss: false, + launchPolicy: .mayLaunch, + convertsRecordedFailure: true, + clearsRememberedTextEntryTap: true + ) } } } @@ -159,6 +251,14 @@ struct Command: Codable { let steps: [SequenceStep]? } +extension Command { + /// How the runner classifies this request, resolved once so every consumer of a + /// payload-dependent fact reads the same decision. + var traits: CommandTraits { + command.traits(for: self) + } +} + enum ScrollReleaseBehavior: String, Codable { case controlled case inertial diff --git a/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/UnitTests/RunnerTests+CommandDispatchTests.swift b/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/UnitTests/RunnerTests+CommandDispatchTests.swift index e9b6c6a1b2..2a56e63a97 100644 --- a/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/UnitTests/RunnerTests+CommandDispatchTests.swift +++ b/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/UnitTests/RunnerTests+CommandDispatchTests.swift @@ -29,8 +29,8 @@ extension RunnerTests { func testXCTestRecordedFailureResponseFailsActionButtonSuccess() throws { // The Action Button press carries no settle and no post-action observation, so this conversion is - // the only evidence the press landed. That is why the press is not classified runner-lifecycle: - // `isLifecycle` would silence the conversion here (#2699, #2702 review). + // the only evidence the press landed. That is why the press declares `convertsRecordedFailure` + // even though its launch policy keeps it out of the app-activation preflight (#2699, #2702). let command = try runnerCommandFixture(#"{"command":"actionButton","commandId":"action-button-1"}"#) let response = Response(ok: true, data: DataPayload(message: "actionButton")) @@ -88,6 +88,58 @@ extension RunnerTests { XCTAssertFalse(snapshotXCTestPenaltyWarmupExemption.isPending) } + /// A command hosted by the surface that already has focus is the activation bypass itself: it + /// resolves its target as it stands, leaves a stopped app stopped, and binds nothing, so the next + /// read of that app is refused instead of answered by a bare launch (#2890). + func testFocusedSurfaceCommandLeavesAStoppedAppStoppedAndUnbound() throws { + let unstarted = XCUIApplication(bundleIdentifier: "com.apple.Preferences") + defer { invalidateCachedTarget(reason: "unit_test_cleanup") } + for request in [ + #"{"command":"actionButton","commandId":"press-1","appBundleId":"com.apple.Preferences"}"#, + #"{"command":"alert","action":"get","commandId":"alert-1","appBundleId":"com.apple.Preferences"}"# + ] { + unstarted.terminate() + pendingTargetActivation = nil + currentApp = nil + currentBundleId = nil + let command = try runnerCommandFixture(request) + + guard case .context(let prepared) = prepareActiveCommandContext(command: command) else { + return XCTFail("\(request) must be prepared, not refused") + } + // Leaving the app stopped is only half of the bypass. The command must still be served against + // the app it names: a hosted alert belongs to that app, and routing the request to SpringBoard + // or to the runner's own host app would answer a different screen than the caller asked about — + // with nothing launched, so no assertion below would notice. The stopped app is the only target + // that reads as `.notRunning`, which is what separates it from every substitute. + XCTAssertEqual( + prepared.app.state, + .notRunning, + "\(request) must be prepared against the stopped app it names, not a live surface" + ) + XCTAssertNil( + prepared.systemSurface, + "\(request) must be served from the named app, not from a surface presented over it" + ) + XCTAssertEqual( + unstarted.state, + .notRunning, + "\(request) may not foreground the app it was told to leave alone" + ) + XCTAssertNil(pendingTargetActivation, "\(request) may not record an activation fact") + XCTAssertNil(currentBundleId, "\(request) may not bind a target it never brought forward") + } + + let read = try runnerCommandFixture( + #"{"command":"snapshot","commandId":"read","appBundleId":"com.apple.Preferences"}"# + ) + guard case .response(let refusal) = prepareActiveCommandContext(command: read), + refusal.error?.code == RunnerWireErrorCode.appNotRunning + else { + return XCTFail("a command that bound no target must leave the next read refused, not launched") + } + } + func testSkipAppActivationPreflightIncludesForegroundCachedCoordinateOnlyTaps() throws { app.launch() currentApp = app @@ -133,17 +185,6 @@ extension RunnerTests { XCTAssertFalse(shouldSkipAppActivationPreflight(coordinateTap)) } - func testActionButtonPressSkipsAppActivationPreflightWithoutBeingRunnerLifecycle() throws { - currentApp = nil - currentBundleId = nil - let press = try runnerCommandFixture(#"{"command":"actionButton","commandId":"action-button-1"}"#) - - // The skip is its own decision, reached without the lifecycle flag that would also drop the - // recorded-failure conversion; no cached target and no foreground app is required for it. - XCTAssertFalse(isRunnerLifecycleCommand(.actionButton)) - XCTAssertTrue(shouldSkipAppActivationPreflight(press)) - } - func testPrepareActiveCommandContextRoutesBlockingSystemModalToSpringboard() throws { blockingSystemModalPresenceOverrideForTesting = true currentApp = nil @@ -392,14 +433,6 @@ extension RunnerTests { XCTAssertFalse(shouldSkipAppActivationPreflight(scroll)) XCTAssertFalse(shouldSkipAppActivationPreflight(sequence)) } - - func testSkipAppActivationPreflightIncludesAlertCommands() throws { - let alert = try runnerCommandFixture( - #"{"command":"alert","commandId":"alert-1","action":"get"}"# - ) - - XCTAssertTrue(shouldSkipAppActivationPreflight(alert)) - } #endif func testDispatchReturnsBusyBeforeQueueingMainThreadWork() throws { diff --git a/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/UnitTests/RunnerTests+LifecycleTests.swift b/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/UnitTests/RunnerTests+LifecycleTests.swift index 352cdbce1c..3c9e933cb9 100644 --- a/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/UnitTests/RunnerTests+LifecycleTests.swift +++ b/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/UnitTests/RunnerTests+LifecycleTests.swift @@ -9,7 +9,7 @@ private struct AlertCommandTraitsFixture: Decodable { } extension RunnerTests { - func testAlertReadOnlyClassificationMatchesGoldenTable() throws { + func testAlertRetryFactMatchesTheSharedGoldenTable() throws { let fixtureURL = URL(fileURLWithPath: #filePath) .deletingLastPathComponent() .deletingLastPathComponent() @@ -24,7 +24,11 @@ extension RunnerTests { ) XCTAssertEqual(cases.map { $0.command.action }, [nil, "get", "accept", "dismiss"]) for fixture in cases { - XCTAssertEqual(isReadOnlyCommand(fixture.command), fixture.readOnly, fixture.name) + XCTAssertEqual( + fixture.command.traits.retryOnSessionLoss, + fixture.readOnly, + fixture.name + ) } } @@ -153,14 +157,19 @@ extension RunnerTests { return (try execute(command: try runnerCommandFixture(json)), target) } - /// Covers a user-level read and a mutation's leading read (a gesture's `gestureViewport`). + /// Covers a user-level read, a mutation's leading read (a gesture's `gestureViewport`), and the read + /// that resolves a selector tap (`querySelector`, whose refusal the retry fact alone used to decide, + /// #2890). 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") + let bundleId = Self.notRunningTargetBundleId + for request in [ + #"{"command":"snapshot","commandId":"read","appBundleId":"\#(bundleId)"}"#, + #"{"command":"gestureViewport","commandId":"read","appBundleId":"\#(bundleId)"}"#, + #"{"command":"querySelector","selectorKey":"label","selectorValue":"Settings","commandId":"read","appBundleId":"\#(bundleId)"}"# + ] { + let (response, target) = try executeOnTerminatedTarget(request) + XCTAssertEqual(response.error?.code, RunnerWireErrorCode.appNotRunning, request) + XCTAssertEqual(target.state, .notRunning, "\(request) must not launch the session app") target.terminate() } } diff --git a/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/UnitTests/RunnerTests+ModelsTests.swift b/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/UnitTests/RunnerTests+ModelsTests.swift index 1931498727..c2986fae3f 100644 --- a/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/UnitTests/RunnerTests+ModelsTests.swift +++ b/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/UnitTests/RunnerTests+ModelsTests.swift @@ -142,5 +142,90 @@ extension RunnerTests { "\(level) fields with no production request" ) } + + /// The five decisions `CommandType.traits(for:)` declares, in declaration order. + private func traits( + _ interaction: Bool, + _ retry: Bool, + _ launch: CommandLaunchPolicy, + _ converts: Bool, + _ clears: Bool + ) -> CommandTraits { + CommandTraits( + isInteraction: interaction, + retryOnSessionLoss: retry, + launchPolicy: launch, + convertsRecordedFailure: converts, + clearsRememberedTextEntryTap: clears + ) + } + + /// Every decision the runner makes from a classification, asserted for every command from one + /// table. `retry` is replay eligibility and `launch` is what the runner may do about a stopped + /// app: `querySelector` is the row that proves one does not set the other (#2890). + func testEveryCommandDeclaresEveryRunnerSideDecisionTogether() throws { + // command interaction retry launch converts clears + let table: [(CommandType, Bool, Bool, CommandLaunchPolicy, Bool, Bool)] = [ + (.tap, true, false, .mayLaunch, true, false), + (.mouseClick, false, false, .mayLaunch, true, true), + (.longPress, true, false, .mayLaunch, true, true), + (.drag, true, false, .mayLaunch, true, true), + (.remotePress, true, false, .mayLaunch, true, true), + (.type, true, false, .mayLaunch, true, false), + (.swipe, true, false, .mayLaunch, true, true), + (.scroll, true, false, .mayLaunch, true, true), + (.desktopScroll, true, false, .mayLaunch, true, true), + (.findText, false, true, .existingApp, false, false), + (.querySelector, false, false, .existingApp, true, true), + (.readText, false, true, .existingApp, false, false), + (.snapshot, false, true, .existingApp, false, false), + (.screenshot, false, true, .noApp, false, false), + (.backInApp, true, false, .mayLaunch, true, true), + (.backSystem, true, false, .mayLaunch, true, true), + (.home, false, false, .mayLaunch, true, true), + (.rotate, true, false, .mayLaunch, true, true), + (.appSwitcher, true, false, .mayLaunch, true, true), + (.actionButton, false, false, .hostedByFocusedSurface, true, true), + (.keyboardDismiss, true, false, .mayLaunch, true, true), + (.keyboardReturn, true, false, .mayLaunch, true, true), + (.alert, false, true, .hostedByFocusedSurface, false, false), + (.sequence, true, false, .mayLaunch, true, true), + (.gesture, true, false, .mayLaunch, true, true), + (.gestureViewport, false, true, .existingApp, false, false), + (.recordStart, false, false, .mayLaunch, true, true), + (.recordStop, false, false, .noApp, false, true), + (.status, false, true, .noApp, false, false), + (.uptime, false, false, .noApp, false, true), + (.activate, false, false, .mayLaunch, true, true), + (.terminate, false, false, .noApp, false, true), + (.targetReset, false, false, .noApp, false, true), + (.shutdown, false, false, .noApp, false, true) + ] + for (type, interaction, retry, launch, converts, clears) in table { + let request = #"{"command":"\#(type.rawValue)"}"# + let command = try runnerCommandFixture(request) + XCTAssertEqual(command.command, type, request) + XCTAssertEqual(command.traits, traits(interaction, retry, launch, converts, clears), request) + } + XCTAssertEqual( + Set(table.map { $0.0 }), + Set(CommandType.allCases), + "every command states its decisions in this table" + ) + + // The one payload-dependent command settles each fact per action: `get` changes nothing and may + // be replayed, while `accept` and `dismiss` mutate and must not be. + let alertCases: [(action: String?, expected: CommandTraits)] = [ + (nil, traits(false, true, .hostedByFocusedSurface, false, false)), + ("get", traits(false, true, .hostedByFocusedSurface, false, false)), + ("accept", traits(false, false, .hostedByFocusedSurface, true, true)), + ("dismiss", traits(false, false, .hostedByFocusedSurface, true, true)) + ] + for alertCase in alertCases { + let request = alertCase.action.map { #"{"command":"alert","action":"\#($0)"}"# } + ?? #"{"command":"alert"}"# + XCTAssertEqual(try runnerCommandFixture(request).traits, alertCase.expected, request) + } + } } #endif diff --git a/docs/adr/0014-session-ref-frame-lifetime.md b/docs/adr/0014-session-ref-frame-lifetime.md index 6205c867c8..3fb81350f4 100644 --- a/docs/adr/0014-session-ref-frame-lifetime.md +++ b/docs/adr/0014-session-ref-frame-lifetime.md @@ -275,8 +275,9 @@ specialized route. The completeness gate covers every command projected to the d generic fallbacks, so a missing facet cannot hide an unclassified mutation. Mutations performed by unrelated external tools remain outside this session guarantee. -This policy is not derived from Apple runner `readOnly`. Runner traits govern retry, liveness, -readiness probes, and preflight skipping at a lower wire-command seam. `refFrameEffect` governs +This policy is not derived from the runner-side classification. Apple runner command traits govern +retry eligibility, launch policy, and the recorded-failure conversion at a lower wire-command seam, +while the TypeScript `readOnly` trait is the one that also gates readiness probes. `refFrameEffect` governs daemon session authorization and includes commands that never reach the Apple runner. Narrow consistency tests may cover direct mappings, but blanket parity would couple different concepts. From f1e0bcbb06a4ebfa32d91aa8afd1daa7d1709002 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Pierzcha=C5=82a?= Date: Thu, 24 Sep 2026 16:49:30 +0200 Subject: [PATCH 2/4] refactor(ios-runner): read the launch policy at the row and at its consumer MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `CommandLaunchPolicy.hostedByFocusedSurface` was a computed `#if` value, so a row for a command hosted by the focused surface resolved to a bare launch off iOS, and the completeness test asserted through the same helper it was meant to check. The enum now has a real `presentedSurface` case, the one platform exception lives where the policy is read, and dispatch switches over the policy — so a new case is a compile error rather than a fall-through, and the request-dependent bypass that queries the cached target is reached only where activation is on the table. `clearsRememberedTextEntryTap` was read by one consumer that nine of its rows never reached, and equalled `convertsRecordedFailure` minus `{ tap, type }` wherever it was read. It is now derived there, from that fact plus a named set. Commands that decide alike share a named group, and `Command.traits` owns the switch directly. The classification test now compares every fact against its own literal instead of a value built by the type under test, so an initializer that swapped two facts or a row re-pointed at another policy goes red. --- .../RunnerTests+CommandDispatch.swift | 154 ++++++----- .../RunnerTests+CommandExecution.swift | 6 +- .../RunnerTests+Models.swift | 261 +++++++++--------- .../RunnerTests+CommandDispatchTests.swift | 11 +- .../UnitTests/RunnerTests+ModelsTests.swift | 158 +++++++---- 5 files changed, 327 insertions(+), 263 deletions(-) diff --git a/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/RunnerTests+CommandDispatch.swift b/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/RunnerTests+CommandDispatch.swift index 04da86b2df..0e532a4514 100644 --- a/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/RunnerTests+CommandDispatch.swift +++ b/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/RunnerTests+CommandDispatch.swift @@ -236,7 +236,7 @@ extension RunnerTests { } } - /// The dispatched snapshot recovery loop: read-only retry + XCTest-recorded-failure invalidation, + /// The dispatched snapshot recovery loop: session-loss retry + XCTest-recorded-failure invalidation, /// matching what `executeOnMainSafely` gives the generic path. `perform` runs the capture and its /// own bounded main-thread work. func executeDispatchedWithRecovery( @@ -418,93 +418,121 @@ extension RunnerTests { ) } + /// The target this command runs against, decided by its `launchPolicy` (#2890). Exhaustive over the + /// policy so a new case is a compile error here rather than a fall-through that quietly launches or + /// quietly refuses. func prepareActiveCommandContext( command: Command, routeToSpringboard: Bool = false ) -> ActiveCommandPreparation { - var activeApp = currentApp ?? app - var systemSurface: SystemSurfaceHost? = nil if routeToSpringboard { - activeApp = springboard - } else if command.traits.launchPolicy == .noApp || shouldSkipAppActivationPreflight(command) { - // A command that answers from an in-place surface or from state the runner already holds, or a - // synthesized coordinate tap whose cached target is already foreground: none of them may bring - // anything forward, so the target is resolved as it stands. - activeApp = resolveAppWithoutActivation(command: command) - } else if let presented = presentedSystemSurfaceHost() { + return .context(ActiveCommandContext(app: springboard)) + } + switch command.traits.launchPolicy { + case .noApp: + // Answers from the runner's own capture and state, so the target is resolved exactly as it + // stands. + return .context(ActiveCommandContext(app: resolveAppWithoutActivation(command: command))) + case .presentedSurface: + // The command is about the surface that already has focus; activating an app under it would + // cancel exactly what the command is about. +#if os(iOS) + return .context(ActiveCommandContext(app: resolveAppWithoutActivation(command: command))) +#else + // The platform exception, written once: `SystemSurfaceHostRegistry` registers no hosts off iOS, + // so nothing is ever served in place there and such a command keeps the activation route this + // axis found it on. + return prepareActivatedTarget(command: command) +#endif + case .existingApp, .mayLaunch: + // Asked only where activation is on the table: the bypass decides by querying the cached + // target's state, and a command that may bring nothing forward has nothing for it to settle. + if shouldSkipAppActivationPreflight(command) { + // The one request-dependent bypass: a coordinate-only synthesized tap whose cached target is + // already foreground needs nothing brought forward. + return .context(ActiveCommandContext(app: resolveAppWithoutActivation(command: command))) + } + return prepareActivatedTarget(command: command) + } + } + + /// The route that may bring something forward: a system surface genuinely on screen is served in + /// place, and otherwise the requested session app is resolved and activated. What happens to a + /// stopped app is the caller's `launchPolicy`; the `.existingApp` refusal belongs to + /// `notRunningRefusal` because it is only meaningful once nothing is presented (#2890). + private func prepareActivatedTarget(command: Command) -> ActiveCommandPreparation { + if let presented = presentedSystemSurfaceHost() { // Serve and drive the presented surface IN PLACE: never activate it (that cancels what it // presents) and never adopt it as the cached session target, so once it is gone the next // command resolves back to the still-bound session app (#2438). - activeApp = presented.app - systemSurface = presented.host if command.traits.isInteraction { applyInteractionStabilizationIfNeeded() } - } else { - // The launch policy decides what happens to a stopped app here: `.existingApp` was refused - // above, and `.mayLaunch` brings the app up through the activation below. - let normalizedBundleId = command.appBundleId? - .trimmingCharacters(in: .whitespacesAndNewlines) - let requestedBundleId = (normalizedBundleId?.isEmpty == true) ? nil : normalizedBundleId - if let bundleId = requestedBundleId, - let notRunning = notRunningRefusal(command: command, bundleId: bundleId) - { - return .response(notRunning) + return .context(ActiveCommandContext(app: presented.app, systemSurface: presented.host)) + } + + let normalizedBundleId = command.appBundleId? + .trimmingCharacters(in: .whitespacesAndNewlines) + let requestedBundleId = (normalizedBundleId?.isEmpty == true) ? nil : normalizedBundleId + if let bundleId = requestedBundleId, + let notRunning = notRunningRefusal(command: command, bundleId: bundleId) + { + return .response(notRunning) + } + if let bundleId = requestedBundleId { + if currentBundleId != bundleId || currentApp == nil { + _ = activateTarget(bundleId: bundleId, reason: "bundle_changed") + } else { + refreshCachedTargetIfProcessChanged(bundleId: bundleId) } + } else { + // Do not reuse stale bundle targets when the caller does not explicitly request one. + invalidateCachedTarget(reason: "missing_app_bundle") + } + + // Read back after the bundle resolution above, which is what may have just bound a target. + var activeApp = currentApp ?? app + if let bundleId = requestedBundleId, targetNeedsActivation(activeApp) { + activeApp = activateTarget(bundleId: bundleId, reason: "stale_target") + } else if requestedBundleId == nil, targetNeedsActivation(activeApp) { + ensureRunnerHostAppActive(reason: "missing_app_bundle") + activeApp = app + } + + let skipExistenceWait = canUseFastForegroundAppGuard( + activeApp: activeApp, + requestedBundleId: requestedBundleId + ) + if !skipExistenceWait && !activeApp.waitForExistence(timeout: appExistenceTimeout) { if let bundleId = requestedBundleId { - if currentBundleId != bundleId || currentApp == nil { - _ = activateTarget(bundleId: bundleId, reason: "bundle_changed") - } else { - refreshCachedTargetIfProcessChanged(bundleId: bundleId) + activeApp = activateTarget(bundleId: bundleId, reason: "missing_after_wait") + guard activeApp.waitForExistence(timeout: appExistenceTimeout) else { + return .response(Response(ok: false, error: .targetAppUnavailable(bundleId: bundleId))) } } else { - // Do not reuse stale bundle targets when the caller does not explicitly request one. - invalidateCachedTarget(reason: "missing_app_bundle") + return .response(Response(ok: false, error: .targetAppUnavailable(bundleId: nil))) } + } - activeApp = currentApp ?? app - if let bundleId = requestedBundleId, targetNeedsActivation(activeApp) { - activeApp = activateTarget(bundleId: bundleId, reason: "stale_target") - } else if requestedBundleId == nil, targetNeedsActivation(activeApp) { - ensureRunnerHostAppActive(reason: "missing_app_bundle") + if command.traits.isInteraction { + if let bundleId = requestedBundleId, activeApp.state != .runningForeground { + activeApp = activateTarget(bundleId: bundleId, reason: "interaction_foreground_guard") + } else if requestedBundleId == nil, activeApp.state != .runningForeground { + ensureRunnerHostAppActive(reason: "interaction_missing_app_bundle") activeApp = app } - - let skipExistenceWait = canUseFastForegroundAppGuard( + let skipInteractionExistenceWait = canUseFastForegroundAppGuard( activeApp: activeApp, requestedBundleId: requestedBundleId ) - if !skipExistenceWait && !activeApp.waitForExistence(timeout: appExistenceTimeout) { - if let bundleId = requestedBundleId { - activeApp = activateTarget(bundleId: bundleId, reason: "missing_after_wait") - guard activeApp.waitForExistence(timeout: appExistenceTimeout) else { - return .response(Response(ok: false, error: .targetAppUnavailable(bundleId: bundleId))) - } - } else { - return .response(Response(ok: false, error: .targetAppUnavailable(bundleId: nil))) - } - } - - if command.traits.isInteraction { - if let bundleId = requestedBundleId, activeApp.state != .runningForeground { - activeApp = activateTarget(bundleId: bundleId, reason: "interaction_foreground_guard") - } else if requestedBundleId == nil, activeApp.state != .runningForeground { - ensureRunnerHostAppActive(reason: "interaction_missing_app_bundle") - activeApp = app - } - let skipInteractionExistenceWait = canUseFastForegroundAppGuard( - activeApp: activeApp, - requestedBundleId: requestedBundleId + if !skipInteractionExistenceWait && !activeApp.waitForExistence(timeout: 2) { + return .response( + Response(ok: false, error: .targetAppUnavailable(bundleId: requestedBundleId)) ) - if !skipInteractionExistenceWait && !activeApp.waitForExistence(timeout: 2) { - return .response( - Response(ok: false, error: .targetAppUnavailable(bundleId: requestedBundleId)) - ) - } - applyInteractionStabilizationIfNeeded() } + applyInteractionStabilizationIfNeeded() } - return .context(ActiveCommandContext(app: activeApp, systemSurface: systemSurface)) + return .context(ActiveCommandContext(app: activeApp)) } /// A registered system surface host that is genuinely on screen, or nil. Presence is foreground diff --git a/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/RunnerTests+CommandExecution.swift b/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/RunnerTests+CommandExecution.swift index 086c61fed6..2735874292 100644 --- a/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/RunnerTests+CommandExecution.swift +++ b/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/RunnerTests+CommandExecution.swift @@ -8,7 +8,11 @@ extension RunnerTests { alertDeadline: Date? = nil ) throws -> Response { var activeApp = activeApp - if command.traits.clearsRememberedTextEntryTap { + // Every command that reaches here with a mutation to prove makes a remembered text-entry tap + // stale; the two commands that own that witness decide for themselves in their own cases below. + if command.traits.convertsRecordedFailure, + !CommandTraits.textEntryWitnessOwners.contains(command.command) + { clearRememberedTextEntryTap() } switch command.command { diff --git a/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/RunnerTests+Models.swift b/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/RunnerTests+Models.swift index 5e95965325..6c65b9986e 100644 --- a/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/RunnerTests+Models.swift +++ b/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/RunnerTests+Models.swift @@ -43,9 +43,14 @@ enum CommandType: String, Codable, CaseIterable { /// decides whether a stopped app is started, so it is declared per command rather than inferred /// from whether the command may be replayed (#2890). enum CommandLaunchPolicy: Equatable { - /// Answers from what is already on screen — an in-place system surface, or state the runner itself - /// holds — and never touches the target app's foreground state. + /// Never brings an app forward: the command answers from the runner's own capture and state, or + /// drives the runner's own lifecycle. case noApp + /// Answers from the surface that already has focus, where activating an app would cancel exactly + /// what the command is about: an in-place system surface, or a press that belongs to the system. + /// Only iOS registers surfaces that can be served in place, and + /// `prepareActiveCommandContext` is where that one platform exception is written. + case presentedSurface /// Refuses with `APP_NOT_RUNNING` rather than starting a stopped app, because `activate()` on a /// not-running app is a bare launch (#2852). The refusal is about a session app, so it answers an /// explicitly requested bundle id on the platform that can read that app's state; a request naming @@ -55,20 +60,6 @@ enum CommandLaunchPolicy: Equatable { case mayLaunch } -extension CommandLaunchPolicy { - /// The policy of a command answered by the surface that already has focus, where activating an app - /// would cancel exactly what the command is about. Only iOS proved that skip before this axis - /// existed, so macOS and tvOS keep the route they had: `mayLaunch`, which still serves a presented - /// surface first and activates only when nothing is presented. - static var hostedByFocusedSurface: CommandLaunchPolicy { -#if os(iOS) - return .noApp -#else - return .mayLaunch -#endif - } -} - /// Runner command traits — see CONTEXT.md ("Runner command traits"). /// /// Single source of truth for how the runner classifies one request. Each fact names the one @@ -76,141 +67,102 @@ extension CommandLaunchPolicy { /// decision alone and cannot silently move another. `Command.traits` resolves them against the /// request, so a payload-dependent fact is settled once instead of re-read per consumer. /// +/// Commands that decide alike share one named group instead of repeating a literal per fact, and a +/// group spells only the facts that reach its commands; a fact an arm answers before reading falls +/// to the initializer's default. The completeness test pins every fact on every command either way, +/// so a default that stopped matching its consumer is a red row rather than a silent one (#2890 +/// review). The groups are file-private so a test of the classification has to spell the facts +/// rather than re-derive them from the same names. +/// /// The classification is load-bearing for ADR-0002 session invalidation: `retryOnSessionLoss` gates /// the retry that nulls currentApp/currentBundleId, and `launchPolicy` — never the retry fact — /// decides whether a stopped app is brought up. -struct CommandTraits: Equatable { +struct CommandTraits { /// Whether the command needs the foreground-guard + stabilization preflight before running. let isInteraction: Bool /// Whether the command is eligible for the session-invalidating retry. let retryOnSessionLoss: Bool - /// What the runner may do when the command's app is not running. + /// What the runner may do when the command's app is not running. The one fact with no default: no + /// command inherits a launch answer from how it was classified for anything else. let launchPolicy: CommandLaunchPolicy /// Whether an XCTest-recorded failure during this command turns its own healthy response into a /// failure and invalidates the session. That conversion is the only evidence a mutation with no /// settle and no post-action observation ever landed, while a command that reports the runner's own /// state or drives its lifecycle has no user-visible mutation to prove. let convertsRecordedFailure: Bool - /// Whether serving this command makes a remembered text-entry tap stale. Consumed by the prepared - /// command path, so a command that answers before that path has a declaration and no consumer. - let clearsRememberedTextEntryTap: Bool -} -extension CommandType { - /// The classification of this command for one request. Exhaustive by construction: a new - /// CommandType cannot compile without declaring every fact, and the facts that depend on the - /// payload are settled here rather than re-read by each consumer. - func traits(for command: Command) -> CommandTraits { - switch self { - // Interaction commands: require the foreground-guard + stabilization preflight. - // keyboardReturn is the sibling of keyboardDismiss (missing from the historical switch — - // drift the table now prevents). .scroll is the fused frame-resolve + drag scroll; same - // classification as .drag. .desktopScroll is the macOS frame-resolve + wheel event sibling. - // .sequence is the fused multi-step gesture batch. - case .longPress, .drag, .remotePress, .swipe, .scroll, .desktopScroll, - .backInApp, .backSystem, .rotate, .appSwitcher, - .keyboardDismiss, .keyboardReturn, .sequence, .gesture: - return CommandTraits( - isInteraction: true, - retryOnSessionLoss: false, - launchPolicy: .mayLaunch, - convertsRecordedFailure: true, - clearsRememberedTextEntryTap: true - ) - - // The two interactions that own the text-entry witness themselves: `tap` records it and clears - // it where a tap demonstrably did not land, and `type` reads the one this command relies on. - case .tap, .type: - return CommandTraits( - isInteraction: true, - retryOnSessionLoss: false, - launchPolicy: .mayLaunch, - convertsRecordedFailure: true, - clearsRememberedTextEntryTap: false - ) - - // Reads of the session app: replayable after session invalidation, and refused rather than - // answered by starting the app. - case .findText, .readText, .snapshot, .gestureViewport: - return CommandTraits( - isInteraction: false, - retryOnSessionLoss: true, - launchPolicy: .existingApp, - convertsRecordedFailure: false, - clearsRememberedTextEntryTap: false - ) - - // Reads the runner answers from its own capture and state, so preparation never brings an app - // forward; a capture aimed at an app still observes that app while it executes. - case .screenshot, .status: - return CommandTraits( - isInteraction: false, - retryOnSessionLoss: true, - launchPolicy: .noApp, - convertsRecordedFailure: false, - clearsRememberedTextEntryTap: false - ) - - // Alert answers from the modal where it sits: activating anything would cancel the surface the - // command is about. Only its `get` action changes nothing, so only `get` may be replayed. - case .alert: - let reads = (command.action ?? "get").lowercased() == "get" - return CommandTraits( - isInteraction: false, - retryOnSessionLoss: reads, - launchPolicy: .hostedByFocusedSurface, - convertsRecordedFailure: !reads, - clearsRememberedTextEntryTap: !reads - ) - - // The runner's own lifecycle: no session app is brought forward, and no mutation is proven. - case .recordStop, .uptime, .terminate, .targetReset, .shutdown: - return CommandTraits( - isInteraction: false, - retryOnSessionLoss: false, - launchPolicy: .noApp, - convertsRecordedFailure: false, - clearsRememberedTextEntryTap: true - ) - - // A hardware press belongs to the system, not to the session app, and the press has no settle - // or post-action observation, so the recorded-failure conversion is its only evidence. - case .actionButton: - return CommandTraits( - isInteraction: false, - retryOnSessionLoss: false, - launchPolicy: .hostedByFocusedSurface, - convertsRecordedFailure: true, - clearsRememberedTextEntryTap: true - ) - - // Selector resolution is an observation: it refuses a stopped app instead of bare-launching it, - // and the runner still must not replay it after session invalidation. Those are two facts about - // one command, which is why they are two declarations (#2890). - case .querySelector: - return CommandTraits( - isInteraction: false, - retryOnSessionLoss: false, - launchPolicy: .existingApp, - convertsRecordedFailure: true, - clearsRememberedTextEntryTap: true - ) - - // NOTE: mouseClick stays non-interaction for now — it is macOS-only and the foreground - // guard interacts with bespoke macOS activation, so classifying it needs a macOS smoke - // check first (tracked as a follow-up). - case .mouseClick, .home, .recordStart, .activate: - return CommandTraits( - isInteraction: false, - retryOnSessionLoss: false, - launchPolicy: .mayLaunch, - convertsRecordedFailure: true, - clearsRememberedTextEntryTap: true - ) - } + init( + isInteraction: Bool = false, + retryOnSessionLoss: Bool = false, + launchPolicy: CommandLaunchPolicy, + convertsRecordedFailure: Bool = false + ) { + self.isInteraction = isInteraction + self.retryOnSessionLoss = retryOnSessionLoss + self.launchPolicy = launchPolicy + self.convertsRecordedFailure = convertsRecordedFailure } } +fileprivate extension CommandTraits { + /// Element interactions: bring the session app forward, run the preflight, and owe the + /// recorded-failure conversion for whatever the gesture did. + static let interaction = CommandTraits( + isInteraction: true, + launchPolicy: .mayLaunch, + convertsRecordedFailure: true + ) + + /// Mutations the runner performs without the element-interaction preflight. NOTE: `mouseClick` + /// stays non-interaction for now — it is macOS-only and the foreground guard interacts with + /// bespoke macOS activation, so classifying it needs a macOS smoke check first (tracked as a + /// follow-up). + static let appMutation = CommandTraits(launchPolicy: .mayLaunch, convertsRecordedFailure: true) + + /// Reads of the session app: replayable after session invalidation, and refused rather than + /// answered by starting the app. + static let appRead = CommandTraits(retryOnSessionLoss: true, launchPolicy: .existingApp) + + /// Selector resolution is an observation: it refuses a stopped app instead of bare-launching it, + /// and the runner still must not replay it after session invalidation. Those are two facts about + /// one command, which is why they are two declarations (#2890). + static let selectorResolution = CommandTraits( + launchPolicy: .existingApp, + convertsRecordedFailure: true + ) + + /// Reads the runner answers from its own capture and state, so preparation never brings an app + /// forward; a capture aimed at an app still observes that app while it executes. + static let runnerCaptureRead = CommandTraits(retryOnSessionLoss: true, launchPolicy: .noApp) + + /// The runner's own lifecycle: no session app is brought forward, and no mutation is proven. + static let runnerLifecycle = CommandTraits(launchPolicy: .noApp) + + /// Commands hosted by the surface that already has focus, which no activation may cancel. A + /// hardware press belongs to the system rather than to the session app, and an alert answers from + /// the modal where it sits; both mutate. + static let presentedSurfaceMutation = CommandTraits( + launchPolicy: .presentedSurface, + convertsRecordedFailure: true + ) + + /// `alert get` changes nothing, so it is the one alert action that may be replayed. + static let presentedSurfaceQuery = CommandTraits( + retryOnSessionLoss: true, + launchPolicy: .presentedSurface + ) +} + +extension CommandTraits { + /// The commands that own the remembered text-entry witness instead of invalidating it: `tap` + /// records it (and clears it where a tap demonstrably did not land), and `type` reads the one this + /// command relies on. Everywhere else on the prepared command path it is having a mutation to + /// prove that makes a remembered tap stale, so clearing is derived from `convertsRecordedFailure` + /// together with this set at that one consumer — not declared as a fifth fact, which the commands + /// answered before that path would have carried without ever being read (#2890 review). + static let textEntryWitnessOwners: Set = [.tap, .type] +} + struct Command: Codable { let command: CommandType let commandId: String? @@ -252,10 +204,43 @@ struct Command: Codable { } extension Command { - /// How the runner classifies this request, resolved once so every consumer of a - /// payload-dependent fact reads the same decision. + /// How the runner classifies this request. Exhaustive by construction: a new CommandType cannot + /// compile without choosing a group, and the facts that depend on the payload are settled here + /// rather than re-read by each consumer. var traits: CommandTraits { - command.traits(for: self) + switch command { + // The gesture families, each classified with what it is built from. keyboardReturn is the + // sibling of keyboardDismiss (missing from the historical switch — drift the table now + // prevents). .scroll is the fused frame-resolve + drag scroll and .desktopScroll the macOS + // frame-resolve + wheel event sibling of .drag; .sequence is the fused multi-step batch. + case .tap, .type, .longPress, .drag, .remotePress, .swipe, .scroll, .desktopScroll, + .backInApp, .backSystem, .rotate, .appSwitcher, + .keyboardDismiss, .keyboardReturn, .sequence, .gesture: + return .interaction + + case .findText, .readText, .snapshot, .gestureViewport: + return .appRead + + case .screenshot, .status: + return .runnerCaptureRead + + case .alert: + return (action ?? "get").lowercased() == "get" + ? .presentedSurfaceQuery + : .presentedSurfaceMutation + + case .recordStop, .uptime, .terminate, .targetReset, .shutdown: + return .runnerLifecycle + + case .actionButton: + return .presentedSurfaceMutation + + case .querySelector: + return .selectorResolution + + case .mouseClick, .home, .recordStart, .activate: + return .appMutation + } } } @@ -485,7 +470,7 @@ struct SnapshotQualityPayload: Codable { } } -/// A runner failure the read-only retry may recover from by re-resolving the target. +/// A runner failure the session-loss retry may recover from by re-resolving the target. enum RetryableResponseFailure: Equatable { case targetAppUnavailable } diff --git a/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/UnitTests/RunnerTests+CommandDispatchTests.swift b/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/UnitTests/RunnerTests+CommandDispatchTests.swift index 2a56e63a97..51e9aa4acd 100644 --- a/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/UnitTests/RunnerTests+CommandDispatchTests.swift +++ b/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/UnitTests/RunnerTests+CommandDispatchTests.swift @@ -88,10 +88,11 @@ extension RunnerTests { XCTAssertFalse(snapshotXCTestPenaltyWarmupExemption.isPending) } - /// A command hosted by the surface that already has focus is the activation bypass itself: it - /// resolves its target as it stands, leaves a stopped app stopped, and binds nothing, so the next - /// read of that app is refused instead of answered by a bare launch (#2890). - func testFocusedSurfaceCommandLeavesAStoppedAppStoppedAndUnbound() throws { + /// A `.presentedSurface` command is the activation bypass itself: it resolves its target as it + /// stands, leaves a stopped app stopped, and binds nothing, so the next read of that app is refused + /// instead of answered by a bare launch (#2890). This is where the table's launch policy is proved + /// on the platform that serves surfaces in place. + func testPresentedSurfaceCommandLeavesAStoppedAppStoppedAndUnbound() throws { let unstarted = XCUIApplication(bundleIdentifier: "com.apple.Preferences") defer { invalidateCachedTarget(reason: "unit_test_cleanup") } for request in [ @@ -502,7 +503,7 @@ extension RunnerTests { drainedCalls += 1 return recovered } - XCTAssertEqual(drainedCalls, 2, "with the channel free the read-only retry runs once") + XCTAssertEqual(drainedCalls, 2, "with the channel free the session-loss retry runs once") } private func setAbandonedMainThreadWork(_ count: Int) { diff --git a/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/UnitTests/RunnerTests+ModelsTests.swift b/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/UnitTests/RunnerTests+ModelsTests.swift index c2986fae3f..addd861206 100644 --- a/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/UnitTests/RunnerTests+ModelsTests.swift +++ b/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/UnitTests/RunnerTests+ModelsTests.swift @@ -143,69 +143,109 @@ extension RunnerTests { ) } - /// The five decisions `CommandType.traits(for:)` declares, in declaration order. - private func traits( - _ interaction: Bool, - _ retry: Bool, - _ launch: CommandLaunchPolicy, - _ converts: Bool, - _ clears: Bool - ) -> CommandTraits { - CommandTraits( + /// One row's expectation, as literals. It deliberately does not build a `CommandTraits`: an + /// expectation constructed by the type under test moves with it, so a declaration that swapped or + /// rewrote a fact would keep such a row green. Each fact is compared below against its own + /// literal, and the named groups the classification resolves through are file-private to it + /// (#2890 review). + private struct ExpectedTraits { + let isInteraction: Bool + let retryOnSessionLoss: Bool + let launchPolicy: CommandLaunchPolicy + let convertsRecordedFailure: Bool + } + + private func expectation( + interaction: Bool, + retry: Bool, + launch: CommandLaunchPolicy, + converts: Bool + ) -> ExpectedTraits { + ExpectedTraits( isInteraction: interaction, retryOnSessionLoss: retry, launchPolicy: launch, - convertsRecordedFailure: converts, - clearsRememberedTextEntryTap: clears + convertsRecordedFailure: converts + ) + } + + private func assertTraits( + _ traits: CommandTraits, + matches expectation: ExpectedTraits, + _ request: String + ) { + XCTAssertEqual(traits.isInteraction, expectation.isInteraction, "\(request) isInteraction") + XCTAssertEqual( + traits.retryOnSessionLoss, + expectation.retryOnSessionLoss, + "\(request) retryOnSessionLoss" + ) + XCTAssertEqual(traits.launchPolicy, expectation.launchPolicy, "\(request) launchPolicy") + XCTAssertEqual( + traits.convertsRecordedFailure, + expectation.convertsRecordedFailure, + "\(request) convertsRecordedFailure" ) } /// Every decision the runner makes from a classification, asserted for every command from one /// table. `retry` is replay eligibility and `launch` is what the runner may do about a stopped - /// app: `querySelector` is the row that proves one does not set the other (#2890). + /// app: `querySelector` is the row that proves one does not set the other (#2890). Each row names + /// a concrete launch case, so re-pointing a command at another policy fails that row. func testEveryCommandDeclaresEveryRunnerSideDecisionTogether() throws { - // command interaction retry launch converts clears - let table: [(CommandType, Bool, Bool, CommandLaunchPolicy, Bool, Bool)] = [ - (.tap, true, false, .mayLaunch, true, false), - (.mouseClick, false, false, .mayLaunch, true, true), - (.longPress, true, false, .mayLaunch, true, true), - (.drag, true, false, .mayLaunch, true, true), - (.remotePress, true, false, .mayLaunch, true, true), - (.type, true, false, .mayLaunch, true, false), - (.swipe, true, false, .mayLaunch, true, true), - (.scroll, true, false, .mayLaunch, true, true), - (.desktopScroll, true, false, .mayLaunch, true, true), - (.findText, false, true, .existingApp, false, false), - (.querySelector, false, false, .existingApp, true, true), - (.readText, false, true, .existingApp, false, false), - (.snapshot, false, true, .existingApp, false, false), - (.screenshot, false, true, .noApp, false, false), - (.backInApp, true, false, .mayLaunch, true, true), - (.backSystem, true, false, .mayLaunch, true, true), - (.home, false, false, .mayLaunch, true, true), - (.rotate, true, false, .mayLaunch, true, true), - (.appSwitcher, true, false, .mayLaunch, true, true), - (.actionButton, false, false, .hostedByFocusedSurface, true, true), - (.keyboardDismiss, true, false, .mayLaunch, true, true), - (.keyboardReturn, true, false, .mayLaunch, true, true), - (.alert, false, true, .hostedByFocusedSurface, false, false), - (.sequence, true, false, .mayLaunch, true, true), - (.gesture, true, false, .mayLaunch, true, true), - (.gestureViewport, false, true, .existingApp, false, false), - (.recordStart, false, false, .mayLaunch, true, true), - (.recordStop, false, false, .noApp, false, true), - (.status, false, true, .noApp, false, false), - (.uptime, false, false, .noApp, false, true), - (.activate, false, false, .mayLaunch, true, true), - (.terminate, false, false, .noApp, false, true), - (.targetReset, false, false, .noApp, false, true), - (.shutdown, false, false, .noApp, false, true) + let table: [(CommandType, ExpectedTraits)] = [ + (.tap, expectation(interaction: true, retry: false, launch: .mayLaunch, converts: true)), + (.mouseClick, expectation(interaction: false, retry: false, launch: .mayLaunch, converts: true)), + (.longPress, expectation(interaction: true, retry: false, launch: .mayLaunch, converts: true)), + (.drag, expectation(interaction: true, retry: false, launch: .mayLaunch, converts: true)), + (.remotePress, expectation(interaction: true, retry: false, launch: .mayLaunch, converts: true)), + (.type, expectation(interaction: true, retry: false, launch: .mayLaunch, converts: true)), + (.swipe, expectation(interaction: true, retry: false, launch: .mayLaunch, converts: true)), + (.scroll, expectation(interaction: true, retry: false, launch: .mayLaunch, converts: true)), + (.desktopScroll, expectation(interaction: true, retry: false, launch: .mayLaunch, converts: true)), + (.findText, expectation(interaction: false, retry: true, launch: .existingApp, converts: false)), + ( + .querySelector, + expectation(interaction: false, retry: false, launch: .existingApp, converts: true) + ), + (.readText, expectation(interaction: false, retry: true, launch: .existingApp, converts: false)), + (.snapshot, expectation(interaction: false, retry: true, launch: .existingApp, converts: false)), + (.screenshot, expectation(interaction: false, retry: true, launch: .noApp, converts: false)), + (.backInApp, expectation(interaction: true, retry: false, launch: .mayLaunch, converts: true)), + (.backSystem, expectation(interaction: true, retry: false, launch: .mayLaunch, converts: true)), + (.home, expectation(interaction: false, retry: false, launch: .mayLaunch, converts: true)), + (.rotate, expectation(interaction: true, retry: false, launch: .mayLaunch, converts: true)), + (.appSwitcher, expectation(interaction: true, retry: false, launch: .mayLaunch, converts: true)), + ( + .actionButton, + expectation(interaction: false, retry: false, launch: .presentedSurface, converts: true) + ), + (.keyboardDismiss, expectation(interaction: true, retry: false, launch: .mayLaunch, converts: true)), + (.keyboardReturn, expectation(interaction: true, retry: false, launch: .mayLaunch, converts: true)), + ( + .alert, + expectation(interaction: false, retry: true, launch: .presentedSurface, converts: false) + ), + (.sequence, expectation(interaction: true, retry: false, launch: .mayLaunch, converts: true)), + (.gesture, expectation(interaction: true, retry: false, launch: .mayLaunch, converts: true)), + ( + .gestureViewport, + expectation(interaction: false, retry: true, launch: .existingApp, converts: false) + ), + (.recordStart, expectation(interaction: false, retry: false, launch: .mayLaunch, converts: true)), + (.recordStop, expectation(interaction: false, retry: false, launch: .noApp, converts: false)), + (.status, expectation(interaction: false, retry: true, launch: .noApp, converts: false)), + (.uptime, expectation(interaction: false, retry: false, launch: .noApp, converts: false)), + (.activate, expectation(interaction: false, retry: false, launch: .mayLaunch, converts: true)), + (.terminate, expectation(interaction: false, retry: false, launch: .noApp, converts: false)), + (.targetReset, expectation(interaction: false, retry: false, launch: .noApp, converts: false)), + (.shutdown, expectation(interaction: false, retry: false, launch: .noApp, converts: false)) ] - for (type, interaction, retry, launch, converts, clears) in table { + for (type, rowExpectation) in table { let request = #"{"command":"\#(type.rawValue)"}"# let command = try runnerCommandFixture(request) XCTAssertEqual(command.command, type, request) - XCTAssertEqual(command.traits, traits(interaction, retry, launch, converts, clears), request) + assertTraits(command.traits, matches: rowExpectation, request) } XCTAssertEqual( Set(table.map { $0.0 }), @@ -215,16 +255,22 @@ extension RunnerTests { // The one payload-dependent command settles each fact per action: `get` changes nothing and may // be replayed, while `accept` and `dismiss` mutate and must not be. - let alertCases: [(action: String?, expected: CommandTraits)] = [ - (nil, traits(false, true, .hostedByFocusedSurface, false, false)), - ("get", traits(false, true, .hostedByFocusedSurface, false, false)), - ("accept", traits(false, false, .hostedByFocusedSurface, true, true)), - ("dismiss", traits(false, false, .hostedByFocusedSurface, true, true)) + let alertCases: [(action: String?, expectation: ExpectedTraits)] = [ + (nil, expectation(interaction: false, retry: true, launch: .presentedSurface, converts: false)), + ("get", expectation(interaction: false, retry: true, launch: .presentedSurface, converts: false)), + ( + "accept", + expectation(interaction: false, retry: false, launch: .presentedSurface, converts: true) + ), + ( + "dismiss", + expectation(interaction: false, retry: false, launch: .presentedSurface, converts: true) + ) ] for alertCase in alertCases { let request = alertCase.action.map { #"{"command":"alert","action":"\#($0)"}"# } ?? #"{"command":"alert"}"# - XCTAssertEqual(try runnerCommandFixture(request).traits, alertCase.expected, request) + assertTraits(try runnerCommandFixture(request).traits, matches: alertCase.expectation, request) } } } From 01d7bf21e77aedf42d4298d66c7f6c9538e909a0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Pierzcha=C5=82a?= Date: Thu, 24 Sep 2026 16:49:30 +0200 Subject: [PATCH 3/4] docs(ios-runner): note the selector-route refusal and name both trait layers A stopped app on the selector route now answers `APP_NOT_RUNNING` instead of launching, which a caller can see. ADR-0014 rejected deriving from "runner read-only traits", a concept this PR deleted, and described the TypeScript `readOnly` trait as gating readiness probes alone. It gates read-only resend, session-invalidation skip, transport error classification, and readiness preflight. --- docs/adr/0014-session-ref-frame-lifetime.md | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/docs/adr/0014-session-ref-frame-lifetime.md b/docs/adr/0014-session-ref-frame-lifetime.md index 3fb81350f4..d5d2e98e88 100644 --- a/docs/adr/0014-session-ref-frame-lifetime.md +++ b/docs/adr/0014-session-ref-frame-lifetime.md @@ -277,9 +277,11 @@ unrelated external tools remain outside this session guarantee. This policy is not derived from the runner-side classification. Apple runner command traits govern retry eligibility, launch policy, and the recorded-failure conversion at a lower wire-command seam, -while the TypeScript `readOnly` trait is the one that also gates readiness probes. `refFrameEffect` governs -daemon session authorization and includes commands that never reach the Apple runner. Narrow -consistency tests may cover direct mappings, but blanket parity would couple different concepts. +while the TypeScript `readOnly` trait is itself consumed as several daemon decisions: read-only +resend, session-invalidation skip, transport error classification, and readiness preflight. +`refFrameEffect` governs daemon session authorization and includes commands that never reach the +Apple runner. Narrow consistency tests may cover direct mappings, but blanket parity would couple +different concepts. Frame admission and transitions are serialized by the existing per-session request lock. The frame is shared session state, not per-client or per-lease history. Generation pins make the rejected epoch @@ -467,8 +469,9 @@ registry claims are necessary but do not substitute for this live evidence. single-current-frame contract. - **Add a per-ref historical ledger immediately:** rejected until evidence requires concurrent generation support; one bounded current frame plus issuance scope is sufficient. -- **Derive the policy from runner read-only traits:** rejected because runner liveness and daemon ref - authorization classify different commands for different reasons. +- **Derive the policy from the runner's command traits:** rejected because those traits classify a + wire command for what the runner may still do about its app, while daemon ref authorization + classifies a command for what it did to the frame. - **Add batch interpolation or an unsafe ref-stability override:** rejected as a new orchestration interface that bypasses the same safety rule. - **Force lifetime into ADR 0011's element path matrix:** rejected because ref lifetime spans commands, From 51513719b80dbaba482232b36788ee6860334ad8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Pierzcha=C5=82a?= Date: Thu, 24 Sep 2026 16:49:30 +0200 Subject: [PATCH 4/4] chore(gates): rename the alert golden table's readOnly column to query MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The runner no longer classifies commands by read-only-ness, so a shared column named after it implied a concept the runner no longer has. `query` names the fact both sides agree on — the request changes nothing — which each maps to its own consumer: replay eligibility in the runner, `readOnly` in the daemon. Also names the `.presentedSurface` dispatch proof in the iOS PR lane, so what the declared policy does to a stopped app is checked on every pull request rather than only in the nightly. --- .../UnitTests/RunnerTests+LifecycleTests.swift | 7 +++++-- contracts/fixtures/alert-command-traits.json | 8 ++++---- .../runner/__tests__/runner-command-traits.test.ts | 11 +++++++---- 3 files changed, 16 insertions(+), 10 deletions(-) diff --git a/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/UnitTests/RunnerTests+LifecycleTests.swift b/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/UnitTests/RunnerTests+LifecycleTests.swift index 3c9e933cb9..93377c219c 100644 --- a/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/UnitTests/RunnerTests+LifecycleTests.swift +++ b/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/UnitTests/RunnerTests+LifecycleTests.swift @@ -2,10 +2,13 @@ import Foundation import XCTest #if AGENT_DEVICE_RUNNER_UNIT_TESTS +/// One row of the cross-language golden table. Its `query` column names the shared fact — this alert +/// request changes nothing — which each side maps to its own consumer: replay eligibility here, and +/// the TypeScript `readOnly` trait there. private struct AlertCommandTraitsFixture: Decodable { let name: String let command: Command - let readOnly: Bool + let query: Bool } extension RunnerTests { @@ -26,7 +29,7 @@ extension RunnerTests { for fixture in cases { XCTAssertEqual( fixture.command.traits.retryOnSessionLoss, - fixture.readOnly, + fixture.query, fixture.name ) } diff --git a/contracts/fixtures/alert-command-traits.json b/contracts/fixtures/alert-command-traits.json index a3303e7be2..63eac9ab5b 100644 --- a/contracts/fixtures/alert-command-traits.json +++ b/contracts/fixtures/alert-command-traits.json @@ -1,18 +1,18 @@ [ - { "name": "default query", "command": { "command": "alert" }, "readOnly": true }, + { "name": "default query", "command": { "command": "alert" }, "query": true }, { "name": "explicit query", "command": { "command": "alert", "action": "get" }, - "readOnly": true + "query": true }, { "name": "accept mutates", "command": { "command": "alert", "action": "accept" }, - "readOnly": false + "query": false }, { "name": "dismiss mutates", "command": { "command": "alert", "action": "dismiss" }, - "readOnly": false + "query": false } ] diff --git a/packages/platform-apple/src/runner/__tests__/runner-command-traits.test.ts b/packages/platform-apple/src/runner/__tests__/runner-command-traits.test.ts index 3f5db74422..671592a694 100644 --- a/packages/platform-apple/src/runner/__tests__/runner-command-traits.test.ts +++ b/packages/platform-apple/src/runner/__tests__/runner-command-traits.test.ts @@ -85,19 +85,22 @@ test('runner command trait helpers read from the shared trait table', () => { }); test('alert actions match the native read-only golden table', () => { + // The fixture's `query` column names the shared fact — the alert request changes nothing — which + // each side consumes under its own name: `readOnly` for this daemon trait, retry eligibility for + // the Apple runner, which no longer classifies commands by read-only-ness at all. const cases = JSON.parse( fs.readFileSync( new URL('../../../../../contracts/fixtures/alert-command-traits.json', import.meta.url), 'utf8', ), - ) as Array<{ name: string; command: RunnerCommand; readOnly: boolean }>; + ) as Array<{ name: string; command: RunnerCommand; query: boolean }>; assert.deepEqual( cases.map(({ command }) => command.action), [undefined, 'get', 'accept', 'dismiss'], ); - for (const { name, command, readOnly } of cases) { - assert.deepEqual(readRunnerCommandTraits(command), { ...defaults(), readOnly }, name); - assert.equal(isReadOnlyRunnerCommand(command), readOnly, name); + for (const { name, command, query } of cases) { + assert.deepEqual(readRunnerCommandTraits(command), { ...defaults(), readOnly: query }, name); + assert.equal(isReadOnlyRunnerCommand(command), query, name); } });