Skip to content

fix(discord): honor auto_thread_free_response in routing - #86351

Open
bertholomus wants to merge 1 commit into
NousResearch:mainfrom
bertholomus:fix/discord-free-response-auto-thread-config
Open

fix(discord): honor auto_thread_free_response in routing#86351
bertholomus wants to merge 1 commit into
NousResearch:mainfrom
bertholomus:fix/discord-free-response-auto-thread-config

Conversation

@bertholomus

Copy link
Copy Markdown

Summary

  • restore the existing discord.auto_thread_free_response opt-in in the current plugin Discord adapter
  • seed the supported YAML key into PlatformConfig.extra while preserving legacy env compatibility
  • keep free-response channels inline by default and keep no_thread_channels as the hard override
  • add focused regression coverage for env and config-extra routing

Reproduction

With free_response_channels containing the parent, auto_thread: true, and auto_thread_free_response: true, current main still executes:

skip_thread = bool(channel_keys & no_thread_channels) or is_free_channel

That unconditional or is_free_channel makes the public opt-in ineffective and routes replies into the parent channel.

Verification

  • python -m pytest tests/gateway/test_discord_free_response.py -q -o addopts= → 23 passed
  • focused routing/config selection → 4 passed
  • direct _apply_yaml_config probe confirms auto_thread_free_response is seeded and legacy env is bridged
  • git diff --check

Related work

Several older PRs address similar behavior using alternate config names or pre-plugin paths. This patch targets the current plugins/platforms/discord/adapter.py implementation and the already-present public key auto_thread_free_response.

@coderabbitai

coderabbitai Bot commented Aug 14, 2026

Copy link
Copy Markdown

Important

Review skipped

Auto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: 667e878c-9195-44b8-bcd4-1e17cadd0a8f

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review

Comment @coderabbitai help to get the list of available commands.

@alt-glitch alt-glitch added type/bug Something isn't working P3 Low — cosmetic, nice to have comp/plugins Plugin system and bundled plugins platform/discord Discord bot adapter labels Aug 14, 2026
@alt-glitch

Copy link
Copy Markdown
Collaborator

This was generated by AI during triage.

Related: #33110 addresses the same free-response auto-threading cluster, but uses a different default behavior and bundles thread-label changes. This patch restores the existing explicit auto_thread_free_response opt-in on the current plugin adapter; maintainers should select the intended behavior.

@Enough1122

Copy link
Copy Markdown
Contributor

AI code review — automated review for reference, author can ignore or act on any point.

fix(discord): honor auto_thread_free_response in routing

No blocking issues found. A few minor observations:

  1. plugins/platforms/discord/adapter.py (L8127-8138): the value is read via _config_value("auto_thread_free_response", False, env_key="DISCORD_AUTO_THREAD_FREE_RESPONSE"), and _apply_yaml_config (L10383-10390) also bridges the YAML value into that env var. If _config_value gives env precedence, a user setting both env and YAML to different values gets the env silently — consistent with the sibling auto_thread handling, so this is fine, just worth a line in the config docs noting precedence.
  2. The string coercion accepts {"true", "1", "yes"} — matches the existing DISCORD_AUTO_THREAD parsing in the same function, so behavior is consistent across the two flags.
  3. Behavior edge confirmed sane: when auto_thread_free_response is true, free-response channels thread, but no_thread_channels still wins and is_reply_message still skips threading — the new flag only relaxes the free-channel branch, nothing else.

@alt-glitch alt-glitch added the sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades label Aug 16, 2026
@trupel

trupel commented Aug 18, 2026

Copy link
Copy Markdown

I compared this with my duplicate PR #89473. Your implementation has the stronger focused coverage and uses the existing _apply_yaml_config path, so I’ll close mine as a duplicate.

Useful additions from my testing:

documentation for auto_thread_free_response;
live Discord verification that each top-level message creates a separate thread and thread follow-ups work without mentions;
coordination with #87358, since rapid top-level messages can hit Discord’s thread-creation rate limit.

@bertholomus

Copy link
Copy Markdown
Author

This regression reproduced again on current main after the local commit was reset during update. The current adapter also scopes Discord gate values per profile under multiplex, so the durable implementation now additionally: (1) adds DISCORD_AUTO_THREAD_FREE_RESPONSE to _GATE_ENV_KEYS, (2) resolves it through _gate_raw("auto_thread_free_response", "DISCORD_AUTO_THREAD_FREE_RESPONSE"), (3) seeds PlatformConfig.extra in _apply_yaml_config while respecting _profile_scoped_config_load(), and (4) covers multiplex gate isolation. Current focused verification: 45 tests across test_discord_free_response.py and test_discord_gate_isolation.py. I cannot update this PR branch because the active OAuth token lacks workflow scope and the current base modifies .github/workflows/ci.yaml; the existing PR remains the upstream durability gate.

@sammy-ye

sammy-ye commented Sep 6, 2026

Copy link
Copy Markdown

Tested the same mechanism locally on current main (2-person server, free-response + auto-thread) — confirms the need. Supporting this PR over #103990: the config.yaml key with env bridge plus tests is the right shape per repo conventions. Happy to help rebase if needed — current main has moved (routing now around adapter.py ~5997).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

comp/plugins Plugin system and bundled plugins P3 Low — cosmetic, nice to have platform/discord Discord bot adapter sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants