Stop mob_dev force-quitting every app on an attached iPhone (MOB-70)

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.**

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.

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

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