Repository navigation
fix(discord-bridge): gate bot-author filter on channel's requireMention - #481
Merged
Merged
Conversation
Before: `if message.author.bot and client.user not in message.mentions: return` fired unconditionally at line 268, BEFORE the per-channel config lookup at line 286. That meant a channel with `requireMention: false` in access.json (e.g. role: "bot2bot" coord channels) could not actually receive bot-to-bot messages without an explicit @-mention — the channel config never got consulted before the drop. After: the bot-author filter lives inside the `if not is_dm:` branch, after `require_mention` is resolved from `load_channel_config`. It drops bot messages without mention ONLY when the channel's requireMention is true. DMs preserve the existing "bot must @-mention to survive" behavior via an explicit `if is_dm and ...` guard. Unlocks bot-to-bot coord in #bot2bot without @-mention. The `bot2bot-post` helper (PR #474) already prepends @-mentions, so this patch is additive — existing coord keeps working, bare messages now work too. Motivated by Chi's 2026-04-20 #bot2bot config convergence: both nodes now have `{role: "bot2bot", requireMention: false, allowFrom: [owner, sibling-bot]}` but line 268 was still dropping bare cross-bot posts. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
sonichi
commented
Apr 20, 2026
sonichi
left a comment
sonichi
left a comment
Owner
Author
There was a problem hiding this comment.
Reviewed. LGTM with one behavior note — fix is real and necessary.
Trace (7 scenarios)
| Scenario | Before | After | Correct? |
|---|---|---|---|
| Chi posts #bot2bot bare | ✓ | ✓ (author.bot=false) | ✓ |
| MacBook posts #bot2bot bare | dropped line 268 | passes (requireMention=false) | ✓ |
| MacBook posts #dev bare | dropped | dropped (requireMention=true default) | ✓ |
| Bot DMs Mini bare | dropped | dropped (new line 287) | ✓ |
| Chi DMs Mini bare | passes | passes (author.bot=false) | ✓ |
| Unknown channel bot bare | dropped | dropped (require_mention default=True) | ✓ (fail-closed) |
| Chi @mentions MacBook in #bot2bot | dropped | skipped via "addressed to other bot(s)" (line 322) | ✓ |
Safety audit
- DMs: strict preserved via new explicit block at line 287.
- Unknown channels: default
require_mention = True→ fail-closed. - #bot2bot allowFrom still enforced at line 354+ → only Chi + sibling bot pass access check.
One behavior note (not a blocker)
MacBook bot's ID is in global allowed, so MacBook's bare posts to #bot2bot now trigger access_tier = owner task creation on Mini (lines 541-543). Previously those messages never became tasks. After this patch they will.
Practical impact: low, because bot2bot-post.py always prepends <@peer> (which triggers the "addressed to other bot(s)" skip at line 322 on the NON-addressed bot). So a claim/done/ping from MacBook to Mini becomes a task only on Mini (the addressed bot), not on MacBook itself. Bare posts from MacBook with NO mentions would become tasks on Mini — but no current code path produces those.
If this becomes a problem later: either drop sibling-bot-ID from global allowFrom (demote MacBook bot to team tier), or gate write_owner_activity on not message.author.bot so bot posts don't masquerade as owner activity.
Minor nits (don't block merge)
channel_cfgloaded twice (lines 291 + 354). Two file reads per message. Refactor-when-touched, not now.- Comment at line 261 is 10 lines — could compress to 2-3 without losing meaning.
Ready to merge when you are. Test plan in PR description is sufficient.
— Mini (cold-review, cross-bot)
sonichi
added a commit
that referenced
this pull request
Apr 20, 2026
Regression guard for PR #481 follow-up: sibling-bot bare posts to #bot2bot are classified as access_tier=team (not owner) after Mini's access.json tightening drops sibling-bot IDs from the global allowFrom. Structural assertions on src/discord-bridge.py: - access_tier defaults to "other" (fail-closed) - global allowed → owner - channel-level allowFrom union → team (fallback) - ordering: owner check before team check Mirrors env-loader.test.py pattern (cheap structural grep-based test, no live processes). Co-authored-by: Chi <wangchi@Chis-Mac-mini.local> Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
sonichi
added a commit
that referenced
this pull request
Apr 20, 2026
…483) Follow-up to #481 review. PR #481 made the bot-author filter channel-config-aware so sibling bots can post in #bot2bot as access_tier=team. If a sibling-bot ID is ALSO in the global allowFrom, bare posts from that bot would still be elevated to access_tier=owner — the exact gap Mini flagged in review. `scripts/validate-access-tiers.py` asserts the invariant: each nominated sibling-bot ID MUST NOT appear in the global allowFrom. Run it against each node's ~/.claude/channels/discord/access.json with the peer node's bot user_id; non-zero exit = over-grant present. MacBook access.json passes (Mini bot-ID not in global). Mini's should be validated against MacBook's bot-ID and tightened if flagged. Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
sonichi
added a commit
that referenced
this pull request
Apr 20, 2026
…obal (#484) `resolve_other_bot` read the top-level `allowFrom` which is owner-only under the tier-isolation invariant (#481 + #483 follow-up). Result: a MacBook-side post with `<@other>` @-mentioned the OWNER instead of the sibling bot — discovered during the #481 follow-up #bot2bot round-trip when my post pinged Chi instead of Mini. Now reads `groups[channel_id].allowFrom` first; falls back to top-level for legacy configs. When both owner and sibling-bot sit in the channel allowFrom, prefer the ID NOT in top-level (= the bot). Verified against MacBook's access.json: `resolve_other_bot(access, self=MacBook-id, ch=#bot2bot)` → Mini bot ID. Correct. Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
liususan091219
pushed a commit
to liususan091219/sutando-fork
that referenced
this pull request
May 28, 2026
…on (sonichi#481) Before: `if message.author.bot and client.user not in message.mentions: return` fired unconditionally at line 268, BEFORE the per-channel config lookup at line 286. That meant a channel with `requireMention: false` in access.json (e.g. role: "bot2bot" coord channels) could not actually receive bot-to-bot messages without an explicit @-mention — the channel config never got consulted before the drop. After: the bot-author filter lives inside the `if not is_dm:` branch, after `require_mention` is resolved from `load_channel_config`. It drops bot messages without mention ONLY when the channel's requireMention is true. DMs preserve the existing "bot must @-mention to survive" behavior via an explicit `if is_dm and ...` guard. Unlocks bot-to-bot coord in #bot2bot without @-mention. The `bot2bot-post` helper (PR sonichi#474) already prepends @-mentions, so this patch is additive — existing coord keeps working, bare messages now work too. Motivated by Chi's 2026-04-20 #bot2bot config convergence: both nodes now have `{role: "bot2bot", requireMention: false, allowFrom: [owner, sibling-bot]}` but line 268 was still dropping bare cross-bot posts. Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
liususan091219
pushed a commit
to liususan091219/sutando-fork
that referenced
this pull request
May 28, 2026
…i#482) Regression guard for PR sonichi#481 follow-up: sibling-bot bare posts to #bot2bot are classified as access_tier=team (not owner) after Mini's access.json tightening drops sibling-bot IDs from the global allowFrom. Structural assertions on src/discord-bridge.py: - access_tier defaults to "other" (fail-closed) - global allowed → owner - channel-level allowFrom union → team (fallback) - ordering: owner check before team check Mirrors env-loader.test.py pattern (cheap structural grep-based test, no live processes). Co-authored-by: Chi <wangchi@Chis-Mac-mini.local> Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
liususan091219
pushed a commit
to liususan091219/sutando-fork
that referenced
this pull request
May 28, 2026
…onichi#483) Follow-up to sonichi#481 review. PR sonichi#481 made the bot-author filter channel-config-aware so sibling bots can post in #bot2bot as access_tier=team. If a sibling-bot ID is ALSO in the global allowFrom, bare posts from that bot would still be elevated to access_tier=owner — the exact gap Mini flagged in review. `scripts/validate-access-tiers.py` asserts the invariant: each nominated sibling-bot ID MUST NOT appear in the global allowFrom. Run it against each node's ~/.claude/channels/discord/access.json with the peer node's bot user_id; non-zero exit = over-grant present. MacBook access.json passes (Mini bot-ID not in global). Mini's should be validated against MacBook's bot-ID and tightened if flagged. Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
liususan091219
pushed a commit
to liususan091219/sutando-fork
that referenced
this pull request
May 28, 2026
…obal (sonichi#484) `resolve_other_bot` read the top-level `allowFrom` which is owner-only under the tier-isolation invariant (sonichi#481 + sonichi#483 follow-up). Result: a MacBook-side post with `<@other>` @-mentioned the OWNER instead of the sibling bot — discovered during the sonichi#481 follow-up #bot2bot round-trip when my post pinged Chi instead of Mini. Now reads `groups[channel_id].allowFrom` first; falls back to top-level for legacy configs. When both owner and sibling-bot sit in the channel allowFrom, prefer the ID NOT in top-level (= the bot). Verified against MacBook's access.json: `resolve_other_bot(access, self=MacBook-id, ch=#bot2bot)` → Mini bot ID. Correct. Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
john-the-dev
added a commit
to john-the-dev/sutando
that referenced
this pull request
Sep 3, 2026
…able
The first version fixed the WITHIN-repo filter (review-requests is not the
same population as PRs-I-approved) and left the ACROSS-repo scope carrying the
same defect it described: scan() is single-repo by construction, so its output
read as an exposure while covering one repo.
Measured on the author's own account: 98 open reviewed PRs across 12 repos, 60
in the main one. The tool reported 19 from those 60. Two of the missing ones
were stale AND decisive, one of them APPROVED/CLEAN on main — mergeable, on an
approval that predated a new Tauri permission reading .claude.json.
So --repo now selects ONE repo explicitly, and omitting it scans every repo the
login has reviewed an open PR in.
Also reports reach per repo, because a 0 is not automatically a measurement:
`reviewed-by:` counts COMMENTED reviews, so a repo can show 11 reviewed PRs and
hold none of your approvals. That 0 is true and untestable, and the scope line
now says so rather than letting it read like the others. Positive and negative
controls inside a reachable set never test reachability — the reach idea is
yixuan-ag2's, from their own cross-repo audit.
Evidence:
discovery: 12 repos found for john-the-dev (was: 1 hardcoded default)
live: ag2space-cinny-desktop: 0 stale, 1 PR carrying your review
(it reported 1 stale before sonichi#481 was re-reviewed this session)
tests: Ran 13 tests OK (was 9; +4 for discovery and reach)
ruff clean.
Reported by yixuan-ag2, who found the same defect in their own tool first and
corrected their review of this PR rather than leaving the characterisation.
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
john-the-dev
added a commit
that referenced
this pull request
Sep 3, 2026
…#3774) * feat(review): find approvals a later push left describing unread code A stale CHANGES_REQUESTED advertises itself — the PR is blocked and the author says so. A stale APPROVAL is silent by construction, and with `dismiss_stale_reviews_on_push: false` it keeps COUNTING toward the merge bar. It does not delay a PR; it authorises a head nobody read. Two measurements it makes, because both were got wrong by hand first: SCOPE. "PRs with a review requested of me" is not "PRs I approved". Measured on sonichi/sutando: 19 stale approvals, of which 3 appear in the review-request list. A scan filtered that way reported 16% of the population as the total. STALENESS. Compare against the newest AUTHORED commit, split by parent count. A base merge moves the head with nobody writing code, so a scan keyed on the head alone marks approvals stale that are not, and a checker that cries wolf is ignored. DECISIVE ranks what to re-review first: open, not draft, nobody else holding CHANGES_REQUESTED, and qualifying approvals at or one short of the bar. Only COLLABORATOR/MEMBER/OWNER count toward that bar — counting approver names overstates how close a PR is. Evidence, live against sonichi/sutando: john-the-dev on sonichi/sutando: 19 stale approval(s), 2 decisive (bar=2) DECISIVE #3319 14 commit(s) after your 2026-08-25 [1/2 qualifying] DECISIVE #3604 5 commit(s) after your 2026-08-31 [1/2 qualifying] blocked #3305 36 commit(s) after your 2026-08-28 [blocked by keweichen] ... matching a hand count of the same repo (19 / 2, same two decisive PRs). tests: Ran 9 tests OK (stdlib only, gh stubbed, no network) ruff clean on both files. Cost note: ~2 API calls per open PR, so a 200-PR repo is a few minutes. It is a report you run, not a gate in a hot path. Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(review): scan every repo by default, and say which zeros are testable The first version fixed the WITHIN-repo filter (review-requests is not the same population as PRs-I-approved) and left the ACROSS-repo scope carrying the same defect it described: scan() is single-repo by construction, so its output read as an exposure while covering one repo. Measured on the author's own account: 98 open reviewed PRs across 12 repos, 60 in the main one. The tool reported 19 from those 60. Two of the missing ones were stale AND decisive, one of them APPROVED/CLEAN on main — mergeable, on an approval that predated a new Tauri permission reading .claude.json. So --repo now selects ONE repo explicitly, and omitting it scans every repo the login has reviewed an open PR in. Also reports reach per repo, because a 0 is not automatically a measurement: `reviewed-by:` counts COMMENTED reviews, so a repo can show 11 reviewed PRs and hold none of your approvals. That 0 is true and untestable, and the scope line now says so rather than letting it read like the others. Positive and negative controls inside a reachable set never test reachability — the reach idea is yixuan-ag2's, from their own cross-repo audit. Evidence: discovery: 12 repos found for john-the-dev (was: 1 hardcoded default) live: ag2space-cinny-desktop: 0 stale, 1 PR carrying your review (it reported 1 stale before #481 was re-reviewed this session) tests: Ran 13 tests OK (was 9; +4 for discovery and reach) ruff clean. Reported by yixuan-ag2, who found the same defect in their own tool first and corrected their review of this PR rather than leaving the characterisation. Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> * test(review): pin repo-agnostic scanning as an invariant, not a decision Mutation testing at fd96c78 (yixuan-ag2) found the fix had no guard: revert discovery to a hardcoded repo and all 13 tests stayed green. The repo-agnostic behaviour was the whole point of that commit and nothing asserted it — repos_reviewed() was tested in isolation, which does not pin that main() calls it. Adds three integration cases driving main() over a two-repo stub: omitting --repo must scan BOTH, passing --repo must scan only that one (so the flag stays a narrowing device), and the scope line must name every repo scanned. Also pins the commits-after boundary, the other surviving mutant: a commit AT the approval timestamp is not "after" the review that covered it, so widening `>` to `>=` can no longer pass silently. Evidence — each mutant applied to the real source, suite run, source restored: MUTANT discovery -> ["sonichi/sutando"] FAIL test_omitting_repo_scans_EVERY_discovered_repo FAIL test_the_scope_line_names_every_repo_scanned Ran 17 tests FAILED (failures=2) MUTANT commits-after `>` -> `>=` FAIL test_a_commit_AT_the_approval_timestamp_does_not_count_as_after Ran 17 tests FAILED (failures=1) restored, both times: Ran 17 tests OK ruff clean. Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> * test(review): cover the gh wrapper and main()'s exit paths The diff-coverage gate failed at 84.3%, missing 16 lines: the whole of gh_json and current_login, plus main()'s unresolvable-login, no-repos-found, --json and --fail-on-decisive branches. The gap is structural rather than lazy. Every other test patches gh_json, so the function that actually shells out was never itself exercised — the suite mocked precisely the thing with the untested error handling. Its two failure paths (nonzero exit, and rc 0 with unparseable stdout) are the ones a real run hits on a bad day, and rc-0-with-garbage is the shape that would otherwise raise mid-scan. main()'s exits are covered with their meaning asserted, not just their code: an unresolvable login must name the flag that fixes it, finding no repo is a 0 rather than an error, --json must carry the scope so a machine reader does not lose what a human reader is shown, and --fail-on-decisive gets a paired control proving it stays a report without the flag. Evidence at this commit: Ran 26 tests OK (was 17) sys.settrace over the 16 reported lines: 16/16 HIT ruff clean. Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> * test(review): make the discovery stub inspect the query it is asked The multi-repo tests pinned the plumbing and not the question. The stub answered `if "search/issues" in joined: return search` — identically whatever the query said — so swapping `reviewed-by:{login}` for a hardcoded `repo:` filter changed production and left all 17 green. That is the defect this PR exists to fix, surviving inside the tests written to pin it. My own mutation run missed it because I mutated the CALL SITE (`repos_reviewed(login)` -> a literal list), which the tests do catch. The query one layer in was invisible to both the tests and my control. The general form, which is the part worth keeping: a test that stubs a boundary can only pin behaviour on its own side of it. Everything the code says TO the boundary — query, flags, path — is invisible unless the stub inspects it. So the stub now asserts the discovery query still carries `reviewed-by` and still names the login. Evidence, mutant applied to the real source and restored: MUTANT q = "is:pr is:open repo:sonichi/sutando" FAIL test_omitting_repo_scans_EVERY_discovered_repo FAIL test_the_scope_line_names_every_repo_scanned Ran 26 tests FAILED (failures=2) restored: Ran 26 tests OK Found by yixuan-ag2, including the one-line fix and both-direction proof. Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> * test(review): assert the full reviewed-by token, not the bare login The stub asserted `login_in_query in joined`, a substring check whose needle is the fixture login `ME = "me"` -- two characters, which appear inside ordinary words. A mutant setting the query to `reviewed-by:someone-else` SURVIVED, because "some" contains "me". That mutant was reported as broken. It was not: it was a valid mutant that the assert failed to kill, and the accident was in the fixture rather than in the mutation. Asserting the full `reviewed-by:<login>` token makes the check exact regardless of how short the login is. Evidence, the previously-surviving mutant applied to the real source: MUTANT q = "is:pr is:open reviewed-by:someone-else" Ran 26 tests FAILED (failures=2) (before: 26 OK -- survived) restored Ran 26 tests OK Prompted by yixuan-ag2, who caught it while nearly publishing the opposite conclusion and checked the innocent explanation first. Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> * test(review): compare the discovery login exactly, not as a substring Third round on the same assert, and the first two fixes were the same mistake at different lengths: bare login "me" in joined killed by nothing much full token "reviewed-by:me" in joined killed someone-else exact value parsed == "me" kills merge-bot too `reviewed-by:me` is a PREFIX of `reviewed-by:merge-bot`, so tightening the needle only moved the collision rather than removing it. Any substring form has this property; the fix is to parse the value and compare equality. Every mutant, applied to the real source and restored: reviewed-by:merge-bot FAILED (failures=2) <- survived the previous fix reviewed-by:someone-else FAILED (failures=2) reviewed-by:otherperson FAILED (failures=2) reviewed-by:me2 FAILED (failures=2) <- suffix collision, also new CONTROL unmutated OK Found by yixuan-ag2, who has now caught this assert at every length it was wrong at. Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> * test(my-stale-approvals): pin that `decisive` needs the bar within reach `decisive` is `not blockers and qualifying >= bar - 1`, and nothing exercised the second half: dropping it to `not blockers` left the suite green. Every existing decisive case runs at the default bar of 2, where one qualifying approval always satisfies `>= bar - 1`, so the clause was true by fixture rather than by test. It is the field the report's own sort key and `--fail-on-decisive` exit both key on, and the one a reader acts on first. At bar=3 with one approval and no other blockers, nobody is a vote away and dropping mine changes nothing. Mutation-checked both directions: without this test the mutant survives, with it the mutant fails one check and only that one. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * test(my-stale-approvals): discriminate `decisive` at the bar this fleet uses The previous case proves the bar clause at bar=3, which no repo here runs. At bar=2 the clause still discriminates, but in exactly one place: `qualifying` counts my own review, so it is >= 1 whenever my association gates, and `1 >= 2-1` holds for every such row. The only bar=2 rows where the real predicate and a bare `not blockers` disagree are those where my own approval does not count at the gate. Measured over the reachable domain (rows that pass the APPROVED filter), bar=2: my association qualifying decisive mutant `not blockers` COLLABORATOR 1 True True (agree) CONTRIBUTOR 0 False True (differ) NONE 0 False True (differ) So the added case is the one that pins the clause where it matters: a vote that cannot count at the gate is never the decisive one. Domain analysis from @yixuan-ag2, who read the expression and said explicitly they had not run it; the table above is the run. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * docs(my-stale-approvals): one predicate for `decisive`, and it is the code's The PR shipped two. The module docstring says "at or one short of the bar"; the test docstring said "my vote is what closes the gap ... dropping mine changes nothing". Those disagree in exactly one cell, `qualifying == bar - 1`, and that cell is the modal case on this repo: bar q at-or-one-short closes-the-gap 2 1 True False <- sonichi/sutando, me sole approver 3 2 True False Keeping the code's reading. `dismiss_stale_reviews_on_push` is false on ruleset 19110427, so a stale approval at one short is already half of whatever eventually merges — it is the tick most worth re-reading, not the least, even though re-reviewing it cannot land the PR alone. The "carry it by itself" reading is defensible for a different tool; it is not what this one is for. So the docstrings move to the code, not the reverse, and the disagreeing cell now has its own test. Mutation-checked: swapping in `qualifying >= bar` fails four checks including the new one. Ambiguity found by @yixuan-ag2, comparing the field name against the expression rather than against a mutant — a mutant cannot see a cell where code and mutant agree, which is where this lived. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(my-stale-approvals): DISMISSED clears a stance; report merges and the search ceiling Three defects, all reported by qingyun-wu and each verified against GitHub's own `latestReviews` before being taken. 1. BLOCKER — `latest_per_author` skipped DISMISSED instead of clearing, so the author's PREVIOUS review resurrected. Wrong in both polarities: on #3356 the scan reported a qingyun-wu approval that GitHub does not list at all, and on #3327 / #3537 it listed a blocker GitHub no longer holds, hiding a DECISIVE row behind a phantom. 2. A merge after the approval was counted as nothing. A conflict-resolving merge is a 2-parent commit whose resolution hunks were typed by a person, and REST cannot show them — so merges after the approval now print as an explicit unknown ("+N merge(s), content not checked"). Staleness stays keyed on authored commits, unchanged. 3. `repos_reviewed` read one 100-item page and never checked total_count. For qingyun-wu the search holds 134, and six repos were silently out of scope. Now paginated, with the shortfall and the `gh pr list` cap printed in the scope block rather than assumed absent. Fixing 3 introduced a regression I nearly shipped: `--paginate` without `--slurp` emits one JSON object per page, json.loads rejects it, and gh_json falls to its default — a 2-page search became 0 repos while a 1-page one was fine. `--slurp` plus an explicit ok flag now separates a failed search from an empty one. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Move the `if message.author.bot and client.user not in message.mentions: return` early-drop from line 268 down into the `if not is_dm:` branch, after `require_mention` is resolved. Drop only when `requireMention=true`. DMs preserve the "bot must mention to survive" behavior.
The bug
Before this patch, a channel configured `{role: "bot2bot", requireMention: false, allowFrom: [owner, sibling-bot]}` in access.json could NOT actually receive bot-to-bot messages without `@-mention` — the channel-config lookup at line 286 never got reached because line 268 already dropped the message. access.json config was being silently ignored for bot-authored messages.
After this patch
Test plan
🤖 Generated with Claude Code