diff --git a/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/RunnerTests+CommandDispatch.swift b/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/RunnerTests+CommandDispatch.swift index f58f712686..0e532a4514 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 @@ -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( @@ -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 @@ -418,88 +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 shouldSkipAppActivationPreflight(command) { - 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 isInteractionCommand(command.command) { + if command.traits.isInteraction { applyInteractionStabilizationIfNeeded() } - } else if !isRunnerLifecycleCommand(command.command) { - 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) + 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 isInteractionCommand(command.command) { - 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 @@ -533,7 +566,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 +579,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..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.command != .tap && command.command != .type && !isReadOnlyCommand(command) { + // 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+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..6c65b9986e 100644 --- a/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/RunnerTests+Models.swift +++ b/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/RunnerTests+Models.swift @@ -39,84 +39,128 @@ 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 { + /// 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 + /// 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 +} + /// 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. +/// 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. +/// +/// 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 { /// 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. 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 + + init( + isInteraction: Bool = false, + retryOnSessionLoss: Bool = false, + launchPolicy: CommandLaunchPolicy, + convertsRecordedFailure: Bool = false + ) { + self.isInteraction = isInteraction + self.retryOnSessionLoss = retryOnSessionLoss + self.launchPolicy = launchPolicy + self.convertsRecordedFailure = convertsRecordedFailure } } -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 { - 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, - .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. - 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). - case .alert: - return CommandTraits(isInteraction: false, readOnly: .conditional, isLifecycle: false) - - // Runner-lifecycle commands: skip the app-activation preflight. - case .recordStop, .uptime, .terminate, .targetReset, .shutdown: - return CommandTraits(isInteraction: false, readOnly: .never, isLifecycle: true) +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 + ) +} - // 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). - case .actionButton: - return CommandTraits(isInteraction: false, readOnly: .never, isLifecycle: false) - - case .status: - return CommandTraits(isInteraction: false, readOnly: .always, isLifecycle: 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) - } - } +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 { @@ -159,6 +203,47 @@ struct Command: Codable { let steps: [SequenceStep]? } +extension Command { + /// 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 { + 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 + } + } +} + enum ScrollReleaseBehavior: String, Codable { case controlled case inertial @@ -385,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 e9b6c6a1b2..51e9aa4acd 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,59 @@ extension RunnerTests { XCTAssertFalse(snapshotXCTestPenaltyWarmupExemption.isPending) } + /// 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 [ + #"{"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 +186,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 +434,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 { @@ -469,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+LifecycleTests.swift b/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/UnitTests/RunnerTests+LifecycleTests.swift index 352cdbce1c..93377c219c 100644 --- a/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/UnitTests/RunnerTests+LifecycleTests.swift +++ b/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/UnitTests/RunnerTests+LifecycleTests.swift @@ -2,14 +2,17 @@ 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 { - func testAlertReadOnlyClassificationMatchesGoldenTable() throws { + func testAlertRetryFactMatchesTheSharedGoldenTable() throws { let fixtureURL = URL(fileURLWithPath: #filePath) .deletingLastPathComponent() .deletingLastPathComponent() @@ -24,7 +27,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.query, + fixture.name + ) } } @@ -153,14 +160,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..addd861206 100644 --- a/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/UnitTests/RunnerTests+ModelsTests.swift +++ b/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/UnitTests/RunnerTests+ModelsTests.swift @@ -142,5 +142,136 @@ extension RunnerTests { "\(level) fields with no production request" ) } + + /// 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 + ) + } + + 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). Each row names + /// a concrete launch case, so re-pointing a command at another policy fails that row. + func testEveryCommandDeclaresEveryRunnerSideDecisionTogether() throws { + 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, rowExpectation) in table { + let request = #"{"command":"\#(type.rawValue)"}"# + let command = try runnerCommandFixture(request) + XCTAssertEqual(command.command, type, request) + assertTraits(command.traits, matches: rowExpectation, 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?, 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"}"# + assertTraits(try runnerCommandFixture(request).traits, matches: alertCase.expectation, request) + } + } } #endif 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/docs/adr/0014-session-ref-frame-lifetime.md b/docs/adr/0014-session-ref-frame-lifetime.md index 6205c867c8..d5d2e98e88 100644 --- a/docs/adr/0014-session-ref-frame-lifetime.md +++ b/docs/adr/0014-session-ref-frame-lifetime.md @@ -275,10 +275,13 @@ 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 -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. +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 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 @@ -466,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, 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); } });