Skip to content

fix(discord-bridge): pop reply-anchor INTO a var before downstream read - #1003

Merged
qingyun-wu merged 1 commit into
mainfrom
fix/discord-bridge-anchor-pop-order
May 22, 2026
Merged

qingyun-wu merged 1 commit into
mainfrom
fix/discord-bridge-anchor-pop-order

Conversation

@sonichi

@sonichi sonichi commented May 22, 2026

Copy link
Copy Markdown
Owner

Bug

PR #1002 added auto-thread of replies to the triggering message via pending_reply_anchors. Live-tested 2026-05-22 ~03:00 UTC: replies still appeared as fresh messages, not quote-replies. Chi: "it's not a quote reply".

Root cause

In poll_results, the anchor was popped immediately after the channel pop:

channel = pending_replies.pop(task_id)
pending_reply_anchors.pop(task_id, None)   # ← removed from dict
...  # ~50 lines later
reply_to_id = pending_reply_anchors.get(task_id)   # ← always None now

Fix

Capture into a local:

channel = pending_replies.pop(task_id)
source_message_anchor = pending_reply_anchors.pop(task_id, None)
...
if reply_to_id is None and not is_thread:
    reply_to_id = source_message_anchor

Why the unit test missed it

tests/discord-bridge-reply-directive.test.py exercises two paths: explicit [reply: <id>] directive (passes) and no-directive (passes). The no-directive case passed coincidentally because the test fixture never populates pending_reply_anchors for that task_id — so the buggy .get() returned None, and the test asserted "no reference set". The bug + the test gap canceled out.

Test plan

  • Unit test still green: python3 tests/discord-bridge-reply-directive.test.py (all 3 cases pass)
  • Live: restart bridge, send a message in a regular channel, agent replies → quote-reply visible
  • In-thread: send a message in a thread, agent replies → no quote anchor (thread context only)
  • Could add a 4th unit test case explicitly populating pending_reply_anchors[task_id] and asserting the reference WAS set, to prevent future regressions

🤖 Generated with Claude Code

…tream

Live-test caught: PR #1002's auto-thread never fired because the result-send
loop popped pending_reply_anchors[task_id] immediately after pending_replies
pop, then ~50 lines later tried to .get(task_id) — which returned None
because the pop already removed it.

Capture the anchor into `source_message_anchor` at pop time; reference that
local in the auto-thread conditional. Unit test still green (covers the
"explicit [reply: <id>] directive present" + "no directive, no reference"
cases — the latter passes because pending_reply_anchors is also empty in
the test fixture, which is why the unit test missed the bug).

Symptom: reply went as a fresh channel message instead of a quote-reply
threaded to the triggering msg. Caught 2026-05-22 ~03:00 UTC by Chi
("it's not a quote reply") on the first live verification round of #1002.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

@qingyun-wu qingyun-wu 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.

LGTM!

@qingyun-wu
qingyun-wu merged commit e7df202 into main May 22, 2026
2 checks passed
@qingyun-wu
qingyun-wu deleted the fix/discord-bridge-anchor-pop-order branch May 22, 2026 03:04
liususan091219 pushed a commit to liususan091219/sutando-fork that referenced this pull request May 28, 2026
…tream (sonichi#1003)

Live-test caught: PR sonichi#1002's auto-thread never fired because the result-send
loop popped pending_reply_anchors[task_id] immediately after pending_replies
pop, then ~50 lines later tried to .get(task_id) — which returned None
because the pop already removed it.

Capture the anchor into `source_message_anchor` at pop time; reference that
local in the auto-thread conditional. Unit test still green (covers the
"explicit [reply: <id>] directive present" + "no directive, no reference"
cases — the latter passes because pending_reply_anchors is also empty in
the test fixture, which is why the unit test missed the bug).

Symptom: reply went as a fresh channel message instead of a quote-reply
threaded to the triggering msg. Caught 2026-05-22 ~03:00 UTC by Chi
("it's not a quote reply") on the first live verification round of sonichi#1002.

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants