fix: support xctrace-only iOS physical devices#1375
Conversation
Review: xctrace-only iOS physical-device supportReviewed the full diff (26 files, +745/−234). This is a clean, well-scoped feature. The CoreDevice path is refactored out verbatim into Overview
Findings1. [design, medium] Backend selection is process-global side-channel state, decoupled from I traced the reachable paths:
The deeper point is the latent invariant "discovery must precede command execution, in-process" that nothing enforces by type. Since the resolved 2. [maintainability, low] case .activate:
return Response(ok: true, data: DataPayload(message: "app activated"))This is correct only because
3. [dead-code, low] 4. [nit] Duplicated constants. 5. [nit] Things I checked that are fine
ValidationLive physical-device validation is still pending (noted in the description). Given the whole feature targets rarely-available iOS 15/16 hardware, I'd treat finding #1's injected-inventory gap and a real-device smoke of open → screenshot → terminate as the two things to close before relying on this in the field. |
|
P1: The new xctrace-only backend cannot reach its XCTest runner. |
Size Report
Startup median (7 runs, lower is better):
Top changed chunks:
|
|
|
Re-reviewed at Readiness is still blocked by the confirmed Fallow failure: |
|
Addressed the remaining review blocker in Also repeated the live source-built smoke on unlocked |
|
Re-reviewed exact head The remaining blocker is practical validation: the |
|
Addressed the final readiness comment. The maintainer explicitly chose to undraft this PR after the lack of iOS 15/16 hardware and the resulting residual validation risk were disclosed; treating that as acceptance of the documented risk rather than claiming live xctrace/usbmux evidence we do not have. The fallback path remains covered by the end-to-end fake-usbmuxd regression, the available CoreDevice physical-device flow passed open → selector interaction → screenshot → close, every CI check is green, and |
|
Coordinator correction: removing |
|
Acknowledged the coordinator correction. |
|
Branch readiness update: exact head 293e902 is now reported CONFLICTING with current main after recent merges. The reviewed code head has not changed, so this is an owner-action branch blocker rather than a new code finding. Rebase onto current main, resolve the conflict, and let checks rerun before readiness can be reassessed. |
293e902 to
9f52c6a
Compare
|
Review at Two contract/docs gaps also need resolution: #1361 explicitly requires backend selection through The branch conflict is resolved and all current checks are green, but this code finding blocks readiness. After fixing it, the new path still needs a real xctrace-only iOS 15/16 open → interaction → screenshot → close run, or explicit maintainer acceptance of that residual risk. |
Summary
Support xctrace-only iOS 15/16 physical devices through a small backend seam while keeping CoreDevice as the default path.
DeviceInfodiscriminant without exposing it publicly.Closes #1361
Closes #1355
Validation
pnpm check:affected --base origin/main --run(format, lint, typecheck, layering, fallow, and build passed; nine unrelated concurrent timeout cases passed in isolated reruns)pnpm test:coverage --maxWorkers=4(540 files; 4,568/4,569 passed, with the sole unrelated timeout passing immediately in isolation; all provider-integration tests passed)pnpm build:xcuitestthymikee-iphonesmoke with the source-built CLI: opened Settings, resolved and tappedid="com.apple.settings.general", settled on General, found the hittable Software Update row, and captured a post-interaction screenshot; session closed cleanly