From 4385ea5703048e9e016251ed12969cb45ea08489 Mon Sep 17 00:00:00 2001 From: Jack Kingsman Date: Wed, 8 Jul 2026 17:08:04 -0700 Subject: [PATCH] Add clearer regional scoping for bots. Closes #300. --- app/fanout/AGENTS_fanout.md | 2 +- app/fanout/bot.py | 8 +- app/fanout/bot_exec.py | 27 +++-- .../settings/SettingsFanoutSection.tsx | 9 +- tests/test_bot.py | 103 +++++++++++++++++ tests/test_fanout_hitlist.py | 109 ++++++++++++++++++ 6 files changed, 246 insertions(+), 12 deletions(-) diff --git a/app/fanout/AGENTS_fanout.md b/app/fanout/AGENTS_fanout.md index 17370d0..892e391 100644 --- a/app/fanout/AGENTS_fanout.md +++ b/app/fanout/AGENTS_fanout.md @@ -120,7 +120,7 @@ Wraps bot code execution via `app/fanout/bot_exec.py`. Config blob: - Executes in a thread pool with timeout and semaphore concurrency control - Rate-limits outgoing messages for repeater compatibility - Channel `message_text` passed to bot code is normalized for human readability by stripping a leading `"{sender_name}: "` prefix when it matches the payload sender. -- The `bot(...)` function receives, in order: `sender_name`, `sender_key`, `message_text`, `is_dm`, `channel_key`, `channel_name`, `sender_timestamp`, `path`, then optionally `is_outgoing`, `path_bytes_per_hop`, `packet_hash`. `region` (resolved region name for region-scoped channel messages; `None` otherwise) is delivered **only** to bots that use `**kwargs` or explicitly name the `region` parameter — it is intentionally not added to the positional call styles so existing bot signatures keep binding unchanged. `_analyze_bot_signature` in `bot_exec.py` picks the call style from the bot's actual signature. +- The `bot(...)` function receives, in order: `sender_name`, `sender_key`, `message_text`, `is_dm`, `channel_key`, `channel_name`, `sender_timestamp`, `path`, then optionally `is_outgoing`, `path_bytes_per_hop`, `packet_hash`. Two further kwargs — `region` (resolved region name; `None` for unscoped flood or a transport code matching no known region) and `scoped` (`bool`: whether the message carried a regional flood scope) — are delivered **only** to bots that use `**kwargs` or explicitly name the parameter; they are intentionally not added to the positional call styles so existing bot signatures keep binding unchanged. `scoped` disambiguates a `None` region: `not scoped` = unscoped, `scoped and region is None` = scoped-but-unknown-region, `scoped and region` = that named region. Unlike `region` (channel-only historically), `scoped` is also set for scoped DMs (flood-direct messages can carry a scope), which resolves the DM half of #300. `_analyze_bot_signature` in `bot_exec.py` picks the call style from the bot's actual signature. - **Return shapes** (`execute_bot_code` → `process_bot_response`): `None` (no reply), a `str`, a `list[str]` (sent in order), or a `dict` `{"region": , "message": }`. The dict form (`BotReply`) scopes the reply send to a region **for that send only**: a region name applies it, `None`/empty clears it (unscoped/plain flood), and an absent `region` key falls back to the channel's persisted `flood_scope_override`. Region scoping applies to channel replies only — it is ignored for DM replies (DMs are not region-scoped). Outgoing scope reuses the existing `send_channel_message_with_effective_scope` set-scope/send/restore machinery via a per-send `flood_scope_override` on `SendChannelMessageRequest`. Note the bot can scope to any region name; whether the echo is *labeled* still depends on the operator's `app_settings.known_regions` (that list only drives decode, not transmit). ### webhook (webhook.py) diff --git a/app/fanout/bot.py b/app/fanout/bot.py index e4e63bd..729df1a 100644 --- a/app/fanout/bot.py +++ b/app/fanout/bot.py @@ -137,9 +137,12 @@ class BotModule(FanoutModule): path_value = paths[0].get("path") if isinstance(paths[0], dict) else None path_bytes_per_hop = _derive_path_bytes_per_hop(paths, path_value) packet_hash = data.get("packet_hash") - # Resolved region name for region-scoped channel messages (None for DMs, - # unscoped flood, or when the transport code matched no known region). + # Resolved region name (None for unscoped flood or a transport code that + # matched no known region). `scoped` disambiguates None: a transport code + # was present iff the message carried a regional flood scope. This is set + # for scoped DMs too (flood-direct messages can carry a scope). region = data.get("region") + scoped = data.get("transport_code") is not None # Wait for message to settle (allows retransmissions to be deduped) await asyncio.sleep(2) @@ -167,6 +170,7 @@ class BotModule(FanoutModule): path_bytes_per_hop, packet_hash, region, + scoped, ), timeout=BOT_EXECUTION_TIMEOUT, ) diff --git a/app/fanout/bot_exec.py b/app/fanout/bot_exec.py index 454b14d..333eb9a 100644 --- a/app/fanout/bot_exec.py +++ b/app/fanout/bot_exec.py @@ -126,7 +126,7 @@ def _analyze_bot_signature(bot_func_or_sig) -> BotCallPlan: has_kwargs = any(p.kind == inspect.Parameter.VAR_KEYWORD for p in param_values) explicit_optional_names = tuple( name - for name in ("is_outgoing", "path_bytes_per_hop", "packet_hash", "region") + for name in ("is_outgoing", "path_bytes_per_hop", "packet_hash", "region", "scoped") if name in params ) unsupported_required_kwonly = [ @@ -134,7 +134,7 @@ def _analyze_bot_signature(bot_func_or_sig) -> BotCallPlan: for p in param_values if p.kind == inspect.Parameter.KEYWORD_ONLY and p.default is inspect.Parameter.empty - and p.name not in {"is_outgoing", "path_bytes_per_hop", "packet_hash", "region"} + and p.name not in {"is_outgoing", "path_bytes_per_hop", "packet_hash", "region", "scoped"} ] if unsupported_required_kwonly: raise ValueError( @@ -164,6 +164,8 @@ def _analyze_bot_signature(bot_func_or_sig) -> BotCallPlan: keyword_args["packet_hash"] = "" if has_kwargs or "region" in params: keyword_args["region"] = None + if has_kwargs or "scoped" in params: + keyword_args["scoped"] = False candidate_specs.append(("keyword", [], keyword_args)) if not has_kwargs and explicit_optional_names: @@ -176,6 +178,8 @@ def _analyze_bot_signature(bot_func_or_sig) -> BotCallPlan: kwargs["packet_hash"] = "" if has_kwargs or "region" in params: kwargs["region"] = None + if has_kwargs or "scoped" in params: + kwargs["scoped"] = False candidate_specs.append(("mixed_keyword", base_args, kwargs)) if has_varargs or positional_capacity >= 11: @@ -201,7 +205,7 @@ def _analyze_bot_signature(bot_func_or_sig) -> BotCallPlan: "Supported trailing parameters are: path; path + is_outgoing; " "path + path_bytes_per_hop; path + is_outgoing + path_bytes_per_hop; " "path + is_outgoing + path_bytes_per_hop + packet_hash; " - "or use **kwargs for forward compatibility (which also receives region)." + "or use **kwargs for forward compatibility (which also receives region and scoped)." ) @@ -219,6 +223,7 @@ def execute_bot_code( path_bytes_per_hop: int | None = None, packet_hash: str | None = None, region: str | None = None, + scoped: bool = False, ) -> str | list[str] | BotReply | None: """ Execute user-provided bot code with message context. @@ -227,8 +232,8 @@ def execute_bot_code( `bot(sender_name, sender_key, message_text, is_dm, channel_key, channel_name, sender_timestamp, path, is_outgoing, path_bytes_per_hop, packet_hash)` or use named parameters / `**kwargs`. - `region` is only delivered to bots that opt in via `**kwargs` or by naming - the parameter (`region`); the positional call styles are unchanged for + `region` and `scoped` are only delivered to bots that opt in via `**kwargs` + or by naming the parameter; the positional call styles are unchanged for backward compatibility. The bot returns either None (no response), a string (single response message), @@ -253,8 +258,14 @@ def execute_bot_code( is_outgoing: True if this is our own outgoing message path_bytes_per_hop: Number of bytes per routing hop (1, 2, or 3), if known packet_hash: MeshCore packet hash (first 16 hex chars of SHA256, uppercase), if known - region: Resolved region name for a region-scoped channel message, or None - for DMs, unscoped flood, or a transport code matching no known region + scoped: True if the message carried a regional flood scope, False for + plain/unscoped flood. Check this first — it is the meaningful signal. + Set for scoped DMs too (flood-direct messages can carry a scope). + region: Only meaningful when scoped is True. When scoped is False, region + is always None and should be ignored. When scoped is True, region is + the decoded region name, or None if the scope matched none of your + known_regions (i.e. scoped, but region unrecognized). region is never + enough on its own to tell "unscoped" from "unrecognized" — use scoped. Returns: Response string, list of strings, or None. @@ -356,6 +367,8 @@ def execute_bot_code( keyword_args["packet_hash"] = packet_hash if "region" in call_plan.keyword_args: keyword_args["region"] = region + if "scoped" in call_plan.keyword_args: + keyword_args["scoped"] = scoped result = bot_func(**keyword_args) else: result = bot_func( diff --git a/frontend/src/components/settings/SettingsFanoutSection.tsx b/frontend/src/components/settings/SettingsFanoutSection.tsx index 9cef069..5a63ef7 100644 --- a/frontend/src/components/settings/SettingsFanoutSection.tsx +++ b/frontend/src/components/settings/SettingsFanoutSection.tsx @@ -89,8 +89,13 @@ const DEFAULT_BOT_CODE = `def bot(**kwargs) -> str | list[str] | None: path: Hex-encoded routing path (may be None) is_outgoing: True if this is our own outgoing message path_bytes_per_hop: Bytes per hop in path (1, 2, or 3) when known - region: Resolved region name for a region-scoped channel - message (None for DMs, unscoped flood, or no match) + scoped: True if the message carried a regional flood scope, + False for plain/unscoped flood. Check this first. Set for + scoped DMs too. + region: Only meaningful when scoped is True (else always None). + When scoped, it's the decoded region name, or None if the + scope matched none of your known_regions. region alone can't + distinguish unscoped from unrecognized — use scoped. Returns: None for no reply, a string for a single reply, diff --git a/tests/test_bot.py b/tests/test_bot.py index 64681b5..0c44fff 100644 --- a/tests/test_bot.py +++ b/tests/test_bot.py @@ -497,6 +497,109 @@ def bot(sender_name, sender_key, message_text, is_dm, channel_key, channel_name, ) assert result == "ok:Hi" + def test_kwargs_bot_receives_scoped(self): + """Bots using **kwargs receive `scoped` for a region-scoped message (#300).""" + code = """ +def bot(sender_name, sender_key, message_text, is_dm, channel_key, channel_name, sender_timestamp, path, **kwargs): + return f"scoped={kwargs.get('scoped', 'missing')}" +""" + result = execute_bot_code( + code=code, + sender_name="Someone", + sender_key=None, + message_text="Hi", + is_dm=False, + channel_key="AABBCCDD", + channel_name="#general", + sender_timestamp=None, + path=None, + region="EU", + scoped=True, + ) + assert result == "scoped=True" + + def test_named_scoped_param_bot_receives_scoped(self): + """Bots may opt into `scoped` by naming the parameter (with a default).""" + code = """ +def bot(sender_name, sender_key, message_text, is_dm, channel_key, channel_name, sender_timestamp, path, scoped=False): + return f"scoped={scoped}" +""" + result = execute_bot_code( + code=code, + sender_name="Someone", + sender_key=None, + message_text="Hi", + is_dm=False, + channel_key="AABBCCDD", + channel_name="#general", + sender_timestamp=None, + path=None, + scoped=True, + ) + assert result == "scoped=True" + + def test_scoped_true_region_none_disambiguates_unknown_region(self): + """scoped=True with region=None means 'scoped but region unknown' (#300.1).""" + code = """ +def bot(**kwargs): + return f"scoped={kwargs.get('scoped')},region={kwargs.get('region')}" +""" + result = execute_bot_code( + code=code, + sender_name="Someone", + sender_key=None, + message_text="Hi", + is_dm=False, + channel_key="AABBCCDD", + channel_name="#general", + sender_timestamp=None, + path=None, + region=None, + scoped=True, + ) + assert result == "scoped=True,region=None" + + def test_scoped_defaults_false_for_unscoped_message(self): + """scoped is delivered as False (not absent) for plain/unscoped flood.""" + code = """ +def bot(**kwargs): + return f"scoped={kwargs.get('scoped', 'missing')}" +""" + result = execute_bot_code( + code=code, + sender_name="Alice", + sender_key="abc123", + message_text="Hi", + is_dm=True, + channel_key=None, + channel_name=None, + sender_timestamp=None, + path=None, + ) + assert result == "scoped=False" + + def test_legacy_positional_bot_unaffected_by_scoped(self): + """Historical positional bots keep binding unchanged; scoped is never passed positionally.""" + code = """ +def bot(sender_name, sender_key, message_text, is_dm, channel_key, channel_name, sender_timestamp, path, is_outgoing): + return f"ok:{message_text}" +""" + result = execute_bot_code( + code=code, + sender_name="Alice", + sender_key="abc123", + message_text="Hi", + is_dm=True, + channel_key=None, + channel_name=None, + sender_timestamp=None, + path=None, + is_outgoing=False, + region="EU", + scoped=True, + ) + assert result == "ok:Hi" + def test_dict_return_with_region_produces_bot_reply(self): """A {"region", "message"} return becomes a BotReply with a normalized scope (#300).""" from app.fanout.bot_exec import BotReply diff --git a/tests/test_fanout_hitlist.py b/tests/test_fanout_hitlist.py index 1b446d0..29fb204 100644 --- a/tests/test_fanout_hitlist.py +++ b/tests/test_fanout_hitlist.py @@ -39,6 +39,7 @@ class TestBotModuleParameterExtraction: path_bytes_per_hop, packet_hash, region, + scoped, ): captured["is_outgoing"] = is_outgoing captured["is_dm"] = is_dm @@ -90,6 +91,7 @@ class TestBotModuleParameterExtraction: path_bytes_per_hop, packet_hash, region, + scoped, ): captured["is_outgoing"] = is_outgoing return None @@ -138,6 +140,7 @@ class TestBotModuleParameterExtraction: path_bytes_per_hop, packet_hash, region, + scoped, ): captured["path"] = path captured["path_bytes_per_hop"] = path_bytes_per_hop @@ -188,6 +191,7 @@ class TestBotModuleParameterExtraction: path_bytes_per_hop, packet_hash, region, + scoped, ): captured["region"] = region return None @@ -238,6 +242,7 @@ class TestBotModuleParameterExtraction: path_bytes_per_hop, packet_hash, region, + scoped, ): captured["region"] = region return None @@ -264,6 +269,107 @@ class TestBotModuleParameterExtraction: assert captured["region"] is None + @pytest.mark.asyncio + async def test_scoped_true_when_transport_code_present(self): + """A message with a transport_code forwards scoped=True (#300).""" + from app.fanout.bot import BotModule + + captured = {} + + def fake_execute( + code, + sender_name, + sender_key, + message_text, + is_dm, + channel_key, + channel_name, + sender_timestamp, + path, + is_outgoing, + path_bytes_per_hop, + packet_hash, + region, + scoped, + ): + captured["scoped"] = scoped + return None + + mod = BotModule("test", {"code": "def bot(**k): pass"}, name="Test") + + with ( + patch("app.fanout.bot_exec.execute_bot_code", side_effect=fake_execute), + patch( + "app.fanout.bot_exec._bot_semaphore", + MagicMock(__aenter__=AsyncMock(), __aexit__=AsyncMock()), + ), + patch("app.fanout.bot.asyncio.sleep", new_callable=AsyncMock), + patch("app.repository.ChannelRepository") as mock_chan, + ): + mock_chan.get_by_key = AsyncMock(return_value=MagicMock(name="#test")) + await mod._run_for_message( + { + "type": "CHAN", + "conversation_key": "ch1", + "text": "Alice: hello", + "sender_name": "Alice", + "channel_name": "#test", + # Scoped but region unresolved: scoped must still be True. + "transport_code": 6789, + "region": None, + } + ) + + assert captured["scoped"] is True + + @pytest.mark.asyncio + async def test_scoped_false_when_transport_code_absent(self): + """A message without a transport_code forwards scoped=False (unscoped flood).""" + from app.fanout.bot import BotModule + + captured = {} + + def fake_execute( + code, + sender_name, + sender_key, + message_text, + is_dm, + channel_key, + channel_name, + sender_timestamp, + path, + is_outgoing, + path_bytes_per_hop, + packet_hash, + region, + scoped, + ): + captured["scoped"] = scoped + return None + + mod = BotModule("test", {"code": "def bot(**k): pass"}, name="Test") + + with ( + patch("app.fanout.bot_exec.execute_bot_code", side_effect=fake_execute), + patch( + "app.fanout.bot_exec._bot_semaphore", + MagicMock(__aenter__=AsyncMock(), __aexit__=AsyncMock()), + ), + patch("app.fanout.bot.asyncio.sleep", new_callable=AsyncMock), + patch("app.repository.ContactRepository") as mock_contact, + ): + mock_contact.get_by_key = AsyncMock(return_value=MagicMock(name="Alice")) + await mod._run_for_message( + { + "type": "PRIV", + "conversation_key": "pk1", + "text": "hello", + } + ) + + assert captured["scoped"] is False + @pytest.mark.asyncio async def test_channel_sender_prefix_stripped(self): """Channel message text has 'SenderName: ' prefix stripped.""" @@ -285,6 +391,7 @@ class TestBotModuleParameterExtraction: path_bytes_per_hop, packet_hash, region, + scoped, ): captured["message_text"] = message_text captured["sender_name"] = sender_name @@ -335,6 +442,7 @@ class TestBotModuleParameterExtraction: path_bytes_per_hop, packet_hash, region, + scoped, ): captured["channel_name"] = channel_name return None @@ -384,6 +492,7 @@ class TestBotModuleParameterExtraction: path_bytes_per_hop, packet_hash, region, + scoped, ): captured["sender_name"] = sender_name captured["sender_key"] = sender_key