From c0350a4a2f16f02ccb49305c15782108d7df64b4 Mon Sep 17 00:00:00 2001 From: GenericJam Date: Sat, 5 Sep 2026 08:52:35 -0600 Subject: [PATCH 1/2] Make mob.uninstall actually honour :ios_bundle_id, and stop the deploy error lying MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Both found by an adversarial review of a documentation PR — the docs claimed these behaved correctly, and checking the claims found that they did not. Two defects I shipped, in the two commits before this one. `mix mob.uninstall` resolved one bundle id for both platforms in the task and passed it down as `project_bundle_id`. `resolve_apps_for_device/3` then did `opts[:project_bundle_id] || default_bundle_id(device)`, so the `||` short-circuited and the per-platform clause — the entire point of the change — was dead code on every real invocation. On a device holding the app under a different `:ios_bundle_id`, uninstall removed nothing and reported success. The test could not see it: it passed `[]` for opts, which is not the shape the task sends. Resolving the id now happens where the device is known, the task passes only whether it is in a Mix project, and the tests use the task's own opts. One of them asserts the two shapes still agree, since that mismatch is what hid the bug rather than the logic itself. Separately, `mix mob.deploy`'s invalid-option error advised that a value beginning with `-` needs the equals form. That was true until `join_dashed_values/1` landed in the same commit as the advice and made the spaced form work. Telling someone their working spelling is wrong is a small version of the thing this whole run has been about. Co-Authored-By: Claude Opus 5 (1M context) --- CHANGELOG.md | 10 ++++++++++ lib/mix/tasks/mob.deploy.ex | 3 +-- lib/mix/tasks/mob.uninstall.ex | 15 +++++++++------ lib/mob_dev/uninstaller.ex | 11 +++++++---- test/mix/tasks/mob_deploy_parsing_test.exs | 10 ++++++++-- test/mob_dev/config_test.exs | 17 +++++++++++++++-- test/mob_dev/wiring_test.exs | 15 +++++++++++++++ 7 files changed, 65 insertions(+), 16 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 7a9881a..a327255 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -21,6 +21,16 @@ Full module documentation: [hexdocs.pm/mob_dev](https://hexdocs.pm/mob_dev). ### Fixed +- **`mix mob.uninstall` ignored `:ios_bundle_id`.** The task resolved one + bundle id for both platforms and passed it down, so the per-platform clause + that was supposed to pick `:ios_bundle_id` for an iOS device never ran. On a + device holding the app under a different iOS id, uninstall removed nothing + and reported success. The accompanying test passed an opts shape the task + never sends, which is why the suite stayed green. +- **`mix mob.deploy`'s error told you the equals form was required for + dash-prefixed values.** It stopped being true in the same commit that wrote + it, once the spaced form was handled. + - **`--beam-flags "-S 4:4 -A 4"` aborted the deploy.** `OptionParser` will not consume a dash-prefixed argument as a string value, so the spelling this repo prints in seven places — the moduledoc, five README recipes and both diff --git a/lib/mix/tasks/mob.deploy.ex b/lib/mix/tasks/mob.deploy.ex index 9d2e2f3..53c073f 100644 --- a/lib/mix/tasks/mob.deploy.ex +++ b/lib/mix/tasks/mob.deploy.ex @@ -495,8 +495,7 @@ defmodule Mix.Tasks.Mob.Deploy do end) "Unrecognized or invalid option(s): #{names}\n\n" <> - "Run `mix help mob.deploy` for the accepted options.\n" <> - "A value beginning with `-` needs the equals form: --beam-flags=\"-S 4:4\"." + "Run `mix help mob.deploy` for the accepted options." end @doc """ diff --git a/lib/mix/tasks/mob.uninstall.ex b/lib/mix/tasks/mob.uninstall.ex index d2839e7..8b78a15 100644 --- a/lib/mix/tasks/mob.uninstall.ex +++ b/lib/mix/tasks/mob.uninstall.ex @@ -104,11 +104,14 @@ defmodule Mix.Tasks.Mob.Uninstall do {opts, _positional, _} = OptionParser.parse(args, strict: @switches, aliases: @aliases) device_ids = Keyword.get_values(opts, :device) - project_bundle_id = - case Mix.Project.get() do - nil -> nil - _ -> MobDev.Config.bundle_id() - end + # Whether we are in a Mix project — NOT which id to use. Resolving that + # here picked one id for both platforms, and the `||` in + # `resolve_apps_for_device/3` then short-circuited past the per-platform + # clause, so the iOS branch was dead on every real invocation and + # `mix mob.uninstall` removed nothing on a device holding the app under + # `:ios_bundle_id`. The test that was supposed to cover it passed `[]` for + # opts, which is not what this task passes. + in_project? = Mix.Project.get() != nil uninstaller_opts = [ device_ids: device_ids, @@ -117,7 +120,7 @@ defmodule Mix.Tasks.Mob.Uninstall do bundle_id: opts[:bundle_id], all_apps: Keyword.get(opts, :all_apps, false), bundle_prefix: opts[:bundle_prefix], - project_bundle_id: project_bundle_id + in_project: in_project? ] case Uninstaller.plan(uninstaller_opts) do diff --git a/lib/mob_dev/uninstaller.ex b/lib/mob_dev/uninstaller.ex index 64cb074..f9d1f14 100644 --- a/lib/mob_dev/uninstaller.ex +++ b/lib/mob_dev/uninstaller.ex @@ -407,11 +407,14 @@ defmodule MobDev.Uninstaller do opts[:all_apps] -> list_matching_packages(device, bundle_prefix) + opts[:in_project] -> + # Per platform, resolved HERE because only here is the device known. + # An iOS device holds the app under `:ios_bundle_id`; uninstalling by + # the Android applicationId removes nothing and reports success. + [default_bundle_id(device)] + true -> - # Per platform: an iOS device holds the app under `ios_bundle_id`, so - # uninstalling by the generic id silently removes nothing and reports - # success. Same defect class as the deploy path this release fixed. - [opts[:project_bundle_id] || default_bundle_id(device)] + [] end end diff --git a/test/mix/tasks/mob_deploy_parsing_test.exs b/test/mix/tasks/mob_deploy_parsing_test.exs index ed11d26..63dd568 100644 --- a/test/mix/tasks/mob_deploy_parsing_test.exs +++ b/test/mix/tasks/mob_deploy_parsing_test.exs @@ -85,8 +85,14 @@ defmodule Mix.Tasks.Mob.DeployParsingTest do assert message =~ "Unrecognized or invalid" end - test "the message points at the equals form for dashed values" do - assert Deploy.invalid_options_message([{"--beam-flags", nil}]) =~ "--beam-flags=" + test "the message does not claim the equals form is required" do + # It was, before `join_dashed_values/1` landed in the same commit that + # wrote this advice. Telling someone their working spelling is wrong is + # its own small version of a tool that lies. + message = Deploy.invalid_options_message([{"--beam-flags", nil}]) + + refute message =~ "equals form" + assert message =~ "mix help mob.deploy" end end end diff --git a/test/mob_dev/config_test.exs b/test/mob_dev/config_test.exs index 276ae14..99acd31 100644 --- a/test/mob_dev/config_test.exs +++ b/test/mob_dev/config_test.exs @@ -97,20 +97,33 @@ defmodule MobDev.ConfigTest do :ok end + # `[]` is not what the task passes, and that is why this suite stayed green + # while the feature did not work: `mix mob.uninstall` sent a + # `project_bundle_id` it had already resolved, the `||` short-circuited, + # and the per-platform clause was dead on every real invocation. These now + # use the task's own opts shape. + @task_opts [in_project: true] + test "an iOS device resolves the iOS id" do device = %MobDev.Device{name: "iPhone", serial: "udid", platform: :ios} - assert MobDev.Uninstaller.resolve_apps_for_device(device, [], "com.") == + assert MobDev.Uninstaller.resolve_apps_for_device(device, @task_opts, "com.") == ["com.genericjam.mishkamob"] end test "an Android device keeps the applicationId" do device = %MobDev.Device{name: "emu", serial: "emulator-5554", platform: :android} - assert MobDev.Uninstaller.resolve_apps_for_device(device, [], "com.") == + assert MobDev.Uninstaller.resolve_apps_for_device(device, @task_opts, "com.") == ["com.example.mishka_mob"] end + test "outside a Mix project there is no project app to uninstall" do + device = %MobDev.Device{name: "iPhone", serial: "udid", platform: :ios} + + assert MobDev.Uninstaller.resolve_apps_for_device(device, [], "com.") == [] + end + test "an explicit --bundle-id still wins" do device = %MobDev.Device{name: "iPhone", serial: "udid", platform: :ios} diff --git a/test/mob_dev/wiring_test.exs b/test/mob_dev/wiring_test.exs index b9e9f04..ac3cb15 100644 --- a/test/mob_dev/wiring_test.exs +++ b/test/mob_dev/wiring_test.exs @@ -211,4 +211,19 @@ defmodule MobDev.WiringTest do ~r/unless invalid == \[\] do\s*Mix\.raise\(invalid_options_message\(invalid\)\)/ end end + + describe "mob.uninstall resolves the bundle id where the device is known" do + @uninstall_task File.read!(Path.expand("../../lib/mix/tasks/mob.uninstall.ex", __DIR__)) + + test "the task passes whether it is in a project, not which id to use" do + # The gap that hid the bug: the task resolved one id for both platforms + # and passed it down, so the `||` in resolve_apps_for_device/3 + # short-circuited past the per-platform clause. The unit test passed an + # opts shape the task never sends, so it could not see any of that. + assert @uninstall_task =~ "in_project: in_project?" + + refute @uninstall_task =~ "project_bundle_id:", + "resolving the id in the task is what made the iOS branch dead code" + end + end end From d762639fecd438601e986a793f119334261fd989 Mon Sep 17 00:00:00 2001 From: GenericJam Date: Sat, 5 Sep 2026 08:59:42 -0600 Subject: [PATCH 2/2] Answer --help instead of refusing it MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Strict option parsing turned `mix mob.deploy --help` into `Unrecognized or invalid option(s): --help` — the flag people try first, on the task they run most, telling them to go and read the help. `mix mob.mutate` shipped with the same hole a day later. `mix mob.uninstall` had already solved this: `MobDev.TaskHelp` exists for it, and its moduledoc explains why (Mix has `mix help `, but people from outside the Elixir ecosystem expect ` --help` to work). Both tasks now use it. Found by an adversarial review of the documentation PR, which noticed that a README paragraph celebrating the new refusal did not mention that it also refused --help. Co-Authored-By: Claude Opus 5 (1M context) --- CHANGELOG.md | 5 +++++ lib/mix/tasks/mob.deploy.ex | 8 ++++++++ lib/mix/tasks/mob.mutate.ex | 8 ++++++++ test/mix/tasks/mob_deploy_parsing_test.exs | 8 ++++++++ test/mob_dev/wiring_test.exs | 13 +++++++++++++ 5 files changed, 42 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index a327255..06cdcae 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -21,6 +21,11 @@ Full module documentation: [hexdocs.pm/mob_dev](https://hexdocs.pm/mob_dev). ### Fixed +- **`mix mob.deploy --help` and `mix mob.mutate --help` refused to help.** + Strict option parsing turned the flag people try first, on the task they run + most, into `Unrecognized or invalid option(s): --help` — telling the reader + to go and read the help. Both now defer to `MobDev.TaskHelp`, as + `mix mob.uninstall` already did. - **`mix mob.uninstall` ignored `:ios_bundle_id`.** The task resolved one bundle id for both platforms and passed it down, so the per-platform clause that was supposed to pick `:ios_bundle_id` for an iOS device never ran. On a diff --git a/lib/mix/tasks/mob.deploy.ex b/lib/mix/tasks/mob.deploy.ex index 53c073f..7158f9b 100644 --- a/lib/mix/tasks/mob.deploy.ex +++ b/lib/mix/tasks/mob.deploy.ex @@ -179,6 +179,14 @@ defmodule Mix.Tasks.Mob.Deploy do @impl Mix.Task def run(args) do + if MobDev.TaskHelp.help_requested?(args) do + MobDev.TaskHelp.print_module_help(__MODULE__) + else + do_run(args) + end + end + + defp do_run(args) do {opts, argv, invalid} = args |> join_dashed_values() diff --git a/lib/mix/tasks/mob.mutate.ex b/lib/mix/tasks/mob.mutate.ex index c00c75b..c0eaa25 100644 --- a/lib/mix/tasks/mob.mutate.ex +++ b/lib/mix/tasks/mob.mutate.ex @@ -60,6 +60,14 @@ defmodule Mix.Tasks.Mob.Mutate do @impl Mix.Task def run(args) do + if MobDev.TaskHelp.help_requested?(args) do + MobDev.TaskHelp.print_module_help(__MODULE__) + else + do_run(args) + end + end + + defp do_run(args) do {opts, _argv, invalid} = OptionParser.parse(args, strict: @switches) unless invalid == [] do diff --git a/test/mix/tasks/mob_deploy_parsing_test.exs b/test/mix/tasks/mob_deploy_parsing_test.exs index 63dd568..d4fab47 100644 --- a/test/mix/tasks/mob_deploy_parsing_test.exs +++ b/test/mix/tasks/mob_deploy_parsing_test.exs @@ -95,4 +95,12 @@ defmodule Mix.Tasks.Mob.DeployParsingTest do assert message =~ "mix help mob.deploy" end end + + describe "--help" do + test "the helper recognises both spellings" do + assert MobDev.TaskHelp.help_requested?(["--help"]) + assert MobDev.TaskHelp.help_requested?(["-h"]) + refute MobDev.TaskHelp.help_requested?(["--device", "x"]) + end + end end diff --git a/test/mob_dev/wiring_test.exs b/test/mob_dev/wiring_test.exs index ac3cb15..f68e2e9 100644 --- a/test/mob_dev/wiring_test.exs +++ b/test/mob_dev/wiring_test.exs @@ -226,4 +226,17 @@ defmodule MobDev.WiringTest do "resolving the id in the task is what made the iOS branch dead code" end end + + describe "--help is answered, not refused" do + # Strict parsing made `mix mob.deploy --help` fail with "Unrecognized or + # invalid option(s): --help" — the flag people try first, on the task they + # run most, telling them to go read the help. `mob.uninstall` had already + # solved this with MobDev.TaskHelp; deploy and mutate had not. + for task <- ~w(mob.deploy mob.mutate) do + test "#{task} defers to MobDev.TaskHelp" do + path = Path.expand("../../lib/mix/tasks/#{unquote(task)}.ex", __DIR__) + assert File.read!(path) =~ "MobDev.TaskHelp.help_requested?(args)" + end + end + end end