From 2b3923ff138f5bd68e576b722ee298a8ce07dfe7 Mon Sep 17 00:00:00 2001 From: YAMAGUCHI Seiji Date: Fri, 24 Apr 2026 14:32:03 +0900 Subject: [PATCH] fix(gateway): coerce scalar free_response_channels to str before split MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit YAML loads a bare numeric value such as discord: free_response_channels: 1491973769726791812 as an int. _discord_free_response_channels() / _slack_free_response_channels() checked `isinstance(raw, list)` and `isinstance(raw, str)` in that order and then fell through to `return set()`, so a single-channel config that happened to be unquoted was silently dropped with no log line — the bot kept demanding @mentions even though the channel was configured to free-response. A multi-channel value like `1234567890,9876543210` does not trip this because the comma forces YAML to parse it as a string. Single-channel configs are the only case that breaks, which is exactly the footgun that's hardest to diagnose (the config "looks right" and the feature just doesn't activate). Note that the old-schema env-var bridge at gateway/config.py:614+ already runs `str(frc)` when forwarding to SLACK_/DISCORD_FREE_RESPONSE_CHANNELS, so the env-var fallback worked. The bug only surfaces on the `config.extra["free_response_channels"]` path populated by the `platforms:` bridge at gateway/config.py:576, which passes the raw YAML value through unchanged. Fix at the reader: treat any non-list value as a scalar, coerce with str(), then apply the same CSV split semantics. This keeps the public contract stable (list or str-like continues to work identically) while accepting the ints that the YAML loader is free to hand us. Added tests for both Discord and Slack covering: - bare int value in config.extra - list of ints in config.extra --- gateway/platforms/discord.py | 11 +++++++++-- gateway/platforms/slack.py | 11 +++++++++-- tests/gateway/test_discord_free_response.py | 20 ++++++++++++++++++++ tests/gateway/test_slack_mention.py | 17 +++++++++++++++++ 4 files changed, 55 insertions(+), 4 deletions(-) diff --git a/gateway/platforms/discord.py b/gateway/platforms/discord.py index fcd2cbc996c..60cfb55ef67 100644 --- a/gateway/platforms/discord.py +++ b/gateway/platforms/discord.py @@ -2851,8 +2851,15 @@ class DiscordAdapter(BasePlatformAdapter): raw = os.getenv("DISCORD_FREE_RESPONSE_CHANNELS", "") if isinstance(raw, list): return {str(part).strip() for part in raw if str(part).strip()} - if isinstance(raw, str) and raw.strip(): - return {part.strip() for part in raw.split(",") if part.strip()} + # Coerce non-list scalars (str/int/float) to str before splitting. + # YAML parses a bare numeric value such as + # `free_response_channels: 1491973769726791812` as int, which was + # previously falling through the isinstance(str) branch and silently + # returning an empty set. str() here accepts whatever scalar the YAML + # loader hands us without changing existing string/CSV semantics. + s = str(raw).strip() if raw is not None else "" + if s: + return {part.strip() for part in s.split(",") if part.strip()} return set() def _thread_parent_channel(self, channel: Any) -> Any: diff --git a/gateway/platforms/slack.py b/gateway/platforms/slack.py index 9aa23871052..d35b703f70f 100644 --- a/gateway/platforms/slack.py +++ b/gateway/platforms/slack.py @@ -2888,6 +2888,13 @@ class SlackAdapter(BasePlatformAdapter): raw = os.getenv("SLACK_FREE_RESPONSE_CHANNELS", "") if isinstance(raw, list): return {str(part).strip() for part in raw if str(part).strip()} - if isinstance(raw, str) and raw.strip(): - return {part.strip() for part in raw.split(",") if part.strip()} + # Coerce non-list scalars (str/int/float) to str before splitting. + # A bare numeric YAML value (`free_response_channels: 1234567890`) is + # loaded as int and was previously falling through the isinstance(str) + # branch to return an empty set. str() here accepts whatever scalar + # the YAML loader hands us without changing existing string/CSV + # semantics. + s = str(raw).strip() if raw is not None else "" + if s: + return {part.strip() for part in s.split(",") if part.strip()} return set() diff --git a/tests/gateway/test_discord_free_response.py b/tests/gateway/test_discord_free_response.py index f1ee99606ec..f3242e3d5d5 100644 --- a/tests/gateway/test_discord_free_response.py +++ b/tests/gateway/test_discord_free_response.py @@ -220,6 +220,26 @@ async def test_discord_free_response_channel_can_come_from_config_extra(adapter, assert event.text == "allowed from config" +def test_discord_free_response_channels_bare_int(adapter, monkeypatch): + # YAML `discord.free_response_channels: 1491973769726791812` (single bare + # integer) is loaded as an int and previously fell through the + # isinstance(str) branch in _discord_free_response_channels, silently + # returning an empty set. Scalar → str coercion makes single-channel + # config work without having to quote the ID in YAML. + monkeypatch.delenv("DISCORD_FREE_RESPONSE_CHANNELS", raising=False) + adapter.config.extra["free_response_channels"] = 1491973769726791812 + + assert adapter._discord_free_response_channels() == {"1491973769726791812"} + + +def test_discord_free_response_channels_int_list(adapter, monkeypatch): + # YAML list form with bare numeric entries — each element should be coerced. + monkeypatch.delenv("DISCORD_FREE_RESPONSE_CHANNELS", raising=False) + adapter.config.extra["free_response_channels"] = [1491973769726791812, 99999] + + assert adapter._discord_free_response_channels() == {"1491973769726791812", "99999"} + + @pytest.mark.asyncio async def test_discord_forum_parent_in_free_response_list_allows_forum_thread(adapter, monkeypatch): monkeypatch.setenv("DISCORD_REQUIRE_MENTION", "true") diff --git a/tests/gateway/test_slack_mention.py b/tests/gateway/test_slack_mention.py index e6ba010de09..892cabef889 100644 --- a/tests/gateway/test_slack_mention.py +++ b/tests/gateway/test_slack_mention.py @@ -215,6 +215,23 @@ def test_free_response_channels_env_var_fallback(monkeypatch): assert OTHER_CHANNEL_ID in result +def test_free_response_channels_bare_int(): + # YAML `free_response_channels: 1491973769726791812` (single bare integer) + # is loaded as an int and would previously fall through the isinstance(str) + # branch to return an empty set. Coerce scalar → str so single-channel + # config without quoting works as users expect. + adapter = _make_adapter(free_response_channels=1491973769726791812) + result = adapter._slack_free_response_channels() + assert result == {"1491973769726791812"} + + +def test_free_response_channels_int_list(): + # YAML list form with bare numeric entries — each element should be coerced. + adapter = _make_adapter(free_response_channels=[1491973769726791812, 99999]) + result = adapter._slack_free_response_channels() + assert result == {"1491973769726791812", "99999"} + + # --------------------------------------------------------------------------- # Tests: mention gating integration (simulating _handle_slack_message logic) # ---------------------------------------------------------------------------