diff --git a/.device-evidence/CHECKLIST-runner-failures.md b/.device-evidence/CHECKLIST-runner-failures.md index e9e6a1d5e4..cc94966a48 100644 --- a/.device-evidence/CHECKLIST-runner-failures.md +++ b/.device-evidence/CHECKLIST-runner-failures.md @@ -1,4 +1,4 @@ -# Runner-failure evidence checklist (#2680, #2683) +# Device evidence checklist Live evidence the coordinator runs serially on the connected iPhone. Each item names the exact command, the environment it needs, and the rendered error that proves the change. Do not paraphrase @@ -100,8 +100,132 @@ said what was wrong with it) or `build_failed_unclassified` — which is also wh mentions the profile it used gets, since a name is not a complaint (#2688 review). Paste the error and the `xcodebuild -version` either way: a capture of the conflicting-settings line is what would let a follow-up name the cause, and the capture must show which build setting disagrees before any hint naming a lever is written. +## #2683 — device-readiness facts from `devicectl device info details` -## Results — coordinator run, 2026-09-20 +Build the CLI first, and stop any warm daemon so the run is on this commit (same preamble as the +#2680 section above). + +### 1. Both facts are readable, and they are two facts + +```sh +xcrun devicectl device info details --device "" --json-output /tmp/device-details.json +node -e 'const d=require("/tmp/device-details.json").result.deviceProperties;console.log(JSON.stringify({developerModeStatus:d.developerModeStatus,ddiServicesAvailable:d.ddiServicesAvailable}))' +xcodebuild -version +``` + +Expected: `{"developerModeStatus":"enabled","ddiServicesAvailable":true}` on a healthy device, plus +the Xcode version. Also record `bootState` and `tunnelState` from the same file: those two are what +make a `ddiServicesAvailable: false` an answer at all rather than a device that was not listening. + +`packages/platform-apple/src/core/__tests__/fixtures/ios-device-info-details.json` holds this payload +and `packages/platform-apple/src/runner/__tests__/runner-startup-failure-fixtures.ts` records the two +state pairings, both at `invented-shape` until this capture moves them to `captured`. Paste the raw +values rather than a summary. + +The committed payload is masked, and the mask is a rule with a test, not a one-time edit: +`hardwareProperties.serialNumber` and `deviceProperties.bootedSnapshotName` carry `MASKED`, +`hardwareProperties.ecid` is `0`, and `connectionProperties.tunnelIPAddress` is a documentation-only +`fd00:` address. Everything the reader consumes — the toggle, the image, `bootState`, `tunnelState`, +OS build — stays verbatim. If you refresh this capture, apply the same mask in both the `result` +block and the mirrored `properties` block, and leave the states alone. + +### 2. A healthy device is left alone + +```sh +node --experimental-strip-types src/bin.ts --json prepare ios-runner --platform ios --device "" +``` + +Expected: success as before, and a `ios_runner_session_startup` diagnostic whose timings include +`verify_device_readiness`. No reason may appear on a healthy device: the probe reads, it does not +guess. Record the `verify_device_readiness` duration next to `verify_host_dev_tools_security`. + +### 3. Developer Mode off on the device -> `device_developer_mode_disabled` + +Turn the toggle off on a device you are willing to re-pair (Settings > Privacy & Security > +Developer Mode, then restart), and rerun the `prepare ios-runner` command above. + +Expected: exit non-zero with + +```json +{ + "code": "COMMAND_FAILED", + "message": "The iOS device reports that Developer Mode is turned off", + "details": { + "reason": "device_developer_mode_disabled", + "developerMode": "disabled", + "developerDiskImage": "unavailable" + } +} +``` + +`hint` is top-level and names `Settings > Privacy & Security > Developer Mode`. Record what +`developerDiskImage` says; either value is acceptable as long as the toggle stays the reason. + +### 4. Developer disk image down with the toggle on -> carried onto the failure, not a refusal + +This is the pairing the old hint got wrong, so it is the evidence that matters. Reach it with a +device whose iOS build is newer than the installed Xcode supports, or before Xcode finishes +installing device support for a freshly paired phone, with Developer Mode on. + +This state does NOT stop the run before the build (#2683 review): since iOS 17 CoreDevice mounts the +personalized disk image on demand during build and launch, a phone that has just been rebooted reports +the image down while the very next build clears it. Run the `prepare ios-runner` command above and +record which of the two outcomes you get — both are evidence, and the second one is the reason the +pre-build refusal was removed: + +- **The build clears it**: the command succeeds. Paste the `ios_runner_session_startup` timings showing + `verify_device_readiness` ran and `ensure_xctestrun` followed it. Nothing may name device support. +- **The build fails and names no cause of its own**: `details.reason` is + `device_developer_disk_image_unavailable` and `details.developerDiskImage` is `unavailable`, and the + hint names device support WITHOUT mentioning `Settings > Privacy & Security`. +- **The build fails and names its own cause** (e.g. `signing_no_development_team`): that reason wins and + `details.developerDiskImage` is still `unavailable`. The device state never overwrites xcodebuild's + sentence. + +If the toggle reason appears here instead, that is the bug this issue exists to fix: paste the whole +error and the `/tmp/device-details.json` payload rather than adjusting a rule. + +Capture the details payload at the same moment as the run, and check it says `tunnelState: +"connected"` and `bootState: "booted"`. A `ddiServicesAvailable: false` read any other way is not this +case: an asleep or unreachable device has the same field and no obstacle, and the run must not name +device support for it. The hint you get here is the one `core/devicectl.ts` owns and the device report +carries, so it is worded identically to the hint `devicectl` output produces for the same complaint — +paste both strings and confirm they match character for character. + +### 5. A device that cannot be read claims nothing + +Unplug the iPhone (or shut it down) after a session exists, then rerun the `prepare ios-runner` +command. + +Expected: the failure names whatever the transport could not reach, and no `details.reason` of +`device_developer_mode_disabled` or `device_developer_disk_image_unavailable` appears anywhere in the +error. An unreadable device is never diagnosed. This is also the case that catches a sleeping phone +reporting `ddiServicesAvailable: false`: if the hint here says `Let Xcode finish preparing this +device`, the corroboration rule has been lost, and pasting the payload with its `tunnelState` and +`bootState` is the evidence — do not adjust the rule to make the run pass. + +### 6. A device and a Mac that are both wrong publish the device's reason + +With the iPhone's Developer Mode toggle off AND `sudo DevToolsSecurity -status` reporting the +developer-tools setting disabled, run the `prepare ios-runner` command. + +Expected: one error, whose `details.reason` is `device_developer_mode_disabled`. The Mac's reason is +the other one (`devtools_security_developer_mode_disabled`) and must not be what you get: the phone's +fix needs no admin rights on the Mac, so the probe that can be acted on has to be the one that speaks. +Record which of the two appears; a captured pairing here is what would let a follow-up drop the +ordering argument. + +### 7. Log evidence belongs to the command that wrote it (optional, needs a crash repro) + +Run any command that crashes the app under test, then a second command that fails for its own +reason (for example a selector that no longer exists). + +Expected: the second error carries no `details.runnerFailureReason`. Before this change it inherited +`target_app_axruntime_coretext_crash` from the first command's lines in `runner.log`. If the repro +is not reachable, say so; the pairing is covered by +`packages/platform-apple/src/runner/__tests__/runner-failure-diagnostics.test.ts`. + +## Results — coordinator run, 2026-09-20 (both families) Built from `15808ae228` on `thymikee-iphone` (iPhone 17 Pro, iOS 27.0, build 24A437), cabled. @@ -111,7 +235,7 @@ Xcode 26.2 Build version 17C52 ``` -Section 3 is captured. A device build pointed at a team with no certificate, on a fresh derived +#2680 §3 is captured. A device build pointed at a team with no certificate, on a fresh derived path so no cached artifact short-circuits it, reaches signing and fails with one long `error:` line per target: @@ -125,7 +249,7 @@ per target: answers the wrapping question the rows were held on: the matched phrase arrives inside one `error:` line, so the sibling rows in the same provisioning family do not split the way a wrapped line would. -Sections 1 and 2 are blocked on this account, and the mechanism is worth recording because it is the +The #2680 sections 1 and 2 are blocked on this account, and the mechanism is worth recording because it is the same for all of them: against a signed-in account with a valid identity, `xcodebuild` is invoked with `-allowProvisioningUpdates`, so the build either signs successfully or dies earlier than the diagnostic a row keys on. @@ -134,7 +258,7 @@ diagnostic a row keys on. installed identity and reuses an installed team profile. So section 1's `signing_no_development_team` cannot be induced here; it needs an account signed in with no development team. - `AGENT_DEVICE_IOS_BUNDLE_ID=com.apple.TestFlight` **succeeds** for the same reason, and a bogus - `AGENT_DEVICE_IOS_PROVISIONING_PROFILE` is repaired rather than honoured. So section 2's + `AGENT_DEVICE_IOS_PROVISIONING_PROFILE` is repaired rather than honoured. So §2's `bundle_identifier_already_registered` needs an app id owned by a different team that automatic signing cannot register. - The same gating applies to `bundle_identifier_unavailable` (`App Identifier` + `not available`), @@ -143,6 +267,44 @@ diagnostic a row keys on. which this account will not produce. Recorded beside the fixtures in `runner-startup-failure-fixtures.ts` so the rows read as host-gated, not unexamined. -Section 4 needs a build that fails for an unrelated reason while naming no signing fact; the +#2680 §4 needs a build that fails for an unrelated reason while naming no signing fact; the classifier's behaviour there is pinned by `runner-startup-failure-reasons.test.ts` and needs no device claim to hold. + +### #2683 — what the phone actually reported + +Developer Mode off, same device, `prepare ios-runner --platform ios --json`: + +``` +"deviceProperties": { "developerModeStatus": "disabled", "ddiServicesAvailable": true } + +details.reason device_developer_mode_disabled +details.deviceReadiness { developerMode: "disabled", developerDiskImage: "available" } +hint Enable Developer Mode on the iOS device (Settings > Privacy & Security > + Developer Mode), restart it when prompted, unlock it, then retry. +``` + +The recognised spelling is the device's own lowercase `"disabled"`, so the one refusal +`preflightIosRunnerDeviceReadiness` raises is proven rather than inferred, and an unrecognised +spelling is no longer a live risk for this state. + +Image-down was reached by rebooting and holding the phone locked, watching `devicectl` until +`ddiServicesAvailable` read `false` while `bootState` was `booted`. Two findings from it: + +- The state is only reachable **while locked**. `ddiServicesAvailable` flips back to `true` within + seconds of unlock, so an unlocked image-down device does not exist on iOS 27 and the capture this + checklist asked for cannot be produced on it. +- Inside that window `prepare` fails at the connect stage — `Runner did not accept connection + (xcodebuild exited early)`, exit 70 — and publishes no `developerDiskImage`. A rerun at `eaf411e`, + where both connect-stage failures carry the device states, gave the same error, exit 70 and + `IOS_RUNNER_CONNECT_TIMEOUT` with no `developerDiskImage`. The cause is the observability rule, not + the call site. Before the first unlock the tunnel never comes up: every `devicectl device info + details` read from 10:56 to 10:58 UTC, before and after that run, said `tunnelState: "unavailable"`, + `bootState: "booted"`, `ddiServicesAvailable: false`. So `readDeviceReadiness` returns + `available: false`, the session has no device states, and there is nothing to carry. On iOS 27 a + locked phone does not publish its image state at all. The connect-stage enrichment carries the + fact only when the preflight could observe it (tunnel connected, device booted). + +The host-deadline invariant also held under a real device fault: on a 25s budget the same +image-down state produced `details.reason: prepare_deadline_expired` with **no** device reason and no +readiness facts, across every sample of two separate windows. diff --git a/packages/kernel/src/errors.ts b/packages/kernel/src/errors.ts index b482a210dd..457abefa93 100644 --- a/packages/kernel/src/errors.ts +++ b/packages/kernel/src/errors.ts @@ -414,11 +414,20 @@ function booleanDetail( return typeof value === 'boolean' ? value : undefined; } +/** + * Facts a publisher leaves for a later catch in the same process, never for a caller: whether a rule + * row named this failure, and whether the host's own deadline ended the command behind it. Both + * describe our machinery rather than the caller's problem, and the caller was already handed the + * verdict those facts produced as `reason` and `hint` (#2690 review). + */ +const INTERNAL_PLUMBING_DETAIL_KEYS = ['startupRuleMatched', 'startupHostDeadlineHit'] as const; + function stripDiagnosticMeta( details: Record | undefined, ): Record | undefined { if (!details) return undefined; const output = { ...details }; + for (const key of INTERNAL_PLUMBING_DETAIL_KEYS) delete output[key]; delete output.hint; delete output.diagnosticId; delete output.logPath; diff --git a/packages/platform-apple/src/core/__tests__/devicectl.test.ts b/packages/platform-apple/src/core/__tests__/devicectl.test.ts index 0bbaf859f9..fe72d42838 100644 --- a/packages/platform-apple/src/core/__tests__/devicectl.test.ts +++ b/packages/platform-apple/src/core/__tests__/devicectl.test.ts @@ -98,15 +98,31 @@ test('parseIosDeviceProcessesPayload maps running process entries', () => { ]); }); -test('resolveIosDevicectlHint points at Developer Mode when the disk image cannot mount', () => { +test('resolveIosDevicectlHint names the developer disk image when that is all it reports', () => { // Observed on a freshly paired iPhone: unlocked, trusted, `available (paired)` - // in Xcode, and still unusable because Developer Mode was off. The default - // hint sent the user to re-check trust, which was already fine. + // in Xcode, and still unusable. This line used to be answered with Developer + // Mode advice, which is right often enough to survive as a guess and wrong + // whenever Xcode simply has not finished installing device support on a phone + // whose toggle is already on (#2683). The device reports both states apart, so + // the hint answers the one the output named. const hint = resolveIosDevicectlHint( '', 'Failed to launch iOS app: The developer disk image could not be mounted on this device. (com.apple.dt.CoreDeviceError error 12040 (0x2F08))', ); + assert.match(String(hint), /device support/i); + assert.doesNotMatch(String(hint), /Developer Mode/); +}); + +test('resolveIosDevicectlHint names Developer Mode when the output says both', () => { + // The pairing this hint was written for: a phone with the toggle off cannot + // mount the image either, so the toggle is the thing to fix and the direction + // that genuinely holds (#2683). + const hint = resolveIosDevicectlHint( + '', + 'The operation failed because Developer Mode is disabled. The developer disk image could not be mounted on this device.', + ); + assert.match(String(hint), /Developer Mode/); assert.match(String(hint), /Privacy & Security/); }); diff --git a/packages/platform-apple/src/core/__tests__/fixtures/ios-device-info-details.json b/packages/platform-apple/src/core/__tests__/fixtures/ios-device-info-details.json new file mode 100644 index 0000000000..a0cd9000a2 --- /dev/null +++ b/packages/platform-apple/src/core/__tests__/fixtures/ios-device-info-details.json @@ -0,0 +1,414 @@ +{ + "info": { + "arguments": [ + "devicectl", + "device", + "info", + "details", + "--device", + "00000000-0000-0000-0000-000000000001", + "--json-output", + "" + ], + "commandType": "devicectl.device.info.details", + "environment": {}, + "jsonVersion": 4, + "outcome": "success", + "version": "629.3" + }, + "result": { + "capabilities": [ + { + "featureIdentifier": "com.apple.coredevice.feature.acquireusageassertion", + "name": "Acquire Usage Assertion" + }, + { + "featureIdentifier": "com.apple.coredevice.feature.applicationcontrol", + "name": "Application Control" + }, + { + "featureIdentifier": "com.apple.coredevice.feature.capturescreenshot", + "name": "Capture Screenshot" + }, + { + "featureIdentifier": "com.apple.coredevice.feature.capturesysdiagnose", + "name": "Capture Sysdiagnose" + }, + { + "featureIdentifier": "com.apple.coredevice.feature.configurationprofiles", + "name": "Configuration Profile Management" + }, + { + "featureIdentifier": "com.apple.dt.serviceconnection.create", + "name": "Create Service Connection" + }, + { + "featureIdentifier": "com.apple.dt.servicesocket.create", + "name": "Create Service Socket" + }, + { + "featureIdentifier": "com.apple.coredevice.feature.customizeappearancesettings", + "name": "Customize Appearance Settings" + }, + { + "featureIdentifier": "com.apple.coredevice.feature.customizeliquidglass", + "name": "Customize Liquid Glass" + }, + { + "featureIdentifier": "com.apple.coredevice.feature.customizeuistyle", + "name": "Customize User Interface Style" + }, + { + "featureIdentifier": "com.apple.coredevice.feature.remote.devicecontrol.orientation", + "name": "Device Orientation" + }, + { + "featureIdentifier": "com.apple.coredevice.feature.disableddiservices", + "name": "Disable Developer Disk Image Services" + }, + { + "featureIdentifier": "com.apple.coredevice.feature.disconnectdevice", + "name": "Disconnect from Device" + }, + { + "featureIdentifier": "com.apple.coredevice.feature.fetchappicons", + "name": "Fetch Application Icons" + }, + { + "featureIdentifier": "com.apple.coredevice.feature.getauthlistingidentifiers", + "name": "Fetch AuthListing Identifiers" + }, + { + "featureIdentifier": "com.apple.coredevice.feature.fetchddimetadata", + "name": "Fetch Developer Disk Image Services Metadata" + }, + { + "featureIdentifier": "com.apple.coredevice.feature.getdeviceinfo", + "name": "Fetch Extended Device Info" + }, + { + "featureIdentifier": "com.apple.coredevice.feature.fetchdyldsharecache", + "name": "Fetch dyld Shared Cache Files" + }, + { + "featureIdentifier": "com.apple.coredevice.feature.filesystemoperation", + "name": "File Handle Operation" + }, + { + "featureIdentifier": "com.apple.coredevice.feature.getdisplayinfo", + "name": "Get Display Information" + }, + { + "featureIdentifier": "com.apple.coredevice.feature.getlockstate", + "name": "Get Lock State" + }, + { + "featureIdentifier": "com.apple.coredevice.feature.getmediastreamserverstatus", + "name": "Get Media Stream Server Status" + }, + { + "featureIdentifier": "com.apple.coredevice.feature.getmediasupportinfo", + "name": "Get Support Info for Media Streams" + }, + { + "featureIdentifier": "com.apple.coredevice.feature.remote.hid.button", + "name": "HID Button" + }, + { + "featureIdentifier": "com.apple.coredevice.feature.remote.hid.digitizer", + "name": "HID Digitizer" + }, + { + "featureIdentifier": "com.apple.coredevice.feature.remote.hid.keyboard", + "name": "HID Keyboard" + }, + { + "featureIdentifier": "com.apple.coredevice.feature.remote.hid.scroll", + "name": "HID Scroll" + }, + { + "featureIdentifier": "com.apple.coredevice.feature.remote.hid.vendordefined", + "name": "HID Vendor Defined" + }, + { + "featureIdentifier": "com.apple.coredevice.feature.installapp", + "name": "Install Application" + }, + { + "featureIdentifier": "com.apple.coredevice.feature.installroot", + "name": "Install Root" + }, + { + "featureIdentifier": "com.apple.coredevice.feature.launchapplication", + "name": "Launch Application" + }, + { + "featureIdentifier": "com.apple.coredevice.feature.listFiles", + "name": "List Files" + }, + { + "featureIdentifier": "com.apple.coredevice.feature.listroots", + "name": "List Roots" + }, + { + "featureIdentifier": "com.apple.coredevice.feature.default.user.credentials", + "name": "Modify Credentials for Default Users for a Device" + }, + { + "featureIdentifier": "com.apple.coredevice.feature.tags", + "name": "Modify Tags" + }, + { + "featureIdentifier": "com.apple.coredevice.feature.monitorfilechanges", + "name": "Monitor File Changes" + }, + { + "featureIdentifier": "com.apple.coredevice.feature.monitorprocesstermination", + "name": "Monitor Process for Termination" + }, + { + "featureIdentifier": "com.apple.coredevice.feature.pasteboard", + "name": "Pasteboard" + }, + { + "featureIdentifier": "com.apple.dt.customer.postdarwinnotification", + "name": "Post Darwin Notification" + }, + { + "featureIdentifier": "com.apple.coredevice.feature.processcontrol", + "name": "Process Control" + }, + { + "featureIdentifier": "com.apple.coredevice.feature.provisioningprofiles", + "name": "Provisioning Profile Management" + }, + { + "featureIdentifier": "com.apple.coredevice.feature.querymobilegestalt", + "name": "Query MobileGestalt" + }, + { + "featureIdentifier": "com.apple.coredevice.feature.rebootdevice", + "name": "Reboot Device" + }, + { + "featureIdentifier": "com.apple.coredevice.feature.renamedevice", + "name": "Rename Device" + }, + { + "featureIdentifier": "com.apple.coredevice.feature.sendmemorywarningtoprocess", + "name": "Send Memory Warning to Process" + }, + { + "featureIdentifier": "com.apple.coredevice.feature.sendsignaltoprocess", + "name": "Send Signal to Process" + }, + { + "featureIdentifier": "com.apple.dt.profile", + "name": "Service Hub Profile" + }, + { + "featureIdentifier": "com.apple.coredevice.feature.servicexpcpeerconnection", + "name": "Service XPC Peer Connection" + }, + { + "featureIdentifier": "com.apple.coredevice.feature.simulatelocation", + "name": "Simulate Device Location" + }, + { + "featureIdentifier": "com.apple.coredevice.feature.spawnexecutable", + "name": "Spawn Executable" + }, + { + "featureIdentifier": "com.apple.coredevice.feature.startaudiooutput", + "name": "Start Audio Output" + }, + { + "featureIdentifier": "com.apple.coredevice.feature.startmediastream", + "name": "Start Media Stream" + }, + { + "featureIdentifier": "com.apple.coredevice.feature.startvideooutput", + "name": "Start Video Output" + }, + { + "featureIdentifier": "com.apple.coredevice.feature.stopmediastream", + "name": "Stop Media Stream" + }, + { + "featureIdentifier": "com.apple.coredevice.feature.streamapplist", + "name": "Stream Application List" + }, + { + "featureIdentifier": "com.apple.coredevice.feature.streamprocesslist", + "name": "Stream Process List" + }, + { + "featureIdentifier": "com.apple.coredevice.feature.transferFiles", + "name": "Transfer Files" + }, + { + "featureIdentifier": "com.apple.coredevice.feature.uninstallapp", + "name": "Uninstall Application" + }, + { + "featureIdentifier": "com.apple.coredevice.feature.uninstallroot", + "name": "Uninstall Root" + }, + { + "featureIdentifier": "com.apple.coredevice.feature.remote.universalhid", + "name": "Universal HID Service Pool" + }, + { + "featureIdentifier": "com.apple.coredevice.feature.remote.universalhidservice", + "name": "UniversalHIDService" + }, + { + "featureIdentifier": "com.apple.coredevice.feature.unpairdevice", + "name": "Unpair Device" + }, + { + "featureIdentifier": "com.apple.coredevice.feature.viewdevicescreen", + "name": "View Device Screen" + } + ], + "connectionProperties": { + "authenticationType": "manualPairing", + "isMobileDeviceOnly": false, + "lastConnectionDate": "2026-09-19T13:16:00.000Z", + "localHostnames": [ + "apex-test-iphone.coredevice.local", + "00000000-0000-0000-0000-000000000001.coredevice.local", + "00000000-0000000000000000.coredevice.local" + ], + "pairingState": "paired", + "potentialHostnames": [ + "apex-test-iphone.coredevice.local", + "00000000-0000-0000-0000-000000000001.coredevice.local", + "00000000-0000000000000000.coredevice.local" + ], + "transportType": "wired", + "tunnelIPAddress": "fd00:0000:0000::1", + "tunnelState": "connected", + "tunnelTransportProtocol": "tcp" + }, + "deviceProperties": { + "bootState": "booted", + "bootedFromSnapshot": true, + "bootedSnapshotName": "com.apple.os.update-MASKED", + "ddiServicesAvailable": true, + "developerModeStatus": "enabled", + "hasInternalOSBuild": false, + "name": "apex-test-iphone", + "osBuildUpdate": "23G90", + "osVersionNumber": "26.6.2", + "rootFileSystemIsWritable": false, + "screenViewingURL": "devices://device/open?id=00000000-0000-0000-0000-000000000001", + "supportsCheckedAllocations": true + }, + "hardwareProperties": { + "cpuType": { + "name": "arm64e", + "subType": -2147483646, + "type": 16777228 + }, + "deviceType": "iPhone", + "ecid": 0, + "hardwareModel": "V53AP", + "internalStorageCapacity": 256000000000, + "isProductionFused": true, + "marketingName": "iPhone 17 Pro", + "platform": "iOS", + "productType": "iPhone18,1", + "reality": "physical", + "serialNumber": "C07MASKED00000", + "supportedCPUTypes": [ + { + "name": "arm64e", + "subType": -2147483646, + "type": 16777228 + }, + { + "name": "arm64", + "subType": 0, + "type": 16777228 + } + ], + "supportedDeviceFamilies": [1], + "thinningProductType": "iPhone18,1", + "udid": "00000000-0000000000000000" + }, + "identifier": "00000000-0000-0000-0000-000000000001", + "properties": { + "connection": { + "lastConnectionDate": 811516560, + "pairingState": "paired", + "transportType": "wired", + "tunnelIPAddressString": "fd00:0000:0000::1", + "tunnelTransportProtocol": "tcp" + }, + "hardware": { + "cpuCount": { + "logicalCores": 6, + "packages": 1, + "physicalCores": 6 + }, + "cpuType": { + "subtype": 18446744071562067970, + "type": 16777228 + }, + "ecid": 0, + "internalStorageCapacity": 256000000000, + "marketingName": "iPhone 17 Pro", + "productType": "iPhone18,1", + "reality": "physical", + "serialNumber": "C07MASKED00000", + "udid": "00000000-0000000000000000" + }, + "software": { + "osBuildVersions": { + "buildVersion": { + "majorLetterComponent": "G", + "majorNumberComponent": 23, + "name": "23G90", + "revisionVersion": { + "components": [23, 7, 90, 0, 0], + "originalComponentsCount": 5, + "stringValue": "23.7.90.0.0" + }, + "trainProgram": "iOS", + "updateNumberComponent": 90 + }, + "supplementalBuildVersion": { + "majorLetterComponent": "G", + "majorNumberComponent": 23, + "name": "23G90", + "revisionVersion": { + "components": [23, 7, 90, 0, 0], + "originalComponentsCount": 5, + "stringValue": "23.7.90.0.0" + }, + "trainProgram": "iOS", + "updateNumberComponent": 90 + } + }, + "osVersionNumber": { + "components": [26, 6, 2, 0, 0], + "originalComponentsCount": 3, + "stringValue": "26.6.2" + } + }, + "state": { + "bootState": "booted", + "developerModeStatus": { + "enabled": { + "mode": 1 + } + }, + "name": "apex-test-iphone" + } + }, + "propertyDisplayNames": null, + "tags": [], + "visibilityClass": "default" + } +} diff --git a/packages/platform-apple/src/core/__tests__/physical-device-coredevice.test.ts b/packages/platform-apple/src/core/__tests__/physical-device-coredevice.test.ts index 299a273258..368c91cc22 100644 --- a/packages/platform-apple/src/core/__tests__/physical-device-coredevice.test.ts +++ b/packages/platform-apple/src/core/__tests__/physical-device-coredevice.test.ts @@ -1,9 +1,33 @@ import assert from 'node:assert/strict'; +import fs from 'node:fs'; +import path from 'node:path'; import { test } from 'vitest'; +import type { DeviceInfo } from '@agent-device/kernel/device'; import { parseIosDeviceDetailsPayload, + readIosDeviceReadiness, resolveIosReadyHint, + type IosDeviceReadiness, } from '../physical-device-coredevice.ts'; +import { + IOS_DEVICE_DEVELOPER_DISK_IMAGE_HINT, + IOS_DEVICE_DEVELOPER_MODE_OFF_HINT, +} from '../devicectl.ts'; +import { resolveIosPhysicalDeviceControl } from '../physical-device-control.ts'; +import { createLocalAppleToolProvider, withAppleToolProvider } from '../tool-provider.ts'; + +/** + * `xcrun devicectl device info details` is the one tool that answers what a device thinks of itself, + * and #2682 read its output for exactly one of those answers. The capture below is the shape that + * made the mistake possible: the toggle and the developer disk image sit side by side in + * `deviceProperties`, so a reader that only looks for one of them reports the other wrongly. + */ +const DEVICE_INFO_DETAILS_CAPTURE = JSON.parse( + fs.readFileSync( + path.join(import.meta.dirname, 'fixtures', 'ios-device-info-details.json'), + 'utf8', + ), +) as unknown; test('parseIosDeviceDetailsPayload reads direct and nested tunnel state', () => { assert.equal( @@ -64,6 +88,65 @@ test('parseIosDeviceDetailsPayload ignores malformed values', () => { ); }); +test('parseIosDeviceDetailsPayload reads the developer mode toggle and the disk image apart', () => { + // The capture is a device that is fine on both counts. Reading them apart is the point: a reader + // that returns one boolean for both cannot tell "toggle off" from "Xcode has not finished". + const captured = parseIosDeviceDetailsPayload(DEVICE_INFO_DETAILS_CAPTURE); + assert.equal(captured.developerModeStatus, 'enabled'); + assert.equal(captured.developerDiskImageServicesAvailable, true); + assert.equal(captured.outcome, 'success'); + + assert.deepEqual( + parseIosDeviceDetailsPayload({ + result: { deviceProperties: { developerModeStatus: 'disabled', ddiServicesAvailable: true } }, + }), + { developerModeStatus: 'disabled', developerDiskImageServicesAvailable: true }, + ); + // The nested shape a `devicectl device list`-style envelope wraps the device in. + assert.deepEqual( + parseIosDeviceDetailsPayload({ + result: { + device: { + deviceProperties: { developerModeStatus: 'enabled', ddiServicesAvailable: false }, + }, + }, + }), + { developerModeStatus: 'enabled', developerDiskImageServicesAvailable: false }, + ); + // `false` is a read answer and has to survive; an absent key must not become `false`. + assert.equal( + parseIosDeviceDetailsPayload({ result: { deviceProperties: { ddiServicesAvailable: false } } }) + .developerDiskImageServicesAvailable, + false, + ); + assert.equal( + 'developerDiskImageServicesAvailable' in + parseIosDeviceDetailsPayload({ result: { deviceProperties: {} } }), + false, + ); +}); + +test('parseIosDeviceDetailsPayload reads one field per fact and not the display mirror', () => { + // `result.properties.state` mirrors the toggle in a structured form the tool renders for humans. + // Reading it too would give two sources for one fact, so the mirror stays unread and a payload + // that carries only the mirror reports nothing (#2683). + assert.deepEqual( + parseIosDeviceDetailsPayload({ + result: { properties: { state: { developerModeStatus: { enabled: { mode: 1 } } } } }, + }), + {}, + ); +}); + +test('parseIosDeviceDetailsPayload ignores malformed device states', () => { + assert.deepEqual( + parseIosDeviceDetailsPayload({ + result: { deviceProperties: { developerModeStatus: {}, ddiServicesAvailable: 'true' } }, + }), + {}, + ); +}); + test('resolveIosReadyHint maps known connection errors', () => { assert.match( resolveIosReadyHint('', 'Device is busy (Connecting to iPhone)'), @@ -77,3 +160,317 @@ test('resolveIosReadyHint falls back to generic guidance', () => { assert.match(hint, /unlocked/i); assert.match(hint, /xcode/i); }); + +/** + * `readIosDeviceReadiness` asks the device the same question through the same payload. The reader + * runs for real here, over the recorded capture, because the mistake #2683 fixes was made by a + * reader that looked at one of these two states and answered for both. + */ + +/** + * Whether an iPhone will host development tooling is a fact the phone holds (#2683), and the + * `devicectl` text path used to answer both of its states with one hint. These cases run the real + * reader over recorded payloads: the state pair that must name the developer disk image is the pair + * that used to send people to a Settings pane that was already correct. + * + * The payload identity fields are masked in the capture; the states the reader consumes are verbatim. + */ +const DEVICE_INFO_DETAILS_TEXT = fs.readFileSync( + path.join(import.meta.dirname, 'fixtures', 'ios-device-info-details.json'), + 'utf8', +); + +const IOS_DEVICE: DeviceInfo = { + platform: 'apple', + id: '00000000-0000-0000-0000-000000000001', + name: 'iPhone', + kind: 'device', + appleOs: 'ios', +}; + +const XCTEST_IOS_DEVICE: DeviceInfo = { ...IOS_DEVICE, iosPhysicalDeviceBackend: 'xctest' }; + +/** The remedies a readable report publishes, spelled once so the assertions below stay readable. */ +const DEVICE_REMEDIES = { + developerModeOff: IOS_DEVICE_DEVELOPER_MODE_OFF_HINT, + developerDiskImageUnavailable: IOS_DEVICE_DEVELOPER_DISK_IMAGE_HINT, +}; + +test('a device reporting its toggle on and its disk image up is ready', async () => { + const calls: string[][] = []; + const readiness = await readDeviceDetails(DEVICE_INFO_DETAILS_TEXT, calls); + + assert.deepEqual(readiness, { + available: true, + developerMode: 'enabled', + developerDiskImage: 'available', + remedies: DEVICE_REMEDIES, + }); + assert.equal(calls.length, 1); + const [cmd, ...args] = calls[0] ?? []; + assert.equal(cmd, 'xcrun'); + assert.deepEqual(args.slice(0, 4), ['devicectl', 'device', 'info', 'details']); + assert.ok(args.includes('--device') && args.includes(IOS_DEVICE.id)); + assert.ok(args.includes('--json-output') && args.includes('--timeout')); +}); + +test('a device reporting its toggle off says so, whatever its disk image says', async () => { + const readiness = await readDeviceDetails( + devicePropertiesPayload({ developerModeStatus: 'disabled', ddiServicesAvailable: false }), + ); + + assert.deepEqual(readiness, { + available: true, + developerMode: 'disabled', + developerDiskImage: 'unavailable', + remedies: DEVICE_REMEDIES, + }); +}); + +test('a device with its toggle on and its disk image down reports the image, not the toggle', async () => { + const readiness = await readDeviceDetails( + devicePropertiesPayload({ developerModeStatus: 'enabled', ddiServicesAvailable: false }), + ); + + assert.deepEqual(readiness, { + available: true, + developerMode: 'enabled', + developerDiskImage: 'unavailable', + remedies: DEVICE_REMEDIES, + }); +}); + +test('a device that answers with a spelling we do not know reports nothing', async () => { + // Both fields present, neither recognised, and the same answer when the payload carries no device + // properties at all: a toolchain that renames or stops sending a state must not be read as its + // owner having switched something off. + const readiness = await readDeviceDetails( + devicePropertiesPayload({ developerModeStatus: 'notDetermined', ddiServicesAvailable: 'yes' }), + ); + + assert.deepEqual(readiness, { + available: true, + developerMode: 'unknown', + developerDiskImage: 'unknown', + remedies: DEVICE_REMEDIES, + }); + assert.deepEqual( + await readDeviceDetails(JSON.stringify({ info: { outcome: 'success' }, result: {} })), + { + available: true, + developerMode: 'unknown', + developerDiskImage: 'unknown', + remedies: DEVICE_REMEDIES, + }, + ); +}); + +test('a device whose details cannot be read is reported unreadable rather than diagnosed', async () => { + for (const payload of [ + 'not json at all', + '', + JSON.stringify({ info: { outcome: 'failure' }, result: {} }), + ]) { + const readiness = await readDeviceDetails(payload); + + assert.equal(readiness.available, false); + if (readiness.available) continue; + assert.equal(readiness.reason, 'device_readiness_unreadable'); + // The hint offers a way to read the device, never a cause to fix. + assert.match(readiness.hint, /devicectl device info details/); + assert.doesNotMatch(readiness.hint, /Developer Mode/i); + } +}); + +test('the device report carries the remedies the devicectl path already owns', async () => { + // One wording per fix (#2683): the runner preflight publishes whatever arrives here, so this is the + // only place the wording can be asserted once instead of at every site that shows it. + const readiness = await readDeviceDetails( + devicePropertiesPayload({ developerModeStatus: 'enabled', ddiServicesAvailable: false }), + ); + + assert.ok(readiness.available); + assert.equal(readiness.remedies.developerModeOff, IOS_DEVICE_DEVELOPER_MODE_OFF_HINT); + assert.equal( + readiness.remedies.developerDiskImageUnavailable, + IOS_DEVICE_DEVELOPER_DISK_IMAGE_HINT, + ); +}); + +test('an image complaint from a device that could not have answered is not a diagnosis', async () => { + // `ddiServicesAvailable: false` with the tunnel down or the phone asleep means the developer disk + // image services were not listening, not that device support is missing. Refusing the run on that + // reading would send a self-healing first launch into a permanent "wait for Xcode" loop (#2683). + const unobservable = [ + { bootState: 'asleep' }, + { tunnelState: 'unavailable' }, + { tunnelState: 'unavailable', bootState: 'asleep' }, + ]; + for (const context of unobservable) { + const readiness = await readDeviceDetails( + devicePropertiesPayload( + { developerModeStatus: 'enabled', ddiServicesAvailable: false }, + context, + ), + ); + + assert.equal(readiness.available, false); + if (readiness.available) continue; + assert.equal(readiness.reason, 'device_readiness_unreadable'); + // The reader says which observation is missing rather than naming a cause. + assert.match(readiness.hint, /tunnelState=/); + assert.match(readiness.hint, /bootState=/); + assert.doesNotMatch(readiness.hint, /Let Xcode finish preparing/); + } +}); + +test('a disabled toggle keeps its answer even from a sleeping device', async () => { + // The exception that keeps the pairing honest: a toggle that is off already explains why the image + // is down, and it comes from the paired record rather than from a live service, so it does not need + // the device to be awake to be believed (#2683). + const readiness = await readDeviceDetails( + devicePropertiesPayload( + { developerModeStatus: 'disabled', ddiServicesAvailable: false }, + { tunnelState: 'unavailable', bootState: 'asleep' }, + ), + ); + + assert.ok(readiness.available); + assert.equal(readiness.developerMode, 'disabled'); + assert.equal(readiness.developerDiskImage, 'unavailable'); +}); + +test('the committed capture holds no identity a real device would recognise', () => { + // This payload was read off a real phone and is committed, so the fields that would identify that + // one unit are masked while the states the reader consumes stay verbatim (#2683). + const capture = DEVICE_INFO_DETAILS_CAPTURE as { + result: { + hardwareProperties: { serialNumber: string; ecid: number }; + deviceProperties: { bootedSnapshotName: string; developerModeStatus: string }; + connectionProperties: { tunnelIPAddress: string }; + }; + }; + const { hardwareProperties, deviceProperties, connectionProperties } = capture.result; + + assert.match(hardwareProperties.serialNumber, /MASKED/); + assert.equal(hardwareProperties.ecid, 0); + assert.match(deviceProperties.bootedSnapshotName, /MASKED/); + assert.match(connectionProperties.tunnelIPAddress, /^fd00:/); + // The states the reader consumes are the captured ones, masks and all. + assert.equal(deviceProperties.developerModeStatus, 'enabled'); +}); + +test('a device that fails the details command is unreadable, not unavailable', async () => { + const readiness = await withAppleToolProvider( + createLocalAppleToolProvider({ + runCommand: async (_cmd: string, args: string[]) => ({ + exitCode: args.includes('--json-output') ? 1 : 0, + stdout: '', + stderr: 'ERROR: The device could not be contacted.', + }), + }), + async () => await readIosDeviceReadiness(IOS_DEVICE), + ); + + assert.equal(readiness.available, false); +}); + +test('a spent budget reads nothing and claims nothing', async () => { + let toolCalls = 0; + const readiness = await withAppleToolProvider( + createLocalAppleToolProvider({ + runCommand: async () => { + toolCalls += 1; + return { exitCode: 0, stdout: '', stderr: '' }; + }, + }), + async () => await readIosDeviceReadiness(IOS_DEVICE, 0), + ); + + assert.equal(toolCalls, 0); + assert.equal(readiness.available, false); +}); + +test('an XCTest-backed device reports that its readiness cannot be read', async () => { + let toolCalls = 0; + const readiness = await withAppleToolProvider( + createLocalAppleToolProvider({ + runCommand: async () => { + toolCalls += 1; + return { exitCode: 0, stdout: '', stderr: '' }; + }, + }), + async () => + await resolveIosPhysicalDeviceControl(XCTEST_IOS_DEVICE).readDeviceReadiness( + XCTEST_IOS_DEVICE, + ), + ); + + assert.equal(toolCalls, 0); + assert.equal(readiness.available, false); + if (readiness.available) return; + assert.equal(readiness.reason, 'device_readiness_unreadable'); + assert.match(readiness.hint, /XCTest/); +}); + +test('the CoreDevice backend publishes the device report', async () => { + const readiness = await withAppleToolProvider( + createLocalAppleToolProvider({ + runCommand: async (_cmd: string, args: string[]) => { + const outputPath = jsonOutputPath(args); + if (outputPath) fs.writeFileSync(outputPath, DEVICE_INFO_DETAILS_TEXT); + return { exitCode: 0, stdout: '', stderr: '' }; + }, + }), + async () => await resolveIosPhysicalDeviceControl(IOS_DEVICE).readDeviceReadiness(IOS_DEVICE), + ); + + assert.equal(readiness.available, true); + if (!readiness.available) return; + assert.equal(readiness.developerMode, 'enabled'); + assert.equal(readiness.developerDiskImage, 'available'); +}); + +/** + * A `deviceProperties` payload together with the context it was read in. Both default to the state a + * developer disk image answer requires — tunnel up, phone booted — because #2683 treats that context + * as part of the answer rather than as background: the same `false` from a sleeping or unreachable + * device says the services were not listening. A caller that wants the weaker reading names it. + */ +function devicePropertiesPayload( + deviceProperties: Record, + context: { tunnelState?: string; bootState?: string } = {}, +): string { + return JSON.stringify({ + info: { outcome: 'success' }, + result: { + deviceProperties: { + ...deviceProperties, + bootState: deviceProperties.bootState ?? context.bootState ?? 'booted', + }, + connectionProperties: { tunnelState: context.tunnelState ?? 'connected' }, + }, + }); +} + +async function readDeviceDetails( + payload: string, + calls: string[][] = [], +): Promise { + return await withAppleToolProvider( + createLocalAppleToolProvider({ + runCommand: async (cmd: string, args: string[]) => { + calls.push([cmd, ...args]); + const outputPath = jsonOutputPath(args); + if (outputPath) fs.writeFileSync(outputPath, payload); + return { exitCode: 0, stdout: '', stderr: '' }; + }, + }), + async () => await readIosDeviceReadiness(IOS_DEVICE), + ); +} + +function jsonOutputPath(args: string[]): string | undefined { + const index = args.indexOf('--json-output'); + return index >= 0 ? args[index + 1] : undefined; +} diff --git a/packages/platform-apple/src/core/devicectl.ts b/packages/platform-apple/src/core/devicectl.ts index 3ee8de82c0..d1a921ff7c 100644 --- a/packages/platform-apple/src/core/devicectl.ts +++ b/packages/platform-apple/src/core/devicectl.ts @@ -343,6 +343,25 @@ export const IOS_DEVICECTL_DEFAULT_HINT = const IOS_DEVICE_PROCESS_LIST_HINT = "This Xcode/CoreDevice toolchain must support 'devicectl device info processes' with JSON runningProcesses so agent-device can resolve app process IDs. Inspect diagnostics for the exact devicectl API failure."; +/** + * What to tell a caller whose device reports its own Developer Mode toggle off (#2683). This is the + * one owner of that remedy: the device-readiness fact publishes these two strings as the `remedies` + * of the report it reads, so the runner preflight and this tool-output path cannot carry two + * wordings of one fix. `core/physical-device-coredevice.ts` imports them rather than restating them. + */ +export const IOS_DEVICE_DEVELOPER_MODE_OFF_HINT = + 'Enable Developer Mode on the iOS device (Settings > Privacy & Security > Developer Mode), restart it when prompted, unlock it, then retry.'; + +/** + * What to tell a caller whose developer disk image is the only thing down (#2683). Deliberately not a + * Developer Mode answer, and it never names that setting: this path only has tool text, which cannot + * say where the toggle is, and sending someone to a setting that is already correct loses the actual + * cause. The device-fact path is where both states are known, and it publishes them as fields rather + * than as prose. Owned by the same pair as {@link IOS_DEVICE_DEVELOPER_MODE_OFF_HINT}. + */ +export const IOS_DEVICE_DEVELOPER_DISK_IMAGE_HINT = + 'Let Xcode finish preparing this device: keep it unlocked and connected by cable, open Xcode > Settings > Platforms (or Window > Devices and Simulators), wait for device support to install, then retry.'; + export function resolveIosDevicectlHint(stdout: string, stderr: string): string | null { const text = `${stdout}\n${stderr}`.toLowerCase(); if (text.includes('device is busy') && text.includes('connecting')) { @@ -356,8 +375,15 @@ export function resolveIosDevicectlHint(stdout: string, stderr: string): string // usual state of a phone that has never been used for development. The // default hint sends people to check trust and Xcode, none of which is wrong // yet none of which is the cause. - if (text.includes('developer disk image') || text.includes('developer mode is disabled')) { - return 'Enable Developer Mode on the iOS device (Settings > Privacy & Security > Developer Mode), restart it when prompted, unlock it, then retry.'; + // + // The two complaints are answered apart (#2683): an image line by itself is not evidence that the + // toggle is off, and the device reports both states directly, so the name of one is never used as + // the name of the other. + if (text.includes('developer mode is disabled')) { + return IOS_DEVICE_DEVELOPER_MODE_OFF_HINT; + } + if (text.includes('developer disk image')) { + return IOS_DEVICE_DEVELOPER_DISK_IMAGE_HINT; } if (text.includes('must be paired')) { return 'Pair the iOS device with this Mac: connect it by cable, unlock it, accept the Trust prompt, and enter the device passcode, then retry.'; diff --git a/packages/platform-apple/src/core/physical-device-control.ts b/packages/platform-apple/src/core/physical-device-control.ts index a2d7f241af..a0feeecc14 100644 --- a/packages/platform-apple/src/core/physical-device-control.ts +++ b/packages/platform-apple/src/core/physical-device-control.ts @@ -13,7 +13,9 @@ import { import { ensureCoreDeviceReady, launchCoreDeviceApp, + readIosDeviceReadiness, resolveCoreDeviceTunnelIp, + type IosDeviceReadiness, } from './physical-device-coredevice.ts'; import { copyCoreDeviceRunnerFile } from './physical-device-files.ts'; import { @@ -88,6 +90,7 @@ const CONTROLS: Record = { resolveTunnel: async (device, timeoutBudgetMs) => ({ tunnelIp: await resolveCoreDeviceTunnelIp(device, timeoutBudgetMs), }), + readDeviceReadiness: readIosDeviceReadiness, }, xctest: { backend: 'xctest', @@ -102,6 +105,7 @@ const CONTROLS: Record = { captureScreenshot: captureXctestDeviceScreenshot, copyRunnerFile: rejectXctestRunnerFileCopy, resolveTunnel: rejectXctestTunnelLookup, + readDeviceReadiness: readXctestDeviceReadiness, }, }; @@ -161,6 +165,14 @@ async function rejectXctestTunnelLookup(device: DeviceInfo): Promise { ); } +async function readXctestDeviceReadiness(device: DeviceInfo): Promise { + return { + available: false, + reason: 'device_readiness_unreadable', + hint: `This device is driven through XCTest (device ${device.id}), which does not report Developer Mode or developer disk image state. Check the device's own Settings if development tooling fails to start on it.`, + }; +} + async function rejectXctestRunnerFileCopy(device: DeviceInfo): Promise { throw new AppError( 'UNSUPPORTED_OPERATION', diff --git a/packages/platform-apple/src/core/physical-device-coredevice.ts b/packages/platform-apple/src/core/physical-device-coredevice.ts index 72918637f6..bc00fa9dc4 100644 --- a/packages/platform-apple/src/core/physical-device-coredevice.ts +++ b/packages/platform-apple/src/core/physical-device-coredevice.ts @@ -9,6 +9,8 @@ import { } from '@agent-device/host-kit/host-file'; import { hostProcessId } from '@agent-device/host-kit/process'; import { + IOS_DEVICE_DEVELOPER_DISK_IMAGE_HINT, + IOS_DEVICE_DEVELOPER_MODE_OFF_HINT, IOS_DEVICECTL_DEFAULT_HINT, resolveIosDevicectlHint, runIosDevicectl, @@ -135,16 +137,33 @@ export async function resolveCoreDeviceTunnelIp( device: DeviceInfo, timeoutBudgetMs?: number, ): Promise { - if (typeof timeoutBudgetMs === 'number' && timeoutBudgetMs <= 0) return null; - const timeoutMs = - typeof timeoutBudgetMs === 'number' - ? Math.max(1, Math.min(IOS_RUNNER_DEVICE_INFO_TIMEOUT_MS, timeoutBudgetMs)) - : IOS_RUNNER_DEVICE_INFO_TIMEOUT_MS; + const details = await readIosDeviceDetails( + device, + timeoutBudgetMs ?? IOS_RUNNER_DEVICE_INFO_TIMEOUT_MS, + ); + return details?.tunnelIp ?? null; +} + +/** + * The device's own report, or `null` when CoreDevice could not answer it. Callers that need a + * verdict out of these fields read it here rather than re-running the tool: this is the one place + * that spells the command out, and an unreadable device stays unreadable instead of becoming an + * assumption about what is wrong with it (#2683). + */ +async function readIosDeviceDetails( + device: DeviceInfo, + timeoutBudgetMs: number, + signal?: AbortSignal, +): Promise { + if (!(timeoutBudgetMs > 0)) return null; + const timeoutMs = Math.max(1, Math.min(IOS_RUNNER_DEVICE_INFO_TIMEOUT_MS, timeoutBudgetMs)); try { - const probe = await runCoreDeviceDetails(device.id, timeoutMs); + const probe = await runCoreDeviceDetails(device.id, timeoutMs, 0, signal); if (probe.result.exitCode !== 0 || !probe.parsed.parsed) return null; if (probe.parsed.outcome && probe.parsed.outcome !== 'success') return null; - return probe.parsed.tunnelIp ?? null; + const { parsed } = probe; + const { parsed: _parsed, ...details } = parsed; + return details; } catch { return null; } @@ -157,7 +176,7 @@ async function runCoreDeviceDetails( signal?: AbortSignal, ): Promise<{ result: Awaited>; - parsed: { parsed: boolean; outcome?: string; tunnelState?: string; tunnelIp?: string }; + parsed: { parsed: boolean } & IosDeviceDetails; }> { const jsonPath = path.join( hostTemporaryDirectory(), @@ -192,7 +211,7 @@ async function runCoreDeviceDetails( async function readCoreDeviceDetails( jsonPath: string, -): Promise<{ parsed: boolean; outcome?: string; tunnelState?: string; tunnelIp?: string }> { +): Promise<{ parsed: boolean } & IosDeviceDetails> { try { const payload = JSON.parse(await readHostTextFile(jsonPath)) as unknown; const details = parseIosDeviceDetailsPayload(payload); @@ -202,41 +221,264 @@ async function readCoreDeviceDetails( } } -export function parseIosDeviceDetailsPayload(payload: unknown): { +/** + * What one `devicectl device info details` payload reports (#2683). Every field is the tool's own + * value, copied rather than interpreted: whether Developer Mode is on, and whether the device + * exposes developer disk image services, are two separate answers the device gives, and a reader + * that turns them into a verdict has to be able to see that one arrived and the other did not. + */ +export type IosDeviceDetails = { outcome?: string; tunnelState?: string; tunnelIp?: string; -} { - const result = (payload as { result?: unknown } | null | undefined)?.result; - if (!result || typeof result !== 'object') return {}; - const direct = ( - result as { - connectionProperties?: { tunnelState?: unknown; tunnelIPAddress?: unknown }; - } - ).connectionProperties; - const nested = ( - result as { - device?: { connectionProperties?: { tunnelState?: unknown; tunnelIPAddress?: unknown } }; - } - ).device?.connectionProperties; - const tunnelState = - readNonEmptyString(direct?.tunnelState) ?? readNonEmptyString(nested?.tunnelState); - const tunnelIp = - readNonEmptyString(direct?.tunnelIPAddress) ?? readNonEmptyString(nested?.tunnelIPAddress); - const outcome = readNonEmptyString( - (payload as { info?: { outcome?: unknown } } | null | undefined)?.info?.outcome, - ); + /** `deviceProperties.developerModeStatus`, spelled as CoreDevice spells it. */ + developerModeStatus?: string; + /** `deviceProperties.ddiServicesAvailable`, which is what the device says about its developer disk image. */ + developerDiskImageServicesAvailable?: boolean; + /** `deviceProperties.bootState`, which says whether the device was awake enough to answer at all. */ + bootState?: string; +}; + +/** + * What one payload reports, whichever of the two shapes CoreDevice used. Fields arrive either on + * `result` or nested under `result.device`, and that is a fact about the payload rather than about + * any field, so the shapes are collapsed into one pair of sections before anything is read: a parser + * that prefers the direct value per field has to spell the fallback out every time it grows a field, + * which is how a new state ends up read from only one of the two shapes (#2683). + */ +/** The payload\'s two sections, after the shape difference between releases is resolved. */ +type ReportedSections = { + connectionProperties?: Record; + deviceProperties?: Record; +}; + +function readReportedSections(result: object): ReportedSections { + const source = result as { + device?: unknown; + connectionProperties?: unknown; + deviceProperties?: unknown; + }; + const nested = asRecord(source.device); return { - ...(outcome ? { outcome } : {}), - ...(tunnelState ? { tunnelState } : {}), - ...(tunnelIp ? { tunnelIp } : {}), + connectionProperties: mergeReportedSections( + asRecord(source.connectionProperties), + asRecord(nested?.connectionProperties), + ), + deviceProperties: mergeReportedSections( + asRecord(source.deviceProperties), + asRecord(nested?.deviceProperties), + ), }; } +/** Direct wins wherever it spelled a value; the nested section fills the fields it left blank. */ +function mergeReportedSections( + direct: Record | undefined, + nested: Record | undefined, +): Record | undefined { + if (!direct) return nested; + if (!nested) return direct; + const merged: Record = { ...nested }; + for (const [key, value] of Object.entries(direct)) { + if (value !== undefined) merged[key] = value; + } + return merged; +} + +function asRecord(value: unknown): Record | undefined { + return value && typeof value === 'object' ? (value as Record) : undefined; +} + +export function parseIosDeviceDetailsPayload(payload: unknown): IosDeviceDetails { + const result = asRecord((payload as { result?: unknown } | undefined)?.result); + if (!result) return {}; + const { connectionProperties, deviceProperties } = readReportedSections(result); + return { + ...withValue( + 'outcome', + readNonEmptyString((payload as { info?: { outcome?: unknown } })?.info?.outcome), + ), + ...withValue('tunnelState', readNonEmptyString(connectionProperties?.tunnelState)), + ...withValue('tunnelIp', readNonEmptyString(connectionProperties?.tunnelIPAddress)), + ...withValue('developerModeStatus', readNonEmptyString(deviceProperties?.developerModeStatus)), + ...withValue( + 'developerDiskImageServicesAvailable', + readBoolean(deviceProperties?.ddiServicesAvailable), + ), + ...withValue('bootState', readNonEmptyString(deviceProperties?.bootState)), + }; +} + +/** + * Reports a field only when the payload carried it. An absent state has to stay absent so the reader + * can tell that the device said nothing from the device saying no. + */ +function withValue( + key: K, + value: IosDeviceDetails[K], +): Pick { + return value === undefined + ? ({} as Pick) + : ({ [key]: value } as Pick); +} + function readNonEmptyString(value: unknown): string | undefined { return typeof value === 'string' && value.trim() ? value.trim() : undefined; } +function readBoolean(value: unknown): boolean | undefined { + return typeof value === 'boolean' ? value : undefined; +} + +/** + * The device's own report on whether it can host development tooling (#2683), published through the + * physical-device control facet. `runner/host.ts` mirrors this shape structurally on its side of the + * host port. + * + * The two states are kept apart because the device reports them apart and they fail apart. A device + * with Developer Mode off cannot serve its developer disk image either; an image that is not up on a + * device with the toggle on is its own failure. Deciding which one to name is the reader's job in + * `runner/runner-device-readiness.ts`. + * + * The remedy travels with the states in {@link IosDeviceReadinessRemedies} rather than being worded + * again at each site that publishes one, so the runner preflight and the `devicectl` output path + * cannot drift apart while describing the same fix. + * + * `available: false` is the answer when no state can be established — the device could not be read, + * or it read an answer that could not have been true at the moment it was asked. It carries no + * verdict, because an unreadable device is not a diagnosed one. + */ +export type IosDeviceReadiness = + | Readonly<{ + available: true; + developerMode: IosDeveloperModeState; + developerDiskImage: IosDeveloperDiskImageState; + remedies: IosDeviceReadinessRemedies; + }> + | Readonly<{ + available: false; + reason: 'device_readiness_unreadable'; + hint: string; + }>; + +/** + * What to tell a caller about each state the device can report, published with the report. Both + * strings are owned by `core/devicectl.ts`, which answers the same two complaints when they arrive as + * tool output instead of as a device fact (#2683). + */ +export type IosDeviceReadinessRemedies = Readonly<{ + developerModeOff: string; + developerDiskImageUnavailable: string; +}>; + +/** How a device reports its own Settings > Privacy & Security > Developer Mode toggle. */ +export type IosDeveloperModeState = 'enabled' | 'disabled' | 'unknown'; + +/** How a device reports the services that serve its developer disk image. */ +export type IosDeveloperDiskImageState = 'available' | 'unavailable' | 'unknown'; + +const IOS_DEVICE_READINESS_TIMEOUT_MS = 10_000; + +const IOS_DEVICE_READINESS_REMEDIES: IosDeviceReadinessRemedies = { + developerModeOff: IOS_DEVICE_DEVELOPER_MODE_OFF_HINT, + developerDiskImageUnavailable: IOS_DEVICE_DEVELOPER_DISK_IMAGE_HINT, +}; + +/** + * What a device whose report could not be read needs: a way to read it, and no diagnosis. No fact + * means no claim, so this shape never names a cause (#2683). + */ +const IOS_DEVICE_READINESS_UNREADABLE_HINT = + 'Read the device state directly with `xcrun devicectl device info details --device --json-output -`, keeping the device unlocked and connected by cable, then retry.'; + +/** + * The developer disk image services are only answerable while the device is running and reachable. + * With the tunnel down or the phone asleep, `ddiServicesAvailable: false` says the services are not + * listening right now, not that device support is missing — and refusing the run on that reading + * would turn a self-healing first launch into a permanent "wait for Xcode" loop (#2683). + */ +function isDeveloperDiskImageAnswerObservable(details: IosDeviceDetails): boolean { + return details.tunnelState === 'connected' && details.bootState === 'booted'; +} + +function buildUnobservableDiskImageHint(details: IosDeviceDetails): string { + return ( + `The device reported its developer disk image as unavailable while it was not observable ` + + `(tunnelState=${details.tunnelState ?? 'unknown'}, bootState=${details.bootState ?? 'unknown'}). ` + + 'Unlock it, keep it connected by cable until `xcrun devicectl device info details` reports ' + + 'tunnelState=connected and bootState=booted, then retry.' + ); +} + +/** + * The device's own answer to "can this iPhone run development tooling right now" (#2683). + * + * This reads and never interprets: `developerModeStatus` is the owner's toggle in Settings > + * Privacy & Security > Developer Mode, and `ddiServicesAvailable` is whether the device exposes + * developer disk image services. Both are copied into their own field so the reader that draws a + * verdict can tell that one arrived and the other did not, which is what stops an image complaint + * from being answered as a toggle problem. What the states mean for a runner is decided by + * `runner/runner-device-readiness.ts`. + * + * The one thing it refuses to report is an image answer that could not have been true: an + * uncorroborated `ddiServicesAvailable: false` publishes the unavailability shape instead. The + * toggle keeps its answer regardless, because a disabled toggle already explains an unavailable + * image and comes from the paired record rather than from a live service. + */ +export async function readIosDeviceReadiness( + device: DeviceInfo, + timeoutBudgetMs = IOS_DEVICE_READINESS_TIMEOUT_MS, + signal?: AbortSignal, +): Promise { + const details = await readIosDeviceDetails(device, timeoutBudgetMs, signal); + if (!details) { + return { + available: false, + reason: 'device_readiness_unreadable', + hint: IOS_DEVICE_READINESS_UNREADABLE_HINT, + }; + } + const developerMode = readDeveloperModeState(details.developerModeStatus); + const developerDiskImage = readDeveloperDiskImageState( + details.developerDiskImageServicesAvailable, + ); + if ( + developerDiskImage === 'unavailable' && + developerMode !== 'disabled' && + !isDeveloperDiskImageAnswerObservable(details) + ) { + return { + available: false, + reason: 'device_readiness_unreadable', + hint: buildUnobservableDiskImageHint(details), + }; + } + return { + available: true, + developerMode, + developerDiskImage, + remedies: IOS_DEVICE_READINESS_REMEDIES, + }; +} + +/** + * Only the two spellings CoreDevice uses are states. A missing key or a spelling we do not know + * stays `unknown`: the point of asking the device is that we repeat what it said, so an answer we + * cannot recognise cannot be read as either permission or accusation. + */ +function readDeveloperModeState(status: string | undefined): IosDeveloperModeState { + const spelled = status?.toLowerCase(); + if (spelled === 'enabled') return 'enabled'; + if (spelled === 'disabled') return 'disabled'; + return 'unknown'; +} + +function readDeveloperDiskImageState(available: boolean | undefined): IosDeveloperDiskImageState { + if (available === true) return 'available'; + if (available === false) return 'unavailable'; + return 'unknown'; +} + export function resolveIosReadyHint(stdout: string, stderr: string): string { const devicectlHint = resolveIosDevicectlHint(stdout, stderr); if (devicectlHint) return devicectlHint; diff --git a/packages/platform-apple/src/core/physical-device-routing.ts b/packages/platform-apple/src/core/physical-device-routing.ts index dc95bb90ae..e0efaa832c 100644 --- a/packages/platform-apple/src/core/physical-device-routing.ts +++ b/packages/platform-apple/src/core/physical-device-routing.ts @@ -1,4 +1,5 @@ import type { DeviceInfo } from '@agent-device/kernel/device'; +import type { IosDeviceReadiness } from './physical-device-coredevice.ts'; /** * The physical-device routing contract, declared below both sides that need it: the runner's @@ -19,4 +20,14 @@ export type IosPhysicalDeviceTunnel = { tunnelIp: string | null }; export type IosPhysicalDeviceRunnerControl = { readonly backend: IosPhysicalDeviceBackend; resolveTunnel(device: DeviceInfo, timeoutBudgetMs?: number): Promise; + /** + * The device's own report on whether it can run development tooling (#2683). Only CoreDevice + * answers this, so an XCTest-backed device reports that it could not be read rather than lending + * the runner a guess to fail on. + */ + readDeviceReadiness( + device: DeviceInfo, + timeoutBudgetMs?: number, + signal?: AbortSignal, + ): Promise; }; diff --git a/packages/platform-apple/src/runner/__tests__/runner-client.test.ts b/packages/platform-apple/src/runner/__tests__/runner-client.test.ts index 8d039437b0..48df249118 100644 --- a/packages/platform-apple/src/runner/__tests__/runner-client.test.ts +++ b/packages/platform-apple/src/runner/__tests__/runner-client.test.ts @@ -10,9 +10,8 @@ import assert from 'node:assert/strict'; import fs from 'node:fs'; import path from 'node:path'; import { fileURLToPath } from 'node:url'; -import { mkdtempForTest, mkdtempForTestSync } from './tmp-dir.ts'; +import { mkdtempForTest } from './tmp-dir.ts'; import { appleRunnerTestHost } from '../test-host.ts'; -import type { DiagnosticEventInput } from '@agent-device/host-kit/diagnostics'; const mockRunCmdStreaming = vi.fn(); const mockRunCmdSync = vi.fn(); @@ -57,7 +56,6 @@ import { resolveExpectedRunnerCacheMetadata, resolveRunnerDerivedPath, } from '../runner-xctestrun.ts'; -import { parseRunnerResponse } from '../runner-session.ts'; const iosSimulator: DeviceInfo = { platform: 'apple', @@ -449,322 +447,6 @@ test('shouldRetryRunnerConnectError retries transient connect errors', () => { assert.equal(shouldRetryRunnerConnectError(err), true); }); -test('parseRunnerResponse preserves runner unsupported-operation codes', async () => { - const response = new Response( - JSON.stringify({ - ok: false, - error: { - code: 'UNSUPPORTED_OPERATION', - message: 'Unable to dismiss the iOS keyboard without a safe native dismiss control', - }, - }), - ); - const session = { state: 'starting' } as const; - - await assert.rejects( - () => parseRunnerResponse(response, session, '/tmp/runner.log'), - (error: unknown) => { - assert.ok(error instanceof AppError); - assert.equal(error.code, 'UNSUPPORTED_OPERATION'); - assert.match(error.message, /Unable to dismiss the iOS keyboard/i); - return true; - }, - ); -}); - -test('parseRunnerResponse surfaces the keyboard-dismiss hint naming the occlusion reason', async () => { - const hint = - "An element whose center sits behind the on-screen keyboard is refused with tap_keyboard_occludes_target; one whose center stays above the keys presses normally. To end editing, tap the app's own Done/Cancel control, or use keyboard enter to press the return key when submission is wanted."; - const response = new Response( - JSON.stringify({ - ok: false, - error: { - code: 'UNSUPPORTED_OPERATION', - message: 'Unable to dismiss the iOS keyboard without a safe native dismiss control', - hint, - }, - }), - ); - const session = { state: 'starting' } as const; - - await assert.rejects( - () => parseRunnerResponse(response, session, '/tmp/runner.log'), - (error: unknown) => { - assert.ok(error instanceof AppError); - assert.equal(error.code, 'UNSUPPORTED_OPERATION'); - assert.equal(error.details?.hint, hint); - assert.match(String(error.details?.hint), /tap_keyboard_occludes_target/); - assert.match(String(error.details?.hint), /center stays above the keys/i); - assert.match(String(error.details?.hint), /keyboard enter/i); - return true; - }, - ); -}); - -test('parseRunnerResponse preserves iOS AX snapshot failure code and hint', async () => { - const hint = - 'Try a smaller read such as snapshot -s -d 8, or use direct selector commands such as find id click.'; - const response = new Response( - JSON.stringify({ - ok: false, - error: { - code: 'IOS_AX_SNAPSHOT_FAILED', - message: 'iOS XCTest snapshot failed with kAXErrorIllegalArgument.', - hint, - }, - }), - ); - const session = { state: 'ready' } as const; - - await assert.rejects( - () => parseRunnerResponse(response, session, '/tmp/runner.log'), - (error: unknown) => { - assert.ok(error instanceof AppError); - assert.equal(error.code, 'IOS_AX_SNAPSHOT_FAILED'); - assert.match(error.message, /kAXErrorIllegalArgument/); - assert.equal(error.details?.hint, hint); - assert.equal(isRetryableRunnerError(error), false); - return true; - }, - ); -}); - -test('parseRunnerResponse preserves XCTest recorded failure code and hint', async () => { - const hint = - 'The iOS runner session was invalidated. Re-observe with a fresh snapshot before retrying; if the accessibility tree is unavailable, use screenshot plus coordinate commands instead of retrying the tap blindly.'; - const response = new Response( - JSON.stringify({ - ok: false, - error: { - code: 'XCTEST_RECORDED_FAILURE', - message: - 'XCTest recorded a failure while executing tap; the action may not have been performed.', - hint, - }, - }), - ); - const session = { state: 'ready' } as const; - - await assert.rejects( - () => parseRunnerResponse(response, session, '/tmp/runner.log'), - (error: unknown) => { - assert.ok(error instanceof AppError); - assert.equal(error.code, 'XCTEST_RECORDED_FAILURE'); - assert.match(error.message, /may not have been performed/); - assert.equal(error.details?.hint, hint); - assert.equal(isRetryableRunnerError(error), false); - return true; - }, - ); -}); - -test('parseRunnerResponse maps RUNNER_BUSY to retriable command failure', async () => { - const hint = 'Wait a few seconds and retry.'; - const response = new Response( - JSON.stringify({ - ok: false, - error: { - code: 'RUNNER_BUSY', - message: 'The runner is still finishing abandoned work.', - hint, - }, - }), - ); - const session = { state: 'ready' } as const; - - await assert.rejects( - () => parseRunnerResponse(response, session, '/tmp/runner.log'), - (error: unknown) => { - assert.ok(error instanceof AppError); - assert.equal(error.code, 'COMMAND_FAILED'); - assert.equal(error.details?.runnerErrorCode, 'RUNNER_BUSY'); - assert.equal(error.details?.retriable, true); - assert.equal(error.details?.hint, hint); - assert.equal(isRetryableRunnerError(error), true); - return true; - }, - ); -}); - -test('parseRunnerResponse preserves RUNNER_WEDGED as a fatal runner code', async () => { - const response = new Response( - JSON.stringify({ - ok: false, - error: { - code: 'RUNNER_WEDGED', - message: 'The runner main thread is wedged.', - hint: 'The runner session will be restarted.', - }, - }), - ); - const session = { state: 'ready' } as const; - - await assert.rejects( - () => parseRunnerResponse(response, session, '/tmp/runner.log'), - (error: unknown) => { - assert.ok(error instanceof AppError); - assert.equal(error.code, 'RUNNER_WEDGED'); - assert.equal(error.details?.runnerErrorCode, 'RUNNER_WEDGED'); - assert.equal(isRetryableRunnerError(error), false); - return true; - }, - ); -}); - -test('parseRunnerResponse classifies target app AXRuntime CoreText font crashes from runner log tail', async () => { - const logPath = writeRunnerLogTail(` -Thread 0 Crashed:: Dispatch queue: com.apple.main-thread -0 libobjc.A.dylib objc_retain + 16 -1 CoreText CreateFontWithFontURL(__CFURL const*, __CFString const*, __CFString const*) + 512 -11 AXRuntime reconstitutedSmuggledCTFontFromDictionary + 192 -12 AXRuntime -[NSDictionary(AXPropertyListCoersion) _axRecursivelyReconstitutedRepresentationFromPropertyListWithError:] + 156 -`); - const response = new Response( - JSON.stringify({ - ok: false, - error: { - code: 'XCTEST_RECORDED_FAILURE', - message: - 'XCTest recorded a failure while executing type; the action may not have been performed.', - }, - }), - ); - const session = { state: 'ready' } as const; - - await assert.rejects( - () => parseRunnerResponse(response, session, logPath), - (error: unknown) => { - assert.ok(error instanceof AppError); - assert.equal(error.code, 'IOS_TARGET_APP_CRASH'); - assert.equal(error.details?.runnerFailureReason, 'target_app_axruntime_coretext_crash'); - assert.match(String(error.details?.hint), /AXRuntime read accessibility attributes/); - assert.match(String(error.details?.hint), /latest stable simulator runtime/); - assert.match(String(error.details?.hint), /exact command, selector\/ref/); - return true; - }, - ); -}); - -test('parseRunnerResponse classifies explicit target app crashes from runner log tail', async () => { - const logPath = writeRunnerLogTail(` -AGENT_DEVICE_RUNNER_COMMAND_FAILED command=snapshot -The application under test terminated unexpectedly. -`); - const response = new Response( - JSON.stringify({ - ok: false, - error: { - code: 'COMMAND_FAILED', - message: 'Runner error', - }, - }), - ); - const session = { state: 'ready' } as const; - - await assert.rejects( - () => parseRunnerResponse(response, session, logPath), - (error: unknown) => { - assert.ok(error instanceof AppError); - assert.equal(error.code, 'IOS_TARGET_APP_CRASH'); - assert.equal(error.details?.runnerFailureReason, 'target_app_crash'); - assert.match(String(error.details?.hint), /target iOS app appears to have crashed/); - assert.equal(isRetryableRunnerError(error), false); - return true; - }, - ); -}); - -test('parseRunnerResponse does not classify incidental XCTest crash text as target app crash', async () => { - const logPath = writeRunnerLogTail(` -XCTest runner recovered from a previous test note: the word crashed appeared in debug output. -AGENT_DEVICE_RUNNER_COMMAND_FAILED command=snapshot error=fetch failed -`); - const response = new Response( - JSON.stringify({ - ok: false, - error: { - code: 'COMMAND_FAILED', - message: 'fetch failed', - }, - }), - ); - const session = { state: 'ready' } as const; - - await assert.rejects( - () => parseRunnerResponse(response, session, logPath), - (error: unknown) => { - assert.ok(error instanceof AppError); - assert.equal(error.code, 'COMMAND_FAILED'); - assert.equal(error.details?.runnerFailureReason, undefined); - assert.equal(error.details?.hint, undefined); - assert.equal(isRetryableRunnerError(error), true); - return true; - }, - ); -}); - -test('parseRunnerResponse hints when XCTest main-thread execution times out', async () => { - const logPath = writeRunnerLogTail( - 'AGENT_DEVICE_RUNNER_COMMAND_FAILED command=type error=main thread execution timed out', - ); - const response = new Response( - JSON.stringify({ - ok: false, - error: { - code: 'COMMAND_FAILED', - message: 'main thread execution timed out', - }, - }), - ); - const session = { state: 'ready' } as const; - - await assert.rejects( - () => parseRunnerResponse(response, session, logPath), - (error: unknown) => { - assert.ok(error instanceof AppError); - assert.equal(error.code, 'COMMAND_FAILED'); - assert.equal(error.details?.runnerFailureReason, 'runner_main_thread_execution_timeout'); - assert.match(String(error.details?.hint), /XCTest timed out waiting for main-thread work/); - assert.match(String(error.details?.hint), /screenshot as visual truth/); - assert.match(String(error.details?.hint), /coordinate presses/); - return true; - }, - ); -}); - -test('parseRunnerResponse emits diagnostics for runner gesture fallbacks', async () => { - const response = new Response( - JSON.stringify({ - ok: true, - data: { - message: 'dragged', - gestureFallback: 'xctest-coordinate-drag', - gestureFallbackMessage: 'Runner synthesized drag is unavailable', - gestureFallbackHint: 'Using XCTest coordinate drag fallback.', - }, - }), - ); - const session = { state: 'starting' } as const; - const diagnosticEvents: DiagnosticEventInput[] = []; - appleRunnerTestHost.update({ emitDiagnostic: (event) => diagnosticEvents.push(event) }); - - const data = await parseRunnerResponse(response, session, '/tmp/runner.log'); - assert.equal(data.gestureFallback, 'xctest-coordinate-drag'); - - assert.equal(session.state, 'ready'); - const diagnostics = JSON.stringify(diagnosticEvents); - assert.match(diagnostics, /ios_runner_gesture_fallback/); - assert.match(diagnostics, /xctest-coordinate-drag/); -}); - -function writeRunnerLogTail(contents: string): string { - const dir = mkdtempForTestSync('agent-device-runner-log-'); - onTestFinished(() => fs.rmSync(dir, { recursive: true, force: true })); - const logPath = path.join(dir, 'runner.log'); - fs.writeFileSync(logPath, contents); - return logPath; -} - test('isRetryableRunnerError does not retry xcodebuild early-exit errors', () => { const err = new AppError( 'COMMAND_FAILED', diff --git a/packages/platform-apple/src/runner/__tests__/runner-device-readiness.test.ts b/packages/platform-apple/src/runner/__tests__/runner-device-readiness.test.ts new file mode 100644 index 0000000000..1c00f2edd0 --- /dev/null +++ b/packages/platform-apple/src/runner/__tests__/runner-device-readiness.test.ts @@ -0,0 +1,273 @@ +import assert from 'node:assert/strict'; +import { beforeEach, test, vi } from 'vitest'; +import { AppError, normalizeError } from '@agent-device/kernel/errors'; +import type { DeviceInfo } from '@agent-device/kernel/device'; +import { appleRunnerTestHost } from '../test-host.ts'; +import type { IosPhysicalDeviceRunnerControl } from '../../core/physical-device-routing.ts'; +import type { IosDeviceReadiness } from '../host.ts'; +import { preflightIosRunnerDeviceReadiness } from '../runner-device-readiness.ts'; +import { RUNNER_DEVICE_READINESS_FAILURE_REASONS } from '../runner-contract.ts'; +import { IOS_DEVICE, IOS_SIMULATOR, MACOS_DEVICE } from './device-fixtures.ts'; +import { + deviceReadinessFixtures, + type IosDeviceReadinessFixture, + type IosDeviceReadinessReport, +} from './runner-startup-failure-fixtures.ts'; + +/** + * Whether an iPhone can host development tooling is a fact the phone holds, not a fact a build log + * implies (#2683). `devicectl` output has always had one hint covering both "Developer Mode is + * disabled" and "developer disk image" complaints and always named the toggle, which sent people to + * a Settings pane that was already correct whenever the image was the actual obstacle. + * + * Only one of those two states may stop a run before the build. The toggle is owner-only and no later + * step turns it on; the developer disk image is mounted on demand by CoreDevice during build and + * launch since iOS 17, so a phone that has just been rebooted reports it down while the very next build + * clears it (#2683 review). These cases drive the recorded device reports through the preflight and + * assert which one refuses, which one is carried forward, and that the two are never confused. + */ + +const REPORTS = deviceReadinessFixtures(); + +/** The one report the preflight is allowed to refuse a build for: the owner's toggle. */ +const REFUSALS = REPORTS.filter( + (fixture): fixture is IosDeviceReadinessFixture => + fixture.reason === 'device_developer_mode_disabled', +); + +/** + * Remedies no other module could produce, so a hint matching one of them can only have come from the + * report this test handed over. That is the claim #2683 has to keep: the preflight reads the wording + * the device fact carries, which `core/devicectl.ts` owns, and never words a fix of its own beside it. + */ +const REMEDIES = { + developerModeOff: 'FIX-DEVELOPER-MODE-TOGGLE', + developerDiskImageUnavailable: 'FIX-DEVELOPER-DISK-IMAGE', +} as const; + +const HINT_FOR_REASON = { + device_developer_mode_disabled: REMEDIES.developerModeOff, + device_developer_disk_image_unavailable: REMEDIES.developerDiskImageUnavailable, +} as const; + +/** The budget `runner-session.ts` hands the probe: its slice of the startup budget and its signal. */ +const BUDGET = { budgetMs: 10_000 } as const; + +const readDeviceReadiness = vi.fn( + (_device: DeviceInfo, _budgetMs?: number, _signal?: AbortSignal): Promise => + Promise.reject(new Error('this case records no device report')), +); + +beforeEach(() => { + readDeviceReadiness.mockReset(); + appleRunnerTestHost.update({ + resolveIosPhysicalDeviceControl: () => fakeDeviceControl(readDeviceReadiness), + }); +}); + +for (const fixture of REFUSALS) { + test(`a device reporting ${fixture.deviceReport.developerMode} mode refuses the run with ${fixture.reason}`, async () => { + readDeviceReadiness.mockResolvedValue(readableReport(fixture.deviceReport)); + + const error = await expectRefusal(IOS_DEVICE); + + assert.equal(error.code, 'COMMAND_FAILED'); + assert.equal(error.details?.reason, fixture.reason); + assert.equal(error.details?.hint, HINT_FOR_REASON[fixture.reason]); + assert.equal(error.details?.deviceId, IOS_DEVICE.id); + // Both states travel with the reason, so a caller can see what the device said rather than only + // which of the two this reader decided to name. + assert.equal(error.details?.developerMode, fixture.deviceReport.developerMode); + assert.equal(error.details?.developerDiskImage, fixture.deviceReport.developerDiskImage); + }); + + test(`the ${fixture.reason} reason reaches rendered CLI JSON`, async () => { + readDeviceReadiness.mockResolvedValue(readableReport(fixture.deviceReport)); + + const error = await expectRefusal(IOS_DEVICE); + const rendered = JSON.parse( + JSON.stringify({ success: false, error: normalizeError(error, { diagnosticId: 'diag-1' }) }), + ) as { success: boolean; error: Record }; + + assert.equal(rendered.success, false); + assert.equal(rendered.error.code, 'COMMAND_FAILED'); + assert.equal(rendered.error.details.reason, fixture.reason); + // `normalizeError` lifts the hint out of `details`, so rendered JSON carries it at top level. + assert.equal(rendered.error.hint, HINT_FOR_REASON[fixture.reason]); + assert.equal(rendered.error.details.hint, undefined); + assert.equal(rendered.error.diagnosticId, 'diag-1'); + }); +} + +for (const fixture of REPORTS.filter((f) => f.reason !== 'device_developer_mode_disabled')) { + test(`a device reporting ${fixture.deviceReport.developerDiskImage} disk image with ${fixture.deviceReport.developerMode} mode builds anyway`, async () => { + // The refusal #2683 shipped with was wrong here (#2683 review): iOS 17+ mounts the image on demand + // during build and launch, so this state has to reach the build rather than stop it. + readDeviceReadiness.mockResolvedValue(readableReport(fixture.deviceReport)); + + await assert.doesNotReject(() => preflightIosRunnerDeviceReadiness(IOS_DEVICE, BUDGET)); + }); + + test(`the ${fixture.reason} report is carried forward for the failure it explains`, async () => { + readDeviceReadiness.mockResolvedValue(readableReport(fixture.deviceReport)); + + const states = await preflightIosRunnerDeviceReadiness(IOS_DEVICE, BUDGET); + + assert.deepEqual(states, { + developerMode: fixture.deviceReport.developerMode, + developerDiskImage: fixture.deviceReport.developerDiskImage, + developerDiskImageHint: REMEDIES.developerDiskImageUnavailable, + }); + }); +} + +test('a device whose report cannot be read is not given a reason', async () => { + readDeviceReadiness.mockResolvedValue({ + available: false, + reason: 'device_readiness_unreadable', + hint: 'Read the device state directly with `xcrun devicectl device info details`.', + } satisfies IosDeviceReadiness); + + await assert.doesNotReject(() => preflightIosRunnerDeviceReadiness(IOS_DEVICE, BUDGET)); +}); + +test('a device whose report cannot be read carries no state forward', async () => { + // Nothing was read, so nothing may be claimed later (#2683). + readDeviceReadiness.mockResolvedValue({ + available: false, + reason: 'device_readiness_unreadable', + hint: 'Read the device state directly with `xcrun devicectl device info details`.', + } satisfies IosDeviceReadiness); + + assert.equal(await preflightIosRunnerDeviceReadiness(IOS_DEVICE, BUDGET), undefined); +}); + +test('a device reporting both states healthy is not a failure and carries an available image', async () => { + readDeviceReadiness.mockResolvedValue( + readableReport({ developerMode: 'enabled', developerDiskImage: 'available' }), + ); + + const states = await preflightIosRunnerDeviceReadiness(IOS_DEVICE, BUDGET); + + assert.equal(states?.developerDiskImage, 'available'); +}); + +test('a device that reports neither state is not read as accusing its owner', async () => { + // A toolchain that spells these fields differently, or omits one, earns no verdict. Reading + // "unknown" as "off" is how a version bump turns into a claim about someone's Settings (#2683). + readDeviceReadiness.mockResolvedValue( + readableReport({ developerMode: 'unknown', developerDiskImage: 'unknown' }), + ); + + await assert.doesNotReject(() => preflightIosRunnerDeviceReadiness(IOS_DEVICE, BUDGET)); +}); + +test('an unavailable disk image on a device with Developer Mode on is never named as the toggle', async () => { + // The conflation #2682 answered with "enable Developer Mode" for a device that had it on. The + // unread half of the states has to stay unread too: only the image may be named here. + for (const developerMode of ['enabled', 'unknown'] as const) { + readDeviceReadiness.mockResolvedValue( + readableReport({ developerMode, developerDiskImage: 'unavailable' }), + ); + + const states = await preflightIosRunnerDeviceReadiness(IOS_DEVICE, BUDGET); + + assert.equal(states?.developerDiskImage, 'unavailable'); + // Only the image remedy may be carried: the toggle remedy mentions the Settings pane. + assert.doesNotMatch(String(states?.developerDiskImageHint), /Privacy & Security/); + } +}); + +test('a device with Developer Mode off names the toggle even when the image is down too', async () => { + // The direction that does hold: the toggle explains the image, so naming the toggle is the claim + // that leaves the reader with one thing to fix. + readDeviceReadiness.mockResolvedValue( + readableReport({ developerMode: 'disabled', developerDiskImage: 'unavailable' }), + ); + + const error = await expectRefusal(IOS_DEVICE); + + assert.equal(error.details?.reason, 'device_developer_mode_disabled'); +}); + +test('a simulator or the desktop target never asks the device', async () => { + for (const device of [IOS_SIMULATOR, MACOS_DEVICE]) { + await assert.doesNotReject(() => preflightIosRunnerDeviceReadiness(device, BUDGET)); + } + + assert.equal(readDeviceReadiness.mock.calls.length, 0); +}); + +test('every device-readiness reason has a recorded device report', () => { + const reasonsWithReports = new Set(REPORTS.map((fixture) => fixture.reason)); + + assert.equal(reasonsWithReports.size, RUNNER_DEVICE_READINESS_FAILURE_REASONS.length); + for (const reason of RUNNER_DEVICE_READINESS_FAILURE_REASONS) { + assert.ok(reasonsWithReports.has(reason), `no device report records the ${reason} reason`); + } +}); + +test('the probe is bounded by the startup budget it runs inside', async () => { + // A preflight that ignores the budget it was given can outlive the command that started it, which + // is how a cancelled `prepare` ends up building anyway (#2683). + const controller = new AbortController(); + readDeviceReadiness.mockResolvedValue( + readableReport({ developerMode: 'enabled', developerDiskImage: 'available' }), + ); + + await preflightIosRunnerDeviceReadiness(IOS_DEVICE, { + budgetMs: 2_500, + signal: controller.signal, + }); + + assert.deepEqual(readDeviceReadiness.mock.lastCall?.[1], 2_500); + assert.equal(readDeviceReadiness.mock.lastCall?.[2], controller.signal); +}); + +test('a budget that ran out during the probe stops the startup even on a healthy device', async () => { + // The read returning just as the caller gave up is not permission to keep going: nobody is waiting + // for a build that cannot be delivered (#2683). + const controller = new AbortController(); + readDeviceReadiness.mockImplementation(() => { + controller.abort(); + return Promise.resolve( + readableReport({ developerMode: 'enabled', developerDiskImage: 'available' }), + ); + }); + + await assert.rejects( + () => + preflightIosRunnerDeviceReadiness(IOS_DEVICE, { + budgetMs: 10_000, + signal: controller.signal, + }), + (error: unknown) => (error as Error).name === 'AbortError', + ); +}); + +function readableReport( + report: IosDeviceReadinessReport, +): Extract { + return { available: true, ...report, remedies: REMEDIES }; +} + +async function expectRefusal(device: DeviceInfo): Promise { + let caught: unknown; + await assert.rejects( + () => preflightIosRunnerDeviceReadiness(device, BUDGET), + (error: unknown) => { + caught = error; + return true; + }, + ); + assert.ok(caught instanceof AppError, 'the preflight must refuse with an AppError'); + return caught; +} + +function fakeDeviceControl(read: typeof readDeviceReadiness): IosPhysicalDeviceRunnerControl { + return { + backend: 'coredevice', + resolveTunnel: async () => ({ tunnelIp: null }), + readDeviceReadiness: read, + }; +} diff --git a/packages/platform-apple/src/runner/__tests__/runner-early-exit-diagnosis.test.ts b/packages/platform-apple/src/runner/__tests__/runner-early-exit-diagnosis.test.ts index 9028cde80c..a04f9a583e 100644 --- a/packages/platform-apple/src/runner/__tests__/runner-early-exit-diagnosis.test.ts +++ b/packages/platform-apple/src/runner/__tests__/runner-early-exit-diagnosis.test.ts @@ -16,7 +16,11 @@ const PROVISIONING_FAILURE_STDERR = [ '** TEST EXECUTE FAILED **', ].join('\n'); -function sessionFailingWith(stdout: string, stderr: string): RunnerSession { +function sessionFailingWith( + stdout: string, + stderr: string, + startupDeviceStates?: RunnerSession['startupDeviceStates'], +): RunnerSession { return { sessionId: 'early-exit-session', device: { platform: 'apple', id: 'device-1', name: 'iPhone', kind: 'device', booted: true }, @@ -27,9 +31,16 @@ function sessionFailingWith(stdout: string, stderr: string): RunnerSession { testPromise: Promise.resolve({ exitCode: 1, stdout, stderr }), child: { pid: 4242, exitCode: 1 } as ExecBackgroundResult['child'], state: 'starting', + startupDeviceStates, }; } +const IMAGE_DOWN_STATES = { + developerMode: 'enabled' as const, + developerDiskImage: 'unavailable' as const, + developerDiskImageHint: 'Unlock the iPhone so it can mount the developer disk image.', +}; + test('the early-exit error a user actually receives names the provisioning cause', async () => { // Regression: the reason was classified correctly while the hint was built // separately and always returned connect-timeout guidance, so the shipped @@ -67,3 +78,44 @@ test('a busy connecting device keeps its own targeted hint', async () => { assert.match(String(error.details?.hint), /still connecting/); }); + +test('an early exit carries the disk-image state the device was read in (#2683)', async () => { + // A locked iPhone lets the build finish and kills `xcodebuild test-without-building` instead, so + // the startup build catch never runs and the readiness facts read before the build would be + // dropped. Captured on hardware: this is the failure an image-down locked phone actually produces. + const error = (await buildRunnerEarlyExitError({ + session: sessionFailingWith( + '', + 'xcodebuild: error: Timed out waiting for the test runner', + IMAGE_DOWN_STATES, + ), + port: 8100, + })) as AppError; + + assert.equal(error.details?.developerDiskImage, 'unavailable'); +}); + +test('an early exit that already names a cause keeps it, and only gains the fact (#2683)', async () => { + const error = (await buildRunnerEarlyExitError({ + session: sessionFailingWith( + '', + 'xcodebuild: error: Timed out waiting for the test runner', + IMAGE_DOWN_STATES, + ), + port: 8100, + })) as AppError; + + // The connect reason was proved by the tool output, so the device's image state is never allowed + // to overwrite a claimed cause or swap the hint beside it. + assert.equal(error.details?.reason, 'IOS_RUNNER_CONNECT_TIMEOUT'); + assert.match(String(error.details?.hint), /Retry runner startup/); +}); + +test('a session that never probed the device publishes no disk-image claim (#2683)', async () => { + const error = (await buildRunnerEarlyExitError({ + session: sessionFailingWith('', 'xcodebuild: error: Timed out waiting for the test runner'), + port: 8100, + })) as AppError; + + assert.equal('developerDiskImage' in (error.details ?? {}), false); +}); diff --git a/packages/platform-apple/src/runner/__tests__/runner-failure-diagnostics.test.ts b/packages/platform-apple/src/runner/__tests__/runner-failure-diagnostics.test.ts new file mode 100644 index 0000000000..cfdf71bc09 --- /dev/null +++ b/packages/platform-apple/src/runner/__tests__/runner-failure-diagnostics.test.ts @@ -0,0 +1,154 @@ +import assert from 'node:assert/strict'; +import fs from 'node:fs'; +import path from 'node:path'; +import { onTestFinished, test } from 'vitest'; +import { AppError } from '@agent-device/kernel/errors'; +import { logChunk } from '../runner-io.ts'; +import { captureRunnerLogAttempt } from '../runner-failure-diagnostics.ts'; +import { parseRunnerResponse } from '../runner-session.ts'; +import { mkdtempForTestSync } from './tmp-dir.ts'; + +/** + * One `runner.log` serves every command sent to one device and is never truncated between them + * (#2683). Classifying a failure from the tail of that file therefore reads whatever the last + * crashed command left behind and hands the blame to whoever failed next — a `snapshot` that timed + * out two commands after an app crash used to be reported as that crash. + * + * The marker under test is where the log had reached when the command was sent, so only bytes this + * command wrote can explain it. + */ + +const AX_RUNTIME_CRASH = `Thread 0 Crashed:: Dispatch queue: com.apple.main-thread +0 libobjc.A.dylib objc_retain + 16 +1 CoreText CreateFontWithFontURL(__CFURL const*, __CFString const*, __CFString const*) + 512 +11 AXRuntime reconstitutedSmuggledCTFontFromDictionary + 192 +`; + +const PRELUDE = 'AGENT_DEVICE_RUNNER_COMMAND_START command=snapshot\n'; + +const FAILED_BODY = JSON.stringify({ + ok: false, + error: { code: 'COMMAND_FAILED', message: 'Runner command timed out' }, +}); + +test('a crash an earlier command wrote is not blamed on the command that failed next', async () => { + const logPath = writeRunnerLog(AX_RUNTIME_CRASH); + const logAttempt = await captureRunnerLogAttempt(logPath); + + const error = await expectFailure(logAttempt); + + assert.equal(error.code, 'COMMAND_FAILED'); + assert.equal(error.details?.runnerFailureReason, undefined); +}); + +test('a crash this command wrote after the marker is still classified', async () => { + // The marker must narrow what is read, not switch the reader off: without this control the + // negative case above would pass by never reading the log at all. + const logPath = writeRunnerLog('AGENT_DEVICE_RUNNER_COMMAND_START command=snapshot\n'); + const logAttempt = await captureRunnerLogAttempt(logPath); + fs.appendFileSync(logPath, AX_RUNTIME_CRASH); + + const error = await expectFailure(logAttempt); + + assert.equal(error.code, 'IOS_TARGET_APP_CRASH'); + assert.equal(error.details?.runnerFailureReason, 'target_app_axruntime_coretext_crash'); +}); + +test('a log truncated behind the marker is not read at all', async () => { + // A runner restarted under the command and rewrote its log, so the file is now shorter than the + // byte this command started at. Nothing in it can be this command's, and guessing is worse than + // staying silent (#2683). + const logPath = writeRunnerLog(`${'x'.repeat(4096)}\n`); + const logAttempt = await captureRunnerLogAttempt(logPath); + fs.writeFileSync(logPath, AX_RUNTIME_CRASH); + + const error = await expectFailure(logAttempt); + + assert.equal(error.details?.runnerFailureReason, undefined); +}); + +test('a log that does not exist yet is read from its first byte', async () => { + const dir = mkdtempForTestSync('agent-device-runner-log-missing-'); + onTestFinished(() => fs.rmSync(dir, { recursive: true, force: true })); + const logPath = path.join(dir, 'runner.log'); + const logAttempt = await captureRunnerLogAttempt(logPath); + assert.equal(logAttempt?.byteOffset, 0); + fs.writeFileSync(logPath, AX_RUNTIME_CRASH); + + const error = await expectFailure(logAttempt); + + assert.equal(error.details?.runnerFailureReason, 'target_app_axruntime_coretext_crash'); +}); + +test('a command with no log configured claims nothing from one', async () => { + const error = await expectFailure(await captureRunnerLogAttempt(undefined)); + + assert.equal(error.code, 'COMMAND_FAILED'); + assert.equal(error.details?.runnerFailureReason, undefined); +}); + +test("an earlier command's bytes still in the writer are not this command's either", async () => { + // `logChunk` queues its write on a promise chain and returns immediately, so the file on disk can + // still be short when the next command measures it. Draining that queue first is what keeps these + // bytes below the marker instead of above it (#2683). + // + // The log starts with bytes already flushed, so a measurement that skipped the queue reports a + // number that is short by exactly the crash rather than reporting zero: the assertion below fails + // for the reason it should. + const logPath = writeRunnerLog(PRELUDE); + logChunk(AX_RUNTIME_CRASH, logPath); + + const logAttempt = await captureRunnerLogAttempt(logPath); + + assert.equal(logAttempt?.byteOffset, Buffer.byteLength(PRELUDE + AX_RUNTIME_CRASH)); + assert.equal(fs.statSync(logPath).size, logAttempt?.byteOffset); + + const error = await expectFailure(logAttempt); + assert.equal(error.details?.runnerFailureReason, undefined); +}); + +test('a log the disk refuses leaves no marker', async () => { + // The writer no longer swallows a failed append (#2683 review), and the boundary has to say so: an + // offset measured over bytes that never landed would credit this command with output it did not + // produce, so the marker is withheld and the tail goes unread. `blocker` is a regular file standing + // where a directory has to be, which is the cheapest way to make every append fail with ENOTDIR. + const dir = mkdtempForTestSync('agent-device-runner-log-refused-'); + onTestFinished(() => fs.rmSync(dir, { recursive: true, force: true })); + fs.writeFileSync(path.join(dir, 'blocker'), ''); + const logPath = path.join(dir, 'blocker', 'runner.log'); + logChunk(AX_RUNTIME_CRASH, logPath); + + assert.equal(await captureRunnerLogAttempt(logPath), undefined); +}); + +test('a command that already stopped waiting draws no marker', async () => { + // The boundary is a prelude to a command, so a caller that gave up gets no further delay and no + // claim about a log it is no longer reading (#2683 review). + const logPath = writeRunnerLog(PRELUDE); + const canceled = AbortSignal.abort(); + + assert.equal(await captureRunnerLogAttempt(logPath, { signal: canceled }), undefined); +}); + +async function expectFailure( + logAttempt: Awaited>, +): Promise { + let caught: unknown; + await assert.rejects( + () => parseRunnerResponse(new Response(FAILED_BODY), { state: 'ready' }, logAttempt), + (error: unknown) => { + caught = error; + return true; + }, + ); + assert.ok(caught instanceof AppError); + return caught; +} + +function writeRunnerLog(contents: string): string { + const dir = mkdtempForTestSync('agent-device-runner-log-'); + onTestFinished(() => fs.rmSync(dir, { recursive: true, force: true })); + const logPath = path.join(dir, 'runner.log'); + fs.writeFileSync(logPath, contents); + return logPath; +} diff --git a/packages/platform-apple/src/runner/__tests__/runner-session-readiness.test.ts b/packages/platform-apple/src/runner/__tests__/runner-session-readiness.test.ts index d616fa1715..96ee609ecb 100644 --- a/packages/platform-apple/src/runner/__tests__/runner-session-readiness.test.ts +++ b/packages/platform-apple/src/runner/__tests__/runner-session-readiness.test.ts @@ -1,6 +1,7 @@ import assert from 'node:assert/strict'; +import fs from 'node:fs'; import { beforeEach, test, vi } from 'vitest'; -import { IOS_SIMULATOR } from './device-fixtures.ts'; +import { IOS_DEVICE, IOS_SIMULATOR } from './device-fixtures.ts'; import { appleRunnerTestHost } from '../test-host.ts'; import { AppError } from '@agent-device/kernel/errors'; import { @@ -14,6 +15,8 @@ import { redirectHandle, } from './runner-session-fixtures.ts'; import { mkdtempForTestSync } from './tmp-dir.ts'; +import { createLocalAppleToolProvider, withAppleToolProvider } from '../../core/tool-provider.ts'; +import { IOS_DEVICE_DEVELOPER_DISK_IMAGE_HINT } from '../../core/devicectl.ts'; const { mockAcquireXcodebuildSimulatorSetRedirect, @@ -113,7 +116,11 @@ vi.mock('../runner-xctestrun.ts', async () => { }; }); -import { abortAllIosRunnerSessions, executeRunnerCommandWithSession } from '../runner-session.ts'; +import { + abortAllIosRunnerSessions, + ensureRunnerSession, + executeRunnerCommandWithSession, +} from '../runner-session.ts'; // Test-only stand-in for the daemon's own runtime lease-owner-state-dir // setter (root-only; the package cannot import it - R11). Backs the @@ -539,3 +546,165 @@ test('runner session preserves structured runner failures', async () => { }, ); }); + +/** + * The two startup probes answer different questions about different machines, and the order they run + * in is a claim (#2683). Both answers can be wrong at once; the phone's is the one the caller can fix + * without admin rights on the Mac, so the device is asked first and gets to speak. Probing the host + * first would publish only the Mac's reason and hide the device's for as long as both held. + */ +test('a device and a Mac that are both wrong publish the device reason', async () => { + const device = { ...IOS_DEVICE, id: 'runner-session-probe-order-device' }; + mockRunAppleToolCommand.mockImplementation(async (cmd: string, args: string[]) => { + if (cmd === 'DevToolsSecurity' && args[0] === '-status') { + return { exitCode: 0, stdout: 'Developer mode is currently disabled.\n', stderr: '' }; + } + return { exitCode: 0, stdout: '', stderr: '' }; + }); + + await assert.rejects( + () => + withAppleToolProvider( + createLocalAppleToolProvider({ + runCommand: async (_cmd: string, args: string[]) => { + const outputPath = jsonOutputPathOf(args); + if (outputPath) { + fs.writeFileSync(outputPath, DEVELOPER_MODE_OFF_DETAILS_PAYLOAD); + } + return { exitCode: 0, stdout: '', stderr: '' }; + }, + }), + () => ensureRunnerSession(device, {}), + ), + (error: unknown) => { + assert.ok(error instanceof AppError); + assert.equal(error.details?.reason, 'device_developer_mode_disabled'); + return true; + }, + ); +}); + +/** A device reporting its own toggle off while awake and connected: the state that makes it a fact. */ +const DEVELOPER_MODE_OFF_DETAILS_PAYLOAD = JSON.stringify({ + info: { outcome: 'success' }, + result: { + deviceProperties: { + developerModeStatus: 'disabled', + ddiServicesAvailable: false, + bootState: 'booted', + }, + connectionProperties: { tunnelState: 'connected' }, + }, +}); + +/** Where `devicectl ... --json-output ` is told to put its payload. */ +function jsonOutputPathOf(args: string[]): string | undefined { + const index = args.indexOf('--json-output'); + return index >= 0 ? args[index + 1] : undefined; +} + +/** + * The startup catch — not the build catch — is where the device's own answer attaches (#2690 review): + * a cold build, a warm derived cache that fails at install, and an external xctestrun that never + * launches are different steps, and a caller told "developer disk image" should not have to know which + * one this run happened to take. What each step throws below is the shape that step publishes; what is + * under test is what the session adds on the way out. + */ +test('a build that named no cause on a device with its image down gets the device answer', async () => { + const device = { ...IOS_DEVICE, id: 'runner-session-image-down-build' }; + mockEnsureXctestrunArtifact.mockRejectedValue( + new AppError('COMMAND_FAILED', 'xcodebuild build-for-testing failed', { + reason: 'build_failed_unclassified', + startupRuleMatched: false, + details: { stdout: "error: cannot find 'AgentDeviceRunnerCommand' in scope\n" }, + }), + ); + + const error = await expectStartupFailure(device, deviceDetailsPayload('enabled', false)); + + assert.equal(error.details?.reason, 'device_developer_disk_image_unavailable'); + assert.equal(error.details?.developerDiskImage, 'unavailable'); + assert.ok(String(error.details?.hint).includes(IOS_DEVICE_DEVELOPER_DISK_IMAGE_HINT)); +}); + +test('a warm cache that fails at launch still carries what the device said', async () => { + // The case the build-catch-only version missed: nothing had to compile, so the device's answer was + // never attached anywhere, and an install that cannot start the runner said only "build failed". + const device = { ...IOS_DEVICE, id: 'runner-session-image-down-launch' }; + mockEnsureXctestrunArtifact.mockResolvedValue({ + xctestrunPath: '/tmp/base-runner.xctestrun', + derived: '/tmp/derived', + cache: 'exact', + artifact: 'valid', + buildMs: 0, + xctestrunPathSource: 'manifest', + }); + mockRunCmdBackground.mockImplementation(() => { + throw new AppError('COMMAND_FAILED', 'xcodebuild test-without-building exited unexpectedly'); + }); + + const error = await expectStartupFailure(device, deviceDetailsPayload('enabled', false)); + + assert.equal(error.details?.reason, 'device_developer_disk_image_unavailable'); + assert.equal(error.details?.developerDiskImage, 'unavailable'); +}); + +test('a device whose image is available claims nothing for a failure it did not cause', async () => { + const device = { ...IOS_DEVICE, id: 'runner-session-image-available' }; + mockEnsureXctestrunArtifact.mockRejectedValue( + new AppError('COMMAND_FAILED', 'xcodebuild build-for-testing failed', { + reason: 'build_failed_unclassified', + startupRuleMatched: false, + details: { stdout: "error: cannot find 'AgentDeviceRunnerCommand' in scope\n" }, + }), + ); + + const error = await expectStartupFailure(device, deviceDetailsPayload('enabled', true)); + + assert.equal(error.details?.reason, 'build_failed_unclassified'); + assert.equal(error.details?.developerDiskImage, 'available'); +}); + +/** `devicectl device info details` for a phone that is awake, connected, and reporting both states. */ +function deviceDetailsPayload( + developerModeStatus: 'enabled' | 'disabled', + ddiServicesAvailable: boolean, +): string { + return JSON.stringify({ + info: { outcome: 'success' }, + result: { + deviceProperties: { + developerModeStatus, + ddiServicesAvailable, + bootState: 'booted', + }, + connectionProperties: { tunnelState: 'connected' }, + }, + }); +} + +/** Starts a session against a device whose `devicectl` answers with `payload`, and returns the failure. */ +async function expectStartupFailure(device: typeof IOS_DEVICE, payload: string): Promise { + let caught: unknown; + await assert.rejects( + () => + withAppleToolProvider( + createLocalAppleToolProvider({ + runCommand: async (_cmd: string, args: string[]) => { + const outputPath = jsonOutputPathOf(args); + if (outputPath) { + fs.writeFileSync(outputPath, payload); + } + return { exitCode: 0, stdout: '', stderr: '' }; + }, + }), + () => ensureRunnerSession(device, {}), + ), + (error: unknown) => { + caught = error; + return true; + }, + ); + assert.ok(caught instanceof AppError, 'startup must fail with an AppError'); + return caught; +} diff --git a/packages/platform-apple/src/runner/__tests__/runner-session-response.test.ts b/packages/platform-apple/src/runner/__tests__/runner-session-response.test.ts new file mode 100644 index 0000000000..d1ad163287 --- /dev/null +++ b/packages/platform-apple/src/runner/__tests__/runner-session-response.test.ts @@ -0,0 +1,335 @@ +import { AppError } from '@agent-device/kernel/errors'; +import { onTestFinished, test } from 'vitest'; +import assert from 'node:assert/strict'; +import fs from 'node:fs'; +import path from 'node:path'; +import type { DiagnosticEventInput } from '@agent-device/host-kit/diagnostics'; +import { mkdtempForTestSync } from './tmp-dir.ts'; +import { appleRunnerTestHost } from '../test-host.ts'; +import { isRetryableRunnerError } from '../runner-contract.ts'; +import type { RunnerLogAttempt } from '../runner-failure-diagnostics.ts'; +import { parseRunnerResponse } from '../runner-session.ts'; + +test('parseRunnerResponse preserves runner unsupported-operation codes', async () => { + const response = new Response( + JSON.stringify({ + ok: false, + error: { + code: 'UNSUPPORTED_OPERATION', + message: 'Unable to dismiss the iOS keyboard without a safe native dismiss control', + }, + }), + ); + const session = { state: 'starting' } as const; + + await assert.rejects( + () => parseRunnerResponse(response, session, runnerLogAttempt('/tmp/runner.log')), + (error: unknown) => { + assert.ok(error instanceof AppError); + assert.equal(error.code, 'UNSUPPORTED_OPERATION'); + assert.match(error.message, /Unable to dismiss the iOS keyboard/i); + return true; + }, + ); +}); + +test('parseRunnerResponse surfaces the keyboard-dismiss hint naming the occlusion reason', async () => { + const hint = + "An element whose center sits behind the on-screen keyboard is refused with tap_keyboard_occludes_target; one whose center stays above the keys presses normally. To end editing, tap the app's own Done/Cancel control, or use keyboard enter to press the return key when submission is wanted."; + const response = new Response( + JSON.stringify({ + ok: false, + error: { + code: 'UNSUPPORTED_OPERATION', + message: 'Unable to dismiss the iOS keyboard without a safe native dismiss control', + hint, + }, + }), + ); + const session = { state: 'starting' } as const; + + await assert.rejects( + () => parseRunnerResponse(response, session, runnerLogAttempt('/tmp/runner.log')), + (error: unknown) => { + assert.ok(error instanceof AppError); + assert.equal(error.code, 'UNSUPPORTED_OPERATION'); + assert.equal(error.details?.hint, hint); + assert.match(String(error.details?.hint), /tap_keyboard_occludes_target/); + assert.match(String(error.details?.hint), /center stays above the keys/i); + assert.match(String(error.details?.hint), /keyboard enter/i); + return true; + }, + ); +}); + +test('parseRunnerResponse preserves iOS AX snapshot failure code and hint', async () => { + const hint = + 'Try a smaller read such as snapshot -s -d 8, or use direct selector commands such as find id click.'; + const response = new Response( + JSON.stringify({ + ok: false, + error: { + code: 'IOS_AX_SNAPSHOT_FAILED', + message: 'iOS XCTest snapshot failed with kAXErrorIllegalArgument.', + hint, + }, + }), + ); + const session = { state: 'ready' } as const; + + await assert.rejects( + () => parseRunnerResponse(response, session, runnerLogAttempt('/tmp/runner.log')), + (error: unknown) => { + assert.ok(error instanceof AppError); + assert.equal(error.code, 'IOS_AX_SNAPSHOT_FAILED'); + assert.match(error.message, /kAXErrorIllegalArgument/); + assert.equal(error.details?.hint, hint); + assert.equal(isRetryableRunnerError(error), false); + return true; + }, + ); +}); + +test('parseRunnerResponse preserves XCTest recorded failure code and hint', async () => { + const hint = + 'The iOS runner session was invalidated. Re-observe with a fresh snapshot before retrying; if the accessibility tree is unavailable, use screenshot plus coordinate commands instead of retrying the tap blindly.'; + const response = new Response( + JSON.stringify({ + ok: false, + error: { + code: 'XCTEST_RECORDED_FAILURE', + message: + 'XCTest recorded a failure while executing tap; the action may not have been performed.', + hint, + }, + }), + ); + const session = { state: 'ready' } as const; + + await assert.rejects( + () => parseRunnerResponse(response, session, runnerLogAttempt('/tmp/runner.log')), + (error: unknown) => { + assert.ok(error instanceof AppError); + assert.equal(error.code, 'XCTEST_RECORDED_FAILURE'); + assert.match(error.message, /may not have been performed/); + assert.equal(error.details?.hint, hint); + assert.equal(isRetryableRunnerError(error), false); + return true; + }, + ); +}); + +test('parseRunnerResponse maps RUNNER_BUSY to retriable command failure', async () => { + const hint = 'Wait a few seconds and retry.'; + const response = new Response( + JSON.stringify({ + ok: false, + error: { + code: 'RUNNER_BUSY', + message: 'The runner is still finishing abandoned work.', + hint, + }, + }), + ); + const session = { state: 'ready' } as const; + + await assert.rejects( + () => parseRunnerResponse(response, session, runnerLogAttempt('/tmp/runner.log')), + (error: unknown) => { + assert.ok(error instanceof AppError); + assert.equal(error.code, 'COMMAND_FAILED'); + assert.equal(error.details?.runnerErrorCode, 'RUNNER_BUSY'); + assert.equal(error.details?.retriable, true); + assert.equal(error.details?.hint, hint); + assert.equal(isRetryableRunnerError(error), true); + return true; + }, + ); +}); + +test('parseRunnerResponse preserves RUNNER_WEDGED as a fatal runner code', async () => { + const response = new Response( + JSON.stringify({ + ok: false, + error: { + code: 'RUNNER_WEDGED', + message: 'The runner main thread is wedged.', + hint: 'The runner session will be restarted.', + }, + }), + ); + const session = { state: 'ready' } as const; + + await assert.rejects( + () => parseRunnerResponse(response, session, runnerLogAttempt('/tmp/runner.log')), + (error: unknown) => { + assert.ok(error instanceof AppError); + assert.equal(error.code, 'RUNNER_WEDGED'); + assert.equal(error.details?.runnerErrorCode, 'RUNNER_WEDGED'); + assert.equal(isRetryableRunnerError(error), false); + return true; + }, + ); +}); + +test('parseRunnerResponse classifies target app AXRuntime CoreText font crashes from runner log tail', async () => { + const logPath = writeRunnerLogTail(` +Thread 0 Crashed:: Dispatch queue: com.apple.main-thread +0 libobjc.A.dylib objc_retain + 16 +1 CoreText CreateFontWithFontURL(__CFURL const*, __CFString const*, __CFString const*) + 512 +11 AXRuntime reconstitutedSmuggledCTFontFromDictionary + 192 +12 AXRuntime -[NSDictionary(AXPropertyListCoersion) _axRecursivelyReconstitutedRepresentationFromPropertyListWithError:] + 156 +`); + const response = new Response( + JSON.stringify({ + ok: false, + error: { + code: 'XCTEST_RECORDED_FAILURE', + message: + 'XCTest recorded a failure while executing type; the action may not have been performed.', + }, + }), + ); + const session = { state: 'ready' } as const; + + await assert.rejects( + () => parseRunnerResponse(response, session, runnerLogAttempt(logPath)), + (error: unknown) => { + assert.ok(error instanceof AppError); + assert.equal(error.code, 'IOS_TARGET_APP_CRASH'); + assert.equal(error.details?.runnerFailureReason, 'target_app_axruntime_coretext_crash'); + assert.match(String(error.details?.hint), /AXRuntime read accessibility attributes/); + assert.match(String(error.details?.hint), /latest stable simulator runtime/); + assert.match(String(error.details?.hint), /exact command, selector\/ref/); + return true; + }, + ); +}); + +test('parseRunnerResponse classifies explicit target app crashes from runner log tail', async () => { + const logPath = writeRunnerLogTail(` +AGENT_DEVICE_RUNNER_COMMAND_FAILED command=snapshot +The application under test terminated unexpectedly. +`); + const response = new Response( + JSON.stringify({ + ok: false, + error: { + code: 'COMMAND_FAILED', + message: 'Runner error', + }, + }), + ); + const session = { state: 'ready' } as const; + + await assert.rejects( + () => parseRunnerResponse(response, session, runnerLogAttempt(logPath)), + (error: unknown) => { + assert.ok(error instanceof AppError); + assert.equal(error.code, 'IOS_TARGET_APP_CRASH'); + assert.equal(error.details?.runnerFailureReason, 'target_app_crash'); + assert.match(String(error.details?.hint), /target iOS app appears to have crashed/); + assert.equal(isRetryableRunnerError(error), false); + return true; + }, + ); +}); + +test('parseRunnerResponse does not classify incidental XCTest crash text as target app crash', async () => { + const logPath = writeRunnerLogTail(` +XCTest runner recovered from a previous test note: the word crashed appeared in debug output. +AGENT_DEVICE_RUNNER_COMMAND_FAILED command=snapshot error=fetch failed +`); + const response = new Response( + JSON.stringify({ + ok: false, + error: { + code: 'COMMAND_FAILED', + message: 'fetch failed', + }, + }), + ); + const session = { state: 'ready' } as const; + + await assert.rejects( + () => parseRunnerResponse(response, session, runnerLogAttempt(logPath)), + (error: unknown) => { + assert.ok(error instanceof AppError); + assert.equal(error.code, 'COMMAND_FAILED'); + assert.equal(error.details?.runnerFailureReason, undefined); + assert.equal(error.details?.hint, undefined); + assert.equal(isRetryableRunnerError(error), true); + return true; + }, + ); +}); + +test('parseRunnerResponse hints when XCTest main-thread execution times out', async () => { + const logPath = writeRunnerLogTail( + 'AGENT_DEVICE_RUNNER_COMMAND_FAILED command=type error=main thread execution timed out', + ); + const response = new Response( + JSON.stringify({ + ok: false, + error: { + code: 'COMMAND_FAILED', + message: 'main thread execution timed out', + }, + }), + ); + const session = { state: 'ready' } as const; + + await assert.rejects( + () => parseRunnerResponse(response, session, runnerLogAttempt(logPath)), + (error: unknown) => { + assert.ok(error instanceof AppError); + assert.equal(error.code, 'COMMAND_FAILED'); + assert.equal(error.details?.runnerFailureReason, 'runner_main_thread_execution_timeout'); + assert.match(String(error.details?.hint), /XCTest timed out waiting for main-thread work/); + assert.match(String(error.details?.hint), /screenshot as visual truth/); + assert.match(String(error.details?.hint), /coordinate presses/); + return true; + }, + ); +}); + +test('parseRunnerResponse emits diagnostics for runner gesture fallbacks', async () => { + const response = new Response( + JSON.stringify({ + ok: true, + data: { + message: 'dragged', + gestureFallback: 'xctest-coordinate-drag', + gestureFallbackMessage: 'Runner synthesized drag is unavailable', + gestureFallbackHint: 'Using XCTest coordinate drag fallback.', + }, + }), + ); + const session = { state: 'starting' } as const; + const diagnosticEvents: DiagnosticEventInput[] = []; + appleRunnerTestHost.update({ emitDiagnostic: (event) => diagnosticEvents.push(event) }); + + const data = await parseRunnerResponse(response, session, runnerLogAttempt('/tmp/runner.log')); + assert.equal(data.gestureFallback, 'xctest-coordinate-drag'); + + assert.equal(session.state, 'ready'); + const diagnostics = JSON.stringify(diagnosticEvents); + assert.match(diagnostics, /ios_runner_gesture_fallback/); + assert.match(diagnostics, /xctest-coordinate-drag/); +}); + +/** + * A log attempt over a log this test just created. Offset 0 is the honest boundary there: the file + * holds nothing but this command's bytes, so everything in it may be read as this attempt's evidence. + */ +function runnerLogAttempt(logPath: string): RunnerLogAttempt { + return { logPath, byteOffset: 0 }; +} + +function writeRunnerLogTail(contents: string): string { + const dir = mkdtempForTestSync('agent-device-runner-log-'); + onTestFinished(() => fs.rmSync(dir, { recursive: true, force: true })); + const logPath = path.join(dir, 'runner.log'); + fs.writeFileSync(logPath, contents); + return logPath; +} diff --git a/packages/platform-apple/src/runner/__tests__/runner-startup-failure-fixtures.ts b/packages/platform-apple/src/runner/__tests__/runner-startup-failure-fixtures.ts index b9685bed7c..b351184b7f 100644 --- a/packages/platform-apple/src/runner/__tests__/runner-startup-failure-fixtures.ts +++ b/packages/platform-apple/src/runner/__tests__/runner-startup-failure-fixtures.ts @@ -1,5 +1,10 @@ import { AppError } from '@agent-device/kernel/errors'; -import type { RunnerStartupFailureReason } from '../runner-contract.ts'; +import type { + RunnerDeviceReadinessFailureReason, + RunnerStartupFailureReason, +} from '../runner-contract.ts'; +import { RUNNER_DEVICE_READINESS_FAILURE_REASONS } from '../runner-contract.ts'; +import type { IosPhysicalDeviceRunnerControl } from '../../core/physical-device-routing.ts'; /** * Recorded startup failures for {@link classifyRunnerStartupFailure} (#2680). @@ -35,14 +40,38 @@ import type { RunnerStartupFailureReason } from '../runner-contract.ts'; * read as waiting on effort this machine can supply. */ -export type RunnerStartupFailureSite = 'build-for-testing' | 'host-dev-tools-security'; +export type RunnerStartupFailureSite = + | 'build-for-testing' + | 'host-dev-tools-security' + | 'device-readiness'; + +/** + * The two states a device reports about itself (#2683), in the shape `readIosDeviceReadiness` + * publishes them. They are recorded as states rather than as payload text because the states are the + * evidence: the payload they came from is captured in + * `packages/platform-apple/src/core/__tests__/fixtures/ios-device-info-details.json`. + */ +/** + * The two states a device payload carries. `remedies` is left out on purpose: that wording is ours and + * arrives on the report, so a fixture that recorded it would be recording our own advice as if the + * phone had said it. + */ +export type IosDeviceReadinessReport = Omit< + Extract< + Awaited>, + { + available: true; + } + >, + 'available' | 'remedies' +>; /** * Whether the text reaches the build catch inside the exec error's `details` (`exec-details`, which * is how a non-zero `xcodebuild` arrives) or only in the thrown message (`message-only`, which is * how anything the exec layer raised as a plain `Error` arrives after the catch wraps `String(err)`). */ -export type RunnerStartupFailureCarrier = 'exec-details' | 'message-only'; +export type RunnerStartupFailureCarrier = 'exec-details' | 'message-only' | 'host-timeout'; const UNOBSERVED = 'unobserved'; @@ -54,6 +83,8 @@ export type RunnerStartupFailureFixture = Readonly<{ /** Which throw site receives this output. */ site: RunnerStartupFailureSite; carrier?: RunnerStartupFailureCarrier; + /** The deadline the host killed this command at, for the `host-timeout` carrier. */ + hostTimeoutMs?: number; /** The invocation that produced {@link RunnerStartupFailureFixture.output}, once one is recorded. */ command?: string; /** `xcodebuild -version` recorded from that run, or `unobserved`. */ @@ -63,10 +94,20 @@ export type RunnerStartupFailureFixture = Readonly<{ output: string; /** The argv the exec reported, which is never evidence of a cause (#2680). */ args?: readonly string[]; + /** + * The device's own states. On the `device-readiness` site this is the evidence the preflight reads; + * on a `build-for-testing` entry it is what the startup carried onto that build, which is the pairing + * the corroborated disk-image reason depends on (#2683). + */ + deviceReport?: IosDeviceReadinessReport; /** What the pending capture still has to show, and how to reach it. */ note?: string; }>; +/** The one command the `device-readiness` site runs, spelled out by `readIosDeviceReadiness`. */ +const DEVICE_INFO_DETAILS_COMMAND = + 'xcrun devicectl device info details --device --json-output --timeout 10'; + export const RUNNER_STARTUP_FAILURE_FIXTURES: readonly RunnerStartupFailureFixture[] = [ { id: 'bundle-id-registration-failed', @@ -229,6 +270,74 @@ export const RUNNER_STARTUP_FAILURE_FIXTURES: readonly RunnerStartupFailureFixtu "note: Using provisioning profile \"match-development\" to sign the app bundle (in target 'AgentDeviceRunner' from project 'AgentDeviceRunner')\nwarning: The certificate \"Apple Development: Example Dev (ABCD1234)\" has expired.\nerror: cannot find 'AgentDeviceRunnerCommand' in scope (in target 'AgentDeviceRunnerUITests' from project 'AgentDeviceRunner')\n** TEST BUILD FAILED **\n", note: 'The cross-line hazard a whole-log AND cannot see (#2688 review): a benign profile note three lines above an unrelated expired-certificate warning. Both phrases are in the captured log and neither qualifies the other, so the profile stays unclassified and the reader keeps cache-recovery advice rather than being sent to replace a profile that is fine.', }, + { + id: 'unclassified-build-on-device-with-image-down', + reason: 'device_developer_disk_image_unavailable', + site: 'build-for-testing', + xcodeVersion: UNOBSERVED, + provenance: 'invented-shape', + output: + "error: cannot find 'AgentDeviceRunnerCommand' in scope (in target 'AgentDeviceRunnerUITests' from project 'AgentDeviceRunner')\n** TEST BUILD FAILED **\n", + deviceReport: { developerMode: 'enabled', developerDiskImage: 'unavailable' }, + note: 'The corroborated pairing (#2683 review): a build that names no cause, on a phone core read directly as reporting its image down. Naming the image beats cache-recovery advice; the state also travels as details.developerDiskImage.', + }, + { + id: 'host-killed-build-on-device-with-image-down', + reason: 'build_failed_unclassified', + site: 'build-for-testing', + carrier: 'host-timeout', + hostTimeoutMs: 900_000, + xcodeVersion: UNOBSERVED, + provenance: 'invented-shape', + output: + "note: Using target 'AgentDeviceRunner' for build-for-testing\nbuilding project 'AgentDeviceRunner' toward destination 'Example iPhone'\nCompileSwiftFile normal (in target 'AgentDeviceRunner' from project 'AgentDeviceRunner')\n", + deviceReport: { developerMode: 'enabled', developerDiskImage: 'unavailable' }, + note: "The build the host killed at its own `buildTimeoutMs`, on a phone reporting its image down (#2690 review). A slow build and a build the device refuses are different facts, and the second one is not available from a command that never finished: the reason stays unclassified with cache-recovery advice, and the image state rides along as a detail only. The shape follows the exec layer's timeout error; the 15-minute budget and the partial log are ours, so no capture stands behind them.", + }, + { + id: 'conflicting-settings-on-device-with-image-down', + reason: 'build_failed_unclassified', + site: 'build-for-testing', + xcodeVersion: UNOBSERVED, + provenance: 'invented-shape', + output: + "error: \"AgentDeviceRunner\" has conflicting provisioning settings (in target 'AgentDeviceRunner' from project 'AgentDeviceRunner')\n** TEST BUILD FAILED **\n", + deviceReport: { developerMode: 'enabled', developerDiskImage: 'unavailable' }, + note: 'The pairing that made the enrichment key on "did a row match" rather than on the unclassified reason (#2690 review): a just-rebooted phone reports its image down while the failure is a settings disagreement a row already looked at and declined to name. The row answer wins and the cache-recovery hint stays; the image state still rides along as a detail.', + }, + { + id: 'team-id-failure-on-device-with-image-down', + reason: 'signing_no_development_team', + site: 'build-for-testing', + xcodeVersion: UNOBSERVED, + provenance: 'shipped-sniff-trigger', + output: + "error: Signing for \"AgentDeviceRunner\" requires a development team (in target 'AgentDeviceRunner' from project 'AgentDeviceRunner')\n** TEST BUILD FAILED **\n", + deviceReport: { developerMode: 'enabled', developerDiskImage: 'unavailable' }, + note: "A build that named its own cause keeps it: a corroborated device state never overwrites xcodebuild's own sentence (#2683).", + }, + { + id: 'device-mode-off', + reason: 'device_developer_mode_disabled', + site: 'device-readiness', + command: DEVICE_INFO_DETAILS_COMMAND, + xcodeVersion: UNOBSERVED, + provenance: 'invented-shape', + output: '"developerModeStatus" : "disabled",\n"ddiServicesAvailable" : false,\n', + deviceReport: { developerMode: 'disabled', developerDiskImage: 'unavailable' }, + note: 'Both states bad, which is what a phone with the toggle off looks like: the toggle has to be the reason named, since it explains the image. No device with the toggle off has been captured.', + }, + { + id: 'device-disk-image-down', + reason: 'device_developer_disk_image_unavailable', + site: 'device-readiness', + command: DEVICE_INFO_DETAILS_COMMAND, + xcodeVersion: UNOBSERVED, + provenance: 'invented-shape', + output: '"developerModeStatus" : "enabled",\n"ddiServicesAvailable" : false,\n', + deviceReport: { developerMode: 'enabled', developerDiskImage: 'unavailable' }, + note: 'The decisive pairing, and the one #2682 used to answer with Developer Mode advice: the toggle is on and only the image is down. It is NOT a pre-build refusal (#2683 review): iOS 17+ mounts the image on demand during build and launch, so this report has to survive to a failure — which is what `unclassified-build-on-device-with-image-down` records. The enabled half is captured on some devices; a device waiting on device support has not been captured.', + }, { id: 'devtools-security-disabled', reason: 'devtools_security_developer_mode_disabled', @@ -251,6 +360,28 @@ export function buildFixtureById(id: string): RunnerStartupFailureFixture { return fixture; } +/** A recorded device report, narrowed to the reasons the device can name about itself. */ +export type IosDeviceReadinessFixture = RunnerStartupFailureFixture & { + reason: RunnerDeviceReadinessFailureReason; + site: 'device-readiness'; + deviceReport: IosDeviceReadinessReport; +}; + +/** The recorded device reports, which the runner preflight reads instead of any tool's text. */ +export function deviceReadinessFixtures(): IosDeviceReadinessFixture[] { + return RUNNER_STARTUP_FAILURE_FIXTURES.filter(isDeviceReadinessFixture); +} + +function isDeviceReadinessFixture( + fixture: RunnerStartupFailureFixture, +): fixture is IosDeviceReadinessFixture { + return ( + fixture.site === 'device-readiness' && + fixture.deviceReport !== undefined && + (RUNNER_DEVICE_READINESS_FAILURE_REASONS as readonly string[]).includes(fixture.reason) + ); +} + /** * What the exec layer hands the build-failure catch: for `exec-details` a COMMAND_FAILED carrying * the tool's output and the argv in `details` (`execFailureDetails` shape), and for `message-only` @@ -263,6 +394,17 @@ export function buildForTestingExecFailure( if ((fixture.carrier ?? 'exec-details') === 'message-only') { return new Error(`xcodebuild exited with code ${exitCode}: ${fixture.output}`); } + if (fixture.carrier === 'host-timeout') { + // The exec layer's own kill-at-deadline error, which `isCommandTimeoutError` answers for. + const timeoutMs = fixture.hostTimeoutMs ?? 900_000; + return new AppError('COMMAND_FAILED', `xcodebuild timed out after ${timeoutMs}ms`, { + cmd: 'xcodebuild', + args: fixture.args ?? ['build-for-testing'], + stdout: fixture.output, + stderr: '', + timeoutMs, + }); + } return new AppError('COMMAND_FAILED', `xcodebuild exited with code ${exitCode}`, { stdout: fixture.output, stderr: '', diff --git a/packages/platform-apple/src/runner/__tests__/runner-startup-failure-reasons.test.ts b/packages/platform-apple/src/runner/__tests__/runner-startup-failure-reasons.test.ts index 08fb735483..e5f3cff58b 100644 --- a/packages/platform-apple/src/runner/__tests__/runner-startup-failure-reasons.test.ts +++ b/packages/platform-apple/src/runner/__tests__/runner-startup-failure-reasons.test.ts @@ -4,10 +4,16 @@ import path from 'node:path'; import { afterEach, beforeEach, test, vi } from 'vitest'; import { AppError, normalizeError, type NormalizedError } from '@agent-device/kernel/errors'; import { resetAllProcessMemosForTests } from '@agent-device/kernel/ttl-memo'; +import { + IOS_DEVICE_DEVELOPER_DISK_IMAGE_HINT, + IOS_DEVICE_DEVELOPER_MODE_OFF_HINT, +} from '../../core/devicectl.ts'; import { appleRunnerTestHost } from '../test-host.ts'; import type { ExecResult } from '@agent-device/host-kit/command'; import { createRunnerPhaseBudget, ensureXctestrunArtifact } from '../runner-xctestrun.ts'; import { + enrichRunnerStartupFailureWithDeviceStates, + RUNNER_DEVICE_READINESS_FAILURE_REASONS, RUNNER_ERROR_RULES, classifyRunnerStartupFailure, RUNNER_STARTUP_FAILURE_REASONS, @@ -42,13 +48,23 @@ import { mkdtempForTestSync } from './tmp-dir.ts'; const CACHE_RECOVERY_HINT = /clean:xcuitest|apple-runner\/derived/; -const HINT_FOR_REASON: Record = { - bundle_identifier_already_registered: /AGENT_DEVICE_IOS_BUNDLE_ID/, - signing_no_development_team: /AGENT_DEVICE_IOS_TEAM_ID/, - signing_provisioning_profile_missing: /AGENT_DEVICE_IOS_PROVISIONING_PROFILE/, - signing_unspecified: /Automatic Signing/, - devtools_security_developer_mode_disabled: /DevToolsSecurity -enable/, - build_failed_unclassified: CACHE_RECOVERY_HINT, +/** + * The phrase each reason's advice has to contain. Kept as text rather than as syntax because two of + * them are quotations from `core/devicectl.ts`, and a fifth escaping helper for a prose remedy with + * parentheses in it is not this suite's job. + */ +const HINT_FOR_REASON: Record = { + bundle_identifier_already_registered: 'AGENT_DEVICE_IOS_BUNDLE_ID', + signing_no_development_team: 'AGENT_DEVICE_IOS_TEAM_ID', + signing_provisioning_profile_missing: 'AGENT_DEVICE_IOS_PROVISIONING_PROFILE', + signing_unspecified: 'Automatic Signing', + devtools_security_developer_mode_disabled: 'DevToolsSecurity -enable', + // Both device remedies are owned by `core/devicectl.ts` and travel on the device report, so this + // table quotes them instead of restating them; `runner-device-readiness.test.ts` is where the + // preflight publishing them is asserted. + device_developer_mode_disabled: IOS_DEVICE_DEVELOPER_MODE_OFF_HINT, + device_developer_disk_image_unavailable: IOS_DEVICE_DEVELOPER_DISK_IMAGE_HINT, + build_failed_unclassified: 'clean:xcuitest', }; const runCmdSync = vi.fn(); @@ -92,10 +108,58 @@ afterEach(() => { for (const fixture of buildForTestingFixtures()) { test(`a build-for-testing failure publishes ${fixture.reason} for ${fixture.id}`, async () => { - assertFailureEnvelope(await driveBuildFailure(fixture), fixture); + const envelope = await driveBuildFailure(fixture); + + assertFailureEnvelope(envelope, fixture); + // The device's answer travels on the failure it explains, and on nothing else: a fixture with no + // recorded device report must not grow one (#2683). + assert.equal(envelope.details?.developerDiskImage, fixture.deviceReport?.developerDiskImage); }); } +/** The states of a phone whose developer disk image is down, as `preflightIosRunnerDeviceReadiness` reads them. */ +const DEVICE_WITH_IMAGE_DOWN = { + developerMode: 'enabled', + developerDiskImage: 'unavailable', + developerDiskImageHint: IOS_DEVICE_DEVELOPER_DISK_IMAGE_HINT, +} as const; + +test('a command the host killed names no device cause even without the threaded fact', () => { + // The install and launch steps fail with the exec's own timeout error, which no build catch has + // wrapped, so the deadline has to be read off the error itself (#2690 review). + const killed = new AppError('COMMAND_FAILED', 'xcodebuild timed out after 900000ms', { + cmd: 'xcodebuild', + timeoutMs: 900_000, + }); + + const enriched = enrichRunnerStartupFailureWithDeviceStates( + killed, + DEVICE_WITH_IMAGE_DOWN, + ) as AppError; + + assert.equal(enriched.details?.reason, undefined); + assert.equal(enriched.details?.hint, undefined); + assert.equal(enriched.details?.developerDiskImage, 'unavailable'); +}); + +test('the device speaking for a failure keeps the error that caused it', () => { + const caused = new AppError( + 'COMMAND_FAILED', + 'xcodebuild build-for-testing failed', + { reason: RUNNER_STARTUP_FAILURE_UNCLASSIFIED_REASON }, + new Error('xcodebuild was killed by the host'), + ); + + const enriched = enrichRunnerStartupFailureWithDeviceStates( + caused, + DEVICE_WITH_IMAGE_DOWN, + ) as AppError; + + assert.equal(enriched.details?.reason, 'device_developer_disk_image_unavailable'); + // The cause is what a reader of the daemon log follows to the command that actually died. + assert.equal(enriched.cause, caused.cause); +}); + /** * Every startup failure reaches a caller through one envelope: the typed reason in `details`, its hint * and the log path hoisted to top level by `normalizeError`, and the tool output still reachable @@ -109,7 +173,10 @@ function assertFailureEnvelope( assert.equal(envelope.code, 'COMMAND_FAILED'); assert.equal(envelope.message, 'xcodebuild build-for-testing failed'); assert.equal(envelope.details?.reason, fixture.reason); - assert.match(String(envelope.hint), HINT_FOR_REASON[fixture.reason]); + assert.ok( + String(envelope.hint).includes(HINT_FOR_REASON[fixture.reason]), + `the ${fixture.reason} hint must carry "${HINT_FOR_REASON[fixture.reason]}"`, + ); // No `logPath` was handed to `normalizeError`: the top-level value can only be the one the // build catch wrote into the error it throws. assert.equal(envelope.logPath, logPath); @@ -118,6 +185,10 @@ function assertFailureEnvelope( assert.equal(envelope.details?.hint, undefined); assert.equal(envelope.details?.logPath, undefined); assert.equal(envelope.details?.diagnosticId, undefined); + // Plumbing one catch leaves for the next, never for a caller: `reason` and `hint` already carry the + // verdict these facts produced (#2690 review). + assert.equal(envelope.details?.startupRuleMatched, undefined); + assert.equal(envelope.details?.startupHostDeadlineHit, undefined); assertToolOutputReachable(envelope, fixture); } @@ -131,7 +202,7 @@ function assertToolOutputReachable( envelope: NormalizedError, fixture: RunnerStartupFailureFixture, ): void { - if ((fixture.carrier ?? 'exec-details') !== 'exec-details') { + if ((fixture.carrier ?? 'exec-details') === 'message-only') { assert.equal(envelope.details?.details, undefined); return; } @@ -156,6 +227,10 @@ test('every reason the classifier can name is produced by a rule row', () => { for (const reason of RUNNER_STARTUP_FAILURE_REASONS) { // The catch-all is the classifier's own answer when no row matched, so it names no row. if (reason === RUNNER_STARTUP_FAILURE_UNCLASSIFIED_REASON) continue; + // The device-readiness members are named by the device's own states in + // `runner-device-readiness.ts`, not by a rule row: no amount of tool text establishes them, + // which is exactly why they are declared as a subset (#2683). + if ((RUNNER_DEVICE_READINESS_FAILURE_REASONS as readonly string[]).includes(reason)) continue; assert.ok(reasonsFromRules.has(reason), `no rule row yields the ${reason} reason`); } }); @@ -240,9 +315,27 @@ test('a conflicting-settings failure is not answered with missing-profile advice assert.doesNotMatch(String(envelope.hint), /AGENT_DEVICE_IOS_PROVISIONING_PROFILE/); }); -/** Drives a recorded fixture through the real build catch and normalizes what it threw. */ +/** + * Drives a recorded fixture through the two steps a real startup runs in order: the build catch turns + * the tool's output into a typed reason, and the session's startup catch hands that failure to the + * device enrichment step (#2690 review). Both are the production functions; nothing here re-implements + * either. + */ async function driveBuildFailure(fixture: RunnerStartupFailureFixture): Promise { - return normalizeThrown(await runBuildCatch(() => buildForTestingExecFailure(fixture))); + const thrown = await runBuildCatch(() => buildForTestingExecFailure(fixture)); + return normalizeThrown( + enrichRunnerStartupFailureWithDeviceStates(thrown, deviceStatesOf(fixture)), + ); +} + +/** The states `preflightIosRunnerDeviceReadiness` would have handed the startup for this fixture. */ +function deviceStatesOf(fixture: RunnerStartupFailureFixture | undefined) { + if (!fixture?.deviceReport) return undefined; + return { + developerMode: fixture.deviceReport.developerMode, + developerDiskImage: fixture.deviceReport.developerDiskImage, + developerDiskImageHint: IOS_DEVICE_DEVELOPER_DISK_IMAGE_HINT, + }; } /** Drives a hand-built rejection through the same real build catch. */ diff --git a/packages/platform-apple/src/runner/__tests__/runner-startup-transport.test.ts b/packages/platform-apple/src/runner/__tests__/runner-startup-transport.test.ts index 97ec4c455d..35afe0eb64 100644 --- a/packages/platform-apple/src/runner/__tests__/runner-startup-transport.test.ts +++ b/packages/platform-apple/src/runner/__tests__/runner-startup-transport.test.ts @@ -253,6 +253,42 @@ test('waitForRunner preserves xcodebuild diagnostics when the runner exits durin assert.equal(mockUsbmuxPostCommand.mock.calls.length, 1); }); +test('waitForRunner carries the disk-image state when the runner is still alive at the connect deadline (#2683)', async () => { + // The alive-child twin of the early exit: `xcodebuild` never exits, the runner never answers, and + // the failure a locked phone produces this way must still say what the phone reported. + const session: RunnerSession = { + sessionId: 'starting-device-session', + device: xctestIosDevice, + deviceId: xctestIosDevice.id, + port: 8100, + xctestrunPath: '/tmp/runner.xctestrun', + jsonPath: '/tmp/runner.json', + testPromise: new Promise(() => {}), + child: { pid: 1234, exitCode: null } as ExecBackgroundResult['child'], + state: 'starting', + startupDeviceStates: { + developerMode: 'enabled', + developerDiskImage: 'unavailable', + developerDiskImageHint: 'Unlock the iPhone so it can mount the developer disk image.', + }, + }; + mockUsbmuxPostCommand.mockRejectedValue(new Error('ECONNREFUSED')); + vi.stubGlobal('fetch', vi.fn().mockRejectedValue(new Error('ECONNREFUSED'))); + + await assert.rejects( + () => + waitForRunner(xctestIosDevice, 8100, { command: 'uptime' }, '/tmp/runner.log', 100, session), + (error: unknown) => { + const appError = error as AppError; + assert.equal(appError.message, 'Runner did not accept connection'); + assert.equal(appError.details?.developerDiskImage, 'unavailable'); + assert.equal(appError.details?.reason, 'IOS_RUNNER_CONNECT_TIMEOUT'); + assert.doesNotMatch(String(appError.details?.hint), /Unlock the iPhone/); + return true; + }, + ); +}); + test('waitForRunner reports the usbmux verdict for xctest devices without retrying', async () => { // Regression: an XCTest device has no tunnel, so retrying cannot attach a // cable. Before this was terminal, readiness preflight and read-only diff --git a/packages/platform-apple/src/runner/__tests__/runner-transport.test.ts b/packages/platform-apple/src/runner/__tests__/runner-transport.test.ts index a1a7d4d2d4..b6af37284e 100644 --- a/packages/platform-apple/src/runner/__tests__/runner-transport.test.ts +++ b/packages/platform-apple/src/runner/__tests__/runner-transport.test.ts @@ -74,6 +74,13 @@ function fakeResolveIosPhysicalDeviceControl(device: { fs.rmSync(jsonPath, { force: true }); } }, + // Device readiness is asserted in runner-device-readiness.test.ts; this suite only ever needs + // the transport route, and an unreadable device is the shape that keeps it out of the way. + readDeviceReadiness: async () => ({ + available: false as const, + reason: 'device_readiness_unreadable' as const, + hint: 'unreadable in this suite', + }), }; } diff --git a/packages/platform-apple/src/runner/host.ts b/packages/platform-apple/src/runner/host.ts index ef41da1ff1..ed6ba3fda7 100644 --- a/packages/platform-apple/src/runner/host.ts +++ b/packages/platform-apple/src/runner/host.ts @@ -105,6 +105,17 @@ export type AppleRunnerHost = Pick< /** The runner's deadline type is the host's read side; the {@link Deadline} shim below builds them. */ export type Deadline = HostRetry.DeadlineClock; +/** + * What an iPhone reports about its own fitness to host development tooling (#2683), under the name the + * module that reads it owns. Re-exported rather than restated or re-derived, so the runner, its tests, + * and the core reader all speak one type for one device report. + */ +export type { + IosDeveloperDiskImageState, + IosDeveloperModeState, + IosDeviceReadiness, +} from '../core/physical-device-coredevice.ts'; + let boundHost: AppleRunnerHost | undefined; /** diff --git a/packages/platform-apple/src/runner/runner-artifact.ts b/packages/platform-apple/src/runner/runner-artifact.ts index ffd0f38e0b..8f332b69fd 100644 --- a/packages/platform-apple/src/runner/runner-artifact.ts +++ b/packages/platform-apple/src/runner/runner-artifact.ts @@ -9,6 +9,7 @@ import { withProcessLock, emitRequestProgress, findProjectRoot, + isCommandTimeoutError, } from './host.ts'; import type { ExecBackgroundResult } from '@agent-device/host-kit/command'; import type { DeviceInfo } from '@agent-device/kernel/device'; @@ -519,13 +520,20 @@ async function buildRunnerXctestrun( error instanceof AppError ? error : new AppError('COMMAND_FAILED', String(error)); // The reason and the hint beside it come from one classifier (#2680), so the reason a caller // switches on can never disagree with the advice it is handed. - const { reason, hint } = classifyRunnerStartupFailure(appErr); + const { reason, hint, matched } = classifyRunnerStartupFailure(appErr); + const hostDeadlineHit = isCommandTimeoutError(appErr); + // `startupRuleMatched` travels with the verdict: this wrapper buries the tool's text a level too + // deep for the rows to read again, and whether a row spoke is not recoverable from the reason + // alone (#2690 review). The device's own state is attached further out, by the startup catch that + // can see this build and the launch after it. throw new AppError('COMMAND_FAILED', 'xcodebuild build-for-testing failed', { reason, error: appErr.message, details: appErr.details, logPath: options.logPath, hint, + startupRuleMatched: matched, + startupHostDeadlineHit: hostDeadlineHit, }); } }); diff --git a/packages/platform-apple/src/runner/runner-contract.ts b/packages/platform-apple/src/runner/runner-contract.ts index 367a56da23..2724c8ad01 100644 --- a/packages/platform-apple/src/runner/runner-contract.ts +++ b/packages/platform-apple/src/runner/runner-contract.ts @@ -14,10 +14,12 @@ import type { ClickButton } from '@agent-device/contracts/click-button'; import type { ElementSelectorKey } from '@agent-device/contracts/interactor-types'; import type { GesturePlan } from '@agent-device/contracts/gesture-plan-types'; import type { ScrollDirection } from '@agent-device/contracts/scroll-gesture'; +import type { IosDeveloperDiskImageState, IosDeveloperModeState } from './host.ts'; import type { ScrollReleaseBehavior } from '@agent-device/contracts/scroll-command'; import { getRequestSignal, isRequestCanceled, + isCommandTimeoutError, bootFailureHint, classifyBootFailure, } from './host.ts'; @@ -224,6 +226,24 @@ type RunnerErrorVerdicts = { artifactSuspect?: boolean; }; +/** + * The two device-readiness members (#2683): what the iPhone itself reports through + * `devicectl device info details`, not what another tool's output implies about it. They are listed + * apart because they are the members {@link classifyRunnerStartupFailure} does NOT produce — no + * xcodebuild or host-tool text establishes them, and the code that reads the device publishes them + * with the hint beside it. The two reach the caller at different moments, which is the whole + * asymmetry of #2683: a disabled Developer Mode toggle refuses the run up front, while an unavailable + * developer disk image is published onto a build that named no cause of its own, because iOS 17+ + * mounts that image on demand during build and launch. A connect-stage failure always claims a cause + * of its own (`IOS_RUNNER_CONNECT_TIMEOUT` unless a provisioning row matches first), so there the + * image state travels only as `details.developerDiskImage`, and + * `device_developer_disk_image_unavailable` is published only from the startup build catch. + */ +export const RUNNER_DEVICE_READINESS_FAILURE_REASONS = [ + 'device_developer_mode_disabled', + 'device_developer_disk_image_unavailable', +] as const; + /** * Why the Apple runner could not reach the point of serving a command (#2680). Published in * `details.reason` on the `COMMAND_FAILED` every one of these paths throws, so a caller branches @@ -247,11 +267,16 @@ export const RUNNER_STARTUP_FAILURE_REASONS = [ 'signing_provisioning_profile_missing', 'signing_unspecified', 'devtools_security_developer_mode_disabled', + ...RUNNER_DEVICE_READINESS_FAILURE_REASONS, 'build_failed_unclassified', ] as const; export type RunnerStartupFailureReason = (typeof RUNNER_STARTUP_FAILURE_REASONS)[number]; +/** The device-readiness subset, typed from the one list above. */ +export type RunnerDeviceReadinessFailureReason = + (typeof RUNNER_DEVICE_READINESS_FAILURE_REASONS)[number]; + /** * The reason a startup failure carries when no rule proves a cause. Its hint is deliberately the * cache-recovery advice rather than anything about signing: an unclassified build is not evidence of @@ -788,10 +813,11 @@ export function buildRunnerConnectError(params: { endpoints: string[]; logPath?: string; lastError: unknown; + deviceStates?: IosRunnerDeviceStates; }): AppError { - const { port, endpoints, logPath, lastError } = params; + const { port, endpoints, logPath, lastError, deviceStates } = params; const message = 'Runner did not accept connection'; - return new AppError('COMMAND_FAILED', message, { + const error = new AppError('COMMAND_FAILED', message, { port, endpoints, logPath, @@ -803,6 +829,9 @@ export function buildRunnerConnectError(params: { }), hint: bootFailureHint('IOS_RUNNER_CONNECT_TIMEOUT'), }); + // The other way the connect stage gives up: `xcodebuild` is still alive at the deadline. It gets + // the same enrichment as the early exit below (#2683). + return enrichRunnerStartupFailureWithDeviceStates(error, deviceStates) as AppError; } export async function buildRunnerEarlyExitError(params: { @@ -822,7 +851,7 @@ export async function buildRunnerEarlyExitError(params: { // exec-guard-allow: xcodebuild can exit 0 and still count as an early exit; // the trio is nested tool context under `xcodebuild`, classified into // `reason`/`hint` above — not a process-exit wrap. - return new AppError('COMMAND_FAILED', message, { + const error = new AppError('COMMAND_FAILED', message, { port, logPath, xcodebuild: { @@ -833,6 +862,11 @@ export async function buildRunnerEarlyExitError(params: { reason, hint: resolveRunnerEarlyExitHint(message, result.stdout, result.stderr, reason), }); + // The build catch is not the only way a runner stops before serving a command. A locked phone lets + // the build finish and kills `xcodebuild test-without-building` instead, so nothing reaches that + // catch and the disk-image state read before the build would be dropped. Same enrichment, applied + // to the failure this path actually produces (#2683). + return enrichRunnerStartupFailureWithDeviceStates(error, session.startupDeviceStates) as AppError; } /** @@ -844,25 +878,137 @@ export async function buildRunnerEarlyExitError(params: { * Callers publish the pair as `details.reason` plus the top-level hint on a `COMMAND_FAILED`; the * code is `COMMAND_FAILED` for every reason, so the reason is the assertion. */ -export function classifyRunnerStartupFailure(error: unknown): { - reason: RunnerStartupFailureReason; - hint: string; -} { +export function classifyRunnerStartupFailure(error: unknown): RunnerStartupClassification { if (error instanceof AppError) { for (const rule of RUNNER_ERROR_RULES) { const buildFailure = rule.buildFailure; if (!buildFailure) continue; if (matchesRunnerErrorRule(error, rule.match)) { - return { reason: buildFailure.reason, hint: buildFailure.hint }; + return { reason: buildFailure.reason, hint: buildFailure.hint, matched: true }; } } } return { reason: RUNNER_STARTUP_FAILURE_UNCLASSIFIED_REASON, hint: RUNNER_CACHE_RECOVERY_HINT, + matched: false, }; } +/** + * The verdict, the advice beside it, and whether a row reached either (#2690 review). `matched` is + * half the answer rather than an implementation detail: {@link RUNNER_STARTUP_FAILURE_UNCLASSIFIED_REASON} + * is also what a row that deliberately claims no cause publishes, so reading the reason alone cannot + * tell "nothing spoke" from "a row spoke and declined to name a cause". + */ +export type RunnerStartupClassification = Readonly<{ + reason: RunnerStartupFailureReason; + hint: string; + /** + * False only when no rule row matched at all. The catch that publishes this verdict carries it on + * `details.startupRuleMatched`, which is the half a caller cannot recover from the reason alone. + */ + matched: boolean; +}>; + +/** + * What the startup carries forward from the device so a later failure can say what the phone said + * (#2683). The one thing that stops a run up front is a disabled Developer Mode toggle, which no + * later step can change; everything else the device reports is only worth publishing beside the + * failure it explains. + */ +export type IosRunnerDeviceStates = Readonly<{ + developerMode: IosDeveloperModeState; + developerDiskImage: IosDeveloperDiskImageState; + /** The remedy for an unavailable image, worded by `core/devicectl.ts` and read, not rewritten. */ + developerDiskImageHint: string; +}>; + +/** + * The device's turn on a startup failure (#2683, #2690 review), applied by the session's startup + * catch so it reaches every path that stops a runner before it serves a command: a cold build, a warm + * derived cache that fails at install, or an external xctestrun that never launches. The phone's own + * state rides along as `details.developerDiskImage` on all of them, because it is a fact whoever is + * reading this failure wants. + * + * It becomes the *reason* only when the failure carries no reason of its own and no rule row matched. + * `devicectl` reports the image only while the tunnel is up and the phone is booted, so an + * unavailable reading that reached here is a fact about the device rather than a snapshot of a sleeping + * phone — but a failure that already named a cause, or that a row looked at and declined to name one + * for, outranks a state that may have been cleared before the failure was written down. And a command + * the host killed at its own deadline says nothing about the device either: the build that never + * finished cannot have been refused for want of developer support, so a timeout outranks a state too. + * An error that is not an `AppError` comes back untouched: a cancellation and a foreign failure keep + * their identity. + */ +export function enrichRunnerStartupFailureWithDeviceStates( + error: unknown, + states: IosRunnerDeviceStates | undefined, +): unknown { + if (!states || !(error instanceof AppError)) return error; + const speaks = + claimedStartupFailureReason(error) === undefined && + states.developerDiskImage === 'unavailable' && + !startupFailureRuleMatched(error) && + !startupFailureHostDeadlineHit(error); + return new AppError( + error.code, + error.message, + { + ...(error.details ?? {}), + ...(speaks + ? { + reason: 'device_developer_disk_image_unavailable', + hint: states.developerDiskImageHint, + } + : {}), + developerDiskImage: states.developerDiskImage, + }, + error.cause, + ); +} + +/** + * The reason a startup failure already carries, discounting the placeholder the classifier publishes + * when nothing proved a cause. Without this discount the build catch's own + * `build_failed_unclassified` would read as a claimed cause and silence the device everywhere. + */ +function claimedStartupFailureReason(error: AppError): RunnerStartupFailureReason | undefined { + const reason = error.details?.reason; + if (typeof reason !== 'string' || reason === RUNNER_STARTUP_FAILURE_UNCLASSIFIED_REASON) { + return undefined; + } + return reason as RunnerStartupFailureReason; +} + +/** + * Whether the host's own execution deadline ended the command behind this failure. A build the host + * killed at `buildTimeoutMs` reaches the startup catch as `build_failed_unclassified` with nothing + * matched, and an unavailable image sitting on the device would then be named as the cause of a build + * that was simply too slow (#2690 review). A catch that published a classification carries the answer + * in `details.startupHostDeadlineHit` for the same reason it carries `startupRuleMatched`: its wrapper + * buries the tool error a level too deep to inspect. A failure that never passed through such a catch + * is read here, where the exec's own `timeoutMs` detail is still in reach. + */ +function startupFailureHostDeadlineHit(error: AppError): boolean { + const published = error.details?.startupHostDeadlineHit; + if (typeof published === 'boolean') return published; + return isCommandTimeoutError(error); +} + +/** + * Whether a rule row already reached this failure. A catch that published a classification carries its + * own answer in `details.startupRuleMatched`, because its wrapper keeps the tool's text one level too + * deep for the rows to read again — re-classifying the wrapper would report "nothing matched" for a + * failure whose cause a row had just declined to name (#2690 review). A failure that never passed + * through such a catch is classified here, which is the same answer its own publisher would have given. + */ +function startupFailureRuleMatched(error: AppError): boolean { + const published = error.details?.startupRuleMatched; + if (typeof published === 'boolean') return published; + return classifyRunnerStartupFailure(error).matched; +} + export function withRunnerCommandId(command: RunnerCommand): RunnerCommand { if (command.command === 'status') return command; if (command.commandId?.trim()) return command; diff --git a/packages/platform-apple/src/runner/runner-device-readiness.ts b/packages/platform-apple/src/runner/runner-device-readiness.ts new file mode 100644 index 0000000000..b887d243f6 --- /dev/null +++ b/packages/platform-apple/src/runner/runner-device-readiness.ts @@ -0,0 +1,77 @@ +import { AppError } from '@agent-device/kernel/errors'; +import { isIosFamily, type DeviceInfo } from '@agent-device/kernel/device'; +import { resolveIosPhysicalDeviceControl, type IosDeviceReadiness } from './host.ts'; +import { + type IosRunnerDeviceStates, + type RunnerDeviceReadinessFailureReason, +} from './runner-contract.ts'; + +const DEVICE_MODE_OFF_MESSAGE = 'The iOS device reports that Developer Mode is turned off'; + +/** + * The device half of "can this iPhone run the runner at all", asked before the runner builds. Returns + * the states worth carrying forward, and refuses only what no later step can clear. + * + * It refuses for exactly one state: Developer Mode off, which no build, install, or launch step can + * change and which `xcodebuild` reports only as a signing or install failure with no mention of the + * setting. The developer disk image is deliberately NOT a refusal: since iOS 17 CoreDevice mounts the + * personalized image on demand during build and launch, a phone that has just been rebooted reports + * `ddiServicesAvailable: false` while the very next build clears it, and refusing there would turn a + * self-clearing state into a failed run (#2683 review). Its state travels on the returned facts + * instead, and lands on whatever failure the build actually produces. + * + * A device that could not answer is left alone. `available: false` carries no verdict, and inventing + * one from a missing read is how a temporarily unplugged cable turns into a claim about someone's + * Settings (#2683). + */ +export async function preflightIosRunnerDeviceReadiness( + device: DeviceInfo, + budget: Readonly<{ budgetMs: number; signal?: AbortSignal }>, +): Promise { + if (!isIosFamily(device) || device.kind !== 'device') return undefined; + const readiness = await resolveIosPhysicalDeviceControl(device).readDeviceReadiness( + device, + budget.budgetMs, + budget.signal, + ); + // A read that returned just as the startup budget ran out is still not permission to keep going: + // the caller that cancelled is not waiting for a build that cannot be delivered (#2683). + budget.signal?.throwIfAborted(); + if (!readiness.available) return undefined; + const obstacle = namePreBuildDeviceObstacle(readiness); + if (obstacle) { + throw new AppError('COMMAND_FAILED', obstacle.message, { + reason: obstacle.reason, + hint: obstacle.hint, + deviceId: device.id, + developerMode: readiness.developerMode, + developerDiskImage: readiness.developerDiskImage, + }); + } + return { + developerMode: readiness.developerMode, + developerDiskImage: readiness.developerDiskImage, + developerDiskImageHint: readiness.remedies.developerDiskImageUnavailable, + }; +} + +/** The device report once it is known to have arrived, which is the only shape with states to weigh. */ +type ReadableIosDeviceReadiness = Extract; + +/** + * The one device state that stops a run before the build: the owner's Developer Mode toggle, which + * no later step turns on and which no build log names. A disabled toggle also explains an + * unavailable developer disk image, so naming it leaves the reader one thing to fix (#2683). + */ +function namePreBuildDeviceObstacle( + readiness: ReadableIosDeviceReadiness, +): + | Readonly<{ reason: RunnerDeviceReadinessFailureReason; message: string; hint: string }> + | undefined { + if (readiness.developerMode !== 'disabled') return undefined; + return { + reason: 'device_developer_mode_disabled', + message: DEVICE_MODE_OFF_MESSAGE, + hint: readiness.remedies.developerModeOff, + }; +} diff --git a/packages/platform-apple/src/runner/runner-failure-diagnostics.ts b/packages/platform-apple/src/runner/runner-failure-diagnostics.ts index 1369cac279..324635fc2d 100644 --- a/packages/platform-apple/src/runner/runner-failure-diagnostics.ts +++ b/packages/platform-apple/src/runner/runner-failure-diagnostics.ts @@ -1,6 +1,7 @@ import fs from 'node:fs/promises'; import type { FileHandle } from 'node:fs/promises'; import { AppError, type AppErrorCode } from '@agent-device/kernel/errors'; +import { flushRunnerLogAppends } from './runner-io.ts'; const RUNNER_LOG_TAIL_BYTES = 64 * 1024; @@ -19,12 +20,69 @@ const IOS_TARGET_APP_CRASH_HINT = const IOS_RUNNER_MAIN_THREAD_TIMEOUT_HINT = 'XCTest timed out waiting for main-thread work on the current iOS screen. The app may still be visually responsive, especially on focused React Native overlays or animating screens. Use screenshot as visual truth, use coordinate presses only to prove or leave the state, and retry snapshot -i after the UI settles or after navigating away.'; +/** + * The one thing a failing command needs to charge log evidence to its own attempt (#2683): which + * `runner.log`, and where that file had reached before anything was sent. The runner writes one log + * per device and never truncates it between commands, so everything an earlier command produced is + * still there when a later one fails, and reading the tail without this boundary blames an older + * command's crash on the command that merely happened to fail next. There is deliberately no + * log-path-only shape: an attempt that skipped the boundary would read the whole file again. + */ +export type RunnerLogAttempt = Readonly<{ logPath: string; byteOffset: number }>; + +/** + * How long drawing the boundary may take. It precedes a command that carries its own timeout, so the + * wait for the log writer is capped far below it rather than inheriting it (#2683 review). + */ +const RUNNER_LOG_FLUSH_BOUND_MS = 2_000; + +/** + * Draws the boundary for one command: everything `runner.log` holds when this returns belongs to an + * earlier command, and {@link enrichRunnerFailureFromLog} reads only what comes after it. Any append + * still queued for that path is awaited first, so an earlier command cannot write its way into this + * one's evidence (#2683). + * + * A log that does not exist yet is reported as `byteOffset: 0` rather than skipped: every byte it + * gets from here on belongs to this command, which is exactly the claim worth keeping. + * + * A writer that will not drain, or that reports the disk refused an append, yields no marker at all. + * That is the safe direction: with no boundary {@link enrichRunnerFailureFromLog} declines to read the + * tail, rather than crediting this command with bytes whose owner is unknown (#2683 review). + */ +export async function captureRunnerLogAttempt( + logPath: string | undefined, + budget: Readonly<{ timeoutMs?: number; signal?: AbortSignal }> = {}, +): Promise { + if (!logPath) return undefined; + // The writer serialises appends on a promise chain, so an earlier command's crash can still be in + // flight and land past whatever size `fs.stat` reports right now. Measuring without draining it + // first is what this whole marker exists to prevent (#2683). Draining is a prelude to sending a + // command, so it is bounded below the command's own clock instead of spending it (#2683 review). + try { + await flushRunnerLogAppends(logPath, { + timeoutMs: Math.min(RUNNER_LOG_FLUSH_BOUND_MS, budget.timeoutMs ?? RUNNER_LOG_FLUSH_BOUND_MS), + signal: budget.signal, + }); + } catch { + return undefined; + } + try { + return { logPath, byteOffset: (await fs.stat(logPath)).size }; + } catch { + return { logPath, byteOffset: 0 }; + } +} + export async function enrichRunnerFailureFromLog(params: { error: AppError; - logPath?: string; + /** + * The failing command's own log boundary. Without one there is no way to tell whose bytes are in + * the tail, so the tail is not read at all and the message keeps whatever the response said (#2683). + */ + logSince?: RunnerLogAttempt; }): Promise { const diagnostic = - (await resolveRunnerFailureDiagnostic(params.logPath)) ?? + (await resolveRunnerFailureDiagnostic(params.logSince)) ?? classifyRunnerFailureError(params.error); if (!diagnostic) return params.error; @@ -44,10 +102,10 @@ export async function enrichRunnerFailureFromLog(params: { } async function resolveRunnerFailureDiagnostic( - logPath: string | undefined, + logSince: RunnerLogAttempt | undefined, ): Promise { - if (!logPath) return undefined; - const tail = await readFileTail(logPath, RUNNER_LOG_TAIL_BYTES); + if (!logSince) return undefined; + const tail = await readFileSince(logSince, RUNNER_LOG_TAIL_BYTES); if (!tail) return undefined; return classifyRunnerFailureLog(tail); } @@ -103,15 +161,21 @@ function isMainThreadExecutionTimeout(message: string): boolean { return message.toLowerCase().includes('main thread execution timed out'); } -async function readFileTail(filePath: string, maxBytes: number): Promise { +async function readFileSince( + logSince: RunnerLogAttempt, + maxBytes: number, +): Promise { let handle: FileHandle | undefined; try { - const stat = await fs.stat(filePath); - const start = Math.max(0, stat.size - maxBytes); + const stat = await fs.stat(logSince.logPath); + // Never reads before the marker, and never reads more than the tail budget of what came after + // it. A log that is shorter than the marker has been replaced underneath us, which is not + // evidence about this command. + const start = Math.max(logSince.byteOffset, stat.size - maxBytes); const length = stat.size - start; if (length <= 0) return undefined; - handle = await fs.open(filePath, 'r'); + handle = await fs.open(logSince.logPath, 'r'); const buffer = Buffer.alloc(length); await handle.read(buffer, 0, length, start); return buffer.toString('utf8'); diff --git a/packages/platform-apple/src/runner/runner-io.ts b/packages/platform-apple/src/runner/runner-io.ts index a4e3798eae..03cd303067 100644 --- a/packages/platform-apple/src/runner/runner-io.ts +++ b/packages/platform-apple/src/runner/runner-io.ts @@ -34,16 +34,35 @@ export function logChunk( const logAppendQueues = new Map>(); +/** + * The append failure each log path is carrying, if any, cleared by the next append that succeeded. A + * lost write is the reason a byte offset stops being a boundary, and the queue outlives it. + */ +const logAppendLosses = new Map(); + function appendLogChunk(logPath: string, chunk: string): void { const previous = logAppendQueues.get(logPath) ?? Promise.resolve(); - const next = previous - .catch(() => {}) - .then(async () => { - await fs.promises.mkdir(path.dirname(logPath), { recursive: true }); - await fs.promises.appendFile(logPath, chunk); - }) - .catch(() => {}); - const queued = next.finally(() => { + // A failed append is kept on the chain instead of being swallowed: whoever waits for these bytes has + // to learn the disk refused them, because an offset measured over bytes that never landed would + // credit the next command with output it did not produce (#2683 review). The failure does not stop + // the queue — later output is still worth recording — and the no-op handler below keeps an append + // nobody waited for from becoming an unhandled rejection. + const written = previous.then( + () => writeChunk(logPath, chunk), + () => writeChunk(logPath, chunk), + ); + // The failure is recorded rather than dropped, and the queue keeps going: later output is still worth + // writing, while everything measured over a lost write is untrustworthy until an append succeeds + // again (#2683 review). + const accounted = written.then( + () => { + logAppendLosses.delete(logPath); + }, + (error: unknown) => { + logAppendLosses.set(logPath, error); + }, + ); + const queued = accounted.finally(() => { if (logAppendQueues.get(logPath) === queued) { logAppendQueues.delete(logPath); } @@ -51,6 +70,56 @@ function appendLogChunk(logPath: string, chunk: string): void { logAppendQueues.set(logPath, queued); } +async function writeChunk(logPath: string, chunk: string): Promise { + await fs.promises.mkdir(path.dirname(logPath), { recursive: true }); + await fs.promises.appendFile(logPath, chunk); +} + +/** How long a log flush may take before whoever asked gives up on the tail. */ +const RUNNER_LOG_FLUSH_TIMEOUT_MS = 2_000; + +/** + * Waits for the appends already queued for `logPath` to reach disk. `appendLogChunk` serialises + * writes on a promise chain, so bytes an earlier command produced can still be in flight when a + * later command marks the end of the log; measuring without this would hand those bytes to the + * command that did not write them (#2683). + * + * Bounded, and honest about both ways it can fail to finish (#2683 review): a wedged append or a + * caller that stopped waiting rejects rather than hanging the caller, and an append the disk refused + * rejects too. Callers that are measuring a log boundary for diagnostics treat any rejection as "this + * tail is unmeasurable" rather than as a clean offset. + */ +export async function flushRunnerLogAppends( + logPath: string, + budget: Readonly<{ timeoutMs?: number; signal?: AbortSignal }> = {}, +): Promise { + const signal = AbortSignal.any([ + budget.signal ?? new AbortController().signal, + AbortSignal.timeout(budget.timeoutMs ?? RUNNER_LOG_FLUSH_TIMEOUT_MS), + ]); + if (signal.aborted) throw signal.reason; + + const pending = logAppendQueues.get(logPath); + if (pending) { + let onAbort: () => void = () => {}; + try { + await Promise.race([ + pending, + new Promise((_resolve, reject) => { + onAbort = () => reject(signal.reason); + signal.addEventListener('abort', onAbort, { once: true }); + }), + ]); + } finally { + signal.removeEventListener('abort', onAbort); + } + } + // Checked whether or not anything was queued, so an append that failed and was forgotten by the queue + // still reaches whoever is about to measure this file. + const lost = logAppendLosses.get(logPath); + if (lost !== undefined) throw lost; +} + export function cleanupTempFile(filePath: string): void { try { if (fs.existsSync(filePath)) fs.unlinkSync(filePath); diff --git a/packages/platform-apple/src/runner/runner-session-types.ts b/packages/platform-apple/src/runner/runner-session-types.ts index 23dfddc6a0..68a05e52b9 100644 --- a/packages/platform-apple/src/runner/runner-session-types.ts +++ b/packages/platform-apple/src/runner/runner-session-types.ts @@ -4,6 +4,7 @@ import type { DeviceInfo } from '@agent-device/kernel/device'; import type { RunnerXctestrunArtifact } from './runner-xctestrun.ts'; import type { RunnerLease } from './runner-lease.ts'; import type { XcodebuildSimulatorSetRedirectHandle } from './runner-device-set.ts'; +import type { IosRunnerDeviceStates } from './runner-contract.ts'; /** * Where one runner process stands in the lifecycle of the session that owns it (#2662). The state @@ -86,6 +87,12 @@ export type RunnerSession = { speculative?: boolean; startupTimings?: Record; startupTimingsReported?: boolean; + /** + * Device-readiness facts the pre-build probe read for this startup. An adopted session has none: + * it skipped the probe. Carried so a failure raised after the build still reports the disk image + * state the device was in, which is the only way a locked phone's early exit says why (#2683). + */ + startupDeviceStates?: IosRunnerDeviceStates; logicalLeaseContext?: RunnerLogicalLeaseContext; simulatorSetRedirect?: XcodebuildSimulatorSetRedirectHandle; lease?: RunnerLease; diff --git a/packages/platform-apple/src/runner/runner-session.ts b/packages/platform-apple/src/runner/runner-session.ts index 2270d12e2a..4e1d3bf753 100644 --- a/packages/platform-apple/src/runner/runner-session.ts +++ b/packages/platform-apple/src/runner/runner-session.ts @@ -36,6 +36,7 @@ import { type RunnerCommand, resolveRunnerFatalErrorReason, isRunnerMainThreadOccupiedError, + enrichRunnerStartupFailureWithDeviceStates, } from './runner-contract.ts'; import { canSkipRunnerReadinessPreflightAfterHealthyMutation, @@ -62,7 +63,11 @@ import { stopRunnerPrepProcesses, type RunnerDisposalOptions, } from './runner-disposal.ts'; -import { enrichRunnerFailureFromLog } from './runner-failure-diagnostics.ts'; +import { + captureRunnerLogAttempt, + enrichRunnerFailureFromLog, + type RunnerLogAttempt, +} from './runner-failure-diagnostics.ts'; import { advanceRunnerSessionState, buildRunnerSessionId, @@ -145,6 +150,9 @@ export async function ensureRunnerSession( }); } +/** How long the device-readiness probe may take, bounded by the startup budget it runs inside. */ +const RUNNER_DEVICE_READINESS_BUDGET_MS = 10_000; + async function startRunnerSessionWithLease( device: DeviceInfo, options: RunnerSessionOptions, @@ -186,10 +194,28 @@ async function startRunnerSessionWithLease( await measureRunnerStartupStep(startupTimings, 'ensure_booted', async () => { await ensureBootedIfNeeded(device); }); + // Device first, host second: both answers can be wrong at once, and the phone's own state is the + // one the caller can act on without admin rights. Probing the host first would publish only the + // Mac's reason and hide the device's (#2683). + // Only a disabled Developer Mode toggle stops the run here; whatever else the device reports rides + // along onto the build below, because iOS 17+ mounts the developer disk image on demand during + // build and launch and refusing that state up front would refuse a state this build clears (#2683). + const deviceStates = await measureRunnerStartupStep( + startupTimings, + 'verify_device_readiness', + async () => + await ( + await import('./runner-device-readiness.ts') + ).preflightIosRunnerDeviceReadiness(device, { + budgetMs: Math.min( + RUNNER_DEVICE_READINESS_BUDGET_MS, + startupBudget.deadline?.remainingMs() ?? RUNNER_DEVICE_READINESS_BUDGET_MS, + ), + signal, + }), + ); await measureRunnerStartupStep(startupTimings, 'verify_host_dev_tools_security', async () => { - // Loaded here rather than at the top of the file: the runner subtree sits in the eager import - // closure of the seven Apple facades (eager-closure-budgets), and a preflight only a physical - // device ever needs has no business being evaluated to answer a simulator request. + // Loaded here for the same reason as the device probe above. const { assertDevToolsSecurityForIosRunner } = await import('./runner-dev-tools-security.ts'); await assertDevToolsSecurityForIosRunner(device); }); @@ -206,39 +232,50 @@ async function startRunnerSessionWithLease( } // Read before the build, which is a phase of its own with its own budget (#2422). const startupTimeoutMs = requireRunnerPhaseRemainingMs(startupBudget, 'runner_session_startup'); - const xctestrunArtifact = await measureRunnerStartupStep( - startupTimings, - 'ensure_xctestrun', - async () => - await ensureXctestrunArtifact(device, { - ...options, - budget: createRunnerPhaseBudget(options.buildTimeoutMs, signal), - }), - ); - startupTimings.build_xctestrun = xctestrunArtifact.buildMs; - const port = await measureRunnerStartupStep( - startupTimings, - 'allocate_port', - async () => await getFreePort(), - ); - const { xctestrunPath, jsonPath } = await measureRunnerStartupStep( - startupTimings, - 'prepare_xctestrun_env', - async () => - await prepareXctestrunWithEnv( - xctestrunArtifact.xctestrunPath, - { AGENT_DEVICE_RUNNER_PORT: String(port) }, - `session-${device.id}-${runnerOwnerToken()}-${port}`, - { iosXctestEnvDir: options.iosXctestEnvDir }, - ), - ); - const simulatorSetRedirect = await measureRunnerStartupStep( - startupTimings, - 'simulator_set_redirect', - async () => await acquireXcodebuildSimulatorSetRedirect(device), - ); + let xctestrunArtifact: Awaited>; + let port: number; + let xctestrunPath: string; + let jsonPath: string; + let simulatorSetRedirect: + | Awaited> + | undefined; let runnerProcess: LaunchedRunnerProcess; + // One catch for everything between here and a runner that answers, because the device's own answer + // belongs on all of it (#2690 review): a cold build, a warm derived cache that fails at install, and + // an external xctestrun that never launches are different steps, and a caller told "developer disk + // image" should not have to know which one this run happened to take. try { + xctestrunArtifact = await measureRunnerStartupStep( + startupTimings, + 'ensure_xctestrun', + async () => + await ensureXctestrunArtifact(device, { + ...options, + budget: createRunnerPhaseBudget(options.buildTimeoutMs, signal), + }), + ); + startupTimings.build_xctestrun = xctestrunArtifact.buildMs; + port = await measureRunnerStartupStep( + startupTimings, + 'allocate_port', + async () => await getFreePort(), + ); + ({ xctestrunPath, jsonPath } = await measureRunnerStartupStep( + startupTimings, + 'prepare_xctestrun_env', + async () => + await prepareXctestrunWithEnv( + xctestrunArtifact.xctestrunPath, + { AGENT_DEVICE_RUNNER_PORT: String(port) }, + `session-${device.id}-${runnerOwnerToken()}-${port}`, + { iosXctestEnvDir: options.iosXctestEnvDir }, + ), + )); + simulatorSetRedirect = await measureRunnerStartupStep( + startupTimings, + 'simulator_set_redirect', + async () => await acquireXcodebuildSimulatorSetRedirect(device), + ); if (xctestrunArtifact.buildMs > 0) { emitRequestProgress({ type: 'command', @@ -260,7 +297,7 @@ async function startRunnerSessionWithLease( ); } catch (error) { await simulatorSetRedirect?.releaseBestEffort(); - throw error; + throw enrichRunnerStartupFailureWithDeviceStates(error, deviceStates); } const sessionId = buildRunnerSessionId(device.id, port); const lease = buildRunnerLease({ @@ -285,6 +322,7 @@ async function startRunnerSessionWithLease( startupRetryWake: runnerProcess.startupRetryWake, startupTimeoutMs: normalizeRunnerStartupTimeoutMs(startupTimeoutMs), startupTimings, + startupDeviceStates: deviceStates, logicalLeaseContext, simulatorSetRedirect: simulatorSetRedirect ?? undefined, lease, @@ -748,6 +786,9 @@ export async function executeRunnerCommandWithSession( signal?: AbortSignal, ): Promise> { emitRunnerStartupTimings(session, command.command); + // Drawn before anything is sent, including the preflight: whatever the runner writes from here on + // is this command's attempt, and whatever is already in the log belongs to an earlier one (#2683). + const logAttempt = await captureRunnerLogAttempt(logPath, { timeoutMs, signal }); const runnerCommand = withRunnerCommandId(command); const readOnlyCommand = isReadOnlyRunnerCommand(runnerCommand); const deadline = Deadline.fromTimeoutMs(timeoutMs); @@ -757,7 +798,7 @@ export async function executeRunnerCommandWithSession( device, session, runnerCommand, - logPath, + logAttempt, deadline, signal, decision: preflightDecision, @@ -787,7 +828,7 @@ export async function executeRunnerCommandWithSession( throw markSkippedPreflightTransportError(error, session, preflightDecision); } try { - const data = await parseRunnerResponse(response, session, logPath); + const data = await parseRunnerResponse(response, session, logAttempt); // Mirror the runner's own main-thread occupancy stamped on this response: a runner that // served a read off the XCTest channel (e.g. a private-AX capture) while a tree crawl it // abandoned still grinds reports busy, so the healthy response must not be read as drained. @@ -910,12 +951,13 @@ async function runRunnerReadinessPreflight(params: { device: DeviceInfo; session: RunnerSession; runnerCommand: RunnerCommand; - logPath: string | undefined; + logAttempt: RunnerLogAttempt | undefined; deadline: Deadline; signal: AbortSignal | undefined; decision: Extract; }): Promise { - const { device, session, runnerCommand, logPath, deadline, signal, decision } = params; + const { device, session, runnerCommand, logAttempt, deadline, signal, decision } = params; + const logPath = logAttempt?.logPath; const readinessTimeoutMs = session.state === 'ready' ? Math.min(RUNNER_READY_PREFLIGHT_TIMEOUT_MS, deadline.remainingMs()) @@ -942,7 +984,7 @@ async function runRunnerReadinessPreflight(params: { timeoutMs: readinessTimeoutMs, }, ); - await parseRunnerResponse(readinessResponse, session, logPath); + await parseRunnerResponse(readinessResponse, session, logAttempt); } catch (error) { throw markRunnerReadinessPreflightError(error); } @@ -978,13 +1020,14 @@ function emitRunnerReadinessPreflightSkipped( export async function parseRunnerResponse( response: Response, session: Pick, - logPath?: string, + /** The command's own log boundary. Absent means no log was configured, so nothing is read. */ + logAttempt?: RunnerLogAttempt, ): Promise> { const payload = decodeRunnerResponseBody(await response.text()); if (!isRunnerResponseOk(payload)) { throw await enrichRunnerFailureFromLog({ - error: buildRunnerResponseError(payload, logPath), - logPath, + error: buildRunnerResponseError(payload, logAttempt?.logPath), + logSince: logAttempt, }); } advanceRunnerSessionState(session, 'ready'); diff --git a/packages/platform-apple/src/runner/runner-startup-transport.ts b/packages/platform-apple/src/runner/runner-startup-transport.ts index d51b9f3708..403b6cdcac 100644 --- a/packages/platform-apple/src/runner/runner-startup-transport.ts +++ b/packages/platform-apple/src/runner/runner-startup-transport.ts @@ -123,7 +123,13 @@ export async function waitForRunner( if (session?.child.exitCode !== null && session?.child.exitCode !== undefined) { throw await buildRunnerEarlyExitError({ session, port, logPath }); } - throw buildRunnerConnectError({ port, endpoints: route.endpoints, logPath, lastError }); + throw buildRunnerConnectError({ + port, + endpoints: route.endpoints, + logPath, + lastError, + deviceStates: session?.startupDeviceStates, + }); } type RunnerRouteResolver = ReturnType['resolveRoute']; diff --git a/scripts/check-affected/model.test.ts b/scripts/check-affected/model.test.ts index bc2b134fcf..357fe6f2b4 100644 --- a/scripts/check-affected/model.test.ts +++ b/scripts/check-affected/model.test.ts @@ -230,6 +230,23 @@ test('unknown path fails open to the full check set', () => { assert.equal(result.failOpenReasons[0]?.rule, 'unknown-path'); }); +test('a payload capture inside a package selects the unit lane', () => { + // Recorded tool responses checked in beside the module that parses them (#2683). Before this rule + // a capture edit failed the gate open, which punished adding evidence rather than the absence of it. + for (const file of [ + 'packages/platform-apple/src/core/__tests__/fixtures/ios-device-info-details.json', + 'packages/platform-apple/src/snapshot-source/fixtures/wire-vocabulary.json', + ]) { + const result = plan([file]); + assert.equal(result.failOpen, false, file); + assert.ok(result.checks.includes('unit'), file); + assert.ok( + result.reasons.some((reason) => reason.rule === 'own:package-capture'), + file, + ); + } +}); + test('a non-.ts fixture under an owned root fails open (format alone is not ownership)', () => { const result = plan(['test/integration/provider-scenarios/fixtures/device.json']); assert.equal(result.failOpen, true); diff --git a/scripts/check-affected/model.ts b/scripts/check-affected/model.ts index e4ae54160f..d21e10de21 100644 --- a/scripts/check-affected/model.ts +++ b/scripts/check-affected/model.ts @@ -529,6 +529,19 @@ const BUILD_OWNERSHIP: ReadonlyArray<{ file.startsWith('examples/test-app/security/') || file === 'examples/test-app/pnpm-workspace.yaml', }, + // In-package payload captures: a recorded tool response checked in under a package's fixture + // directory (`packages/*/**/__tests__/fixtures/*.json`, or a `fixtures/` dir beside the module that + // reads it). Nothing builds them and no `.ts` sibling names them, so without this a capture edit + // fails the gate open even though exactly one suite asserts against it. + { + check: 'unit', + rule: 'own:package-capture', + detail: 'the vitest unit suite reads the captured payload', + owns: (file) => + file.startsWith('packages/') && + file.endsWith('.json') && + (file.includes('/__tests__/fixtures/') || file.includes('/fixtures/')), + }, // TS/Swift golden tables (`contracts/fixtures/*.json`): the vitest parity test and the // runner XCTest twin both read them, so a table edit owns the unit lane and both runner // builds. Without this a `.json` under contracts/ has no derivable owner and fails open. diff --git a/src/commands/schema/cli-help.ts b/src/commands/schema/cli-help.ts index 2e39479c60..645f91bf48 100644 --- a/src/commands/schema/cli-help.ts +++ b/src/commands/schema/cli-help.ts @@ -689,7 +689,7 @@ iOS physical-device prerequisites: If Xcode cannot choose a profile, set AGENT_DEVICE_IOS_PROVISIONING_PROFILE to the profile name/specifier, not a file path. AGENT_DEVICE_IOS_SIGNING_IDENTITY is optional; omit it unless xcodebuild asks for a specific identity. The profile/team must allow AGENT_DEVICE_IOS_BUNDLE_ID and .uitests. - A runner build failure names its class in error details.reason rather than only in prose: signing_no_development_team, signing_provisioning_profile_missing, bundle_identifier_already_registered, signing_unspecified, devtools_security_developer_mode_disabled (the Mac's DevToolsSecurity setting, which says nothing about the device's Developer Mode toggle), or build_failed_unclassified when nothing proved a cause. Branch on details.reason and follow hint; the message is for humans. + A runner startup failure names its class in error details.reason rather than only in prose: signing_no_development_team, signing_provisioning_profile_missing, bundle_identifier_already_registered, signing_unspecified, devtools_security_developer_mode_disabled (the Mac's DevToolsSecurity setting, which says nothing about the device's Developer Mode toggle), device_developer_mode_disabled (read from an iPhone's own report before the runner builds, and the only device state that stops a run up front), device_developer_disk_image_unavailable (also read from the device, and published on a startup failure that named no cause of its own, since iOS 17+ mounts the developer disk image on demand during build and launch rather than gating the build), or build_failed_unclassified when nothing proved a cause. The two device reasons are never inferred from tool output or from each other. Branch on details.reason and follow hint; the message is for humans. First-run XCTest setup/build can take longer than normal commands; keep the device connected and use --debug to inspect signing/build diagnostics if setup times out. Android physical-device prerequisites: diff --git a/website/docs/docs/installation.md b/website/docs/docs/installation.md index 62e39676f3..cd3c749502 100644 --- a/website/docs/docs/installation.md +++ b/website/docs/docs/installation.md @@ -108,7 +108,8 @@ vega device list - `AGENT_DEVICE_IOS_PROVISIONING_PROFILE` - `AGENT_DEVICE_IOS_BUNDLE_ID` (optional runner bundle-id base override) - Free Apple Developer (Personal Team) accounts can fail with "bundle identifier is not available" for generic IDs; set `AGENT_DEVICE_IOS_BUNDLE_ID` to a unique reverse-DNS value (for example `com.yourname.agentdevice.runner`). -- A runner build failure is typed, not prose: `error.details.reason` is one of `signing_no_development_team`, `signing_provisioning_profile_missing`, `bundle_identifier_already_registered`, `signing_unspecified`, `devtools_security_developer_mode_disabled` (the Mac's `DevToolsSecurity` setting, which says nothing about the device's Developer Mode toggle), or `build_failed_unclassified` when nothing proved a cause. Branch on `details.reason` and follow `hint`; the code stays `COMMAND_FAILED` for every reason. +- A runner startup failure is typed, not prose: `error.details.reason` is one of `signing_no_development_team`, `signing_provisioning_profile_missing`, `bundle_identifier_already_registered`, `signing_unspecified`, `devtools_security_developer_mode_disabled` (the Mac's `DevToolsSecurity` setting, which says nothing about the device's Developer Mode toggle), `device_developer_mode_disabled`, `device_developer_disk_image_unavailable`, or `build_failed_unclassified` when nothing proved a cause. Branch on `details.reason` and follow `hint`; the code stays `COMMAND_FAILED` for every reason. +- The two `device_*` reasons come from the iPhone itself, read over `xcrun devicectl device info details` before the runner builds: `developerModeStatus` for the Settings toggle and `ddiServicesAvailable` for the developer disk image. They are reported apart on purpose. A phone with Developer Mode off cannot serve its disk image either, so it gets the toggle reason; a phone with the toggle on and only the image down gets the disk-image reason, which is a device-support install that has not finished rather than a setting anyone turned off. - If device setup is slow, keep the device connected and inspect daemon diagnostics after retrying. - If daemon startup reports stale metadata, remove stale files and retry: - `/daemon.json`