mirror of
https://github.com/jkingsman/Remote-Terminal-for-MeshCore.git
synced 2026-07-30 21:43:14 +02:00
Add clearer regional scoping for bots. Closes #300.
This commit is contained in:
@@ -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": <name|None>, "message": <str|list[str]>}`. 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)
|
||||
|
||||
+6
-2
@@ -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,
|
||||
)
|
||||
|
||||
+20
-7
@@ -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(
|
||||
|
||||
@@ -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,
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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
|
||||
|
||||
Reference in New Issue
Block a user