feat(ios-runner): read developer mode and disk image state from the device - #2690
Conversation
|
Size Report
Startup median (7 runs, lower is better):
|
|
Reviewed at 62c0fa5. Reading Developer Mode and disk image state from the device is useful, but three issues block this. On iOS 17+, CoreDevice mounts the personalized developer disk image on demand. So a healthy phone right after a reboot can report The readiness fact is not published once, as #2683 asks. The device path keeps its hint text in There is no device evidence yet. The Could core's Not blocking: the Compatibility & Provenance fails on the exports and parser complexity that this PR adds (#L151, #L238, #L65), so it comes from this diff. The Smoke Tests failure is a simulator scroll: the long-press fixture element never became visible. The readiness preflight returns early for simulators, so that failure is likely unrelated. Next: share one hint and verdict owner between the device and devicectl paths, stop refusing before the build on |
62c0fa5 to
b2b8694
Compare
|
Reviewed at 27f3836. This follows up on the earlier review at 62c0fa5 (#2690 (comment)). The disk-image refusal and the evidence gap from that review remain. The disk-image refusal still runs before the build. The Smoke Tests fails on the iOS simulator at Next: before the build, refuse only when Developer Mode is disabled, carry the disk image state on build or launch failures, and attach the #2683 checklist run on a real iPhone, including the post-reboot state. |
27f3836 to
f2d605a
Compare
|
This is a follow-up on the earlier review at 27f3836 (#2690 (comment)). At f2d605a the code still has one behavior bug and the device evidence is still missing. The pre-build refusal in runner-device-readiness.ts:90 is unchanged. With Developer Mode on, tunnelState The #2683 section of .device-evidence/CHECKLIST.md:94 (steps 1-7) still has not been run, and the PR body itself says the pairings for Not blocking: I could not test iOS 17+ CoreDevice mount-on-demand behavior myself, so the post-reboot triple is inferred from CoreDevice semantics and stays a likely finding until the capture above lands. I did not re-review the payload parser, remedies plumbing, or log-boundary tests beyond the four focus invariants; those were settled at 27f3836. I ran no tests locally. CI reports all 21 checks passing at f2d605a, the Smoke Tests simulator failure seen at 27f3836 is gone, and the only simulator-path change in this delta, the per-command runner.log boundary, is byte-identical to the earlier revision. The diff view for this PR is polluted by the base move from f4930d6 to 85bd4de, so I reconstructed the logical delta by diffing the two per-ref patches instead. Before merge, the pre-build disk-image refusal needs to change to refuse only on |
f2d605a to
120ae34
Compare
|
Rebased onto #2688 The pre-build DDI refusal is removed.
The helper lives beside The two non-blocking items are in. Gates at the new head: Still owed, and not runnable from here: no physical captures. |
|
Reviewed at 120ae34. The checklist at https://github.com/callstack/agent-device/blob/120ae34/.device-evidence/CHECKLIST.md#L102 still shows section #2683 steps 1-6 as unrun, and runner-startup-failure-fixtures.ts still marks the Not blocking, can be taken or left: Does this delta need its own I ran no tests and no device commands for this round; the red-without-the-fix claims were judged by reading the flipped assertions and the pre-change code, not by mutating it. iOS 17+ CoreDevice mount-on-demand behavior is still inferred, not observed, so the checklist run is what settles whether the post-reboot triple occurs at all. Per follow-up scope I did not re-review the devicectl payload parser, the remedies plumbing, or physical-device-coredevice beyond the observability gate that the earlier finding and the corroboration rule depend on; those were settled at 27f3836. I did not exercise the launch/install failure route end to end, so that finding's impact is read from the call graph, not observed. CI is green: GitHub reports 21 checks, 0 not passing, at 120ae34. The Smoke Tests simulator failure seen at 27f3836 is gone, and the only simulator-touching change here, the per-command runner.log boundary, is exercised by the unit lane. Before merge, run CHECKLIST section #2683 steps 1, 3 and 4 on a physical iPhone at 120ae34 and paste the raw payloads and JSON output; fixing the corroboration rule's keying is worth doing alongside that. |
120ae34 to
a371ad2
Compare
|
All four items in, rebased onto #2688 The device now keys on "did a row match", not on the unclassified literal. You had the failure exactly right: The enrichment moved up to the session's startup catch. The enricher never overwrites: a failure that already carries a reason keeps it, and a non-
Help clause fixed. Gates at the new head: Phase B is still yours and still owed at this head: the three §#2680 prepare runs, and §#2683 steps 1, 3 and 4 — step 4 including the post-reboot reading (which of the three outcomes a real image-down device gives) and the dev-mode-off pairing. Nothing moved to |
|
This is a follow-up to the earlier findings at 120ae34 (#2690 (comment)), reviewed here at a371ad2.
Not blocking: the startup try in Live evidence per .device-evidence/CHECKLIST.md §#2683 items 1, 3 and 4 is still missing on a physical iPhone at a371ad2; fixtures remain provenance 'invented-shape' and the author has confirmed evidence is pending. Since the enrichment now lives in the session startup catch, the image-down run needs to actually reach that catch. What's needed: raw No tests and no device commands were run for this review; the read-without-the-fix judgment came from reading the three new session-readiness tests against the pre-change build catch, not from mutating the code. Whether f1 is reachable in practice — a build timeout coinciding with Both non-passing checks are Smoke Tests, queued at a371ad2 with no failure excerpt yet, so there's nothing to attribute — but this delta touches the iOS session-startup route that lane exercises, so the result is worth reading once it lands. The device-speaks window for host-killed timeout failures needs to close per the first finding, and then CHECKLIST §#2683 items 1, 3 and 4 need a run on a physical iPhone at the shipped head, with the raw payloads and the image-down JSON pasted here, before this is ready to merge. |
a371ad2 to
52ab61e
Compare
A build the host killed at its own `buildTimeoutMs` arrives with no rule matched, and on a phone that happens to report its developer image down the enrichment answered with the image: a build that never finished cannot have been refused for want of device support, and the caller was sent to mount an image while their real problem was a 15-minute budget (#2690 review). The device now speaks only where nothing matched AND the host's deadline did not end the command. The build catch threads that fact the way it already threads `startupRuleMatched` — its wrapper buries the exec error too deep to inspect — and a failure that never passed through a catch is read with host-kit's `isCommandTimeoutError`, so the install and launch steps are covered by the exec's own `timeoutMs`. Two things the same catch was doing to the error it rebuilt: the cause is now passed through, so the reader of a daemon log still reaches the command that died, and the plumbing keys — whether a row matched, whether the deadline hit — are internal and stripped with the hoisted diagnostic meta, because a caller already holds the verdict they produced rather than the machinery that reached it.
|
Rebased onto Blocking — the host-timeout window is closed. The device now names a cause only where no rule row matched and the host's own deadline did not end the command. The fixture is
On the predicted Gates at this head: Still owed: the physical-device captures in |
|
Follow-up on
Not blocking, and take it or leave it: the kernel now enumerates two apple-runner detail keys through On CI: Smoke Tests was still running at Two limits on this pass: I did not run the unit suite, so the pre-fix redness of the new fixture is argued from the classifier rows and the prior enrichment rather than from a run; and whether xcodebuild's own install step can produce a host-deadline error that reaches the enriched catch needs a device, since that spawn is background and its failures surface in Next blocker: the live physical-iPhone evidence above. |
Phase B — #2690 captured on a real iPhone (Developer Mode OFF)Head This closes the gap that "the only captured 1. Raw
|
|
The Developer Mode half is now proven end to end at The disk image half is the one thing left, and your own run is what shows it. Inside the verified image-down window Your note that adoption runs before the readiness check is worth keeping in the PR body too — it explains why a retained lease can return success where a fresh start refuses. I am holding |
52ab61e to
8951f93
Compare
A build the host killed at its own `buildTimeoutMs` arrives with no rule matched, and on a phone that happens to report its developer image down the enrichment answered with the image: a build that never finished cannot have been refused for want of device support, and the caller was sent to mount an image while their real problem was a 15-minute budget (#2690 review). The device now speaks only where nothing matched AND the host's deadline did not end the command. The build catch threads that fact the way it already threads `startupRuleMatched` — its wrapper buries the exec error too deep to inspect — and a failure that never passed through a catch is read with host-kit's `isCommandTimeoutError`, so the install and launch steps are covered by the exec's own `timeoutMs`. Two things the same catch was doing to the error it rebuilt: the cause is now passed through, so the reader of a daemon log still reaches the command that died, and the plumbing keys — whether a row matched, whether the deadline hit — are internal and stripped with the hoisted diagnostic meta, because a caller already holds the verdict they produced rather than the machinery that reached it.
|
The disk-image question is implemented at
Behaviour is deliberately asymmetric, and that asymmetry is the invariant you asked for:
Three tests in So the reason and the detail now have an observed publishing path rather than a declared one, and it is the path the hardware run actually walked. Your other note — that adoption precedes the readiness check, which is why a retained lease can return success where a fresh start refuses — is now in the PR body alongside the capture that showed it. Local gates: |
|
Reviewed at 8951f93. The early-exit failure now carries the device facts, and a claimed cause keeps its reason and hint. Thank you, that closes the route I asked about at 52ab61e. Is the other connect-stage failure covered too? When the xcodebuild child is still alive at the connect deadline, The checklist (CHECKLIST-runner-failures.md#L76) records the pre-fix image-down run and then says the connect failure now routes through the enrichment. No run at this head shows that yet. Could you rerun A small note: on this route CI is green and there are no conflicts. I am holding |
|
Addressed at eaf411e.
New test: The Validation: Still open: the live |
|
Live run at eaf411e on a cabled iPhone 17 Pro (iOS 27.0, 24A437), rebooted and held locked. The healthy run first: with the phone unlocked, After the reboot, still locked, the same command failed as before: {
"code": "COMMAND_FAILED",
"message": "Runner did not accept connection (xcodebuild exited early)",
"details": { "reason": "IOS_RUNNER_CONNECT_TIMEOUT", "xcodebuild": { "exitCode": 70 } }
}There is no So both connect-stage failures now carry the device states whenever the preflight could observe them, and the unit tests pin that. But no hardware state I can reach produces |
Pull Request is not mergeable
Pull Request is not mergeable
|
Reviewed at f5526ba. The alive-child connect timeout now carries the device states too: A small question, not blocking: All checks pass. The PR has merge conflicts with main after #2688 was squash-merged, so it needs a rebase before it can get |
`xcrun devicectl device info details` has always carried two separate answers about a device — `deviceProperties.developerModeStatus` for the Settings toggle and `ddiServicesAvailable` for the developer disk image — and #2682 read the payload for the tunnel only, so both states stayed invisible to the runner. `core/ios-device-readiness.ts` now reads them apart and publishes them through the physical-device control facet, which is the seam the runner already reaches devices through. The parser copies the tool's values without interpreting them; a state it does not recognise stays `unknown` rather than becoming either permission or accusation. `resolveCoreDeviceTunnelIp` now shares that one reader instead of re-running the command. The two reasons are declared as a subset of the startup vocabulary, apart from the rule-driven members, because no amount of tool text establishes them. The captured payload is committed with its device identifiers, hostnames and capture path masked.
One `devicectl` hint covered both "developer mode is disabled" and "developer disk image", and it named Developer Mode. That pairing runs one way only: a phone with the toggle off cannot mount the image either. The reverse is common — a device whose toggle is on, waiting for Xcode to finish installing device support — and people were sent to a Settings pane that was already correct while the actual wait went unmentioned. The two complaints are answered apart now. When the output names both, the toggle still wins, since it is the one that explains the other. #2683 adds the device's own reading of both states, which is what lets the tool-output path stop guessing which one it is looking at.
One `runner.log` serves every command sent to a device and is never truncated between them, and a failing response was classified from the last 64 KB of it. A crash line an earlier command wrote therefore explained whatever failed next: two commands after an app died in AXRuntime, an unrelated timeout was reported as `target_app_axruntime_coretext_crash`, sending the reader to hunt a crash that had already been dealt with. A command now marks where the log had reached before anything is sent, including before its own readiness preflight, and only bytes past that mark are read. A log that has been rewritten behind the mark is shorter than it and is not read at all — that is not evidence about this command.
A phone that refuses to host development tooling fails opaquely today: the build cannot install, or the runner never reaches its port, and the caller reads a build log for a phone problem. The device already states the reason, so startup now asks it — after the host's own `DevToolsSecurity` probe and for physical iPhones only — and publishes `device_developer_mode_disabled` or `device_developer_disk_image_unavailable` with the hint beside the rule that names it. The toggle is weighed first because a phone with it off cannot serve its developer disk image either; the reverse never holds, and an image that is down on a phone with the toggle on is never restated as a toggle problem. A device that could not answer is left alone: `available: false` carries no verdict, and inventing one from a missing read turns an unplugged cable into a claim about someone's Settings. The probe is loaded at its step rather than imported at the top, for the same reason as the host probe above it: the runner subtree sits in the eager closure of seven Apple facades, and a check only a device ever needs has no business being evaluated to answer a simulator request.
…ner port Two gates decided this shape, and both are worth recording because neither is visible from the code: - `scripts/layering` R9/R10: a type-only import of `runner/runner-contract.ts` from `core/physical-device-control.ts` closed a 13-file type-level cycle across the runner subtree, because pulling that module pulls the whole runner contract graph. The fact types now live with the module that reads the payload, and `runner/host.ts` mirrors the shape structurally — which is what that file prescribes for a host port anyway, with the composition-root assignment as the conformance check. - `eager-closure-budgets`: a separate `core/ios-device-readiness.ts` added one module to the eager closure of six Apple facades, because the physical-device control facet reaches it statically. The reader now sits beside the payload parse it consumes, where it adds nothing to any closure. Behaviour is unchanged: same two states, same unreadable shape, same tests carried over.
…it lane A recorded tool response checked in under a package (`…/src/**/__tests__/fixtures/*.json`, or a `fixtures/` dir beside the module that parses it) has no derivable owner: nothing builds a `.json` and no `.ts` sibling names it, so editing one failed the affected gate open to the full check set. That punished adding evidence — the capture in #2683 tripped it — while the pre-existing `snapshot-source/fixtures/wire-vocabulary.json` has been sitting in the same gap. The rule names the one suite that reads such a file. Root-level and `test/` fixtures still fail open as before, which is right: nothing outside a package says who consumes them. Editing the selector model fails the gate open by design (`selector-owning`), so this change cannot narrow its own selection; the full set runs for it.
…roduced it Review round on the readiness work (#2683): - Drain the runner's queued log appends before drawing a log boundary, so bytes an earlier command has not finished writing cannot be read as this command's. The boundary is now one object (`RunnerLogAttempt`) with no log-path-only shape, and every call site reads through it. - Treat an uncorroborated `ddiServicesAvailable: false` as unreadable rather than as a developer-disk-image obstacle: the services only answer while the tunnel is up and the phone is booted. A disabled toggle keeps its answer regardless, since it already explains the image. - Publish each remedy once: the device report carries the wording `core/devicectl.ts` owns, and the preflight reads it instead of wordings its own. - Bound the readiness probe by the startup budget and its signal, and abort after the read if the caller gave up. - Probe the device before the host's developer-tools setting, so a device and a Mac that are both wrong publish the reason the caller can act on. - Mask the identity fields in the committed device payload and give the mask a test.
…ir modules The Fallow audit that CI runs on changed files was red on this branch: - Collapse the two payload shapes CoreDevice has shipped into one pair of sections before any field is read, so the parser stops spelling the nested fallback out per field and drops below the complexity threshold. Direct values still win. - Report a reported field through one helper that omits it when absent, keeping "the device said nothing" distinct from "the device said no". - Stop exporting `readIosDeviceDetails` and `nameIosDeviceReadinessObstacle`: each has one caller, in its own module, and a caller that needs a verdict reads `readIosDeviceReadiness` or the preflight that publishes the reason. - Move the build-fixture envelope assertions out of the test body into named helpers so the recorded shapes are asserted through the same path.
… host port #2689 rewrote the runner host port to derive its types from host-kit and the owning core modules instead of mirroring them, which left this branch naming types that no longer exist there: - Derive `IosDeviceRunnerReadiness` from the control port's own return type rather than restating the device's states and remedies beside them. - Take `IosPhysicalDeviceRunnerControl` from `core/physical-device-routing.ts` and `ExecResult` from `@agent-device/host-kit/command`, which is where the port now points callers.
Developer Mode off was the only device state no later step could change, and it is the only one that keeps refusing a run before the build. 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: refusing there turned a self-clearing state into a failed run. The preflight now hands the states back, the build carries them, and an unavailable image is published onto a build that named no cause of its own — beside xcodebuild's own sentence, never in place of it. A log flush gained the deadline and abort it was missing, and a refused append is recorded instead of dropped, because an offset measured over bytes that never landed is not a boundary.
Keying the enrichment on `build_failed_unclassified` read the absence of a row as a positive claim: a row that deliberately names no cause publishes the same reason, so a just-rebooted phone's disk-image state would have been published over a conflicting-settings failure. The classifier now reports whether a row matched, the catch that classifies carries that answer on `details.startupRuleMatched`, and the enrichment reads it. The enrichment also moves out of the build catch into the session's startup catch, which already holds the device's states and sees every step from the artifact to a runner that answers. A warm derived cache that fails at install and an external xctestrun that never launches now carry `details.developerDiskImage` too, which the build-catch-only version could not reach.
A build the host killed at its own `buildTimeoutMs` arrives with no rule matched, and on a phone that happens to report its developer image down the enrichment answered with the image: a build that never finished cannot have been refused for want of device support, and the caller was sent to mount an image while their real problem was a 15-minute budget (#2690 review). The device now speaks only where nothing matched AND the host's deadline did not end the command. The build catch threads that fact the way it already threads `startupRuleMatched` — its wrapper buries the exec error too deep to inspect — and a failure that never passed through a catch is read with host-kit's `isCommandTimeoutError`, so the install and launch steps are covered by the exec's own `timeoutMs`. Two things the same catch was doing to the error it rebuilt: the cause is now passed through, so the reader of a daemon log still reaches the command that died, and the plumbing keys — whether a row matched, whether the deadline hit — are internal and stripped with the hoisted diagnostic meta, because a caller already holds the verdict they produced rather than the machinery that reached it.
Co-Authored-By: Apex <noreply@callstack.com>
…ublishes no disk-image state
…on module they cover
f5526ba to
66bac95
Compare
1 participant
Summary
Stacked on #2688. Three things were guesses.
verifyDeveloperModeForIosRunnerread the Mac'sDevToolsSecurity -status— a host setting, never the iPhone's toggle — and threw with no reason;resolveIosDevicectlHintanswered "developer disk image" with Developer Mode advice; andrunner.log, append-only across commands, let an earlier command's crash explain whatever failed next.devicectl device info detailscarries the two device facts apart, so they become two reasons in #2680's vocabulary while the host probe keepsdevtools_security_developer_mode_disabledand is asked second: when a device and a Mac are both wrong, the caller gets the reason they can act on without admin rights. An image complaint is believed only whiletunnelState=connectedandbootState=booted, so a sleeping phone publishes the unreadable shape, not a verdict. Each remedy is worded once incore/devicectl.tsand travels on the report. Each command draws itsrunner.logboundary after draining queued appends and reads only bytes past it, under the startup budget and signal.Only the toggle refuses the run before the build (#2683 review). Since iOS 17 CoreDevice mounts the personalized developer disk image on demand during build and launch, a phone that has just been rebooted reports
ddiServicesAvailable: falsewhile the very next build clears it — refusing there turned a self-clearing state into a failed run. So the preflight hands the states back and the session's startup catch attaches them to whatever startup threw, which is what makes a warm derived cache that fails at install and an external xctestrun that never launches say the same thing a cold build does (#2690 review).It becomes the reason only when the failure carries no reason of its own and no rule row matched (#2690 review).
classifyRunnerStartupFailurenow reportsmatched, and the catch that classifies carries that answer onward asdetails.startupRuleMatched— necessary because a wrapper buries the tool's text a level too deep for the rows to read again. Without it, a row that deliberately names no cause publishesbuild_failed_unclassifiedlike any unmatched failure, and a just-rebooted phone's image state would have been published over a conflicting-settings failure.Each command's log boundary is bounded too: draining is a prelude to sending, so it waits on a deadline and the caller's signal rather than the command's own clock, and an append the disk refused is recorded instead of dropped — with bytes unaccounted for there is no trustworthy offset, so no marker is drawn and the tail goes unread. The device report has one type name, re-exported by
runner/host.tsunder the name the reader owns.Closes #2683. 24 files, 604 production lines added.
Validation
At
a371ad284f(rebased onto #2688bcfed35b4c, which is onmainfc59620932):build,format,check:quick,check:layering,check:gate-manifest,check:command-docs,check:production-exports,apple-runner(533) and the full unit lane (10,665) pass, andfallow audit --base origin/mainreports no findings across the stack's 28 changed files.check:affected --runstays with CI.Seven claims fail red when the fix is removed: unmarked log-tail reads, a boundary drawn before queued appends,
ddiServicesAvailable: falsefrom a sleeping device, a remedy worded outside its owner, host-before-device probing, a device state published where a rule row had already looked at the failure, and a marker measured over an append the disk refused. Making the enrichment stop at the build catch instead of the startup catch takes the warm-cache launch case down with it.The committed payload masks serial, ECID, snapshot name and tunnel IP behind a test. Live evidence is pending (
.device-evidence/CHECKLIST.md§#2683), so the state pairings stayinvented-shape; item 4 now records which of three outcomes a real image-down device produces, since "the build clears it" is the outcome this round removed the refusal for. Startup spends one best-effortdevicectlread per physical-device session; it can see a stale tunnel IP, which is why it never gates a run alone.Added after the Phase B device run (
8951f93496)An early exit now carries the disk-image state too.
enrichRunnerStartupFailureWithDeviceStateshad exactly one call site, the
launch_xcodebuildcatch, so the only image-down state iOS 27 actuallyproduces — a locked phone that lets the build finish and kills
xcodebuild test-without-buildinginstead — never passed through it and published no
developerDiskImage. The facts read before thebuild now travel on the session (
startupDeviceStates) and the early-exit builder routes through thesame enrichment, so the fact lands on the failure a real device produces. A failure that already
proved a cause keeps that reason and hint; the device only adds its fact, never overwrites a claim.
Captured on
thymikee-iphoneat52ab61e: inside a verified image-down windowpreparefailed asRunner did not accept connection (xcodebuild exited early)(exit 70) with nodeveloperDiskImage—which is what this closes.
Why adoption runs before the readiness check, and what that means for a retained lease.
createRunnerSessionmeasuresadopt_detached_runnerfirst (runner-session.ts:176) andverify_device_readinessonly afterwards (:198-209). A live runner already serving commands is adoptedwithout a readiness re-check, so with a retained lease this command returns success where a fresh
start refuses. Defensible — Developer Mode was on when that runner started — but it is why the first
attempt at the Developer-Mode-off capture succeeded until the lease was cleared.