Skip to content

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

Closed
valda wants to merge 1 commit into
NousResearch:mainfrom
valda:fix/free-response-channels-coerce
Closed

fix(gateway): coerce scalar free_response_channels to str before split#14932
valda wants to merge 1 commit into
NousResearch:mainfrom
valda:fix/free-response-channels-coerce

Conversation

@valda

@valda valda commented Apr 24, 2026

Copy link
Copy Markdown
Contributor

Problem

A single-channel Discord/Slack free_response_channels entry configured as a bare numeric YAML value is silently dropped. The bot keeps demanding @mentions even though the channel was configured to free-response, with no log line to point at why.

discord:
  free_response_channels: 1491973769726791812   # bug: ignored
  # free_response_channels: '1491973769726791812'   # workaround: treated as str
  # free_response_channels: 1491973769726791812,9876543210  # coincidentally works — comma forces YAML to parse as str

YAML loads the bare integer as int. _discord_free_response_channels() / _slack_free_response_channels() check isinstance(raw, list) then isinstance(raw, str), and if neither matches fall through to return set(). So any single-channel unquoted numeric ID is silently dropped — which is exactly the footgun that's hardest to diagnose, because the config file "looks right" and the feature just doesn't activate.

A multi-channel value like 1234567890,9876543210 doesn't trip this because the comma forces YAML to parse it as a string. And 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

Coerce any non-list scalar to str() at the reader before applying the existing CSV split:

if isinstance(raw, list):
    return {str(part).strip() for part in raw if str(part).strip()}
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()

This keeps the public contract stable — lists, CSV strings, empty strings, None, and env-var fallbacks all continue to behave identically — while also accepting the int / float scalars that the YAML loader is free to hand us.

Applied symmetrically to both Discord and Slack since they share the exact same pattern.

Tests

Added unit tests covering:

  • bare int value in config.extra (the failure case)
  • list of int entries in config.extra

for both Discord (test_discord_free_response_channels_bare_int, test_discord_free_response_channels_int_list) and Slack (test_free_response_channels_bare_int, test_free_response_channels_int_list).

Existing test_free_response_channels_list / test_free_response_channels_csv_string / test_free_response_channels_empty_string / test_free_response_channels_env_var_fallback / test_discord_free_response_channel_can_come_from_config_extra still pass unchanged.

tests/gateway/test_discord_free_response.py tests/gateway/test_slack_mention.py
50 passed, 2 warnings in 12.42s

Alternative considered

Coercing at the bridge (gateway/config.py:576) would fix this single field but would leave other platforms:-bridged fields with the same pattern (allow_from, group_allow_from, ignored_channels, …) still vulnerable to a different bare-int YAML footgun. Fixing at the reader is minimal, defensive, and scoped to the two adapters that actually consume this field. If the bridge ever grows a general normalization step that's a strictly bigger change, and this reader-level fix is still correct then.

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
@alt-glitch alt-glitch added type/bug Something isn't working P2 Medium — degraded but workaround exists comp/gateway Gateway runner, session dispatch, delivery platform/discord Discord bot adapter platform/slack Slack app adapter area/config Config system, migrations, profiles labels Apr 24, 2026
@kshitijk4poor

Copy link
Copy Markdown
Collaborator

Merged via PR #18553. Your commit was cherry-picked onto current main with your authorship preserved (rebase merge). Thanks for the contribution! 🎉

kshitijk4poor added a commit that referenced this pull request May 1, 2026
Adds email→username mappings for:
- priveperfumes (PR #18456)
- amroessam (PR #17798)
- Hinotoi-agent (PR #9361)
- valda (PR #14932)
donald131 pushed a commit to donald131/hermes-agent that referenced this pull request May 2, 2026
Adds email→username mappings for:
- priveperfumes (PR NousResearch#18456)
- amroessam (PR NousResearch#17798)
- Hinotoi-agent (PR NousResearch#9361)
- valda (PR NousResearch#14932)
nickdlkk pushed a commit to nickdlkk/hermes-agent that referenced this pull request May 11, 2026
Adds email→username mappings for:
- priveperfumes (PR NousResearch#18456)
- amroessam (PR NousResearch#17798)
- Hinotoi-agent (PR NousResearch#9361)
- valda (PR NousResearch#14932)
jsboige pushed a commit to jsboige/hermes-agent that referenced this pull request May 14, 2026
Adds email→username mappings for:
- priveperfumes (PR NousResearch#18456)
- amroessam (PR NousResearch#17798)
- Hinotoi-agent (PR NousResearch#9361)
- valda (PR NousResearch#14932)
dannyJ848 pushed a commit to dannyJ848/hermes-agent that referenced this pull request May 17, 2026
Adds email→username mappings for:
- priveperfumes (PR NousResearch#18456)
- amroessam (PR NousResearch#17798)
- Hinotoi-agent (PR NousResearch#9361)
- valda (PR NousResearch#14932)
gweeteve pushed a commit to gweeteve/hermes-agent that referenced this pull request Jun 2, 2026
Adds email→username mappings for:
- priveperfumes (PR NousResearch#18456)
- amroessam (PR NousResearch#17798)
- Hinotoi-agent (PR NousResearch#9361)
- valda (PR NousResearch#14932)
Seven74AI pushed a commit to Seven74AI/hermes-agent that referenced this pull request Jun 13, 2026
Adds email→username mappings for:
- priveperfumes (PR NousResearch#18456)
- amroessam (PR NousResearch#17798)
- Hinotoi-agent (PR NousResearch#9361)
- valda (PR NousResearch#14932)
waefrebeorn pushed a commit to waefrebeorn/slermes that referenced this pull request Jul 2, 2026
Adds email→username mappings for:
- priveperfumes (PR NousResearch#18456)
- amroessam (PR NousResearch#17798)
- Hinotoi-agent (PR NousResearch#9361)
- valda (PR NousResearch#14932)
Gravezzz pushed a commit to Gravezzz/hermes-agent that referenced this pull request Jul 21, 2026
Adds email→username mappings for:
- priveperfumes (PR NousResearch#18456)
- amroessam (PR NousResearch#17798)
- Hinotoi-agent (PR NousResearch#9361)
- valda (PR NousResearch#14932)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/config Config system, migrations, profiles comp/gateway Gateway runner, session dispatch, delivery P2 Medium — degraded but workaround exists platform/discord Discord bot adapter platform/slack Slack app adapter type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants