From beab5cbb9719261952964c8ba6b9bcc250e0cd81 Mon Sep 17 00:00:00 2001 From: GenericJam Date: Fri, 28 Aug 2026 14:31:22 -0600 Subject: [PATCH 1/2] =?UTF-8?q?MOB-111:=20listener=20process=20=E2=80=94?= =?UTF-8?q?=20single=20inbound=20entry=20point=20from=20native?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Mob.Renderer named a screen process directly at ~35 call sites, one per interactive prop: nif.register_tap({screen_pid, tag}). That hard-wires the inbound path to whichever process rendered the tree, and it looked like the half of the epic that would force a native change, since native stores and dispatches those handles. It does not. nif_register_tap stores an arbitrary term as the handle's tag (enif_make_copy into the handle's own env) and mob_send_tap / mob_send_event echo it back verbatim as {event, tag}. So the tag can carry more than a label. The renderer now registers {listener_pid, {:mob_route, screen_pid, tag}} and Mob.Listener forwards {:tap, tag} to the screen. The screen sees exactly the message it saw before, and no .m, .zig, or generator-template change is needed — the epic's constraint. The envelope is unwrapped on the event atom, so :tap, :change, :focus, :blur, :submit, :dismiss, :select, :scroll, :drag and the rest go through one clause. A new native event kind needs no listener change. The envelope carries a pid rather than the screen ref the epic sketched. It needs no registry, and it produces the behaviour the epic asked for: a handle registered by a screen that has since stopped delivers to a dead pid, which the BEAM drops, instead of being delivered into whatever screen is current with that screen's socket. That is the MOB-107 misrouting. A ref earns its cost when a screen can be restarted and keep identity across a new pid, which is MOB-112. :mob_screen is deliberately untouched. The back gesture, alert actions and launch-notification fallback resolve through enif_whereis_pid on both platforms, and MOB-113's router takes that name over — keeping the two changes separable. handler/1 returns its target unchanged when no listener is running, so any boot path without one behaves exactly as before. That is why the renderer's existing tests needed no changes. Being honest about what this buys: with one screen process the hop is pure overhead. The value is the indirection — the inbound path stops naming a screen process in 35 places, so MOB-112 and MOB-113 change one function instead. The escape hatch for high-frequency streams is simply not calling handler/1; nothing uses it, because the hop has not been measured and carving out an exception first would be guessing. Rationale in decisions/2026-08-28-listener-single-inbound-entry.md. Device-verified both platforms, since this rewires how every interaction reaches Elixir: sheetprobe cold-started on the iOS simulator and the Android emulator, and a real tap on each routed native -> listener -> screen -> re-render -> sender -> native, presenting the correct sheet. Tests: 17 new, including a round trip that replays what mob_send_tap does. Suite 1198 passed, format and credo --strict clean. Co-Authored-By: Claude Opus 5 (1M context) --- ...026-08-28-listener-single-inbound-entry.md | 89 +++++++ lib/mob/app.ex | 8 + lib/mob/listener.ex | 123 ++++++++++ lib/mob/renderer.ex | 83 ++++--- lib/mob/screen.ex | 3 + test/mob/listener_test.exs | 227 ++++++++++++++++++ 6 files changed, 499 insertions(+), 34 deletions(-) create mode 100644 decisions/2026-08-28-listener-single-inbound-entry.md create mode 100644 lib/mob/listener.ex create mode 100644 test/mob/listener_test.exs diff --git a/decisions/2026-08-28-listener-single-inbound-entry.md b/decisions/2026-08-28-listener-single-inbound-entry.md new file mode 100644 index 00000000..7d0f88e4 --- /dev/null +++ b/decisions/2026-08-28-listener-single-inbound-entry.md @@ -0,0 +1,89 @@ +# The listener: one inbound entry point, without touching native + +- Date: 2026-08-28 +- Status: accepted +- Implements: MOB-111, third step of MOB-108 +- Builds on: `2026-08-27-screen-process-architecture.md` + +## Context + +`Mob.Renderer` registered interaction handlers by naming a screen process +directly — `nif.register_tap({screen_pid, tag})`, at ~35 call sites, one per +interactive prop. That hard-wires the inbound path to whichever process rendered +the tree. It is also the half of the epic that looked like it would force a +native change, since native is what stores and dispatches those handles. + +## Decision + +It does not force one. `nif_register_tap` stores an arbitrary term as the +handle's tag (`enif_make_copy` into the handle's own env) and `mob_send_tap` / +`mob_send_event` echo it back verbatim as `{event, tag}`. The tag can therefore +be a nested tuple carrying more than a screen's label. + +`Mob.Renderer` now registers + + {listener_pid, {:mob_route, screen_pid, tag}} + +Native delivers `{:tap, {:mob_route, screen_pid, tag}}` to `Mob.Listener`, which +forwards `{:tap, tag}` to the screen. The screen sees exactly the message it saw +before. **No `.m`, `.zig` or generator-template change**, which was the epic's +constraint. + +### One clause, not one per event + +The envelope is unwrapped on the event atom, so `:tap`, `:change`, `:focus`, +`:blur`, `:submit`, `:dismiss`, `:select`, `:scroll`, `:drag` and the rest are +handled by a single `handle_info/2` clause. Adding a native event kind needs no +listener change. + +### The envelope carries a pid, not a screen ref + +The epic sketched `{listener_pid, {screen_ref, tag}}` with the listener +resolving the ref. Carrying the pid needs no registry and no resolution step, +and it produces the behaviour the epic actually asked for: a handle registered +by a screen that has since been stopped delivers to a dead pid, which the BEAM +drops. That is precisely the MOB-107 fix — the event is dropped rather than +delivered into whatever screen happens to be current with that screen's socket. + +A ref becomes worth its cost when a screen can be *restarted* and keep its +identity across a new pid, which is MOB-112. The change is confined to +`handler/1` and `handle_info/2`. + +### `:mob_screen` is untouched + +The other thing native knows is `enif_whereis_pid("mob_screen")` — back gesture, +alert actions, launch-notification fallback, both platforms. That name still +belongs to the screen process. MOB-113's router takes it over; this step +deliberately leaves it alone so the two changes stay separable. + +### No listener means no envelope + +`handler/1` returns its target unchanged when no listener is running, so events +go straight to the screen exactly as before. That keeps every boot path working +whether or not it starts a listener, and it is why the renderer's existing tests +needed no changes. + +## Consequences + +- The ~35 call sites now go through one `register_handler/2`. That indirection, + not the listener itself, is what makes MOB-112 and MOB-113 tractable: the + inbound path stops naming a screen process in 35 places. +- **The hop buys nothing yet.** With one screen process, carrying its pid + through the listener and forwarding is pure overhead. It is worth stating + plainly rather than implying otherwise — the value is entirely in where the + next two steps get to make their change. +- **The escape hatch is "do not call `handler/1`."** A high-frequency stream + (drag, scroll, `mob_touch` at display rate) pays one hop and one copy per + event; registering `{screen_pid, tag}` directly bypasses the listener and + still works, because that is what the renderer did before. Nothing bypasses it + today: the hop has not been measured, and carving out an exception before + there is a number would be guessing. +- `Mob.Event.Bridge` is unaffected. It translates the `{:tap, tag}` a screen + receives, and the listener unwraps before the screen sees anything. +- The listener is started by `Mob.App.start/0` and, for boot paths that skip it, + by `Mob.Screen.init/1` — unlinked, for the same reason as `Mob.Sender`: the + caller is a screen, and a screen crash must not take down the process every + screen's events arrive through. +- Like the sender, it has no supervisor. Its death is less severe — `handler/1` + falls back to direct registration on the *next* render — but handles already + baked with the dead listener's pid go nowhere until then. diff --git a/lib/mob/app.ex b/lib/mob/app.ex index 2cd5f4e1..f3ff3b98 100644 --- a/lib/mob/app.ex +++ b/lib/mob/app.ex @@ -126,6 +126,14 @@ defmodule Mob.App do {:error, {:already_started, _}} -> :ok end + # The single inbound entry point from native. Must also be up before the + # first render, because that render is what bakes the listener's pid + # into the native tap handles. + case Mob.Listener.start_link() do + {:ok, _} -> :ok + {:error, {:already_started, _}} -> :ok + end + # Mob.Device dispatcher + platform fan-out modules. Order matters: # the IOS / Android modules must exist before Mob.Device starts, # because Mob.Device forwards platform-tagged messages to them. diff --git a/lib/mob/listener.ex b/lib/mob/listener.ex new file mode 100644 index 00000000..60718db9 --- /dev/null +++ b/lib/mob/listener.ex @@ -0,0 +1,123 @@ +defmodule Mob.Listener do + @moduledoc """ + The single process the native layer delivers interaction events to. + + Native knows two things and neither of them is a screen: the registered name + `:mob_screen` (used by `enif_whereis_pid` for the back gesture, alert actions + and the launch-notification fallback, on both platforms) and whatever pid was + stored in a tap handle by `register_tap/1`. This module takes over the second. + + ## The envelope + + `nif_register_tap` stores an arbitrary term as the handle's tag and echoes it + back verbatim — `mob_send_tap` sends `{:tap, tag}`, `mob_send_event` sends + `{event, tag}`. The tag is copied with `enif_make_copy`, so it can be any + shape, including a nested tuple. + + So instead of registering `{screen_pid, tag}`, `Mob.Renderer` registers + + {listener_pid, {:mob_route, screen_pid, tag}} + + Native then delivers `{:tap, {:mob_route, screen_pid, tag}}` here, and the + listener forwards `{:tap, tag}` to the screen. Native remains ignorant that + screens exist, and **no `.m`, `.zig` or generator-template change is + required** to move the inbound path off a single hard-wired screen process. + + Because the envelope is unwrapped generically on the event atom, every event + the native layer sends this way — `:tap`, `:change`, `:focus`, `:blur`, + `:submit`, `:dismiss`, `:select`, `:scroll`, `:drag`, and the rest — is + handled by one clause rather than one per event. + + ## Why a hop at all + + Today there is one screen process, so carrying its pid through the envelope + and forwarding is, on its own, a hop that buys nothing. What it buys is that + the ~35 `register_tap` call sites in `Mob.Renderer` stop naming a screen + process directly. When MOB-112 makes screens processes and MOB-113 adds the + router, the change is confined to `handler/1` and `handle_info/2` here rather + than spread across every interactive prop in the renderer. + + ## The escape hatch + + A high-frequency stream — drag, scroll, `mob_touch` at display rate — pays one + extra hop and one extra copy per event. Registering the screen pid directly + bypasses this module entirely and still works, because that is exactly what + the renderer did before: + + nif.register_tap({screen_pid, tag}) # direct, no listener + + Nothing bypasses it today. The hop has not been measured, and adding an + exception before there is a number to point at would be guessing. + """ + + use GenServer + + @doc "Start the listener. Named, so there is exactly one." + @spec start_link(keyword()) :: GenServer.on_start() + def start_link(opts \\ []), do: GenServer.start_link(__MODULE__, opts, name: __MODULE__) + + @doc "Whether the listener is running." + @spec running?() :: boolean() + def running?, do: is_pid(Process.whereis(__MODULE__)) + + @doc """ + Start the listener if it is not already running. + + Unlinked, for the same reason `Mob.Sender.ensure_started/0` is: the caller is + usually a screen, and a screen crash must not take down the process every + screen's events arrive through. + """ + @spec ensure_started() :: :ok + def ensure_started do + if running?() do + :ok + else + case GenServer.start(__MODULE__, [], name: __MODULE__) do + {:ok, _pid} -> :ok + {:error, {:already_started, _pid}} -> :ok + end + end + end + + @doc """ + Wrap a `register_tap/1` target so native delivers the event here instead of + straight to the screen. + + Accepts either shape the renderer uses — a bare pid, or `{pid, tag}` — and + returns the term to hand to `register_tap/1`. + + Returns the target **unchanged** when the listener is not running, so events + go directly to the screen exactly as they did before this module existed. + That is the fallback for any boot path that does not start a listener, and it + is what keeps the renderer's own tests working without one. + """ + @spec handler(pid() | {pid(), term()}) :: pid() | {pid(), term()} + def handler(target) do + case Process.whereis(__MODULE__) do + nil -> target + listener -> {listener, envelope(target)} + end + end + + # A bare pid registers with no tag; native substitutes the atom :ok and the + # screen receives {:tap, :ok}. Preserved exactly. + defp envelope(pid) when is_pid(pid), do: {:mob_route, pid, :ok} + defp envelope({pid, tag}) when is_pid(pid), do: {:mob_route, pid, tag} + + # ── GenServer ───────────────────────────────────────────────────────────── + + @impl GenServer + def init(_opts), do: {:ok, %{}} + + @impl GenServer + def handle_info({event, {:mob_route, pid, tag}}, state) when is_atom(event) do + # Sending to a dead pid is a no-op in the BEAM, which is the behaviour we + # want: a handle registered by a screen that has since been popped and + # stopped drops its event rather than delivering it to whatever screen + # happens to be current. That is the misrouting MOB-107 reported. + send(pid, {event, tag}) + {:noreply, state} + end + + def handle_info(_message, state), do: {:noreply, state} +end diff --git a/lib/mob/renderer.ex b/lib/mob/renderer.ex index 8947c203..1ae454ff 100644 --- a/lib/mob/renderer.ex +++ b/lib/mob/renderer.ex @@ -254,6 +254,13 @@ defmodule Mob.Renderer do {:ok, :json_tree} end + # Every interactive prop registers through here rather than calling + # nif.register_tap/1 directly, so the inbound path can be moved off a single + # hard-wired screen process in one place instead of ~35. Mob.Listener.handler/1 + # returns the target unchanged when no listener is running, which is what the + # renderer's own tests rely on. See Mob.Listener. + defp register_handler(nif, target), do: nif.register_tap(Mob.Listener.handler(target)) + @doc "Return the full color palette map (token → ARGB integer)." @spec colors() :: %{atom() => non_neg_integer()} def colors, do: @colors @@ -344,31 +351,31 @@ defmodule Mob.Renderer do final |> Enum.flat_map(fn {:on_tap, pid} when is_pid(pid) -> - [{"on_tap", nif.register_tap(pid)}] + [{"on_tap", register_handler(nif, pid)}] {:on_tap, {pid, tag}} when is_pid(pid) and is_atom(tag) -> - [{"on_tap", nif.register_tap({pid, tag})}, {"accessibility_id", Atom.to_string(tag)}] + [{"on_tap", register_handler(nif, {pid, tag})}, {"accessibility_id", Atom.to_string(tag)}] {:on_tap, {pid, tag}} when is_pid(pid) -> - [{"on_tap", nif.register_tap({pid, tag})}] + [{"on_tap", register_handler(nif, {pid, tag})}] {:on_change, {pid, tag}} when is_pid(pid) -> - [{"on_change", nif.register_tap({pid, tag})}] + [{"on_change", register_handler(nif, {pid, tag})}] {:on_focus, {pid, tag}} when is_pid(pid) -> - [{"on_focus", nif.register_tap({pid, tag})}] + [{"on_focus", register_handler(nif, {pid, tag})}] {:on_blur, {pid, tag}} when is_pid(pid) -> - [{"on_blur", nif.register_tap({pid, tag})}] + [{"on_blur", register_handler(nif, {pid, tag})}] {:on_submit, {pid, tag}} when is_pid(pid) -> - [{"on_submit", nif.register_tap({pid, tag})}] + [{"on_submit", register_handler(nif, {pid, tag})}] # Sheet dismissal — swipe-down, back gesture, or outside tap. Native # fires this exactly once per presentation (see ios/MobRootView.swift # and mob_new's generated MobSheet composable). {:on_dismiss, {pid, tag}} when is_pid(pid) -> - [{"on_dismiss", nif.register_tap({pid, tag})}] + [{"on_dismiss", register_handler(nif, {pid, tag})}] {:detents, detents} -> encoded_detents = @@ -382,19 +389,19 @@ defmodule Mob.Renderer do # combine on_change + on_compose: ignore on_change while a composition # is active, replace text on :committed. {:on_compose, {pid, tag}} when is_pid(pid) -> - [{"on_compose", nif.register_tap({pid, tag})}] + [{"on_compose", register_handler(nif, {pid, tag})}] {:on_end_reached, {pid, tag}} when is_pid(pid) -> - [{"on_end_reached", nif.register_tap({pid, tag})}] + [{"on_end_reached", register_handler(nif, {pid, tag})}] {:on_tab_select, {pid, tag}} when is_pid(pid) -> - [{"on_tab_select", nif.register_tap({pid, tag})}] + [{"on_tab_select", register_handler(nif, {pid, tag})}] # Generic selection event — used by pickers, menus, segmented controls. # Lists use a structured tag (see Mob.List) and emit on_tap; this is for # widgets where "selection" is the only meaningful interaction. {:on_select, {pid, tag}} when is_pid(pid) -> - [{"on_select", nif.register_tap({pid, tag})}] + [{"on_select", register_handler(nif, {pid, tag})}] # ── Gestures (Batch 4) ──────────────────────────────────────────────── # Each maps to a UIGestureRecognizer (iOS) / GestureDetector (Android). @@ -403,25 +410,25 @@ defmodule Mob.Renderer do # gesture overhead by default. {:on_long_press, {pid, tag}} when is_pid(pid) -> - [{"on_long_press", nif.register_tap({pid, tag})}] + [{"on_long_press", register_handler(nif, {pid, tag})}] {:on_double_tap, {pid, tag}} when is_pid(pid) -> - [{"on_double_tap", nif.register_tap({pid, tag})}] + [{"on_double_tap", register_handler(nif, {pid, tag})}] {:on_swipe, {pid, tag}} when is_pid(pid) -> - [{"on_swipe", nif.register_tap({pid, tag})}] + [{"on_swipe", register_handler(nif, {pid, tag})}] {:on_swipe_left, {pid, tag}} when is_pid(pid) -> - [{"on_swipe_left", nif.register_tap({pid, tag})}] + [{"on_swipe_left", register_handler(nif, {pid, tag})}] {:on_swipe_right, {pid, tag}} when is_pid(pid) -> - [{"on_swipe_right", nif.register_tap({pid, tag})}] + [{"on_swipe_right", register_handler(nif, {pid, tag})}] {:on_swipe_up, {pid, tag}} when is_pid(pid) -> - [{"on_swipe_up", nif.register_tap({pid, tag})}] + [{"on_swipe_up", register_handler(nif, {pid, tag})}] {:on_swipe_down, {pid, tag}} when is_pid(pid) -> - [{"on_swipe_down", nif.register_tap({pid, tag})}] + [{"on_swipe_down", register_handler(nif, {pid, tag})}] # ── Batch 5: high-frequency events ──────────────────────────────────── # `on_scroll` is the prototype: native side throttles + delta-thresholds @@ -435,63 +442,71 @@ defmodule Mob.Renderer do # which the native side reads alongside the registered handle. {:on_scroll, {pid, tag}} when is_pid(pid) -> - [{"on_scroll", nif.register_tap({pid, tag})}] + [{"on_scroll", register_handler(nif, {pid, tag})}] {:on_scroll, {pid, tag, opts}} when is_pid(pid) and is_list(opts) -> cfg = Mob.Event.Throttle.parse(:scroll, opts) - [{"on_scroll", nif.register_tap({pid, tag})}, {"scroll_config", encode_throttle(cfg)}] + + [ + {"on_scroll", register_handler(nif, {pid, tag})}, + {"scroll_config", encode_throttle(cfg)} + ] {:on_drag, {pid, tag}} when is_pid(pid) -> - [{"on_drag", nif.register_tap({pid, tag})}] + [{"on_drag", register_handler(nif, {pid, tag})}] {:on_drag, {pid, tag, opts}} when is_pid(pid) and is_list(opts) -> cfg = Mob.Event.Throttle.parse(:drag, opts) - [{"on_drag", nif.register_tap({pid, tag})}, {"drag_config", encode_throttle(cfg)}] + [{"on_drag", register_handler(nif, {pid, tag})}, {"drag_config", encode_throttle(cfg)}] {:on_pinch, {pid, tag}} when is_pid(pid) -> - [{"on_pinch", nif.register_tap({pid, tag})}] + [{"on_pinch", register_handler(nif, {pid, tag})}] {:on_pinch, {pid, tag, opts}} when is_pid(pid) and is_list(opts) -> cfg = Mob.Event.Throttle.parse(:pinch, opts) - [{"on_pinch", nif.register_tap({pid, tag})}, {"pinch_config", encode_throttle(cfg)}] + [{"on_pinch", register_handler(nif, {pid, tag})}, {"pinch_config", encode_throttle(cfg)}] {:on_rotate, {pid, tag}} when is_pid(pid) -> - [{"on_rotate", nif.register_tap({pid, tag})}] + [{"on_rotate", register_handler(nif, {pid, tag})}] {:on_rotate, {pid, tag, opts}} when is_pid(pid) and is_list(opts) -> cfg = Mob.Event.Throttle.parse(:rotate, opts) - [{"on_rotate", nif.register_tap({pid, tag})}, {"rotate_config", encode_throttle(cfg)}] + + [ + {"on_rotate", register_handler(nif, {pid, tag})}, + {"rotate_config", encode_throttle(cfg)} + ] {:on_pointer_move, {pid, tag}} when is_pid(pid) -> - [{"on_pointer_move", nif.register_tap({pid, tag})}] + [{"on_pointer_move", register_handler(nif, {pid, tag})}] {:on_pointer_move, {pid, tag, opts}} when is_pid(pid) and is_list(opts) -> cfg = Mob.Event.Throttle.parse(:pointer_move, opts) [ - {"on_pointer_move", nif.register_tap({pid, tag})}, + {"on_pointer_move", register_handler(nif, {pid, tag})}, {"pointer_config", encode_throttle(cfg)} ] # ── Batch 5 Tier 2: semantic scroll events (single-fire, no payload) ── {:on_scroll_began, {pid, tag}} when is_pid(pid) -> - [{"on_scroll_began", nif.register_tap({pid, tag})}] + [{"on_scroll_began", register_handler(nif, {pid, tag})}] {:on_scroll_ended, {pid, tag}} when is_pid(pid) -> - [{"on_scroll_ended", nif.register_tap({pid, tag})}] + [{"on_scroll_ended", register_handler(nif, {pid, tag})}] {:on_scroll_settled, {pid, tag}} when is_pid(pid) -> - [{"on_scroll_settled", nif.register_tap({pid, tag})}] + [{"on_scroll_settled", register_handler(nif, {pid, tag})}] {:on_top_reached, {pid, tag}} when is_pid(pid) -> - [{"on_top_reached", nif.register_tap({pid, tag})}] + [{"on_top_reached", register_handler(nif, {pid, tag})}] # `on_scrolled_past` requires a threshold; native side fires once when # scroll y crosses the boundary (latched: re-emits only after going back # below and past again). {:on_scrolled_past, {pid, tag, threshold}} when is_pid(pid) and is_number(threshold) -> [ - {"on_scrolled_past", nif.register_tap({pid, tag})}, + {"on_scrolled_past", register_handler(nif, {pid, tag})}, {"scrolled_past_threshold", threshold} ] diff --git a/lib/mob/screen.ex b/lib/mob/screen.ex index 5d73efcd..59dad848 100644 --- a/lib/mob/screen.ex +++ b/lib/mob/screen.ex @@ -233,6 +233,9 @@ defmodule Mob.Screen do Process.register(self(), :mob_screen) # Renders are casts, so a missing sender would blank the screen silently. Mob.Sender.ensure_started() + # Started before the first render: that render is what bakes the + # listener's pid into the native tap handles. + Mob.Listener.ensure_started() end socket = diff --git a/test/mob/listener_test.exs b/test/mob/listener_test.exs new file mode 100644 index 00000000..21cb007d --- /dev/null +++ b/test/mob/listener_test.exs @@ -0,0 +1,227 @@ +defmodule Mob.ListenerTest do + use ExUnit.Case, async: false + + alias Mob.Listener + + setup do + case Process.whereis(Listener) do + nil -> :ok + pid -> GenServer.stop(pid) + end + + :ok + end + + defp start_listener do + {:ok, pid} = Listener.start_link([]) + on_exit(fn -> if Process.alive?(pid), do: GenServer.stop(pid) end) + pid + end + + describe "handler/1 without a listener" do + test "returns a tagged target unchanged" do + target = {self(), :save} + assert Listener.handler(target) == target + end + + test "returns a bare pid unchanged" do + assert Listener.handler(self()) == self() + end + end + + describe "handler/1 with a listener" do + test "addresses native at the listener, not the screen" do + listener = start_listener() + assert {^listener, _envelope} = Listener.handler({self(), :save}) + end + + test "carries the screen and tag in the envelope" do + start_listener() + screen = self() + assert {_listener, {:mob_route, ^screen, :save}} = Listener.handler({screen, :save}) + end + + test "a bare pid keeps the :ok tag native would have substituted" do + start_listener() + screen = self() + assert {_listener, {:mob_route, ^screen, :ok}} = Listener.handler(screen) + end + + test "a non-atom tag survives the envelope" do + start_listener() + screen = self() + tag = {:list, "id", :select, 3} + assert {_listener, {:mob_route, ^screen, ^tag}} = Listener.handler({screen, tag}) + end + end + + describe "forwarding" do + test "unwraps and delivers to the screen named in the envelope" do + listener = start_listener() + send(listener, {:tap, {:mob_route, self(), :save}}) + assert_receive {:tap, :save} + end + + test "every native event atom uses the same clause" do + listener = start_listener() + + for event <- [:tap, :change, :focus, :blur, :submit, :dismiss, :select, :scroll, :drag] do + send(listener, {event, {:mob_route, self(), :tag}}) + assert_receive {^event, :tag} + end + end + + test "delivers to the screen in the envelope, not the current one" do + # The MOB-107 shape: an event registered by one screen must not land in + # whichever screen happens to be active now. + listener = start_listener() + test_pid = self() + + other = + spawn(fn -> + receive do: (msg -> send(test_pid, {:other_got, msg})) + end) + + send(listener, {:tap, {:mob_route, other, :belongs_to_other}}) + + assert_receive {:other_got, {:tap, :belongs_to_other}} + refute_receive {:tap, :belongs_to_other} + end + + test "an event for a dead screen is dropped, not redirected" do + listener = start_listener() + dead = spawn(fn -> :ok end) + ref = Process.monitor(dead) + assert_receive {:DOWN, ^ref, :process, ^dead, _} + + send(listener, {:tap, {:mob_route, dead, :gone}}) + + # Still alive and still serving — a dead target must not take it down. + send(listener, {:tap, {:mob_route, self(), :mine}}) + assert_receive {:tap, :mine} + assert Process.alive?(listener) + end + + test "an unrecognised message is ignored" do + listener = start_listener() + send(listener, :garbage) + send(listener, {:tap, :not_an_envelope}) + send(listener, {:tap, {:mob_route, self(), :still_working}}) + assert_receive {:tap, :still_working} + assert Process.alive?(listener) + end + end + + describe "renderer round trip" do + # Stands in for the native layer: records what register_tap/1 was given, and + # replays it the way mob_send_tap does — {event, tag} to the stored pid. + defmodule FakeNative do + def start, do: Agent.start(fn -> [] end, name: __MODULE__) + def registered, do: __MODULE__ |> Agent.get(& &1) |> Enum.reverse() + + def clear_taps, do: :ok + def set_transition(_), do: :ok + def set_root(_), do: :ok + + def register_tap(target) do + Agent.update(__MODULE__, &[target | &1]) + length(Agent.get(__MODULE__, & &1)) - 1 + end + + # mob_send_tap: sends {event, tag} to the pid stored in the handle. + def fire(handle, event) do + case Enum.at(registered(), handle) do + {pid, tag} -> send(pid, {event, tag}) + pid when is_pid(pid) -> send(pid, {event, :ok}) + end + end + end + + setup do + case Process.whereis(FakeNative) do + nil -> :ok + pid -> Agent.stop(pid) + end + + FakeNative.start() + :ok + end + + defp button(tag), + do: %{type: :button, props: %{text: "go", on_tap: {self(), tag}}, children: []} + + test "with no listener, native is given the screen directly" do + screen = self() + Mob.Renderer.render(button(:save), :ios, FakeNative, :none) + assert [{^screen, :save}] = FakeNative.registered() + end + + test "with a listener, native is given the listener and the screen moves into the tag" do + listener = start_listener() + screen = self() + Mob.Renderer.render(button(:save), :ios, FakeNative, :none) + assert [{^listener, {:mob_route, ^screen, :save}}] = FakeNative.registered() + end + + test "a native tap reaches the owning screen unchanged" do + start_listener() + Mob.Renderer.render(button(:save), :ios, FakeNative, :none) + + # What mob_send_tap does on a real tap. + FakeNative.fire(0, :tap) + + # The screen sees exactly what it saw before the listener existed. + assert_receive {:tap, :save} + end + + test "other event kinds round trip too" do + start_listener() + screen = self() + + tree = %{ + type: :text_field, + props: %{on_change: {screen, :changed}, on_submit: {screen, :submitted}}, + children: [] + } + + Mob.Renderer.render(tree, :ios, FakeNative, :none) + + for {handle, event, tag} <- [{0, :change, :changed}, {1, :submit, :submitted}] do + FakeNative.fire(handle, event) + assert_receive {^event, ^tag} + end + end + end + + describe "ensure_started/0" do + test "starts the listener when missing and is a no-op when present" do + refute Listener.running?() + assert :ok = Listener.ensure_started() + assert Listener.running?() + pid = Process.whereis(Listener) + assert :ok = Listener.ensure_started() + assert Process.whereis(Listener) == pid + on_exit(fn -> if Listener.running?(), do: GenServer.stop(Listener) end) + end + + test "does not link to the caller" do + test_pid = self() + + caller = + spawn(fn -> + Listener.ensure_started() + send(test_pid, :started) + receive do: (:die -> exit(:boom)) + end) + + assert_receive :started + listener = Process.whereis(Listener) + ref = Process.monitor(caller) + send(caller, :die) + assert_receive {:DOWN, ^ref, :process, ^caller, _} + + assert Process.alive?(listener) + on_exit(fn -> if Listener.running?(), do: GenServer.stop(Listener) end) + end + end +end From 58570669addc3c7526c734c49e980860a6c7c462 Mon Sep 17 00:00:00 2001 From: GenericJam Date: Fri, 28 Aug 2026 15:04:28 -0600 Subject: [PATCH 2/2] MOB-111: fix a total regression of every value-carrying native event MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The listener unwrapped one envelope shape, {event, tag}, on the assumption that every handle-addressed native event looked alike. Native has two families, and the second was silently dropped: {event, tag} mob_send_tap, mob_send_event, mob_send_scrolled_past {event, tag, payload} mob_send_change, mob_send_compose, mob_send_swipe_with_direction, mob_send_scroll, mob_send_drag, mob_send_pinch, mob_send_rotate, mob_send_pointer_move So on the previous commit every text field, toggle and slider on_change, every tab selection (on_tab_select is wired to mob_send_change_str), and every gesture stream stopped working on both platforms — no crash, no log, the control simply did nothing. Mob.Listener is started unconditionally by Mob.App.start/0, so every real app was affected. Two things let it through. The device verification was a button tap, the one shape that still worked. And the test double modelled only the 2-tuple family, so `test "other event kinds round trip too"` passed with :change — a green test asserting the opposite of production behaviour, which is worse than no test at all. Fixed: a second handle_info/2 clause for the 3-tuple family. FakeNative now has fire/3 replaying the value-carrying senders, and all eight have a round-trip test. Verified as a negative control — both new tests fail without the clause. An unmodelled envelope shape is now logged at error rather than discarded, since that failure is otherwise invisible. Checked that no fourth arity exists: every enif_make_tuple4 in ios/mob_nif.m is a NIF return value, not a message. Also from the review: envelope/1 gained a catch-all so handler/1's two branches agree on what they accept (previously a non-pid target passed through with no listener and raised mid-render with one), and the test teardown no longer uses the racy `if Process.alive?, do: GenServer.stop` idiom, registering on_exit at start so a mid-test failure cannot leak a globally-named listener into unrelated files. Device-verified the actual broken path this time, not just a tap: typing into a text_field on the iOS simulator shows `on_change=Change wor` echoed back through the screen. Android's sendChange builds the identical 3-tuple, so the same Elixir path covers it. Suite 1201 passed, format and credo --strict clean. Co-Authored-By: Claude Opus 5 (1M context) --- ...026-08-28-listener-single-inbound-entry.md | 34 ++++- lib/mob/listener.ex | 56 +++++++-- test/mob/listener_test.exs | 116 ++++++++++++++++-- 3 files changed, 180 insertions(+), 26 deletions(-) diff --git a/decisions/2026-08-28-listener-single-inbound-entry.md b/decisions/2026-08-28-listener-single-inbound-entry.md index 7d0f88e4..d49acff6 100644 --- a/decisions/2026-08-28-listener-single-inbound-entry.md +++ b/decisions/2026-08-28-listener-single-inbound-entry.md @@ -29,12 +29,29 @@ forwards `{:tap, tag}` to the screen. The screen sees exactly the message it saw before. **No `.m`, `.zig` or generator-template change**, which was the epic's constraint. -### One clause, not one per event - -The envelope is unwrapped on the event atom, so `:tap`, `:change`, `:focus`, -`:blur`, `:submit`, `:dismiss`, `:select`, `:scroll`, `:drag` and the rest are -handled by a single `handle_info/2` clause. Adding a native event kind needs no -listener change. +### Two shapes, not one + +The first cut of this change unwrapped a single shape, `{event, tag}`, on the +assumption that every handle-addressed native event looked alike. It does not, +and the cost of being wrong is invisible: an unmatched envelope reaches the +screen as nothing at all, so the control simply stops working with no crash and +no log. + +Native has two families, both reading a tap handle: + +* `{event, tag}` — `mob_send_tap`, `mob_send_event`, `mob_send_scrolled_past`: + `:tap`, `:focus`, `:blur`, `:submit`, `:dismiss`, `:select`. +* `{event, tag, payload}` — `mob_send_change`, `mob_send_compose`, + `mob_send_swipe_with_direction`, `mob_send_scroll`, `mob_send_drag`, + `mob_send_pinch`, `mob_send_rotate`, `mob_send_pointer_move`: every text + field, toggle and slider `on_change`, tab selection (which is wired to + `mob_send_change_str`), and every gesture stream. + +Both are unwrapped on the event atom, so a new event *kind* needs no change +here — but a new *arity* would, which is why anything else carrying a +`{:mob_route, _, _}` is now logged at error rather than discarded. No sender +uses a 4-tuple: every `enif_make_tuple4` in `ios/mob_nif.m` is a NIF return +value, not a message. ### The envelope carries a pid, not a screen ref @@ -84,6 +101,11 @@ needed no changes. by `Mob.Screen.init/1` — unlinked, for the same reason as `Mob.Sender`: the caller is a screen, and a screen crash must not take down the process every screen's events arrive through. +- **The native double in the tests has to model both families.** The version + that modelled only `{event, tag}` produced a passing test asserting that + `on_change` worked while it was in fact being dropped — worse than no test. + `FakeNative.fire/3` now replays the 3-tuple senders, and every one of the + eight has a round-trip test. - Like the sender, it has no supervisor. Its death is less severe — `handler/1` falls back to direct registration on the *next* render — but handles already baked with the dead listener's pid go nowhere until then. diff --git a/lib/mob/listener.ex b/lib/mob/listener.ex index 60718db9..043634c4 100644 --- a/lib/mob/listener.ex +++ b/lib/mob/listener.ex @@ -23,10 +23,22 @@ defmodule Mob.Listener do screens exist, and **no `.m`, `.zig` or generator-template change is required** to move the inbound path off a single hard-wired screen process. - Because the envelope is unwrapped generically on the event atom, every event - the native layer sends this way — `:tap`, `:change`, `:focus`, `:blur`, - `:submit`, `:dismiss`, `:select`, `:scroll`, `:drag`, and the rest — is - handled by one clause rather than one per event. + Native has **two** message shapes for handle-addressed events, and the + listener has to unwrap both: + + * `{event, tag}` — `mob_send_tap`, `mob_send_event`, `mob_send_scrolled_past`. + Covers `:tap`, `:focus`, `:blur`, `:submit`, `:dismiss`, `:select` and the + other payload-free events. + * `{event, tag, payload}` — `mob_send_change`, `mob_send_compose`, + `mob_send_swipe_with_direction`, `mob_send_scroll`, `mob_send_drag`, + `mob_send_pinch`, `mob_send_rotate`, `mob_send_pointer_move`. This is + everything carrying a value: text-field and toggle and slider `on_change`, + tab selection, and every gesture stream. + + Both are unwrapped on the event atom rather than one clause per event, so a + new event kind needs no change here — but a new *arity* would. Anything else + is logged rather than silently discarded, because an unmodelled shape is + invisible otherwise: the widget simply stops working. ## Why a hop at all @@ -52,6 +64,8 @@ defmodule Mob.Listener do use GenServer + require Logger + @doc "Start the listener. Named, so there is exactly one." @spec start_link(keyword()) :: GenServer.on_start() def start_link(opts \\ []), do: GenServer.start_link(__MODULE__, opts, name: __MODULE__) @@ -93,9 +107,10 @@ defmodule Mob.Listener do """ @spec handler(pid() | {pid(), term()}) :: pid() | {pid(), term()} def handler(target) do - case Process.whereis(__MODULE__) do - nil -> target - listener -> {listener, envelope(target)} + case {Process.whereis(__MODULE__), envelope(target)} do + {nil, _} -> target + {_listener, ^target} -> target + {listener, envelope} -> {listener, envelope} end end @@ -103,6 +118,10 @@ defmodule Mob.Listener do # screen receives {:tap, :ok}. Preserved exactly. defp envelope(pid) when is_pid(pid), do: {:mob_route, pid, :ok} defp envelope({pid, tag}) when is_pid(pid), do: {:mob_route, pid, tag} + # Anything else passes through untouched, matching what the no-listener + # branch does. The two branches disagreeing would mean a shape that works + # without a listener and raises mid-render with one. + defp envelope(other), do: other # ── GenServer ───────────────────────────────────────────────────────────── @@ -119,5 +138,26 @@ defmodule Mob.Listener do {:noreply, state} end - def handle_info(_message, state), do: {:noreply, state} + def handle_info({event, {:mob_route, pid, tag}, payload}, state) when is_atom(event) do + send(pid, {event, tag, payload}) + {:noreply, state} + end + + def handle_info(message, state) do + # An envelope shape we do not model reaches the screen as nothing at all — + # the control just stops responding, with no crash and no log. Say so. + if routed?(message) do + Logger.error("[mob] Mob.Listener received an unhandled routed event: #{inspect(message)}") + end + + {:noreply, state} + end + + defp routed?(message) when is_tuple(message) do + message + |> Tuple.to_list() + |> Enum.any?(&match?({:mob_route, _pid, _tag}, &1)) + end + + defp routed?(_message), do: false end diff --git a/test/mob/listener_test.exs b/test/mob/listener_test.exs index 21cb007d..f1b3cdd7 100644 --- a/test/mob/listener_test.exs +++ b/test/mob/listener_test.exs @@ -1,6 +1,8 @@ defmodule Mob.ListenerTest do use ExUnit.Case, async: false + import ExUnit.CaptureLog + alias Mob.Listener setup do @@ -14,10 +16,19 @@ defmodule Mob.ListenerTest do defp start_listener do {:ok, pid} = Listener.start_link([]) - on_exit(fn -> if Process.alive?(pid), do: GenServer.stop(pid) end) + on_exit(fn -> stop_safely(pid) end) pid end + # `if Process.alive?, do: GenServer.stop` races: the process can exit between + # the check and the stop, and the :noproc exit then fails the test from inside + # the on_exit runner. + defp stop_safely(pid) do + GenServer.stop(pid) + catch + :exit, _ -> :ok + end + describe "handler/1 without a listener" do test "returns a tagged target unchanged" do target = {self(), :save} @@ -102,6 +113,22 @@ defmodule Mob.ListenerTest do assert Process.alive?(listener) end + test "an unmodelled routed shape is logged, not silently dropped" do + # A native sender with an arity the listener does not model delivers + # nothing to the screen: the control just stops working, with no crash. + # The log line is the only way that becomes visible. + listener = start_listener() + + log = + capture_log(fn -> + send(listener, {:change, {:mob_route, self(), :tag}, :extra, :unmodelled}) + send(listener, {:tap, {:mob_route, self(), :ping}}) + assert_receive {:tap, :ping} + end) + + assert log =~ "unhandled routed event" + end + test "an unrecognised message is ignored" do listener = start_listener() send(listener, :garbage) @@ -128,11 +155,24 @@ defmodule Mob.ListenerTest do length(Agent.get(__MODULE__, & &1)) - 1 end - # mob_send_tap: sends {event, tag} to the pid stored in the handle. + # mob_send_tap / mob_send_event / mob_send_scrolled_past: {event, tag}. def fire(handle, event) do + {pid, tag} = target(handle) + send(pid, {event, tag}) + end + + # mob_send_change / compose / swipe / scroll / drag / pinch / rotate / + # pointer_move: {event, tag, payload}. Modelling only the 2-tuple family + # is what let the missing 3-tuple clause pass review the first time. + def fire(handle, event, payload) do + {pid, tag} = target(handle) + send(pid, {event, tag, payload}) + end + + defp target(handle) do case Enum.at(registered(), handle) do - {pid, tag} -> send(pid, {event, tag}) - pid when is_pid(pid) -> send(pid, {event, :ok}) + {pid, tag} -> {pid, tag} + pid when is_pid(pid) -> {pid, :ok} end end end @@ -174,21 +214,69 @@ defmodule Mob.ListenerTest do assert_receive {:tap, :save} end - test "other event kinds round trip too" do + test "a payload-free event round trips" do + start_listener() + screen = self() + + tree = %{type: :text_field, props: %{on_submit: {screen, :submitted}}, children: []} + Mob.Renderer.render(tree, :ios, FakeNative, :none) + + FakeNative.fire(0, :submit) + assert_receive {:submit, :submitted} + end + + test "a value-carrying event round trips with its payload" do + # The regression this file missed: mob_send_change sends {change, tag, + # value}, so a listener that only unwraps {event, tag} drops every text + # field, toggle, slider and tab selection silently. + start_listener() + screen = self() + + tree = %{type: :text_field, props: %{on_change: {screen, :email}}, children: []} + Mob.Renderer.render(tree, :ios, FakeNative, :none) + + FakeNative.fire(0, :change, "hello@example.com") + assert_receive {:change, :email, "hello@example.com"} + end + + test "every 3-tuple native sender round trips" do start_listener() screen = self() tree = %{ - type: :text_field, - props: %{on_change: {screen, :changed}, on_submit: {screen, :submitted}}, + type: :canvas, + props: %{ + on_change: {screen, :changed}, + on_compose: {screen, :composed}, + on_swipe: {screen, :swiped}, + on_scroll: {screen, :scrolled}, + on_drag: {screen, :dragged}, + on_pinch: {screen, :pinched}, + on_rotate: {screen, :rotated}, + on_pointer_move: {screen, :moved} + }, children: [] } Mob.Renderer.render(tree, :ios, FakeNative, :none) - for {handle, event, tag} <- [{0, :change, :changed}, {1, :submit, :submitted}] do - FakeNative.fire(handle, event) - assert_receive {^event, ^tag} + handles = + for {target, i} <- Enum.with_index(FakeNative.registered()), + into: %{}, + do: {elem(elem(target, 1), 2), i} + + for {event, tag} <- [ + change: :changed, + compose: :composed, + swipe: :swiped, + scroll: :scrolled, + drag: :dragged, + pinch: :pinched, + rotate: :rotated, + pointer_move: :moved + ] do + FakeNative.fire(handles[tag], event, %{payload: tag}) + assert_receive {^event, ^tag, %{payload: ^tag}} end end end @@ -197,11 +285,14 @@ defmodule Mob.ListenerTest do test "starts the listener when missing and is a no-op when present" do refute Listener.running?() assert :ok = Listener.ensure_started() + # Registered before the assertions below, so a failure cannot leak a + # listener into unrelated test files. + on_exit(fn -> if pid = Process.whereis(Listener), do: stop_safely(pid) end) + assert Listener.running?() pid = Process.whereis(Listener) assert :ok = Listener.ensure_started() assert Process.whereis(Listener) == pid - on_exit(fn -> if Listener.running?(), do: GenServer.stop(Listener) end) end test "does not link to the caller" do @@ -216,12 +307,13 @@ defmodule Mob.ListenerTest do assert_receive :started listener = Process.whereis(Listener) + on_exit(fn -> stop_safely(listener) end) + ref = Process.monitor(caller) send(caller, :die) assert_receive {:DOWN, ^ref, :process, ^caller, _} assert Process.alive?(listener) - on_exit(fn -> if Listener.running?(), do: GenServer.stop(Listener) end) end end end