fix(discord): anchor handoff threads in parent channel - #63459
fix(discord): anchor handoff threads in parent channel#63459SEStarkman wants to merge 3 commits into
Conversation
teknium1
left a comment
There was a problem hiding this comment.
Thanks for addressing both the discoverability and continuation-state gaps. The current-main premise is verified: plugins/platforms/discord/adapter.py:5498-5507 creates an unanchored thread and :6212-6220 requires the id to be in _threads before permitting mentionless replies.
Problems
plugins/platforms/discord/adapter.py:5519sends the visible seed beforeseed_msg.create_thread(). If that call fails, the direct fallback at:5540succeeds but leaves the seed in the parent timeline even though it does not anchor the returned thread. The PR's own fallback test attests/gateway/test_discord_send.py:545-572exercises this exact sequence without checking cleanup.
Suggested changes
- Keep the seed message and, after a successful direct fallback, best-effort delete it or revise it to make the unanchored fallback explicit. Add an assertion to the fallback test.
Automated hermes-sweeper review.
| return None | ||
| seed_msg = await send(f"\U0001f9f5 Hermes handoff: **{thread_name}**") | ||
| thread = await seed_msg.create_thread( | ||
| thread = await create( |
There was a problem hiding this comment.
If seed_msg.create_thread() failed after the send at line 5519, this direct fallback returns an unanchored thread while leaving a visible “Hermes handoff” seed that does not open it. Please retain the seed and best-effort delete it (or edit it to accurately describe the fallback) after direct creation succeeds; extend the fallback test to cover that outcome.
There was a problem hiding this comment.
Confirmed and fixed in 789076d25. The adapter now retains the seed, marks the successfully created direct-fallback thread, then best-effort deletes the stale seed. Discord cleanup refusal is debug-logged and cannot turn a successful fallback into failure.
Regression coverage now asserts both successful deletion and cleanup-failure/non-fatal behavior; the anchored path and participation tracking remain covered.
Verification:
scripts/run_tests.sh tests/gateway/test_discord_send.py tests/e2e/test_discord_adapter.py -q— 35 passedscripts/run_tests.sh tests/cron/test_scheduler.py tests/gateway/test_discord_*.py tests/e2e/test_discord_adapter.py -q— 747 passed.venv/bin/ruff check plugins/platforms/discord/adapter.py tests/gateway/test_discord_send.py tests/e2e/test_discord_adapter.py— passed.venv/bin/python -m py_compile plugins/platforms/discord/adapter.py tests/gateway/test_discord_send.py tests/e2e/test_discord_adapter.py cron/scheduler.py— passedgit diff --check— passed
|
The seed-message-first path is a good improvement, but the direct fallback still has a correctness gap.
thread = await create(
name=thread_name,
auto_archive_duration=1440,
reason=reason,
)Therefore, when the visible seed path fails but direct creation succeeds, Hermes can create a private thread instead of the intended public continuable thread. We reproduced this against the installed discord.py signature/source; Please make the direct fallback explicit: thread = await create(
name=thread_name,
type=discord.ChannelType.public_thread,
auto_archive_duration=1440,
reason=reason,
)Suggested regression assertion: assert create_thread.await_args.kwargs["type"] is discord.ChannelType.public_threadWe currently carry this one-line fix locally; it applies cleanly to current |
Summary
thread_require_mention=falsethread_require_mention=true, plus DM and failed-create behaviorRoot causes
create_handoff_thread()preferredparent.create_thread(), producing an anchorless public thread that Discord mobile did not expose reliably in the parent timeline.self._threads.mark(thread_id), so continuable cron threads were absent fromdiscord_threads.jsonand mentionless replies were dropped before session routing.Verification
scripts/run_tests.sh tests/gateway/test_discord_send.py tests/e2e/test_discord_adapter.py -q(34 passed)scripts/run_tests.sh tests/cron/test_scheduler.py tests/gateway/test_discord_*.py tests/e2e/test_discord_adapter.py -q(765 passed).venv/bin/ruff check plugins/platforms/discord/adapter.py tests/gateway/test_discord_send.py tests/e2e/test_discord_adapter.py.venv/bin/python -m py_compile plugins/platforms/discord/adapter.py tests/gateway/test_discord_send.py tests/e2e/test_discord_adapter.py cron/scheduler.pygit diff --checkDeployment
No migration or config change. Restart the Hermes gateway after installing/checking out the merged version; this PR does not restart, update, merge, or deploy anything.