fix(ios): never launch a not-running session app from an observation - #2852
Conversation
|
Reviewed at 5b9d54d. The refusal is keyed on the runner wire command ( Which rule do you want? Either no runner read may launch the app, including reads before a mutation; then the CHANGELOG and help should say selector and gesture interactions also refuse, and a Also worth checking: for a bundle that is not installed, does The 5 reported checks are green. The PR has a merge conflict with main in |
5b9d54d to
4e93713
Compare
thymikee
changed the title
|
Size Report
Startup median (7 runs, lower is better):
|
The refusal now keys on the runner's own read trait, not on a host-stamped per-request mark: isReadOnlyCommand covers findText, readText, snapshot, gestureViewport, and alert get, so a mutation's leading read -- the viewport read a gesture starts with, the capture that resolves a selector click/fill -- refuses with APP_NOT_RUNNING exactly like a user-level read. A bare activate over a launch SpringBoard holds behind its "Open in ...?" confirmation is a launch without the URL, whatever command asked for the read. The daemon-side request-app-intent plumbing and the observationOnly wire field are removed: the runner's command trait table is the single source of the rule. The refusal stays retriable for a wait poll while the transport reads it as a definite answer; open, activate, and interactions that mutate without a leading read keep the foreground repair. This adopts the rule picked in the #2852 review (option 1) and says so in the CHANGELOG, the foreground-repairs docs section, and the physical-device help topic's lifecycle facts.
4e93713 to
6d41dd4
Compare
|
Rule picked: option 1 — no runner read may launch the app, including reads before a mutation. Pushed as Your reading was right about the mechanism at the reviewed head; by the time you commented, the tip had moved to the option-2 shape (a host-stamped
Still outstanding: CI on |
|
This is ready to merge at 6d41dd4. The guard now refuses a leading read before it ever reaches activateTarget, closing the gap from the earlier review (5b9d54d, #2852 (comment)). I know of no conflicts. All 20 checks pass on this commit, and the new XCTest cases run in the ios.yml -only-testing lane. I did not reproduce the author's live measurements (the 20 s wait-poll deadline, the not-installed-bundle |
thymikee
added
the
ready-for-human
The refusal now keys on the runner's own read trait, not on a host-stamped per-request mark: isReadOnlyCommand covers findText, readText, snapshot, gestureViewport, and alert get, so a mutation's leading read -- the viewport read a gesture starts with, the capture that resolves a selector click/fill -- refuses with APP_NOT_RUNNING exactly like a user-level read. A bare activate over a launch SpringBoard holds behind its "Open in ...?" confirmation is a launch without the URL, whatever command asked for the read. The daemon-side request-app-intent plumbing and the observationOnly wire field are removed: the runner's command trait table is the single source of the rule. The refusal stays retriable for a wait poll while the transport reads it as a definite answer; open, activate, and interactions that mutate without a leading read keep the foreground repair. This adopts the rule picked in the #2852 review (option 1) and says so in the CHANGELOG, the foreground-repairs docs section, and the physical-device help topic's lifecycle facts.
6d41dd4 to
a06b951
Compare
|
Rebased onto main at a06b951, and squashed |
The refusal now keys on the runner's own read trait, not on a host-stamped per-request mark: isReadOnlyCommand covers findText, readText, snapshot, gestureViewport, and alert get, so a mutation's leading read -- the viewport read a gesture starts with, the capture that resolves a selector click/fill -- refuses with APP_NOT_RUNNING exactly like a user-level read. A bare activate over a launch SpringBoard holds behind its "Open in ...?" confirmation is a launch without the URL, whatever command asked for the read. The daemon-side request-app-intent plumbing and the observationOnly wire field are removed: the runner's command trait table is the single source of the rule. The refusal stays retriable for a wait poll while the transport reads it as a definite answer; open, activate, and interactions that mutate without a leading read keep the foreground repair. This adopts the rule picked in the #2852 review (option 1) and says so in the CHANGELOG, the foreground-repairs docs section, and the physical-device help topic's lifecycle facts.
a06b951 to
b5a510d
Compare
|
Simplified at
Not done here: the review also proposed narrowing Validation: |
|
Reviewed at b5a510d. The squashed and rebased commit keeps the reviewed behavior from 6d41dd4. The Swift guard in RunnerTests+CommandDispatch.swift and the host's I checked this by reading the diff. I did not run the XCTest lane or the Node tests. The live run from the 6d41dd4 review still covers the device behavior, because the Swift guard did not change. Smoke Tests is still running and has not failed. This diff changes the iOS runner read dispatch, so please check that job before merge. |
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
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
1 participant
Summary
Targets iOS smoke flake
smoke:automation-inputstep 13wait for Automation lab(12 failures 09-16..22, 1 on main).open --relaunch --launch-urlterminates the app;simctl openurlcan hold the launch behind SpringBoard's "Open in …?" prompt. The next read hit the runner's foreground repair, andactivate()on a not-running app is a bare launch without the URL (runner log:AGENT_DEVICE_RUNNER_ACTIVATE … state=1).No runner read launches the session app. The refusal keys on the runner's own read trait, so user-level reads (
snapshot,wait,is,get, a readingfind) and a mutation's leading reads — agesture'sgestureViewport, the capture that resolves a selectorclick/fill— refuse a not-running session app withAPP_NOT_RUNNING(COMMAND_FAILED, retriable sowaitpolls; the transport reads it as a definite answer and never resends). The first round's daemon-side request-app-intent plumbing and theobservationOnlywire field are removed: the runner's command trait table is the single source.open,activate, and interactions that mutate without a leading read keep the foreground repair. Help:website/docs/docs/commands.md"Foreground repairs on iOS" and thephysical-devicehelp topic's lifecycle facts.Validation
Tested
6d41dd483e(rebased ontob3b58caac1), iPhone 17 Simulator on iOS 26.2 (a purpose-built device, destroyed after the runs):AGENT_DEVICE_RUNNER_OPTIONAL_PROBE_WAIT_COMPLETEDassertion passes. Two batch failures on the first pass (testAbandonedTreeCaptureSkipsQuerySweepAndHonorsWarmupExemption,testAlertAcceptDoesNotActivateAReplacementWithASharedButton) passed alone under no load; the clean re-run was 106/106.testObservationRefusesANotRunningSessionAppInsteadOfLaunchingItandtestInteractionLeadingReadRefusesToLaunchANotRunningSessionAppfail on the previous rule (both reads launched Settings: staterunningForeground,error == nil) and pass on the read-trait rule;testNonReadCommandStillLaunchesANotRunningSessionApppins that a mutation still launches.execute(command:)off the main queue, which asserts.onQueue(.main)— thedispatchPreconditionSIGTRAP restarted the batch and is what silenced the smoke marker. The main-thread helper removes it (3/3 with no restart).XCUIApplication.statereads.notRunning(raw 1) and the runner refuses asnapshotover it withAPP_NOT_RUNNINGin ~6 ms. A livewait text … 20000against a not-running session app polled the retriable refusal to the full 20 s deadline (28 typed refusals inrunner.log) and exitedCOMMAND_FAILED/details.runnerErrorCode: "APP_NOT_RUNNING"/retriable: true.runner-recovery-wiring23/23,daemon-command-registry9/9;pnpm format:check,pnpm lint,pnpm typecheckclean.pnpm check:xctest-selection: 106 PR-lane selections, 0 unreachable.pnpm check:affected --runon the pushed head: all runnable checks passed.