Skip to content

fix(discord): only send turn limit warning from participating bot#728

Merged
thepagent merged 2 commits intoopenabdev:mainfrom
JARVIS-coding-Agent:fix/bot-turn-limit-warning-all-instances
May 4, 2026
Merged

fix(discord): only send turn limit warning from participating bot#728
thepagent merged 2 commits intoopenabdev:mainfrom
JARVIS-coding-Agent:fix/bot-turn-limit-warning-all-instances

Conversation

@JARVIS-coding-Agent
Copy link
Copy Markdown
Contributor

Context

When multiple bot instances share a channel, the bot turn limit warning (⚠️ Bot turn limit reached) is sent by every instance — not just the one that actually participated in the thread. This spams the thread with duplicate warnings and pulls uninvolved bots into the conversation.

Closes #727

Summary

Gate the turn limit warning behind a bot_participated_in_thread() check so only the bot that actually replied in the thread sends the warning message.

Changes

  • Added bot_participated_in_thread() call in the WarnAndStop path before sending the warning message
  • Uninvolved bots now silently return instead of posting a warning they have no business sending

Notes

  • bot_participated_in_thread() is fail-closed: on API error it returns (false, false), so the warning is suppressed rather than sent — safe default
  • The extra API call only happens on the WarnAndStop path (at most once per soft/hard limit per thread), so performance impact is negligible
  • The method is already used in 3 other places in the same file with the same signature

Discord Discussion URL: https://discord.com/channels/1395707092077248593/1395707093364768850/1500805519630274701

Before this fix, when multiple bots shared a channel and the bot turn
soft limit was reached, every bot instance sent its own warning message
into the thread — even bots that never participated. This caused
duplicate warnings and pulled uninvolved bots into the thread.

Now the WarnAndStop path calls bot_participated_in_thread() before
sending the warning, so only the bot that actually replied in the
thread will post the limit message.

Closes openabdev#727
@github-actions github-actions Bot added the pending-screening PR awaiting automated screening label May 4, 2026
@shaun-agent
Copy link
Copy Markdown
Contributor

OpenAB PR Screening

This is auto-generated by the OpenAB project-screening flow for context collection and reviewer handoff.
Click 👍 if you find this useful. Human review will be done within 24 hours. We appreciate your support and contribution 🙏

Screening report ## Intent

PR #728 tries to stop duplicate Discord turn-limit warnings when multiple OpenAB bot instances share the same channel.

The user-visible problem is that every bot instance currently posts ⚠️ Bot turn limit reached, even if that bot never participated in the thread. This creates noisy duplicate warnings and makes uninvolved bots appear to join conversations incorrectly.

Feat

This is a Discord bug fix.

Behavioral change: when the Discord turn-limit handler reaches the WarnAndStop path, the bot first checks whether it participated in the thread. Only a participating bot sends the warning. Non-participating bots silently return.

Who It Serves

Primary beneficiaries:

  • Discord end users, who avoid duplicate warning spam
  • Discord workspace operators running multiple OpenAB bot instances
  • Maintainers, because bot ownership of thread behavior becomes more predictable

Rewritten Prompt

Fix Discord turn-limit warning fan-out when multiple bot instances observe the same channel.

In src/discord.rs, update the WarnAndStop turn-limit path so the warning message is only sent by a bot instance that has already participated in the Discord thread. Reuse the existing bot_participated_in_thread() helper and preserve its fail-closed behavior: if participation cannot be confirmed, suppress the warning rather than posting from an uninvolved bot.

Add or update focused tests if the Discord handler has test coverage for turn-limit behavior. Verify that participating bots still warn once, while non-participating bots do not post anything.

Merge Pitch

This is a small, targeted fix for a real multi-bot Discord deployment issue. It reduces user-facing noise without changing normal single-bot behavior.

Risk profile is low: the change is scoped to the warning path and reuses an existing helper already used elsewhere in the same file. The main reviewer concern is likely whether suppressing the warning on API failure is acceptable. Given the helper already fails closed and this path is specifically about avoiding uninvolved bot chatter, that default is reasonable.

Best-Practice Comparison

Relevant OpenClaw principles:

  • Explicit delivery routing is directly relevant. The warning should be emitted only by the bot instance that owns or participated in the thread.
  • Retry/backoff and run logs are only lightly relevant. This PR does not introduce durable execution or retry semantics.
  • Gateway-owned scheduling, durable job persistence, and isolated executions are not central to this fix.

Relevant Hermes Agent principles:

  • Fresh session per scheduled run and self-contained prompts are not relevant here.
  • File locking and atomic writes are not relevant because this is not persisted scheduler state.
  • The broader principle of preventing overlapping or duplicate agent action does apply. This PR narrows the actor allowed to post the terminal warning.

Overall, the PR aligns with the shared best-practice theme of explicit ownership before side effects.

Implementation Options

Option 1: Conservative participation gate
Keep the PR as proposed: before sending the turn-limit warning, call bot_participated_in_thread() and suppress the warning unless participation is confirmed.

Option 2: Balanced ownership helper
Introduce a small helper around warning delivery, such as should_send_turn_limit_warning(), which encapsulates participation checks and documents fail-closed behavior. Use it in the WarnAndStop path and add focused tests if practical.

Option 3: Ambitious per-thread bot ownership model
Track explicit thread ownership or participation state in a durable or shared layer, so all side effects in multi-bot channels route through a single responsible instance. Use that state for warnings, follow-ups, retries, and future Discord delivery decisions.

