Make a dist deploy durable: hot-load and write, not one or the other (MOB-118) - #69
Conversation
`Deployer.deploy_all/1` picked one transport per device — distribution when the device answered, filesystem copy otherwise. The dist branch called `HotPush.push_all/1`, which loads modules into the running VM over RPC and never touches disk. So a dist deploy updated the running app, printed `✓ (dist, no restart)`, and left the on-disk BEAMs stale. The app reverted to the last filesystem deploy on its next restart — and `mix mob.connect` restarts the app, so connecting to inspect your change was enough to undo it. That is why this presented as intermittent "the deploy did nothing" rather than as a clear bug, and why it survived: every step reported success and there was no error to notice. `--native` was unaffected because it passes `force_fs: true` and skips dist. The two paths were never genuinely alternatives. The hot load is the latency win; the file write is what makes it survive. The dist branch now does both, calling the existing platform deploy with `restart: false` — restarting there would discard exactly the state the hot load exists to preserve. A failed write is an error even though the running app is correct at that moment, because reporting success for an app that will silently revert is the failure being fixed rather than a smaller version of it. Verified on the iOS simulator with `mix mob.attest`, which gives this an acceptance test it did not previously have: deploy, let `mix mob.connect` restart the app, attest. 67 modules, 0 stale, exit 0. The same sequence reported 12 stale before. Also improves attest's unreachable-device error, which now shows the `--node` invocation to scope a run — every discovered device is a candidate, so connecting to one of four and attesting failed with no hint about how to say which one you meant. Fixes MOB-118. MOB-161 is a duplicate of it, filed from the same symptom before I searched the tracker. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
An adversarial review found the first version turned a working deploy into a hard failure, and gave a hot load the ability to brick a device. `Discovery.IOS` finds physical iPhones by probing EPMD across the LAN, and its own comment says LAN-only devices "will fall back to dist-only in the deployer". That fallback is the invariant the first version removed: such a device answers dist, hot-loads fine, then `deploy_ios_physical` bails with "device only reachable via WiFi — use `mix mob.push` for BEAM-only updates", which the persist step wrapped as an error. Green deploy, exit 0, became red deploy, exit 1 — and the tool now contradicted its own error message. Worse, on a USB-attached iPhone the write is `xcrun devicectl ... --remove-existing-content` with no undo. A dist deploy used to be a pure in-memory hot load that could not damage the device; the first version made every one of them wipe and rewrite `Documents/otp/<app>`, so a cable knock mid-copy leaves the app unbootable with no way back but another successful deploy. The decision record claimed the only cost was "an rsync". Physical iOS is therefore excluded: it gets the hot load and a warning that the change will not survive a restart. The documented physical-iOS dist workflow is USB unplugged anyway, which is exactly the state in which the write cannot run. A `:skipped` persist no longer fails the device either. It means "app not installed for that platform", but the device just answered over dist, so it plainly is — failing there reports a device as unreached when it was reached and updated. Two more from the review. The Android persist calls `adb root`, which on a non-root adbd restarts adbd and drops every `adb forward` — including the dist tunnels an open `mix mob.connect` session runs over. Harmless when this ran only on the fallback path; now that a dist deploy persists too, it would kill the user's IEx session from another terminal. It checks `adb shell id -u` first, which is read-only. And the attest hint I had smuggled into this commit pointed at a node from the *unreachable* list, so following it was guaranteed to fail differently; it now names a node that answered, or says to connect. The three tests were source-text greps, and the review demonstrated a one-line mutation that restores the original bug while passing all of them: pass `[]` for `beam_dirs` at the call site and nothing is written, while every body those tests read stays identical. Replaced with behavioural tests over two extracted decisions — `persistable?/1` and `dist_outcome/2` — which is the pattern `categorize_results/1` already sets in this module. The decision record now says which platforms were measured and which were not, and corrects the cost claim: "milliseconds" was true only of the iOS simulator, the one platform verified. Android moves several MB and sleeps ~1.8s. Re-verified after the changes: deploy, let mix mob.connect restart the app, attest — 67 modules, 0 stale, exit 0. Refs MOB-118 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
Adversarial review done and acted on — it found that the fix broke a path that worked, which is why this is not merging as originally written. Three blockersA LAN-discovered physical iPhone went from green to exit 1. A hot load gained the ability to brick a device. On a USB-attached iPhone the write is Physical iOS is now excluded — hot load plus a warning that the change won't survive a restart. The documented physical-iOS dist workflow is USB unplugged anyway, which is exactly when the write can't run. The attest hint I smuggled in was wrong: it named a node from the unreachable list, so following it was guaranteed to fail differently. AlsoA TestsAll three were source-text greps, and the review demonstrated a one-line mutation that restores the original bug while passing them: pass VerificationBoth platforms this time, since the review's concerns were Android-specific and I had only measured the simulator:
adb forwards intact after the Android deploy (3 before, 3 after). The decision record now states which platforms were measured and corrects the cost claim — "milliseconds" was true only of the simulator. 2371 tests, credo and format clean. |
Deployer.deploy_all/1picked one transport per device — distribution when the device answered, filesystem copy otherwise. The dist branch calledHotPush.push_all/1, which loads modules into the running VM over RPC and never touches disk.So a dist deploy updated the running app, printed
✓ (dist, no restart), and left the on-disk BEAMs stale. The app reverted to the last filesystem deploy on its next restart — andmix mob.connectrestarts the app, so connecting to inspect your change was enough to undo it. That is why this presented as intermittent "the deploy did nothing" rather than a clear bug, and why it survived so long: every step reported success and there was no error to notice.--nativewas unaffected because it passesforce_fs: trueand skips dist.The two paths were never genuinely alternatives. The hot load is the latency win; the file write is what makes it survive. The dist branch now does both, calling the existing platform deploy with
restart: false— restarting there would discard exactly the state the hot load exists to preserve.A failed write is an error even though the running app is correct at that moment, because reporting success for an app that will silently revert is the failure being fixed, not a smaller version of it.
Verification
mix mob.attestgives this an acceptance test it never had — deploy, letmix mob.connectrestart the app, attest:Also improves attest's unreachable-device error, which now shows the
--nodeinvocation to scope a run: every discovered device is a candidate, so connecting to one of four and attesting failed with no hint about how to say which one you meant.2368 tests, credo and format clean, decision record included.
MOB-161 is a duplicate of this, filed from the same symptom before I searched the tracker.