Skip to content

fix: restore Discord auto-threading with free-response channels - #33110

Open
hdiesel323 wants to merge 1 commit into
NousResearch:mainfrom
hdiesel323:feat/discord-thread-routing-labels
Open

fix: restore Discord auto-threading with free-response channels#33110
hdiesel323 wants to merge 1 commit into
NousResearch:mainfrom
hdiesel323:feat/discord-thread-routing-labels

Conversation

@hdiesel323

Copy link
Copy Markdown

Summary

  • Decouples Discord free_response_channels from auto-thread creation so free_response_channels: '*' no longer bypasses thread routing.
  • Adds a WT-style Discord thread label registry for machine-readable thread metadata.
  • Adds regression coverage for free-response auto-threading and thread-label loading.

Test Plan

  • pytest tests/ → 369 passed, 2 warnings

Linear

@hdiesel323
hdiesel323 requested a review from a team May 27, 2026 08:00
@alt-glitch alt-glitch added type/bug Something isn't working P2 Medium — degraded but workaround exists comp/gateway Gateway runner, session dispatch, delivery comp/plugins Plugin system and bundled plugins platform/discord Discord bot adapter duplicate This issue or pull request already exists labels May 27, 2026
@alt-glitch

Copy link
Copy Markdown
Collaborator

Duplicate of #13958 (oldest open). This is the 8th+ PR attempting to decouple free_response_channels from auto-thread suppression in the Discord adapter. Competing open PRs: #13958, #18455, #18592, #27651, #29981, #31852. The root issue is #26058.

@austinpickett austinpickett left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Request Changes

The core fix (removing or is_free_channel from skip_thread) is correct — the bug is still present on main and this is the right one-line change. However, this PR is over-scoped relative to the stated bug fix:

🔴 Scope creep / smuggled content:

  1. thread_labels.py + thread_labels.json — a new thread-metadata registry (wtupdate/wtsignoff labels, [labels: ...] appended to thread names). This is a new feature, not a bug fix. Belongs in its own PR.
  2. vercel==0.5.7 added to pyproject.toml optional deps — unexplained. Unrelated to Discord threading. Please remove.
  3. No CI run on this branch.

Coordination note: This is the 8th+ attempt at this fix (related canonical PRs: #13958 [P1, open], #31852). Before merging, this should either consolidate into or supersede the canonical issue. I'd recommend: \n- Strip to just the 1-line fix in adapter.py + the updated test\n- Remove vercel dep + thread-labels feature (separate PR)\n- Then resolve against #13958/#31852 for deduplication

@teknium1 teknium1 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the regression coverage and for identifying the configuration tension.

Problems

  • The routing change at plugins/platforms/discord/adapter.py:4558 reverses main's intentional default: current plugins/platforms/discord/adapter.py:6223, docs at website/docs/user-guide/messaging/discord.md:377, and test tests/gateway/test_discord_free_response.py:658 keep free-response channels inline. Commit 232f5079f introduced that behavior specifically to avoid creating a thread for every mention-free message.
  • The label registry only appends parsed bracket tokens to formatted chat names in this diff; it is unrelated to routing and should not be coupled to this policy change. The vercel extra in pyproject.toml is likewise unrelated.

Suggested changes

  • Keep the current inline default and, if maintainers select this product direction, re-scope to an explicit documented discord.* opt-in for threaded free-response channels, with coverage for both default and opt-in behavior.
  • Split the label-registry and dependency changes into separate, independently justified work.

Automated hermes-sweeper review.

@@ -4552,7 +4558,7 @@ async def _handle_message(self, message: DiscordMessage) -> None:
if not is_thread and not isinstance(message.channel, discord.DMChannel):

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This removes the intentional free-response inline guard. Current main retains the equivalent condition at plugins/platforms/discord/adapter.py:6223; commit 232f5079f and tests/gateway/test_discord_free_response.py:658 establish it as the documented default. Please preserve that default and add a separate explicit opt-in if threaded free-response channels are selected as a supported mode.

@teknium1 teknium1 added sweeper:risk-session-state Sweeper risk: may lose/corrupt/mis-associate session or context state sweeper:risk-message-delivery Sweeper risk: may drop, duplicate, misroute, or suppress messages sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform labels Jul 13, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

comp/gateway Gateway runner, session dispatch, delivery comp/plugins Plugin system and bundled plugins duplicate This issue or pull request already exists P2 Medium — degraded but workaround exists platform/discord Discord bot adapter sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades sweeper:risk-message-delivery Sweeper risk: may drop, duplicate, misroute, or suppress messages sweeper:risk-session-state Sweeper risk: may lose/corrupt/mis-associate session or context state type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants