diff --git a/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/RunnerTests+CommandDispatch.swift b/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/RunnerTests+CommandDispatch.swift index cc0cbaa84e..83915aacde 100644 --- a/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/RunnerTests+CommandDispatch.swift +++ b/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/RunnerTests+CommandDispatch.swift @@ -468,9 +468,14 @@ extension RunnerTests { } 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))) + // Serves a genuinely presented surface in place with its provenance, else the standing cached + // target, activating nothing and binding nothing (#2438); `Command.traits` is the member list. + if let presented = presentedSystemSurfaceHost() { + return .context(ActiveCommandContext(app: presented.app, systemSurface: presented.host)) + } + // The standing target, not the request's bundle id: this route never resolves a bundle it has + // not already bound, which is what keeps an observation from deciding which app it is about. + return .context(ActiveCommandContext(app: mainOwned.app ?? app)) 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. @@ -482,9 +487,11 @@ extension RunnerTests { // 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. + case .existingApp: + // No request-dependent bypass here: it decides by querying the cached target's state, and a + // command that may bring nothing forward has nothing for it to settle. + return prepareActivatedTarget(command: command) + case .mayLaunch: if shouldSkipAppActivationPreflight(command) { // The one request-dependent bypass: a coordinate-only synthesized tap whose cached target is // already foreground needs nothing brought forward. @@ -581,9 +588,8 @@ extension RunnerTests { private func presentedSystemSurfaceHost() -> (host: SystemSurfaceHost, app: XCUIApplication)? { #if os(iOS) for host in SystemSurfaceHostRegistry.hosts { - let candidate = XCUIApplication(bundleIdentifier: host.bundleId) - if candidate.state == .runningForeground { - return (host, candidate) + if systemSurfaceHostState(host) == .runningForeground { + return (host, XCUIApplication(bundleIdentifier: host.bundleId)) } } return nil @@ -592,6 +598,21 @@ extension RunnerTests { #endif } + /// Whether a registered host is on screen. A registered host is an out-of-process service that only + /// comes up because some app presented it, and `open` refuses to launch one, so no in-bundle test + /// can make the system report one foreground; the override answers that one question, and when it + /// is set it is authoritative for every registered host — members are foreground, non-members are + /// not — so a test pins the whole registry walk rather than the live state of what it left out. + /// The registry order and the foreground condition above stay the production ones. + private func systemSurfaceHostState(_ host: SystemSurfaceHost) -> XCUIApplication.State { + #if AGENT_DEVICE_RUNNER_UNIT_TESTS + if let override = presentedSystemSurfaceForegroundOverrideForTesting { + return override.contains(host.bundleId) ? .runningForeground : .notRunning + } + #endif + return XCUIApplication(bundleIdentifier: host.bundleId).state + } + func currentXCTestFailureCount() -> Int { return testRun?.failureCount ?? 0 } diff --git a/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/RunnerTests+CommandExecution.swift b/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/RunnerTests+CommandExecution.swift index e8389af402..a96560e417 100644 --- a/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/RunnerTests+CommandExecution.swift +++ b/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/RunnerTests+CommandExecution.swift @@ -9,11 +9,7 @@ extension RunnerTests { alertDeadline: Date? = nil ) throws -> Response { var activeApp = activeApp - // 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) - { + if command.invalidatesRememberedTextEntryTap { clearRememberedTextEntryTap() } switch command.command { diff --git a/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/RunnerTests+Models.swift b/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/RunnerTests+Models.swift index b1b707e7d7..73c7c52761 100644 --- a/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/RunnerTests+Models.swift +++ b/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/RunnerTests+Models.swift @@ -44,8 +44,15 @@ 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 { - /// Never brings an app forward: the command answers from the runner's own capture and state, or - /// drives the runner's own lifecycle. + /// Preparation brings no app forward and binds no target: the command answers from the runner's own + /// capture and state, or drives the runner's own lifecycle, so it is served the standing cached + /// target. A surface that is genuinely presented is served in place instead, which is the prepared + /// contract those commands had before the launch-policy axis (#2438). No member's response body + /// reads the prepared target or the disclosed surface: the names a `.noApp` command answers from + /// are its own capture and state, or the bundle it names — which is why neither prepared fact + /// needs a consumer here, and why a proof of this arm is a preparation test. Scoped to + /// preparation either way: a command body may still go to the app it names, as macOS `screenshot` + /// does. 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. @@ -54,8 +61,10 @@ enum CommandLaunchPolicy: Equatable { 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. + /// explicitly requested bundle id; a request naming no app has no session app to refuse. It is + /// enforced on iOS only: `notRunningRefusal` is `#if os(iOS)`, the platform that can read an app's + /// state without launching it. Off iOS these commands keep the activation route they had before + /// this axis existed, and no refusal can occur there. case existingApp /// Brings the app forward, which bare-launches it when it is not running. case mayLaunch @@ -126,7 +135,8 @@ fileprivate extension CommandTraits { /// 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). + /// one command, which is why they are two declarations (#2890). The refusal is the iOS-enforced + /// half: off iOS a selector read of a stopped app still activates it, as it did before this axis. static let selectorResolution = CommandTraits( launchPolicy: .existingApp, convertsRecordedFailure: true @@ -157,13 +167,21 @@ fileprivate extension CommandTraits { 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). + /// command relies on. static let textEntryWitnessOwners: Set = [.tap, .type] } +extension Command { + /// Whether arriving at the prepared command path invalidates a remembered text-entry tap. Not a + /// fifth trait: everywhere but the two owner commands, it is having a mutation to prove that makes + /// the witness stale, so this reads `convertsRecordedFailure` and that set rather than declaring a + /// fact no command would answer for itself (#2890 review). `executeOnMainPrepared` is its only + /// consumer, and the exhaustive table test pins the answer for every command. + var invalidatesRememberedTextEntryTap: Bool { + traits.convertsRecordedFailure && !CommandTraits.textEntryWitnessOwners.contains(command) + } +} + struct Command: Codable { let command: CommandType let commandId: String? diff --git a/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/RunnerTests.swift b/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/RunnerTests.swift index cb8c0e2cb1..3140827424 100644 --- a/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/RunnerTests.swift +++ b/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/RunnerTests.swift @@ -202,6 +202,13 @@ final class RunnerTests: XCTestCase { var blockingSystemModalPresenceOverrideForTesting: Bool? var alertResolutionOverrideForTesting: (@MainActor (Date) -> RunnerAlert?)? var alertButtonHittabilityProbeOverrideForTesting: (@MainActor (Date) -> Bool)? + // Unit-test-only seam for the in-place surface probe (`presentedSystemSurfaceHost`): a registered + // host is an out-of-process XPC service that only comes up because some app presented it, and + // `open` refuses to launch one, so the foreground answer for every registered host is supplied + // here when set — members are foreground, non-members are not — which lets a test pin the whole + // registry walk. The probe's registry order and its `.runningForeground` condition stay the + // production ones. Production never compiles this property. + var presentedSystemSurfaceForegroundOverrideForTesting: Set? // Runs on the waiting thread after `runMainThreadWork`'s wait timed out and before it takes the // lock that decides between finished and abandoned, so a test can finish the work in that window. var mainThreadWorkTimedOutForTesting: (@Sendable () -> Void)? diff --git a/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/UnitTests/RunnerTests+CommandDispatchTests.swift b/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/UnitTests/RunnerTests+CommandDispatchTests.swift index 49980d0968..76c6a642a9 100644 --- a/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/UnitTests/RunnerTests+CommandDispatchTests.swift +++ b/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/UnitTests/RunnerTests+CommandDispatchTests.swift @@ -143,6 +143,99 @@ extension RunnerTests { } } + /// Pins the `.noApp` preparation contract in `prepareActiveCommandContext`: a presented host is + /// served in place with its surface disclosed, and with nothing presented the standing cached + /// target is served rather than one resolved from the request. The request names a bundle the + /// session never bound because that is the only shape separating the two targets — when the + /// request agrees with the cache, both answers name the same app. The override is authoritative + /// for every registered host while set, so no live state is read; the probe's registry walk and + /// foreground condition stay the production ones. Deleting the presented arm fails the first + /// block, and resolving the target from the request instead of the cache fails the last. + @MainActor + func testNoAppCommandStillServesAPresentedSurfaceInPlaceAndOtherwiseTheStandingTarget() throws { + let cachedBundleId = "com.example.session" + let requestedBundleId = "com.example.requested-but-never-bound" + defer { + presentedSystemSurfaceForegroundOverrideForTesting = nil + invalidateCachedTarget(reason: "unit_test_cleanup") + } + + let host = try XCTUnwrap(SystemSurfaceHostRegistry.hosts.first) + let screenshot = try runnerCommandFixture( + #"{"command":"screenshot","commandId":"capture-1","appBundleId":"\#(requestedBundleId)"}"# + ) + mainOwned.app = app + mainOwned.bundleId = cachedBundleId + + presentedSystemSurfaceForegroundOverrideForTesting = [host.bundleId] + guard case .context(let presented) = prepareActiveCommandContext(command: screenshot) else { + return XCTFail("screenshot must be prepared, not refused") + } + XCTAssertEqual( + presented.systemSurface, + host, + "a capture prepared under a presented surface must name that surface as its prepared subject (#2438)" + ) + XCTAssertFalse( + presented.app === app, + "the prepared subject is the presented host, not the standing session target" + ) + XCTAssertFalse( + presented.app === springboard, + "a presented surface is served in place, never through SpringBoard" + ) + XCTAssertNil(pendingTargetActivation, "serving a surface in place may not record an activation") + XCTAssertEqual( + mainOwned.bundleId, + cachedBundleId, + "serving a surface in place may not rebind the session target" + ) + + // A second host reported instead of the first: an arm that returned the registry's first entry + // rather than walking it would pass everything above and fail here. The total override makes the + // first host's not-foreground answer pinned too, not merely observed. + let secondHost = SystemSurfaceHostRegistry.hosts[1] + presentedSystemSurfaceForegroundOverrideForTesting = [secondHost.bundleId] + guard case .context(let other) = prepareActiveCommandContext(command: screenshot) else { + return XCTFail("screenshot must be prepared, not refused") + } + XCTAssertEqual( + other.systemSurface, + secondHost, + "the probe must serve the host that is reported foreground, not the registry's first entry" + ) + + // Both reported: the registry's own order decides, because live state cannot be told to present + // two hosts at once. Totality is what keeps the block's answer free of what the sim happens to + // report for any host a future registry entry adds. + presentedSystemSurfaceForegroundOverrideForTesting = [host.bundleId, secondHost.bundleId] + guard case .context(let both) = prepareActiveCommandContext(command: screenshot) else { + return XCTFail("screenshot must be prepared, not refused") + } + XCTAssertEqual( + both.systemSurface, + host, + "with every host foreground the probe must serve them in registry order" + ) + + // The other half, with nothing presented — an empty total override, so this half pins the + // hosts' answers too. Without this the arm could pass by serving a surface that is not there. + presentedSystemSurfaceForegroundOverrideForTesting = [] + guard case .context(let standing) = prepareActiveCommandContext(command: screenshot) else { + return XCTFail("screenshot must be prepared, not refused") + } + XCTAssertNil( + standing.systemSurface, + "no surface is presented, so nothing may be disclosed as one" + ) + XCTAssertTrue( + standing.app === app, + "naming another bundle is no licence to point the observation away from the standing target" + ) + XCTAssertNil(pendingTargetActivation) + XCTAssertEqual(mainOwned.bundleId, cachedBundleId, "preparing a capture binds nothing") + } + @MainActor func testSkipAppActivationPreflightIncludesForegroundCachedCoordinateOnlyTaps() throws { app.launch() diff --git a/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/UnitTests/RunnerTests+ModelsTests.swift b/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/UnitTests/RunnerTests+ModelsTests.swift index 69f1170c25..e018dd5e39 100644 --- a/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/UnitTests/RunnerTests+ModelsTests.swift +++ b/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/UnitTests/RunnerTests+ModelsTests.swift @@ -188,6 +188,55 @@ extension RunnerTests { ) } + /// The commands the merge-base classified read-only, copied from its `CommandType.traits` + /// (`git show 6428c54853:…/RunnerTests+Models.swift`) rather than read from anything under test: + /// `readOnly: .always`, plus `alert`'s `get` action, which its `.conditional` case resolved to the + /// same answer. Both consumers that fact had — replay eligibility and the prepared path's witness + /// rule — are pinned against this literal below, so neither can move with the table (#2890 review). + private static let mergeBaseReadOnlyCommands: Set = [ + .findText, .readText, .snapshot, .gestureViewport, .screenshot, .status, .alert, + ] + + /// Commands that did not exist at the merge-base, so no classification of its is compared with + /// theirs. `appState` arrived with #2929. + private static let commandsNewerThanTheMergeBase: Set = [.appState] + + /// The commands production never runs through the prepared path's body: `executeOnMain` answers + /// these before `executeOnMainPrepared` runs, and `executeDispatched` answers `snapshot` earlier + /// still, on both this and the merge-base chain. The merge-base witness predicate was therefore + /// never evaluated for them and neither is the derived one. If a command starts reaching the + /// prepared path, removing it here is a claim the equivalence assertion below has to keep proving. + private static let commandsAnsweredBeforeThePreparedPath: Set = [ + .status, .uptime, .appState, .activate, .terminate, .targetReset, .shutdown, + .recordStart, .recordStop, .snapshot, + ] + + private func assertRememberedTextEntryWitnessInvalidation( + _ command: Command, + type: CommandType, + wasReadOnlyAtMergeBase: Bool, + _ request: String + ) { + if !Self.commandsNewerThanTheMergeBase.contains(type) { + XCTAssertEqual( + command.traits.retryOnSessionLoss, + wasReadOnlyAtMergeBase, + "\(request) must stay replayable exactly where the merge-base classified it read-only" + ) + } + // The merge-base rule (`RunnerTests+CommandExecution.swift:11`): + // `command != .tap && command != .type && !isReadOnlyCommand(command)`. `querySelector` is the + // row this review round was about: it was never read-only, so the merge-base cleared a + // remembered tap for it too and this column says so for every command, not just that one. + let mergeBaseClears = type != .tap && type != .type && !wasReadOnlyAtMergeBase + guard !Self.commandsAnsweredBeforeThePreparedPath.contains(type) else { return } + XCTAssertEqual( + command.invalidatesRememberedTextEntryTap, + mergeBaseClears, + "\(request) must invalidate a remembered text-entry tap exactly as the merge-base did" + ) + } + /// 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 @@ -247,6 +296,12 @@ extension RunnerTests { let command = try runnerCommandFixture(request) XCTAssertEqual(command.command, type, request) assertTraits(command.traits, matches: rowExpectation, request) + assertRememberedTextEntryWitnessInvalidation( + command, + type: type, + wasReadOnlyAtMergeBase: Self.mergeBaseReadOnlyCommands.contains(type), + request + ) } XCTAssertEqual( Set(table.map { $0.0 }), @@ -271,7 +326,16 @@ extension RunnerTests { for alertCase in alertCases { let request = alertCase.action.map { #"{"command":"alert","action":"\#($0)"}"# } ?? #"{"command":"alert"}"# - assertTraits(try runnerCommandFixture(request).traits, matches: alertCase.expectation, request) + let command = try runnerCommandFixture(request) + assertTraits(command.traits, matches: alertCase.expectation, request) + // The merge-base resolved `alert` through `readOnly: .conditional`, whose rule was this same + // action test, so only `get` — and the missing action it defaults to — was read-only there. + assertRememberedTextEntryWitnessInvalidation( + command, + type: .alert, + wasReadOnlyAtMergeBase: (command.action ?? "get").lowercased() == "get", + request + ) } } } 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 68b7276ef8..e698119eb3 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 @@ -86,9 +86,10 @@ 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. + // The fixture's `query` column records whether the alert request changes anything — `get` is the + // one action that is side-effect-free — and each side consumes it 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),