Skip to content

fix(discord): track handoff-created threads for replies - #90348

Open
ryanlatham wants to merge 2 commits into
NousResearch:mainfrom
ryanlatham:codex/fix-discord-handoff-thread-tracking
Open

fix(discord): track handoff-created threads for replies#90348
ryanlatham wants to merge 2 commits into
NousResearch:mainfrom
ryanlatham:codex/fix-discord-handoff-thread-tracking

Conversation

@ryanlatham

@ryanlatham ryanlatham commented Aug 19, 2026

Copy link
Copy Markdown

What does this PR do?

DiscordAdapter.create_handoff_thread() did not register successfully created Discord threads in the existing ThreadParticipationTracker.

With require_mention=true and thread_require_mention=false, the first ordinary, unmentioned message in a handoff-created thread could therefore be rejected. This change records both direct-created and seed-fallback-created threads before returning them.

Thread creation remains authoritative over bookkeeping: if participation-tracker persistence fails after Discord creates the thread, Hermes logs a warning (including the thread ID, traceback, and restart-durability consequence) and still returns the real thread ID. It does not report a false creation failure or attempt a duplicate fallback thread.

Failed and unsupported creation attempts remain untracked. No configuration change is required.

Related Issue

Related PR: #63459

Based on work by @SEStarkman in #63459, especially commit 9d269402d5. This PR ports the tracking behavior onto current main's direct-first/fallback implementation.

It intentionally does not change creation order, stale-seed cleanup, Discord public/private thread behavior, or thread discoverability. Those broader changes remain separate.

Type of Change

  • 🐛 Bug fix (non-breaking change that fixes an issue)
  • ✨ New feature
  • 🔒 Security fix
  • 📝 Documentation update
  • ✅ Tests
  • ♻️ Refactor
  • 🎯 New skill

Changes Made

  • plugins/platforms/discord/adapter.py
    • Track successful direct and seed-fallback handoff-thread creation.
    • Catch tracker failures, warn, and return the successfully created Discord thread ID.
  • tests/gateway/test_discord_send.py
    • Cover successful direct/fallback tracking, failed creation, and tracker failures after both creation paths without duplicate creation.
    • Exercise a real tracker _save() failure and verify the in-memory mark remains usable.
  • tests/e2e/test_discord_adapter.py
    • Verify an unmentioned message in a tracked handoff thread is admitted.
    • Verify the same message in an untracked thread remains rejected.

How to Test

scripts/run_tests.sh tests/gateway/test_discord_send.py tests/e2e/test_discord_adapter.py -q
scripts/run_tests.sh tests/gateway tests/e2e/test_discord_adapter.py -q
scripts/run_tests.sh
ruff check .
python scripts/check-windows-footguns.py --all
git diff --check upstream/main...HEAD
python3 scripts/audit_pr_attribution.py

Automated evidence:

  • PR head: 8fcbb1374fbd68b7340bfa0b0c44cfade98a0bfd.
  • Current upstream/main: 13f4cfebfafbce8ac9d1bf29f66731858ed638b5.
  • Local current-main integration tested: 6a46804520d589656972812e750ca88f10bd2851.
  • GitHub merge result: 05243f87a727a7bedd3c072865c6f7cb801fdc1c; it has the two SHAs above as parents and the same tree as the tested local integration.
  • Focused regressions: 25 passed.
  • Relevant Discord/gateway suite: 6,112 passed, 4 failed, 30 skipped. All four failures reproduce unchanged on the exact current upstream/main SHA in this macOS environment.
  • Complete repository suite: 36,688 passed, 43 failed, 307 skipped across 3,181 files; one additional file failed once and passed on the runner's retry.
    • 41 of the 43 final failures reproduce unchanged on exact current upstream/main.
    • The other two are timing-sensitive under full-suite load and pass in isolated reruns on both current main and the integration commit (12/12 passed on each).
    • The failures are outside this PR's three changed files and cover macOS/Linux path or socket assumptions, missing optional lazy-installed SDKs under the installed policy, host-specific command behavior, active-service discovery, and unrelated timing/config-state tests.
  • ruff check .: passed (with two pre-existing invalid-noqa warnings).
  • Windows-footgun scan: passed (1,003 files scanned).
  • PR-only git diff --check: passed.
  • Contributor-attribution audit: passed.

