diff --git a/CHANGELOG.md b/CHANGELOG.md index 2168aa8..074c346 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,27 @@ ## [Unreleased] +### Fixed +- **`mix mob.connect` no longer force-quits every app on an attached iPhone** + (MOB-70). Clearing other Mob apps off the device before a launch is + necessary — each one starts an in-process EPMD on `0.0.0.0:4369`, so only one + can run at a time — but the code decided what to clear by matching every + process under `Bundle/Application/`, which is where **all** third-party apps + live. Plugging in a personal iPhone and running `mix mob.connect` terminated + every app its owner had open, and the `except_bundle` argument meant to spare + the target app was discarded outright. + + Measured against an attached iPhone SE: the old code would have killed 15 + user apps, TestFlight among them. It now kills 0 unless mob_dev installed + them itself. + + mob_dev records what it installs (`MobDev.IOSInstalls`, `~/.mob/ios_installs.json`) + and kills only that. An absent or unreadable record means kill nothing. + **Behaviour change:** a Mob app installed by another route — Xcode, + TestFlight, a colleague's build — is no longer cleared, so it will still hold + EPMD 4369 and the launch will fail as it did before mob_dev cleared anything. + See `decisions/2026-09-06-mob-dev-kills-only-what-it-installed.md`. + + ### Changed - **`mix mob.deploy` rejects unrecognised options instead of ignoring them.** diff --git a/decisions/2026-09-06-mob-dev-kills-only-what-it-installed.md b/decisions/2026-09-06-mob-dev-kills-only-what-it-installed.md new file mode 100644 index 0000000..cb49b61 --- /dev/null +++ b/decisions/2026-09-06-mob-dev-kills-only-what-it-installed.md @@ -0,0 +1,77 @@ +# mob_dev kills only the apps mob_dev installed + +Date: 2026-09-06 +Status: accepted +Ticket: MOB-70 + +## Context + +Launching a Mob app on a physical iPhone first cleared other apps off the +device. The reason is real: each physical-device Mob app starts an in-process +EPMD bound to `0.0.0.0:4369` (`mob/ios/mob_beam.m`), so only one can run at a +time — a second gets `EADDRINUSE` and never boots. + +The implementation decided what to clear by pattern-matching the running +process list for `Bundle/Application/`, and terminated each match with +`devicectl ... --kill`. Every third-party iOS app runs from that path. So +plugging in a personal iPhone and running `mix mob.connect` force-quit every +app its owner had open, losing whatever in-memory state they held. The +`except_bundle` parameter that was supposed to spare the target app was +discarded outright (`_ = except_bundle`), and the function's own doc claimed it +was honoured. + +Measured on the iPhone attached while writing this: the old code would have +killed **15** user apps, TestFlight among them. + +Anchoring the match to `Bundle/Application/` removes 24 system processes from +consideration, but that is tidiness, not safety: Apple's `MobileCal.app` runs +from `Bundle/Application/` too, and app names come from +`Macro.camelize(project)`, so a project named `mobile_cal` collides exactly. +**The registry is the safety property.** The anchor only narrows what the +registry then has to be right about. + +## Decision + +**mob_dev may kill an app on an attached device only if mob_dev installed it +there.** A device is someone's phone; nothing else on it is ours to touch. + +`MobDev.IOSInstalls` records `{udid → [{bundle_id, app_name}]}` at install +time. `kill_other_user_apps_physical/2` asks it what belongs to us, excludes +the app about to be launched, and kills only what remains. The decision itself +is a pure function, `IOS.mob_pids_to_kill/3`, so the thing that was previously +untestable is now the tested part. + +**Not knowing is a reason to do nothing.** An absent, empty or corrupt registry +returns `[]`, and `[]` means kill nothing. The alternative — treating "I have +no record" as licence to clear the device — is the bug this replaces. + +Matching is on the `.app` bundle name, whole, because the process listing +carries no bundle ids. `"Demo"` therefore does not match `"DemoOne.app"`; a +short recorded name must not widen the blast radius to everything sharing its +prefix. + +## Consequences + +- A Mob app installed by some other route — Xcode, TestFlight, a colleague's + build — is no longer cleared, and will still hold EPMD 4369. The launch then + fails as it would have before mob_dev ever cleared anything. Losing an + automatic recovery is the correct trade against force-quitting a stranger's + banking app, but the failure is silent in a nasty way: `devicectl launch` + reports success, because it is the BEAM *inside* the app that dies. So an + empty record prints a warning naming EPMD 4369 as the likely cause. Left + unexplained, this would be a worse failure than the one being fixed. +- The target app is no longer `--kill`ed before launch, only + `--terminate-existing`ed by the launch itself. Killing it separately raced + the launch that immediately followed. +- Matching is by app name, so the record has to be forgotten on uninstall + (`IOSInstalls.forget/2`). A stale entry is not inert: it stays killable, and + a third-party app that later takes that name inherits it — the original bug + in miniature. +- The registry is a cache, not a source of truth. Deleting it costs a stale Mob + app surviving a launch; it never costs correctness. Losing a write never + fails the install that is happening. +- The general rule this is an instance of: **a tool operating on someone's + device kills what it created and nothing else.** The same reasoning already + applies to this project's own agents, who are told never to `pkill -f` or + `killall` and to kill only PIDs they spawned. mob_dev was doing to users' + phones precisely what we forbid ourselves to do to our machines. diff --git a/lib/mob_dev/discovery/ios.ex b/lib/mob_dev/discovery/ios.ex index 84af715..fdafcd2 100644 --- a/lib/mob_dev/discovery/ios.ex +++ b/lib/mob_dev/discovery/ios.ex @@ -534,8 +534,11 @@ defmodule MobDev.Discovery.IOS do @doc """ Restarts the app on a physical iOS device via xcrun devicectl. - Kills any other user-installed app first (they all share EPMD port 4369 and - only one can run at a time), then launches the target app fresh. + + First clears other Mob apps that `mob_dev` installed on this device — they + each hold EPMD 4369 and only one can run at a time — then launches the + target app fresh. Apps `mob_dev` did not install are never touched, whoever + they belong to. See `MobDev.IOSInstalls` and MOB-70. """ @spec restart_app_physical(String.t(), String.t()) :: {String.t(), non_neg_integer()} def restart_app_physical(udid, bundle_id) do @@ -558,25 +561,96 @@ defmodule MobDev.Discovery.IOS do ) end - # Kill any user-installed app that is not `except_bundle`. - # User apps run from /private/var/containers/Bundle/Application/. - # All physical-device Mob apps share in-process EPMD on port 4369, so only - # one can run at a time. We kill the others before launching to avoid the - # EADDRINUSE crash that would otherwise prevent BEAM from starting. + @doc """ + Process ids on the device that belong to Mob apps we may kill. + + Pure, so the decision that used to be untestable is now the testable part. + `process_output` is `devicectl device info processes` output; `ours` is what + `MobDev.IOSInstalls` says we installed on this device; `except_app_name` is + the app about to be launched, which the caller launches with + `--terminate-existing` anyway. + + Matching is on the `.app` bundle name in the executable path, because that + is the only identifier the process listing carries — it has no bundle ids. + + **Anything not in `ours` is left alone.** The bug this replaced matched every + process under `Bundle/Application/`, which is where *all* third-party apps + live, so running `mix mob.connect` with a personal iPhone attached force-quit + every app the owner had open (MOB-70). An empty `ours` returns `[]`: not + knowing what is ours means killing nothing. + """ + @spec mob_pids_to_kill(String.t(), [String.t()], String.t() | nil) :: [pos_integer()] + def mob_pids_to_kill(process_output, ours, except_app_name \\ nil) do + killable = MapSet.new(ours) |> MapSet.delete(except_app_name) + + process_output + |> String.split("\n") + |> Enum.flat_map(fn line -> + # Anchored to Bundle/Application/, which is where third-party apps live. + # Without the anchor this matches system processes too — SpringBoard, + # Preferences, Spotlight, News — and the only thing standing between us + # and killing one is that no project happens to camelize to its name. + # That is not a safety property. A project named `news` produces + # `News.app`; Apple ships `News.app` too, and `MobileCal.app` really does + # run from Bundle/Application/, so a project named `mobile_cal` would + # collide exactly. The registry is what protects the user here — the + # anchor only removes the 24 system processes that were never candidates. + case Regex.run(~r{^\s*(\d+)\s+.*/Bundle/Application/[^/]+/([^/]+)\.app/}, line) do + [_, pid_str, app_name] -> + if MapSet.member?(killable, app_name), do: [String.to_integer(pid_str)], else: [] + + _ -> + [] + end + end) + end + + @doc """ + The `.app` name for `bundle_id`, or `nil` if we have no record of it. + + Extracted so the translation is testable. Getting it wrong is not + cosmetic: returning `nil` for the app about to be launched puts that app + back in the kill set, so mob_dev `--kill`s it moments before `devicectl + launch` targets it — the exact race the caller avoids by excluding it. + """ + @spec except_app_name_for([MobDev.IOSInstalls.app()], String.t() | nil) :: String.t() | nil + def except_app_name_for(ours, bundle_id) do + Enum.find_value(ours, &if(&1.bundle_id == bundle_id, do: &1.app_name)) + end + + # Clear other Mob apps off the device before launching. + # + # Physical-device Mob apps each start an in-process EPMD on 0.0.0.0:4369 + # (mob/ios/mob_beam.m), so only one can run at a time — a second gets + # EADDRINUSE and never boots. Clearing the others is genuinely required. + # + # What is not required is guessing. See `mob_pids_to_kill/3`. defp kill_other_user_apps_physical(udid, except_bundle) do + ours = MobDev.IOSInstalls.installed(udid) + app_names = Enum.map(ours, & &1.app_name) + + # The trade this fix makes, said out loud. A Mob app installed by some + # other route — Xcode, TestFlight, a colleague's build — is no longer + # cleared, so it keeps EPMD 4369 and the incoming app's BEAM dies inside a + # launch that otherwise reports success. Left unexplained that is a worse + # failure than the one being fixed, because it is silent. + if app_names == [] do + IO.puts( + " ⚠ No record of mob_dev installs on this device — nothing was cleared.\n" <> + " If the app launches but never joins the network, another Mob app may\n" <> + " be holding EPMD 4369; quit it on the device and retry." + ) + end + + except_app_name = except_app_name_for(ours, except_bundle) + {out, 0} = System.cmd("xcrun", ["devicectl", "device", "info", "processes", "--device", udid], stderr_to_stdout: true ) out - |> String.split("\n") - |> Enum.flat_map(fn line -> - case Regex.run(Regex.compile!("^\\s*(\\d+)\\s+(.+Bundle/Application/.+\\.app/.+)$"), line) do - [_, pid_str, _path] -> [String.to_integer(pid_str)] - _ -> [] - end - end) + |> mob_pids_to_kill(app_names, except_app_name) |> Enum.each(fn pid -> System.cmd( "xcrun", @@ -595,7 +669,6 @@ defmodule MobDev.Discovery.IOS do ) end) - _ = except_bundle :ok rescue _ -> :ok diff --git a/lib/mob_dev/ios_installs.ex b/lib/mob_dev/ios_installs.ex new file mode 100644 index 0000000..d04aab9 --- /dev/null +++ b/lib/mob_dev/ios_installs.ex @@ -0,0 +1,155 @@ +defmodule MobDev.IOSInstalls do + @moduledoc """ + A record of which apps `mob_dev` has installed on which physical iOS device. + + This exists for exactly one reason: so that `mob_dev` can clear other Mob + apps off a device without touching anything else on it. + + Physical-device Mob apps each start an in-process EPMD bound to + `0.0.0.0:4369` (`mob/ios/mob_beam.m`), so only one can run at a time — + launching a second gets EADDRINUSE and no BEAM. Clearing the others before + launch is therefore necessary. What is *not* necessary, and what this module + exists to prevent, is deciding which processes to kill by pattern-matching + the running process list: every third-party app on the phone runs from + `/private/var/containers/Bundle/Application/`, so that pattern matches + Spotify, a banking app and everything else the owner had open (MOB-70). + + A device is someone's phone. `mob_dev` gets to kill what `mob_dev` put there, + and nothing else. If this record is missing or empty, the correct behaviour + is to kill nothing. + + Entries are dropped on uninstall (`forget/2`). Since the process listing + carries no bundle ids, matching is by `.app` name — so a stale entry is not + inert: it would stay killable, and a third-party app that later took that + name would inherit it. + + Stored as JSON at `~/.mob/ios_installs.json`, keyed by device UDID. It is a + cache, not a source of truth: deleting it costs a stale Mob app surviving a + launch, not correctness. + """ + + @type app :: %{bundle_id: String.t(), app_name: String.t()} + + @doc "Path to the registry file. Override with `MOB_IOS_INSTALLS` in tests." + @spec path() :: Path.t() + def path do + System.get_env("MOB_IOS_INSTALLS") || Path.expand("~/.mob/ios_installs.json") + end + + @doc """ + Record that `bundle_id` (whose bundle is `app_name`.app) is installed on + `udid`. + + Idempotent: re-installing the same app does not duplicate the entry, and a + changed `app_name` for a known bundle id replaces it rather than accumulating. + """ + @spec record(String.t(), String.t(), String.t()) :: :ok + def record(udid, bundle_id, app_name) + when is_binary(udid) and is_binary(bundle_id) and is_binary(app_name) do + all = read_all() + existing = Map.get(all, udid, []) + + updated = + [%{"bundle_id" => bundle_id, "app_name" => app_name}] ++ + Enum.reject(existing, &(&1["bundle_id"] == bundle_id)) + + write_all(Map.put(all, udid, updated)) + end + + def record(_, _, _), do: :ok + + @doc """ + Apps `mob_dev` has installed on `udid`, newest first. `[]` when unknown — + which callers must treat as "kill nothing", not "kill everything". + """ + @spec installed(String.t()) :: [app()] + def installed(udid) when is_binary(udid) do + all = read_all() + + # Shape-wrong-but-valid JSON is a different failure from unparseable JSON, + # and `Map.get/3` happily returns a binary that `Enum.flat_map/2` then + # raises on. Both mean the same thing here: we do not know what is ours. + entries = + case Map.get(all, udid, []) do + list when is_list(list) -> list + _ -> [] + end + + entries + |> Enum.flat_map(fn + %{"bundle_id" => b, "app_name" => n} when is_binary(b) and is_binary(n) -> + [%{bundle_id: b, app_name: n}] + + _ -> + [] + end) + end + + def installed(_), do: [] + + @doc """ + Drop `bundle_id` from `udid`'s record, after uninstalling it. + + Matching is by app NAME, so a stale entry is not inert: it stays killable + for ever, and a third-party app that later takes that name inherits the + entry. Forgetting on uninstall is what makes the module's promise — we kill + what we put there — true rather than approximately true. + """ + @spec forget(String.t(), String.t()) :: :ok + def forget(udid, bundle_id) when is_binary(udid) and is_binary(bundle_id) do + all = read_all() + + case Map.get(all, udid) do + list when is_list(list) -> + write_all(Map.put(all, udid, Enum.reject(list, &(&1["bundle_id"] == bundle_id)))) + + _ -> + :ok + end + end + + def forget(_, _), do: :ok + + # A corrupt or unreadable registry is indistinguishable from an absent one, + # and both mean the same thing to every caller: we do not know what is ours, + # so we touch nothing. + defp read_all do + with {:ok, body} <- File.read(path()), + {:ok, %{} = decoded} <- decode(body) do + decoded + else + _ -> %{} + end + end + + defp decode(body) do + case :json.decode(body) do + %{} = m -> {:ok, m} + _ -> :error + end + rescue + _ -> :error + end + + defp write_all(map) do + file = path() + File.mkdir_p!(Path.dirname(file)) + + # Write-then-rename. Two `mix mob.deploy` runs against different devices is + # normal here, and `File.write!` is not atomic — a concurrent reader can + # otherwise observe a half-written file, fail to decode, and treat the + # whole registry as absent. Rename is atomic within a filesystem. + # Unique per write. A shared `.tmp` defeats the point: two concurrent + # deploys write the same path, and one can rename while the other is + # mid-write — publishing a torn file atomically, which is worse than the + # torn read this was meant to prevent. + tmp = file <> ".tmp." <> Integer.to_string(System.unique_integer([:positive])) + File.write!(tmp, :json.encode(map) |> IO.iodata_to_binary()) + File.rename!(tmp, file) + :ok + rescue + # Losing the record costs a stale app surviving a later launch. It must + # never cost the deploy that is happening now. + _ -> :ok + end +end diff --git a/lib/mob_dev/native_build.ex b/lib/mob_dev/native_build.ex index 6e8941b..272f047 100644 --- a/lib/mob_dev/native_build.ex +++ b/lib/mob_dev/native_build.ex @@ -6117,11 +6117,36 @@ defmodule MobDev.NativeBuild do stderr_to_stdout: true, into: IO.stream() ) do - {_, 0} -> :ok - {_, code} -> {:error, "devicectl install failed (exit #{code}) — check output above"} + {_, 0} -> + # Record what we put on this device, so a later launch can clear other + # Mob apps off EPMD 4369 without guessing from the process list — the + # guess used to match every third-party app on the phone (MOB-70). + MobDev.IOSInstalls.record(udid, bundle_id_for(app_path), app_name_for(app_path)) + :ok + + {_, code} -> + {:error, "devicectl install failed (exit #{code}) — check output above"} end end + @doc false + # The .app bundle name is what a running process exposes in its executable + # path; the bundle id is what we address the app by. The registry needs both + # to translate between them. + # + # Public-but-@doc false so the invariant these rest on is testable: the + # bundle directory basename equals the executable name equals + # `ios_display_name/0`, by construction (the binary is copied to + # `.app/` and CFBundleExecutable is set to the same). If that + # ever stops holding, the registry records a name that never matches a + # running process and the kill scope silently becomes empty. + @spec app_name_for(Path.t()) :: String.t() + def app_name_for(app_path), do: Path.basename(app_path, ".app") + + @doc false + @spec bundle_id_for(Path.t()) :: String.t() + def bundle_id_for(_app_path), do: MobDev.Config.ios_bundle_id() + defp throw_bundle_id_error, do: throw({:error, "bundle_id not set in mob.exs"}) diff --git a/lib/mob_dev/uninstaller.ex b/lib/mob_dev/uninstaller.ex index f9d1f14..895f416 100644 --- a/lib/mob_dev/uninstaller.ex +++ b/lib/mob_dev/uninstaller.ex @@ -482,6 +482,13 @@ defmodule MobDev.Uninstaller do {output, exit_code} = System.cmd("xcrun", args, stderr_to_stdout: true) {outcome, reason} = interpret_devicectl_uninstall(output, exit_code) + + # Stop claiming this app is ours. The kill-scope registry matches by app + # NAME (the process listing carries no bundle ids), so a stale entry is not + # inert — it stays killable for ever, and a third-party app that later + # takes that name inherits it. See MOB-70. + if outcome == :uninstalled, do: MobDev.IOSInstalls.forget(udid, bundle_id) + %{device: d, bundle_id: bundle_id, outcome: outcome, reason: reason} end diff --git a/test/mob_dev/discovery/ios_kill_scope_test.exs b/test/mob_dev/discovery/ios_kill_scope_test.exs new file mode 100644 index 0000000..f3e5c6c --- /dev/null +++ b/test/mob_dev/discovery/ios_kill_scope_test.exs @@ -0,0 +1,115 @@ +defmodule MobDev.Discovery.IOSKillScopeTest do + @moduledoc """ + Guards MOB-70: `mix mob.connect` with a personal iPhone attached force-quit + every third-party app on the phone. + + The old code matched any process under `Bundle/Application/` and killed it. + Every user app lives there, so a banking app, a messaging app and whatever + else the owner had open were all terminated to free EPMD 4369. + """ + use ExUnit.Case, async: true + + alias MobDev.Discovery.IOS + + # Shaped exactly like `xcrun devicectl device info processes` output, which + # is columnar: pid, whitespace, absolute executable path. + @processes """ + 412 /usr/libexec/backboardd + 7911 /private/var/containers/Bundle/Application/353039DD/DemoOne.app/DemoOne + 7914 /private/var/containers/Bundle/Application/89670CCD/DemoTwo.app/DemoTwo + 7919 /private/var/containers/Bundle/Application/2DE7229A/Ledger Pro.app/Ledger Pro + 7932 /private/var/containers/Bundle/Application/79CF638A/Chatterbox.app/Chatterbox + 7940 /private/var/containers/Bundle/Application/AABBCCDD/TestFlight.app/TestFlight + 35 /System/Library/CoreServices/SpringBoard.app/SpringBoard + 36 /Applications/News.app/News + """ + + describe "only our own apps are killable" do + test "a third-party app is never selected, however it is spelled" do + # The bug in one line: these are the apps that were being killed. + pids = IOS.mob_pids_to_kill(@processes, ["DemoOne", "DemoTwo"], nil) + + assert pids == [7911, 7914] + + refute 7919 in pids, "killed a third-party app (name contains a space)" + refute 7932 in pids, "killed a third-party app" + refute 7940 in pids, "killed TestFlight" + end + + test "an unknown device kills nothing rather than everything" do + # The whole point. No record of what we installed is not licence to + # clear the phone; it is the reason to touch none of it. + assert IOS.mob_pids_to_kill(@processes, [], nil) == [] + end + + test "the app being launched is left alone" do + # The caller launches it with --terminate-existing straight after, so + # killing it here is redundant, and racing that is how you get a launch + # into a process that is still dying. + pids = IOS.mob_pids_to_kill(@processes, ["DemoOne", "DemoTwo"], "DemoOne") + + assert pids == [7914] + refute 7911 in pids + end + + test "a recorded app that is not running contributes no pid" do + assert IOS.mob_pids_to_kill(@processes, ["NotRunning"], nil) == [] + end + + test "system processes are never matched, even on an exact name collision" do + # The dangerous case, and the reason the pattern is anchored to + # Bundle/Application/. `ios_display_name/0` is Macro.camelize of the + # project name, so a project called :news or :springboard produces + # exactly the bundle name Apple ships. Name equality alone is not a + # safety property. + refute 412 in IOS.mob_pids_to_kill(@processes, ["backboardd", "DemoOne"], nil) + assert IOS.mob_pids_to_kill(@processes, ["SpringBoard"], nil) == [] + assert IOS.mob_pids_to_kill(@processes, ["News"], nil) == [] + end + end + + describe "parsing" do + test "an app name is matched whole, not as a substring" do + # "Demo" must not match "DemoOne.app" — otherwise a short recorded name + # silently widens the blast radius to every app sharing its prefix. + assert IOS.mob_pids_to_kill(@processes, ["Demo"], nil) == [] + end + + test "empty and malformed output are handled without raising" do + assert IOS.mob_pids_to_kill("", ["DemoOne"], nil) == [] + assert IOS.mob_pids_to_kill("garbage\n\n \n", ["DemoOne"], nil) == [] + end + end + + describe "which app is spared" do + @ours [ + %{bundle_id: "com.example.one", app_name: "DemoOne"}, + %{bundle_id: "com.example.two", app_name: "DemoTwo"} + ] + + test "the launching app's bundle id resolves to its .app name" do + assert IOS.except_app_name_for(@ours, "com.example.one") == "DemoOne" + end + + test "an unknown bundle id spares nothing" do + assert IOS.except_app_name_for(@ours, "com.example.other") == nil + assert IOS.except_app_name_for(@ours, nil) == nil + assert IOS.except_app_name_for([], "com.example.one") == nil + end + + test "matching on the wrong field would kill the app we are about to launch" do + # Guards the mutation: comparing app_name instead of bundle_id returns + # nil here, which puts the target app back in the kill set — mob_dev + # then --kills it microseconds before devicectl launch targets it. + refute IOS.except_app_name_for(@ours, "DemoOne") == "DemoOne" + assert IOS.except_app_name_for(@ours, "DemoOne") == nil + end + + test "end to end: the launching app survives, its sibling does not" do + except = IOS.except_app_name_for(@ours, "com.example.one") + pids = IOS.mob_pids_to_kill(@processes, ["DemoOne", "DemoTwo"], except) + + assert pids == [7914] + end + end +end diff --git a/test/mob_dev/ios_install_record_test.exs b/test/mob_dev/ios_install_record_test.exs new file mode 100644 index 0000000..7ee7451 --- /dev/null +++ b/test/mob_dev/ios_install_record_test.exs @@ -0,0 +1,67 @@ +defmodule MobDev.IOSInstallRecordTest do + @moduledoc """ + Pins the invariant the MOB-70 kill scope rests on. + + The registry translates between a bundle id (how we address an app) and a + `.app` name (all a running process exposes). If those two helpers ever + disagree with how the app is actually built, the registry records a name that + never matches a running process — the kill scope silently becomes empty, and + every second Mob app on a device fails to boot with no diagnostic. Nothing + crashes; it just stops working. + """ + use ExUnit.Case, async: false + + alias MobDev.NativeBuild + + test "the recorded app name is the .app bundle basename" do + # The build copies the binary to `.app/` and sets + # CFBundleExecutable to the same, so bundle dir basename == executable + # name == what shows up in `devicectl device info processes`. + assert NativeBuild.app_name_for("/tmp/build/Demo.app") == "Demo" + assert NativeBuild.app_name_for("Demo.app") == "Demo" + assert NativeBuild.app_name_for("/a/b/My App.app") == "My App" + end + + test "the recorded bundle id is the iOS one, where it differs from the Android one" do + # Must be ios_bundle_id/0, not bundle_id/0. Apple forbids underscores and a + # com.example.* id is often already claimed by another team, so the two + # legitimately differ — and recording the Android id would make the entry + # unmatchable when the launcher later looks the app up by its iOS id. + # + # Asserted against a config where they actually differ, because in a + # project where they happen to coincide this test cannot fail and would be + # evidence of nothing. + dir = Path.join(System.tmp_dir!(), "mob70_cfg_#{System.unique_integer([:positive])}") + File.mkdir_p!(dir) + + File.write!(Path.join(dir, "mob.exs"), """ + import Config + config :mob_dev, bundle_id: "com.example.android_id", ios_bundle_id: "com.example.iosid" + """) + + original = File.cwd!() + + try do + File.cd!(dir) + assert MobDev.Config.bundle_id() == "com.example.android_id" + assert MobDev.Config.ios_bundle_id() == "com.example.iosid" + assert NativeBuild.bundle_id_for("/tmp/build/Demo.app") == "com.example.iosid" + after + File.cd!(original) + File.rm_rf(dir) + end + end + + test "an app name round-trips through the registry into a kill decision" do + # The whole chain in one assertion: what install records is what the + # process listing is matched against. + app_path = "/tmp/build/Demo.app" + name = NativeBuild.app_name_for(app_path) + + processes = """ + 7911 /private/var/containers/Bundle/Application/AAAA/#{name}.app/#{name} + """ + + assert MobDev.Discovery.IOS.mob_pids_to_kill(processes, [name], nil) == [7911] + end +end diff --git a/test/mob_dev/ios_install_wiring_test.exs b/test/mob_dev/ios_install_wiring_test.exs new file mode 100644 index 0000000..a88ec0a --- /dev/null +++ b/test/mob_dev/ios_install_wiring_test.exs @@ -0,0 +1,73 @@ +defmodule MobDev.IOSInstallWiringTest do + @moduledoc """ + Pins the two call sites the MOB-70 kill scope depends on. + + Source-asserted, following the convention this repo already uses for + behaviour that cannot be executed in a test: both sites live inside + functions that shell out to `xcrun devicectl` against a physical device. + + This exists because the first attempt at pinning them did not. Helper + functions were made public and tested, which felt like coverage — but + deleting the line that *calls* them left the whole suite green and the + feature silently disabled: the registry stays empty for ever, so nothing is + ever recognised as ours, so nothing is ever cleared off EPMD 4369, and every + second Mob app on a device fails to boot. Nothing crashes. It just stops + working, which is exactly the failure mode MOB-70's fix is supposed to make + impossible. + """ + use ExUnit.Case, async: true + + @native_build Path.expand("../../lib/mob_dev/native_build.ex", __DIR__) + @uninstaller Path.expand("../../lib/mob_dev/uninstaller.ex", __DIR__) + + defp code_only(path) do + path + |> File.read!() + |> String.split("\n") + |> Enum.map_join("\n", &Regex.replace(~r|^\s*#.*$|, &1, "")) + end + + test "a successful physical install records the app" do + body = function_body(code_only(@native_build), "defp devicectl_install(") + + assert body =~ "MobDev.IOSInstalls.record(", + """ + devicectl_install/2 no longer records what it installed. + + The kill scope reads that registry to decide what is ours. Without + the record it stays empty, nothing is ever recognised, and other Mob + apps are never cleared off EPMD 4369 — so the second app on a device + never boots, with no error anywhere. + """ + end + + test "a successful physical uninstall forgets the app" do + body = + function_body( + code_only(@uninstaller), + "defp uninstall_one(%Device{platform: :ios, type: :physical" + ) + + assert body =~ "MobDev.IOSInstalls.forget(", + """ + The physical-device uninstall path no longer forgets the app. + + Matching is by app NAME, so a stale entry is not inert: it stays + killable for ever, and a third-party app that later takes that name + inherits it — MOB-70 in miniature. + """ + end + + test "the record is keyed on the iOS bundle id, not the Android one" do + # They legitimately differ (Apple forbids underscores), and the launcher + # looks the app up by its iOS id. Recording the other one makes every + # entry unmatchable. + assert code_only(@native_build) =~ + "def bundle_id_for(_app_path), do: MobDev.Config.ios_bundle_id()" + end + + defp function_body(src, marker) do + [_, rest] = String.split(src, marker, parts: 2) + rest |> String.split("\n end", parts: 2) |> hd() + end +end diff --git a/test/mob_dev/ios_installs_test.exs b/test/mob_dev/ios_installs_test.exs new file mode 100644 index 0000000..625ced6 --- /dev/null +++ b/test/mob_dev/ios_installs_test.exs @@ -0,0 +1,110 @@ +defmodule MobDev.IOSInstallsTest do + @moduledoc """ + The registry behind MOB-70. Its only job is to answer "did we put this here", + and the answer when it does not know must be "no". + """ + use ExUnit.Case, async: false + + alias MobDev.IOSInstalls + + setup do + file = Path.join(System.tmp_dir!(), "ios_installs_#{System.unique_integer([:positive])}.json") + System.put_env("MOB_IOS_INSTALLS", file) + + on_exit(fn -> + File.rm(file) + System.delete_env("MOB_IOS_INSTALLS") + end) + + {:ok, registry: file} + end + + test "records and reads back an install" do + :ok = IOSInstalls.record("UDID-1", "com.example.demo", "Demo") + + assert [%{bundle_id: "com.example.demo", app_name: "Demo"}] = IOSInstalls.installed("UDID-1") + end + + test "an unknown device returns [] — callers read that as 'kill nothing'" do + assert IOSInstalls.installed("NEVER-SEEN") == [] + end + + test "re-installing the same app does not duplicate it" do + :ok = IOSInstalls.record("UDID-1", "com.example.demo", "Demo") + :ok = IOSInstalls.record("UDID-1", "com.example.demo", "Demo") + + assert length(IOSInstalls.installed("UDID-1")) == 1 + end + + test "a renamed bundle replaces its entry rather than accumulating" do + :ok = IOSInstalls.record("UDID-1", "com.example.demo", "OldName") + :ok = IOSInstalls.record("UDID-1", "com.example.demo", "NewName") + + assert [%{app_name: "NewName"}] = IOSInstalls.installed("UDID-1") + end + + test "devices are kept apart" do + :ok = IOSInstalls.record("UDID-1", "com.example.one", "One") + :ok = IOSInstalls.record("UDID-2", "com.example.two", "Two") + + assert [%{app_name: "One"}] = IOSInstalls.installed("UDID-1") + assert [%{app_name: "Two"}] = IOSInstalls.installed("UDID-2") + end + + test "a corrupt registry reads as empty, not as a crash", %{registry: file} do + File.mkdir_p!(Path.dirname(file)) + File.write!(file, "{ this is not json") + + assert IOSInstalls.installed("UDID-1") == [] + end + + test "an unwritable location does not take down the install that is happening" do + # Losing the record costs a stale app surviving a later launch. It must + # never cost the deploy in progress. + System.put_env("MOB_IOS_INSTALLS", "/proc/nonexistent/nope.json") + + assert IOSInstalls.record("UDID-1", "com.example.demo", "Demo") == :ok + end + + describe "forgetting" do + test "an uninstalled app stops being killable" do + # Matching is by app NAME, so a stale entry is not inert: it stays + # killable for ever and a third-party app that later takes that name + # inherits it. + :ok = IOSInstalls.record("UDID-1", "com.example.demo", "Demo") + :ok = IOSInstalls.forget("UDID-1", "com.example.demo") + + assert IOSInstalls.installed("UDID-1") == [] + end + + test "forgetting one app leaves the others" do + :ok = IOSInstalls.record("UDID-1", "com.example.one", "One") + :ok = IOSInstalls.record("UDID-1", "com.example.two", "Two") + :ok = IOSInstalls.forget("UDID-1", "com.example.one") + + assert [%{app_name: "Two"}] = IOSInstalls.installed("UDID-1") + end + + test "forgetting something unknown is not an error" do + assert IOSInstalls.forget("NEVER-SEEN", "com.example.demo") == :ok + end + end + + describe "malformed registries" do + test "shape-wrong but valid JSON reads as empty rather than raising", %{registry: file} do + # Map.get/3 will happily hand back a binary, which Enum.flat_map/2 then + # raises on. Syntactic validity is not the same as usable shape. + File.mkdir_p!(Path.dirname(file)) + File.write!(file, ~s({"UDID-1":"oops"})) + + assert IOSInstalls.installed("UDID-1") == [] + end + + test "a JSON array instead of an object reads as empty", %{registry: file} do + File.mkdir_p!(Path.dirname(file)) + File.write!(file, ~s(["not a map"])) + + assert IOSInstalls.installed("UDID-1") == [] + end + end +end