diff --git a/decisions/2026-09-11-mob-deploy-refuses-ambiguous-multi-device.md b/decisions/2026-09-11-mob-deploy-refuses-ambiguous-multi-device.md new file mode 100644 index 0000000..4ed04df --- /dev/null +++ b/decisions/2026-09-11-mob-deploy-refuses-ambiguous-multi-device.md @@ -0,0 +1,64 @@ +# mob.deploy refuses an ambiguous multi-device fan-out + +- Date: 2026-09-11 +- Status: accepted +- Linear: MOB-182 +- GitHub: mob_dev#53 + +## Context + +`mix mob.deploy` without `--device` used to target every reachable device. +When only one device was around, this was ergonomic: `mix mob.deploy` +deployed to it. When more than one was around — a laptop's own simulator +plus a teammate's paired phone, a spare emulator left running by another +worktree, a Bluetooth-linked device the author forgot about — the same +command silently fanned out to all of them. The behavior was documented +but the failure mode was invisible: the author saw their expected device +succeed and had no reason to check whether *others* also received a build. + +mob_dev already fixed the adjacent case in this class — `mix mob.deploy -d +X` where `-d` was never aliased silently deployed to every device, and +`mix mob.deploy --native ABC123` (positional-arg fumble of `--device`) +did the same. That's the same discipline as MOB-150 (deploy exits +honestly): the failure mode a caller can't see is worse than an error at +the tin. Refusing the ambiguous case moves this class of bug from +"invisible until the teammate notices" to "loud at run start." + +## Decision + +`mix mob.deploy` refuses to proceed when **two or more** devices are +reachable (after `--android` / `--ios` platform narrowing) unless the run +narrows explicitly: + +- `--device ` or `--ios-device ` — target a specific device. + Either flag disables the gate. +- `--all` — new flag. Explicit opt-in to fan out to every reachable + device. Preserves the prior default behavior for callers who wanted it. + +Single-device runs proceed exactly as before. The gate lives in a pure +predicate (`MobDev.Deployer.check_fanout_gate/2`) so the matrix is unit- +testable without adb/simctl. The mix task does one lightweight discovery +pass ahead of compile / native-build so an ambiguous run is refused before +those costs. + +## Consequences + +- Multi-device CI configurations must add `--all` (or `--device X` if they + really wanted one target). Documented in the `mix mob.deploy` help. +- Single-device dev flow (one Android emulator OR one iOS sim) is + unchanged. +- Mixed dev flow (one Android emulator AND one iOS sim, the common + parallel-platform case) now requires `--all`. This is a real ergonomic + cost — kept small by the flag being a single character (`-a` is left + free; not aliased in this change to keep the surface obvious). If daily + friction is high we can revisit an "authenticated devices" concept + (mob.exs pins the two devices you always deploy to; anything else is a + guest and requires --all). +- `mob.push` still fans out silently; the issue thread (mob_dev#53) called + it "the first fan-out hazard." That's a separate follow-up; MOB-182 is + scoped to `mob.deploy` because that's the flow that also builds and + installs, which is the more expensive mistake. +- The gate does not fire when `deploy_all` runs headlessly (some scripts + do that); those callers pass `all: true` to `deploy_all`'s underlying + discovery, which the check consults through the task's option-parsing + layer. diff --git a/lib/mix/tasks/mob.deploy.ex b/lib/mix/tasks/mob.deploy.ex index 7158f9b..9e116fb 100644 --- a/lib/mix/tasks/mob.deploy.ex +++ b/lib/mix/tasks/mob.deploy.ex @@ -24,6 +24,10 @@ defmodule Mix.Tasks.Mob.Deploy do * `--native` — build native binaries before pushing BEAMs * `--no-restart` — push BEAMs but don't restart the app * `-d`, `--device ` — target a specific device; use `mix mob.devices` to find IDs + * `--all` — allow deploying to every reachable device. Required when + two or more devices match the resolved platform set and no + `--device` / `--ios-device` filter narrows the target. Single- + device runs proceed without it. See MOB-182. * `--dist-port ` — pin the BEAM dist listen port (default: auto-allocated per device, `9100 + index`). Use to resolve EPMD collisions when multiple sims/emulators are running the same app concurrently @@ -139,12 +143,17 @@ defmodule Mix.Tasks.Mob.Deploy do * `mix mob.deploy --device NOPE` matching no device — exit 1. * `mix mob.deploy --android --native` that built the APK with no device attached — exit 0. The artifact is what was asked for. + * `mix mob.deploy` with two or more devices reachable and no `--device` / + `--ios-device` / `--all` — exit 1 before build (MOB-182). Refusing an + ambiguous fan-out is the safer default than silently deploying to a + teammate's phone; add `--all` when the fan-out is intended. `--native` fails the run when a platform you named built nothing at all, which is what a missing `sdk.dir` in `android/local.properties` produces. """ alias MobDev.Device + alias MobDev.Discovery.{Android, IOS} @switches [ native: :boolean, @@ -157,6 +166,11 @@ defmodule Mix.Tasks.Mob.Deploy do android: :boolean, ios: :boolean, device: :string, + # `--all` opts into fanning out to every reachable device when the run + # would otherwise touch two or more of them. Without it, an ambiguous + # multi-device run is refused up front (MOB-182). Single-device runs + # ignore the flag. + all: :boolean, schedulers: :integer, beam_flags: :string, # Manual overrides for the BEAM-distribution surface — useful when @@ -248,6 +262,32 @@ defmodule Mix.Tasks.Mob.Deploy do if native and :ios in platforms, do: MobDev.NativeBuild.detect_physical_ios() + # MOB-182 safety gate: refuse a fan-out to multiple devices unless the + # caller opted in with `--all` or narrowed with `--device` / `--ios-device`. + # The gate itself lives inside `MobDev.Deployer.deploy_all/1` so a device + # plugged in between here and the deploy loop can't sneak past it. That's + # the authoritative check. + # + # We ALSO run it here on a cheap pre-scan so the refusal lands before the + # compile / native-build tax rather than after. The task's `deploy_all` + # call re-verifies against fresh discovery — a device state that changes + # between the pre-scan and the deploy is caught there. + reachable_preview = discover_targeted_devices(platforms, device_id, effective_device_id) + + case MobDev.Deployer.check_fanout_gate(reachable_preview, + device: device_id, + ios_device: effective_device_id, + all: opts[:all] == true + ) do + :ok -> + :ok + + {:refuse, lines} -> + Enum.each(lines, &IO.puts/1) + emit_json(opts, [], [], [], "Refused: multiple devices without --device / --all") + Mix.raise("Refused ambiguous multi-device deploy") + end + # Validate every targeted device against the project's enabled # features (Pythonx, etc.) BEFORE we waste time on a multi-minute # native build that the device couldn't have run anyway. See @@ -321,7 +361,11 @@ defmodule Mix.Tasks.Mob.Deploy do # nil → auto-allocation (per-device port + auto-derived suffix). # Set → all targeted devices use these values verbatim. dist_port: opts[:dist_port], - node_suffix: opts[:node_suffix] + node_suffix: opts[:node_suffix], + # MOB-182: opt-in to the multi-device fan-out. `deploy_all` runs the + # authoritative refuse-if-ambiguous check on its own discovery — the + # pre-scan above is only an ergonomic early-refuse. + all: opts[:all] == true ) Enum.each(format_summary(deployed, failed, skipped, restart: restart), &IO.puts/1) @@ -433,6 +477,38 @@ defmodule Mix.Tasks.Mob.Deploy do defp names(platforms), do: platforms |> Enum.map(&"--#{&1}") |> Enum.join(", ") + # MOB-182: shape the discovered device list the same way + # `MobDev.Deployer.deploy_all/1` does (drop unauthorized Android, apply + # any `--device` / `--ios-device` filter). Returning the exact set that + # `deploy_all` would target keeps the fan-out gate honest — a mismatch + # here would either let a genuinely-ambiguous run through or refuse one + # that would have narrowed to a single device inside the deployer. + defp discover_targeted_devices(platforms, device_id, ios_device_id) do + android = + if :android in platforms do + Android.list_devices() + |> Enum.reject(&(&1.status == :unauthorized)) + |> filter_by_id(device_id) + else + [] + end + + ios = + if :ios in platforms do + IOS.list_devices() |> filter_by_id(ios_device_id || device_id) + else + [] + end + + android ++ ios + end + + defp filter_by_id(devices, nil), do: devices + + defp filter_by_id(devices, id) do + Enum.filter(devices, &Device.match_id?(&1, id)) + end + # Written to the REAL stdout, not the group leader — which under --json now # points at stderr so the progress prose gets out of the document's way. defp emit_json(opts, deployed, failed, skipped, message) do diff --git a/lib/mob_dev/deployer.ex b/lib/mob_dev/deployer.ex index 2345d3e..cde00ec 100644 --- a/lib/mob_dev/deployer.ex +++ b/lib/mob_dev/deployer.ex @@ -84,6 +84,24 @@ defmodule MobDev.Deployer do all = android ++ ios + # MOB-182: authoritative fan-out gate. `mix mob.deploy` runs the same + # check up front on a pre-scan so the refusal lands before the compile + # tax, but a device plugged in between that pre-scan and this + # discovery would sneak past it. This is the check that closes that + # window. See `check_fanout_gate/2` for the decision matrix. + case check_fanout_gate(all, + device: device_id, + ios_device: ios_device_id, + all: Keyword.get(opts, :all, false) + ) do + :ok -> + :ok + + {:refuse, lines} -> + Enum.each(lines, &IO.puts/1) + Mix.raise("Refused ambiguous multi-device deploy") + end + if all == [] do IO.puts(" #{color(:yellow)}No devices found.#{color(:reset)}") {[], [], []} @@ -200,6 +218,67 @@ defmodule MobDev.Deployer do String.contains?(pm_output, "package:#{package_name}") end + @doc """ + Refuses an ambiguous multi-device deploy. + + Fanning out to every reachable device is a safety hazard: a run that + meant to touch a laptop's own simulator can also reach a teammate's + connected phone, a spare emulator, or a Cellular-linked device the + author forgot was still paired. MOB-182 makes the ambiguity explicit + — a run with two or more reachable devices must either name one via + `--device` / `--ios-device` or opt in to the fan-out with `--all`. + + Returns `:ok` when the run may proceed, or `{:refuse, lines}` where + `lines` is the human-readable multi-line explanation. Pure so the + matrix can be tested without discovery. + """ + @spec check_fanout_gate([Device.t()], keyword()) :: :ok | {:refuse, [String.t()]} + def check_fanout_gate(devices, opts) do + device_id = opts[:device] + ios_device_id = opts[:ios_device] + all? = opts[:all] == true + + cond do + # A caller who already named a device (either flag) has been + # explicit; deploy_all narrows to that device inside + # `filter_by_device_id`. Nothing to gate. An empty string doesn't + # count as narrowing — it's what a shell script produces from an + # unset variable, and passing it through as "filter set" would + # bypass the gate silently. + nonempty_string?(device_id) or nonempty_string?(ios_device_id) -> + :ok + + all? -> + :ok + + length(devices) <= 1 -> + :ok + + true -> + {:refuse, refusal_lines(devices)} + end + end + + defp nonempty_string?(s), do: is_binary(s) and s != "" + + defp refusal_lines(devices) do + listing = + Enum.map(devices, fn %Device{} = d -> + " - #{Device.summary(d)}" + end) + + [ + "Refusing to deploy: #{length(devices)} devices reachable and no --device / --ios-device filter set.", + "Reachable devices:" + ] ++ + listing ++ + [ + "", + "Pass `--device ` (or `--ios-device `) to target one, or `--all` to fan out.", + "Run `mix mob.devices` to see the full list." + ] + end + # ── Device filtering ───────────────────────────────────────────────────────── defp filter_by_device_id(devices, nil), do: devices diff --git a/test/mob_dev/deployer_test.exs b/test/mob_dev/deployer_test.exs index cc50102..5ef57db 100644 --- a/test/mob_dev/deployer_test.exs +++ b/test/mob_dev/deployer_test.exs @@ -332,4 +332,96 @@ defmodule MobDev.DeployerTest do assert Deployer.__sqlite_nif_target__(lines) == "/data/app/x/lib/arm/libsqlite3_nif.so" end end + + # MOB-182: `mix mob.deploy` used to fan out silently to every reachable + # device when no --device / --all filter was given, so a run intended for + # the local sim could also land on a teammate's paired phone. The pure + # predicate below decides refuse-vs-proceed independent of adb / simctl. + describe "check_fanout_gate/2" do + setup do + android = %MobDev.Device{platform: :android, serial: "emulator-5554"} + ios1 = %MobDev.Device{platform: :ios, serial: "78354490-EF38-1111"} + ios2 = %MobDev.Device{platform: :ios, serial: "78354490-EF38-2222"} + {:ok, %{android: android, ios1: ios1, ios2: ios2}} + end + + test "single device runs without any flag", %{android: a} do + assert Deployer.check_fanout_gate([a], []) == :ok + end + + test "no devices reachable is not the gate's problem", %{} do + # deploy_all already prints "No devices found" and exits; the gate + # only cares about ambiguity when there are targets. + assert Deployer.check_fanout_gate([], []) == :ok + end + + test "two devices without any filter is refused", %{android: a, ios1: i} do + assert {:refuse, lines} = Deployer.check_fanout_gate([a, i], []) + joined = Enum.join(lines, "\n") + assert joined =~ "Refusing to deploy: 2 devices reachable" + assert joined =~ "--device" + assert joined =~ "--all" + end + + test "two devices with --all fans out", %{android: a, ios1: i} do + assert Deployer.check_fanout_gate([a, i], all: true) == :ok + end + + test "two devices with --device is allowed (deployer narrows further)", + %{android: a, ios1: i} do + # We pass the reachable list unfiltered; the deployer applies + # filter_by_device_id after the gate. The gate just trusts that an + # explicit --device is a real narrowing intent. + assert Deployer.check_fanout_gate([a, i], device: "emulator-5554") == :ok + end + + test "two devices with --ios-device is allowed", %{ios1: i1, ios2: i2} do + assert Deployer.check_fanout_gate([i1, i2], ios_device: "78354490-EF38-1111") == :ok + end + + test "refusal lines list every reachable device", %{android: a, ios1: i1, ios2: i2} do + {:refuse, lines} = Deployer.check_fanout_gate([a, i1, i2], []) + joined = Enum.join(lines, "\n") + assert joined =~ "emulator-5554" + assert joined =~ "78354490-EF38-1111" + assert joined =~ "78354490-EF38-2222" + end + + test "all: false is the same as omitting the flag", %{android: a, ios1: i} do + # A missing flag defaults to false in OptionParser, and an explicit + # `--no-all` is the same thing — either should refuse a multi-device run. + assert {:refuse, _} = Deployer.check_fanout_gate([a, i], all: false) + end + + # The permissive-path tests above (single-device, --all, --device) all pass + # against a trivial `def check_fanout_gate(_, _), do: :ok` — they only + # assert what the gate should NOT do. Pair each with a matching refuse on + # a bit-flipped input so a mutation that removes the gate entirely fails + # both halves. + test "paired: --all polarity", %{android: a, ios1: i} do + assert Deployer.check_fanout_gate([a, i], all: true) == :ok + assert {:refuse, _} = Deployer.check_fanout_gate([a, i], all: false) + end + + test "paired: single-vs-multi with no filter", %{android: a, ios1: i} do + assert Deployer.check_fanout_gate([a], []) == :ok + assert {:refuse, _} = Deployer.check_fanout_gate([a, i], []) + end + + test "paired: --device narrowing polarity", %{android: a, ios1: i} do + assert Deployer.check_fanout_gate([a, i], device: "emulator-5554") == :ok + assert {:refuse, _} = Deployer.check_fanout_gate([a, i], device: nil) + end + + # `--device ""` is what a shell script produces from an unset variable. + # Treating it as "filter set" (which naive `is_binary` did) would bypass + # the gate silently — the empty string never narrows the device list, + # deploy_all would then fall through to `No device matched ""` and the + # user has a green pre-check that yielded a red deploy. The gate rejects + # the empty string like the missing flag it stands in for. + test "empty string --device does not count as a filter", %{android: a, ios1: i} do + assert {:refuse, _} = Deployer.check_fanout_gate([a, i], device: "") + assert {:refuse, _} = Deployer.check_fanout_gate([a, i], ios_device: "") + end + end end