Skip to content

fix(discord): auto-thread free-response channels - #18592

Closed
andyylin wants to merge 5 commits into
NousResearch:mainfrom
andyylin:fix/discord-free-response-auto-thread
Closed

fix(discord): auto-thread free-response channels#18592
andyylin wants to merge 5 commits into
NousResearch:mainfrom
andyylin:fix/discord-free-response-auto-thread

Conversation

@andyylin

@andyylin andyylin commented May 2, 2026

Copy link
Copy Markdown
Contributor

Summary

  • adds deterministic smart names for Discord auto-created threads
  • keeps discord.free_response_channels from disabling auto-thread creation
  • preserves discord.no_thread_channels as the explicit auto-thread opt-out
  • retitles Discord auto-thread sessions after the first exchange when smart thread titles are enabled
  • hardens Discord auto-thread tests against host env leakage

Runtime verification

  • Confirmed in the live Discord gateway after restart: a newly created thread used the smarter title path instead of the raw first-message slice.

Test plan

  • python -m pytest tests/agent/test_title_generator.py tests/gateway/test_discord_slash_commands.py -q -o 'addopts=' — 62 passed
  • Discord live smoke test: new auto-created thread title looked correct

@alt-glitch alt-glitch added type/bug Something isn't working P2 Medium — degraded but workaround exists platform/discord Discord bot adapter comp/gateway Gateway runner, session dispatch, delivery labels May 2, 2026
@andyylin

andyylin commented May 2, 2026

Copy link
Copy Markdown
Contributor Author

Pushed follow-up commit 16dcd437e to retitle Discord auto-thread sessions after the first exchange. Local targeted tests pass: python -m pytest tests/agent/test_title_generator.py tests/gateway/test_discord_slash_commands.py -q -o 'addopts=' → 62 passed. Runtime smoke check from live Discord also looks good: a new thread used the smart title path.

@andyylin
andyylin force-pushed the fix/discord-free-response-auto-thread branch from 16dcd43 to 3d8c57c Compare May 2, 2026 08:57
@andyylin

andyylin commented May 2, 2026

Copy link
Copy Markdown
Contributor Author

CI failure fix pushed.

Root cause: the PR branch had drifted behind current main; after rebasing, CI exposed stale expectations outside the original Discord title patch:

  • Discord free-response tests still expected free-response channels to skip auto-threading, but the PR behavior is intentionally: bypass mention gating while still auto-threading top-level messages.
  • ACP/TUI/concurrent-interrupt/gateway-restart/Teams/Dockerfile expectations had moved with main.

Fix:

  • Rebases PR fix(discord): auto-thread free-response channels #18592 onto current origin/main.
  • Updates the Discord free-response regression test to assert top-level free-response messages are dispatched through the auto-created thread.
  • Aligns affected tests/compat code with current CI behavior.
  • Adds a pending-title cleanup guard in the TUI gateway deferred session build path.

Local verification:

  • 84 passed in 2.63s for title + Discord targeted suite.
  • 365 passed in 13.34s for the previously failing-area suite.

Pushed commit: 3d8c57c0d702c333768158f7b8d218b9784dd95b.

@andyylin
andyylin force-pushed the fix/discord-free-response-auto-thread branch from 34a8a43 to 353d8ec Compare May 3, 2026 01:20
@andyylin
andyylin force-pushed the fix/discord-free-response-auto-thread branch 6 times, most recently from 0396192 to f868b44 Compare May 11, 2026 08:18
@andyylin
andyylin force-pushed the fix/discord-free-response-auto-thread branch from 39788e0 to 4674b9c Compare May 18, 2026 03:32

@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 Discord coverage and the live verification.

Problems

  • The free-response change conflicts with current behavior and stated intent: plugins/platforms/discord/adapter.py:6223 keeps is_free_channel in skip_thread, while website/docs/user-guide/messaging/discord.md:377 documents inline replies for free-response channels. Commit d55754456 added that guard specifically to prevent a thread per free-response message.
  • The semantic-title work is now provided by 0d9ed9214: auto-created-thread provenance is carried by plugins/platforms/discord/adapter.py:5355-5359,6338-6342, and gateway/run.py:13527-13610,19021-19026 retitles after the first exchange without overwriting a human rename.

Suggested changes

  • Preserve the current free-response inline policy unless maintainers choose a documented design change. Any remaining title issue should be re-scoped to a verified gap in the current semantic-title path.

Automated hermes-sweeper review.

no_thread_channels_raw = os.getenv("DISCORD_NO_THREAD_CHANNELS", "")
no_thread_channels = {ch.strip() for ch in no_thread_channels_raw.split(",") if ch.strip()}
skip_thread = bool(channel_ids & no_thread_channels) or is_free_channel
skip_thread = bool(channel_ids & no_thread_channels)

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 reverses the later explicit free-response contract. Current main retains or is_free_channel in the active adapter (plugins/platforms/discord/adapter.py:6223); commit d55754456 and website/docs/user-guide/messaging/discord.md:377 define free-response channels as intentionally inline to avoid creating one thread per message.

@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 12, 2026
@andyylin

Copy link
Copy Markdown
Contributor Author

Closing because current upstream intentionally keeps free-response channels inline, while the still-useful semantic-title behavior has already landed separately. The remaining patch is obsolete and conflicts with documented behavior.

@andyylin andyylin closed this Jul 16, 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 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.

3 participants