fix(gateway): coerce scalar free_response_channels to str before split

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
This commit is contained in:
YAMAGUCHI Seiji 2026-04-24 14:32:03 +09:00 committed by kshitij
parent a717199bbf
commit 2b3923ff13
4 changed files with 55 additions and 4 deletions

View file

@ -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:

View file

@ -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()

View file

@ -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")

View file

@ -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)
# ---------------------------------------------------------------------------