Comparison Table

Option Speed to ship Complexity Reliability Maintainability User impact Fit for OpenAB right now
Conservative participation gate High Low Good Good High for affected Discord channels Strong
Balanced ownership helper Medium Medium-low Good Better if more warning paths emerge High Strong if tests are easy
Durable thread ownership model Low High Best long-term Potentially best, but larger surface High across future multi-bot cases Premature for this PR

Recommendation

Advance the conservative participation gate, preferably with a tiny helper or test only if it fits the existing code structure cleanly.

This PR addresses a concrete Discord bug with minimal blast radius and follows the right ownership principle: only the bot that participated should produce conversation-visible side effects. A broader thread ownership model may be useful later, but it should be split into a separate design item rather than blocking this targeted fix.

@chaodu-agent

This comment has been minimized.

@chaodu-agent
Copy link
Copy Markdown
Collaborator

LGTM ✅ — Four-monk unanimous approval. Clean, minimal fix for a real multi-bot deployment issue.

Consolidated Review (超渡 × 普渡 × 擺渡 × 覺渡)

1. What problem does this solve?

Issue #727: when multiple bot instances share a channel, ALL instances send the ⚠️ Bot turn limit reached warning when any single-bot thread hits the soft limit. This spams the thread with duplicate warnings and — worse — causes bot_participated_in_thread() to consider those uninvolved bots as "involved", pulling them into subsequent replies.

2. How does it solve it?

One surgical change in src/discord.rs (10 additions, 1 deletion): before sending the WarnAndStop warning, call bot_participated_in_thread() and only send if the bot actually has prior messages in the thread. Uninvolved bots silently return.

3. What was considered?

  • Reuses the existing bot_participated_in_thread() helper already used in 3 other places in the same file
  • Fail-closed behavior preserved: API error → (false, false) → warning suppressed (safe default)
  • The extra API call only happens on the WarnAndStop path, at most once per limit hit per thread — negligible performance impact
  • The is_multibot return value is correctly ignored with _

4. Is this the best approach?

Yes. The existing allowed_here check (channel permissions) was necessary but insufficient — it only gates on channel access, not thread participation. Adding the participation check is the correct additional gate.

Traffic Light

🟢 INFO — Excellent use of existing infrastructure. No new methods, no new state, no new dependencies.

🟢 INFO — Comment referencing #727 is helpful for future readers.

🟢 INFO — Prevents the secondary cascade: uninvolved bots posting warnings → becoming "involved" → responding to subsequent human messages.

Copy link
Copy Markdown
Collaborator

@chaodu-agent chaodu-agent left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Four-monk unanimous LGTM. Clean fix, reuses existing helper, fail-closed safe default.

@pahud pahud added pending-maintainer and removed pending-contributor pending-screening PR awaiting automated screening labels May 4, 2026
@chaodu-agent
Copy link
Copy Markdown
Collaborator

🟢 LGTM — Clean, minimal, correct

Verdict: Approve-ready. Single-purpose fix that reuses existing infrastructure with zero new concepts.

Four-Question Review

1. What problem does this solve?
When multiple bot instances share a channel, the WarnAndStop turn limit warning is sent by every instance — not just the one that participated. This spams threads with duplicate warnings. (Closes #727)

2. How does it solve it?
Gates the warning message behind a bot_participated_in_thread() call inside the existing WarnAndStop path. Only the bot that actually replied in the thread sends the warning.

3. What was considered?

  • bot_participated_in_thread() is fail-closed — API errors return (false, false), suppressing the warning rather than sending it. Safe default.
  • The extra API call only fires on the WarnAndStop path (at most once per soft/hard limit per thread). Negligible performance impact.
  • The method is already used in 3 other places in the same file with the same signature — consistent pattern.

4. Is this the best approach?
Yes. Minimal change (+10/-1), fully reuses existing bot_participated_in_thread() infra, no new dependencies or concepts.

Traffic Light

Signal Item
🟢 INFO Correct placement — inside the existing msg.author.id != bot_id && allowed_here guard, so channel permission checks still run first
🟢 INFO Fail-closed design matches the existing pattern across the codebase
🟢 INFO Good inline comment referencing #727 for future readers
NIT (non-blocking)

No new tests added. However, bot_participated_in_thread() is async and requires mocking the Discord HTTP client, which the current unit test framework does not support. The existing detect_thread_rejects_category_child_in_warn_and_stop test covers the synchronous channel-gating logic. Not blocking.


Review by 超渡法師 🔮 — 四問框架 applied, baseline verified against main

@thepagent thepagent merged commit 6a399a2 into openabdev:main May 4, 2026
11 checks passed
canyugs pushed a commit to canyugs/openab that referenced this pull request May 5, 2026
…enabdev#728)

* fix(discord): only send turn limit warning from participating bot

Before this fix, when multiple bots shared a channel and the bot turn
soft limit was reached, every bot instance sent its own warning message
into the thread — even bots that never participated. This caused
duplicate warnings and pulled uninvolved bots into the thread.

Now the WarnAndStop path calls bot_participated_in_thread() before
sending the warning, so only the bot that actually replied in the
thread will post the limit message.

Closes openabdev#727

* fix(discord): annotate ignored is_multibot return value

Co-reviewed-by: VISION

---------

Co-authored-by: JARVIS-coding-Agent <jarvis-coding-agent@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Bot turn limit warning sent by all instances in shared channel, pulling uninvolved bots into thread

5 participants