Repository navigation
fix(discord): a human @-mentioning another agent is not addressing everyone - #2710
Conversation
…eryone
In a requireMention:false channel the addressee gate had three ways to say "not
for us" -- another bot's post, a reply to someone else, a reply to us -- and no
way to notice that a HUMAN named a different agent. So it fell through to the
"addresses nobody, process it" branch.
Observed live twice in five minutes, in a channel named "arcade -- Pro
iterating, Chi reviews":
<@Pro> can you make the sutando nightshift more minecraftish? 32x32?
<@Pro> keep improving the arcade. ...
Both were queued here as owner tasks this instance could only discard, and the
progress-streamer posted "working..." into the lane the owner had pointed at one
agent. Pro was live and answered both within 9s, so the owner got a second bot
narrating in a review thread that was not its own.
Gate, at origin/main vs this branch, on that exact message shape:
PARENT ingested as MY task: True
HEAD ingested as MY task: False
The peer-mention alone never silences us: naming us AND a peer, role-mentioning
us alongside a peer, and replying to us while naming a peer all still resolve to
ours. An unaddressed owner message is unchanged -- who owns those belongs to the
channel-IFC design, not this gate.
`other_agent_mentioned` defaults to False, so a caller that does not pass it
behaves exactly as before. The adapter resolves which mentioned users are bots
other than us; the policy stays in the pure module. The skip log gained the new
field -- without it a skip from this branch prints author_bot=False, reply=False
and reads as unexplained.
Tests: 6 new cases in the existing assertion suite (exit 1 at parent, 0 here),
plus two that pin the bridge wiring. All 31 other discord-bridge suites and
progress-stream green.
Stand: Echo Act IV Mini
qingyun-wu
left a comment
qingyun-wu
left a comment
There was a problem hiding this comment.
Approved at exact head 70f14aa2fd477d1f49efabcc6ddbd07e2d1af8b0.
The reported bug exists on main: in a requireMention:false shared channel, a human message that mentions only a different bot reaches the unaddressed-human fallback. This patch keeps the provider-specific mention resolution in the Discord adapter and the policy in discord_addressee, while preserving every reachability case that should still route here (direct/role mention of us, reply to us, multi-addressee including us, and an unaddressed human post).
I reviewed the full activated gate and ran all tests/discord-bridge*.test.py suites plus tests/progress-stream.test.py; they passed. git diff --check and the hardcoded-path scan also pass, with a separately verified firing positive control. Worst-case disruption is a false skip when a human explicitly names another bot without naming this one; that is the intended addressed-elsewhere boundary, while explicit mentions/replies to this bot take precedence and remain reachable.
Code is ready to merge after the still-running hosted checks finish green and the repository's full approval gate is satisfied.
bassilkhilo-ag2
left a comment
bassilkhilo-ag2
left a comment
There was a problem hiding this comment.
Independently verified on head commit — cloned fresh, traced the logic myself rather than trusting the description alone.
Ordering is correct (the part that matters for reachability): is_addressed_in_shared_channel checks bot_mentioned or role_mentioned or replying_to_me FIRST and returns True immediately — other_agent_mentioned is only consulted after that, so naming us alongside a peer (<@Mini> and <@PRO> both look at this) still resolves to ours. Confirmed this isn't just asserted in the PR body but actually exercised: 4 explicit test cases cover bot_mentioned+peer, role_mentioned+peer, reply-to-us+peer, and the plain False default (back-compat for callers that don't pass the new kwarg).
Ran the test file locally, all 18 assertions pass including the new ones:
ok owner @-mentions another agent -> not ours
ok owner @-mentions us AND a peer -> still ours
ok role-mention of us alongside a peer -> still ours
ok reply to us that also names a peer -> still ours
ok owner addresses nobody -> still ours (unchanged)
ok parameter defaults to False (back-compatible)
All addressee-gate cases pass.
Bridge-side derivation is also sound: _other_agent_mentioned in discord-bridge.py checks getattr(u, "bot", False) and getattr(u, "id", None) != _self_id over message.mentions — correctly scoped to other bots, doesn't misfire on the owner's own mention or a human co-mention.
Small note, not a blocker: the CI redness on this PR right now (eslint, smoke → "Failed to resolve action download info" / "Service Unavailable"; several checks timing out at 2m and getting cancelled) is a GitHub Actions infra issue — the same failure signatures are showing up on #2706 too, on completely unrelated files. Not this PR's fault; worth a re-run once Actions recovers rather than digging into the diff further.
This fix is exactly the same root cause I hit live in this instance's own task queue this session (a <@PRO>-addressed message that should never have reached me) — good to see it closed at the addressee-gate level rather than patched per-symptom.
Coverage Gate✅ Diff coverage PASSES the 95% bar. Whole-tree (informational): 78%. Diff CoverageDiff: origin/main...HEAD, staged and unstaged changes
Summary
|
The defect
In a
requireMention:falsechannel the addressee gate had three ways to say "not for us" — another bot's post, a reply to someone else, a reply to us — and no way to notice that a human named a different agent. So<@PRO> ...fell through to the "addresses nobody, process it" branch.Observed live, twice in five minutes
Channel: "arcade — Pro iterating, Chi reviews" (the name states whose lane it is).
Both arrived here as owner tasks this instance could only discard, and the progress-streamer narrated into a review thread the owner had pointed at one agent. Pro was live and answered both within nine seconds.
This is the same noise class the suite's own docstring already cites — "another agent's own posts (e.g. a sibling bot's ⏳ working… status)" — one step upstream: the human doing the addressing.
Before / after — the gate, on that exact message shape
Test suite: exit 1 at parent, exit 0 here (unpiped exit codes —
| tailwould have reported the pipe's status).Reachability is preserved — the part worth reviewing
Guarding on "a peer was mentioned" alone would silence this bot whenever the owner addresses two agents at once. All of these still resolve to ours:
An unaddressed owner message is deliberately unchanged — who owns those is the channel-IFC question, not this gate's.
Shape
other_agent_mentioneddefaults toFalse, so any caller that doesn't pass it behaves exactly as before.message.mentions); the policy stays in the pure, unit-testeddiscord_addressee.author_bot=False, reply=Falseand reads as unexplained.Tests
6 new cases in the existing assertion-style suite, plus 2 pinning the bridge wiring (that it passes the kwarg, and derives it from
message.mentions). All 31 otherdiscord-bridge-*suites andprogress-streamgreen.