Manual Discord evidence:

  • Tested through the standard installed gateway with the current-main integration SHA above.
  • With require_mention=true and thread_require_mention=false, the first message sent in the handoff-created thread—without a mention and without replying to the bot—received a response and correctly recalled BLUE-COMET-90348.
  • Discord required the direct thread link to locate the created thread. Discoverability and public/private-thread behavior are unchanged by this PR and intentionally out of scope.

Checklist

Code

  • I've read the Contributing Guide.
  • My commit messages follow Conventional Commits.
  • I searched for existing PRs; fix(discord): anchor handoff threads in parent channel #63459 is credited and the narrower scope is explained above.
  • My PR contains only changes related to this fix.
  • I've run the complete repository suite; all PR-relevant tests pass, with baseline/transient failures documented above.
  • I've added tests for the bug and both reviewer-requested edge cases.
  • I've tested on macOS.

Documentation & Housekeeping

  • Documentation changes are not required for this runtime-only bug fix.
  • No configuration keys were added or changed.
  • No architecture or workflow documentation changes are required.
  • Cross-platform checks were considered and the Windows-footgun scan passed.
  • No tool descriptions or schemas changed.

Screenshots / Logs

Not applicable. Automated regressions and a live Discord test are summarized above.

@alt-glitch alt-glitch added type/bug Something isn't working comp/plugins Plugin system and bundled plugins platform/discord Discord bot adapter P3 Low — cosmetic, nice to have sweeper:risk-message-delivery Sweeper risk: may drop, duplicate, misroute, or suppress messages labels Aug 19, 2026
@Enough1122

Copy link
Copy Markdown
Contributor

AI code review — automated review for reference; please use your judgment.

Reviewed by reviewer-e (AI automated review).

Correct root-cause fix: a handoff thread the bot itself created was invisible to the thread registry, so follow-up replies inside it were gated by channel-level mention policy and silently dropped under REQUIRE_MENTION. The try/except/else restructure puts _threads.mark exactly where success is proven on both creation paths (direct and seed-message fallback), keeps failure paths mark-free, and three gateway tests pin mark-once/no-seed-send-on-direct/failure-doesn't-mark, plus an end-to-end test proving an unmentioned reply in a freshly created handoff thread is now admitted. Findings below are minor:

  1. plugins/platforms/discord/adapter.py:7391 — _threads.mark(thread_id) sits unprotected inside the else block: if the registry write ever raises (disk-backed store, unexpected shape), the function throws after Discord already created the thread — callers see an exception instead of a usable id, and the fallback path never runs despite the direct creation having succeeded. Wrapping the two mark calls in a log-and-continue keeps thread creation authoritative over bookkeeping.

  2. tests/e2e/test_discord_adapter.py — the e2e test asserts admission via handle_message being awaited, but nothing asserts the negative control (an unmentioned reply in an untracked thread still rejected), which is the behavior this fix is supposed to change; one assertion there would guard against the registry check being loosened globally later.

Copy link
Copy Markdown
Author

Thanks — both findings are addressed in follow-up commit 8fcbb1374fbd68b7340bfa0b0c44cfade98a0bfd.

  1. Successful Discord creation is now authoritative over participation-tracker persistence. A tracking failure is caught after creation, logged at warning level with the thread ID, traceback, and restart-durability consequence, and the real thread ID is returned. The direct-path regression uses a real _save() failure and verifies that the in-memory mark remains active and no seed fallback/duplicate creation occurs; the fallback path has equivalent failure coverage.

  2. The e2e coverage now includes the requested negative control: an unmentioned message in an untracked thread remains rejected, while the equivalent message in a tracked handoff-created thread is admitted.

Verification on current-main integration:

  • Focused regressions: 25/25 passed.
  • Relevant Discord/gateway suite: 6,112 passed; its four failures reproduce unchanged on exact current upstream/main.
  • Complete suite: 36,688 passed, 43 failed, 307 skipped. 41 final failures reproduce unchanged on exact current main; the other two timing-sensitive failures pass isolated on both main and the integration commit (12/12 each).
  • Ruff, Windows-footgun, PR-only whitespace, and contributor-attribution checks all pass.
  • Live standard-install test passed: the first message in the handoff-created thread was neither a mention nor a reply, it received a response, and the session recalled BLUE-COMET-90348.

The direct thread still required its link to locate. Discoverability and Discord public/private-thread behavior remain unchanged and intentionally separate from this PR.

@ryanlatham
ryanlatham marked this pull request as ready for review August 22, 2026 19:14
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-message-delivery Sweeper risk: may drop, duplicate, misroute, or suppress messages type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants