fix(ios): keep devicectl hints on physical install and delete the dead install paths - #2857
Conversation
Physical iOS install and uninstall ran through devicectl without ever consulting resolveIosDevicectlHint, so a failure lost the Developer Mode, developer-disk-image, and pairing guidance every other devicectl call site attaches. Route both failures through the same resolver. Delete the install/uninstall/assertAppInstallationSupported members of IosPhysicalDeviceControl and their XCTest rejection functions: nothing calls them since #1758 moved physical install to deployment/runtime.ts. Delete pushIosNotification, which deployment/runtime.ts's pushAppleNotification replaced and only its own tests still called. Repoint the C.push iOS-simulator coverage declaration at the deployment-runtime contract test that now exercises simctl push.
installAppleApp's devicectl and simctl branches, and pushAppleNotification's simctl request, omitted allowFailure. The host command runner rejects a non-zero exit before assertAppleToolSuccess runs, so its curated message and the devicectl Developer Mode hint were dead code on those paths. uninstallAppleApp already set allowFailure on both branches. Route every appleTools.run request that feeds assertAppleToolSuccess through a single runAppleTool helper that forces allowFailure: true, so the invariant holds at every call site instead of per-branch.
… by #2785 Their only caller was the IosPhysicalDeviceControl install/uninstall wiring, which the prior commit removed as dead. fallow dead-code --unused-exports now flags both as unused; delete them and their now-unused devicectl/config/apps-simctl imports.
runAppleTool forced allowFailure: true on every request so the devicectl hint would attach, but that made the host command runner return the failed result instead of rejecting it. The local assertAppleToolSuccess then built COMMAND_FAILED details by hand, without processExitError, so normalizeError no longer appended the stderr excerpt to simulator install, physical install, and simulator push failures, and their details lost cmd/args. Uninstall enrichment was pre-existing dead code before this branch; the other three requests regressed on it. Fold assertAppleToolSuccess into runAppleTool: it still forces allowFailure, but now also guards the result and throws through execFailureDetails itself, so a result can't be tolerated without also being checked. A caller passes the message and an optional hint function; uninstall additionally passes a tolerate predicate for its "already missing" case, which is the only one that still needs to inspect a non-zero result before deciding to fail. Mirror the same exit-error shape in the runtime.test.ts xcrunLikeRun fake (cmd/args/processExitError via execFailureDetails, matching createExitError) so it can catch a call site that drops the excerpt. Add regression tests for simulator install and simulator push failures, and assert the normalized message on the existing physical install/uninstall Developer Mode tests, not just the hint.
runAppleTool rebuilt the exec-failure shape locally instead of reusing requireExecSuccess, so the two copies could drift (as the deleted assertAppleToolSuccess already had). Widen requireExecSuccess to accept any result shaped like HostCommandResult (exitCode: number | null) and call it from runAppleTool, keeping one COMMAND_FAILED shape for every allowFailure exec call site.
… re-declaring them runtime.ts's runAppleTool consolidation left IOS_DEVICE_INSTALL_TIMEOUT_MS (core/config.ts) and isMissingAppErrorOutput (core/apps-simctl.ts) with no reader after the deleted CoreDevice install/uninstall paths were removed, while runtime.ts kept a local 120_000 literal and a duplicate substring check. Import and reuse both instead, and drop the issue-number references from the surrounding doc comments.
… left behind The rebase onto origin/main landed on top of the rename that deleted core/apps-simctl.ts and moved isMissingAppErrorOutput into physical-device-apps.ts as a private function. runtime.ts kept importing from the deleted module, and the base's copy in physical-device-apps.ts lost its only reader once this branch deletes uninstallCoreDeviceApp, leaving an unused private declaration. Export isMissingAppErrorOutput from physical-device-apps.ts and import it there from runtime.ts, so the missing-app check has exactly one definition and one caller: the uninstall tolerate predicate.
…yApp
The uninstall tolerate predicate and its lowercasing had no test
driving createAppleAppDeploymentOperations, so removing either left
the whole suite green. Add a reinstall test, for both a physical
CoreDevice and a simulator, where uninstall exits non-zero with
mixed-case stderr ("ERROR: App Not Installed"): deployApp must still
resolve and the install request must have run.
Size Report
Startup median (7 runs, lower is better):
|
|
Reviewed at 6d55663. CI is green: 18 checks reported, none failing on this commit. Physical install and uninstall now force allowFailure:true and guard the result locally through runAppleTool/requireExecSuccess (https://github.com/callstack/agent-device/blob/6d55663/packages/platform-apple/src/deployment/runtime.ts#L146). Both the success path and the failure-hint path changed, and neither ran on a physical device yet. A device-facing route needs live evidence before merge (docs/agents/pull-requests.md), and fixtures do not stand in for that run. Can you run Not blocking: the simulator install-failure test at packages/platform-apple/src/deployment/runtime.test.ts#L352 never actually checks that the hint is absent, since assertRejectsAppError skips the hint check when hint is undefined, so deleting the I could not reach a physical device, so the devicectl route was only exercised against the unit fake, and I did not run check:production-exports or check:fallow; I'm relying on the PR body and green CI for the rest. I also don't know whether real devicectl stderr for Developer Mode matches the fixture string that resolveIosDevicectlHint classifies — any failure that doesn't match still gets IOS_DEVICECTL_DEFAULT_HINT, so is that mapping worth double-checking against a real device error? The PR body's live run was on simulator, which doesn't reach the devicectl route, so a physical-device run covering both a successful install and a failing install with the devicectl hint in the JSON error is what's needed before this is ready to merge. |
|
Addressed at edd6977 (no rebase: MERGEABLE/CLEAN, main did not touch these files). Blocking: live physical-device evidence. Ran on a CoreDevice iPhone 17 Pro (
Non-blocking: simulator hint not asserted. Did both parts:
Validation. |
|
Reviewed at edd6977. The code looks correct. The physical-device install success and failure-hint results are your reported runs; I did not reproduce them. The Developer Mode and developer-disk-image branches of Smoke Tests, Repo Guards and Coverage were still running, and nothing had failed yet. Simulator install and uninstall go through the same Not blocking: the PR body still says the hint is gated on |
thymikee
added
the
ready-for-human
|
1 participant
Summary
Physical-device installs/uninstalls route through
runAppleTool→requireExecSuccess, attachinga
devicectlHint()(Developer Mode / DDI / pairing guidance) on failure; simulator installs get nohint (
device.kind === 'simulator'). Deletes the dead install paths this superseded:installApp/uninstallApp/assertAppInstallationSupported(physical-device-control.ts),pushIosNotification(app-device-io.ts),installCoreDeviceApp/uninstallCoreDeviceApprepo-wide; updates the
iosSimulatorpush-notification coverage declaration to point atdeployment/runtime.test.ts. Closes #2785.Touched files: 10.
Validation
Tested SHA:
edd6977b26036afce88ec457b659bac403bb922e.deployment/runtime.test.ts27/27. Physical install/uninstall failures asserthint: /Developer Mode/against a devicectl stderr fixture; the simulator install and pushfailures now assert
hint: null(normalized hint equals the COMMAND_FAILED code default).Mutation: making the hint unconditional fails both simulator tests. A
test.eachpins theuninstall-tolerate branch through
deployAppfor CoreDevice and simulator devices.00008150-001849640CF8401C), isolated--state-dir:install com.callstack.agentdevicelab <signed Debug-iphoneos AgentDeviceTester.app> --json:success: true;--debugrequest log showsxcrun devicectl device install app --device ....CFBundleIdentifierrewritten (broken signature):error.codeCOMMAND_FAILED, messageApple app install failed: Failed to install the app on the device. (com.apple.dt.CoreDeviceError error 3002 (0xBBA)),details.stderrhas the devicectlexcerpt (
IXUserPresentableErrorDomain error 14),details.argsis the devicectl installargv, and
error.hintisIOS_DEVICECTL_DEFAULT_HINT(no classifier match).a reboot, so the Developer Mode classifier string stays fixture-proven only.
pnpm check:affected --runonedd6977b26: all static stages passed; vitest had 12 timeoutsin
platform-apple/src/runner/__tests__/*(5 s test timeout) at host load average ~450-520.These files do not import
deployment/runtime; the unchanged base6d5566359calso fails themat that load. CI is the clean run.
6d5566359c:check:affectedclean,check:production-exportsexit 0.Remaining risks