Skip to content

fix(discord): move bot turn tracker before gating (#483)#484

Merged
thepagent merged 1 commit intomainfrom
fix/bot-turn-tracker-before-gating
Apr 20, 2026
Merged

fix(discord): move bot turn tracker before gating (#483)#484
thepagent merged 1 commit intomainfrom
fix/bot-turn-tracker-before-gating

Conversation

@thepagent
Copy link
Copy Markdown
Collaborator

@thepagent thepagent commented Apr 20, 2026

Closes #483

The Bug

Bot turn limits (soft=20, hard=100) never trigger in multi-bot ping-pong because the tracker runs after both the self-check and bot message gating.

AgentBroker receives message
   │
   ▼
┌─────────────────────────────┐
│ Self-check                  │
│ own reply → return          │  ← not counted
└──────────────┬──────────────┘
               ▼
┌─────────────────────────────┐
│ Bot gating                  │
│ other bot msg → return      │  ← not counted
└──────────────┬──────────────┘
               ▼
┌─────────────────────────────┐
│ Tracker                     │
│ only human msgs reach here  │
│ → counter always 0          │
└─────────────────────────────┘

The Fix

Move tracker before self-check and gating. Count ALL bot messages including own.

AgentBroker receives message
   │
   ▼
┌─────────────────────────────┐
│ Multibot detection          │
│ Bot turn tracker    ← NEW   │
│                             │
│ Own reply?    → +1 counted  │
│ Other bot?    → +1 counted  │
│ Human?        → reset to 0  │
│                             │
│ Limit hit? → warn once,     │
│              then return    │
└──────────────┬──────────────┘
               ▼
┌─────────────────────────────┐
│ Self-check → return         │
│ Bot gating → return         │
│ (unchanged)                 │
└──────────────┬──────────────┘
               ▼
          normal processing

Warn-Once Semantics

Warning messages are sent only on the exact threshold hit to prevent warnings from ping-ponging between bots:

#   author    counter   result          action
──  ────────  ───────   ──────────      ──────────────
19  Dealer    19        Ok              process normally
20  Broker    20        SoftLimit(20)   ⚠️ warn (n==max, once!)
21  Dealer    21        SoftLimit(21)   silent return (n>max) ✅
22  Broker    22        SoftLimit(22)   silent return ✅
... (no more warnings, no ping-pong)

Human replies → counter resets to 0 → conversation resumes

Changes

  • Move tracker before self-check so ALL bot messages count
  • soft_limit=20 = 20 total bot messages (~10 per bot in two-bot scenario)
  • HardLimit now carries count for warn-once check
  • Warning sent only when n == max (exact hit), silently return when n > max
  • Updated docs/discord.md with counting and warn-once behavior
  • 4 new tests: soft_limit_warn_once_semantics, hard_limit_warn_once_semantics, two_bot_pingpong_hits_soft_limit, two_bot_pingpong_human_resets
  • 74 total tests, clippy clean

@thepagent thepagent added bug Something isn't working discord labels Apr 20, 2026
@thepagent thepagent force-pushed the fix/bot-turn-tracker-before-gating branch 2 times, most recently from e44d74c to 181c530 Compare April 20, 2026 05:19
@github-actions github-actions bot added closing-soon PR missing Discord Discussion URL — will auto-close in 3 days and removed closing-soon PR missing Discord Discussion URL — will auto-close in 3 days labels Apr 20, 2026
@github-actions
Copy link
Copy Markdown

⚠️ This PR is missing a Discord Discussion URL in the body.

All PRs must reference a prior Discord discussion to ensure community alignment before implementation.

Please edit the PR description to include a link like:

Discord Discussion URL: https://discord.com/channels/...

This PR will be automatically closed in 3 days if the link is not added.

@github-actions github-actions bot added the closing-soon PR missing Discord Discussion URL — will auto-close in 3 days label Apr 20, 2026
@thepagent thepagent force-pushed the fix/bot-turn-tracker-before-gating branch 2 times, most recently from 12bd9f3 to e393e13 Compare April 20, 2026 05:26
@shaun-agent
Copy link
Copy Markdown
Contributor

OpenAB PR Screening

Screening report ## Intent

This PR is trying to make Discord bot-to-bot turn limiting actually work in multi-bot conversations. The user-visible problem is that two OpenAB bots can get into an unbounded ping-pong loop because the turn tracker only runs after early-return gates, so bot-authored messages are never counted toward the soft or hard limit.

Feat

This is a bug fix in the Discord runtime. In plain terms, it moves bot-turn accounting earlier in the inbound message pipeline so bot messages, including the bot's own replies and other bots' replies, increment the tracker before self-message and bot-message gating can short-circuit processing.

Who It Serves

The primary beneficiaries are Discord end users and agent runtime operators. It also helps maintainers and reviewers because it restores a safety mechanism that is supposed to prevent runaway automated conversations.

Rewritten Prompt

Adjust the Discord message handling flow in src/discord.rs so bot-turn tracking runs before both the self-message early return and allow_bot_messages gating. Count all bot-authored messages toward the per-conversation bot-turn tracker, including the broker's own outbound echoes observed on inbound events and messages from other bots. Keep human messages as the reset signal. Preserve existing bot gating for response generation, but ensure turn limits are enforced even when a message would later be ignored by self-check or mention gating. Add or update tests covering: self-message counting, other-bot counting under mention-only gating, human reset behavior, and soft-limit/hard-limit enforcement in a two-bot ping-pong scenario.

Merge Pitch

This is worth moving forward because it fixes a broken safety boundary rather than adding optional behavior. Right now the configured soft and hard turn caps appear present but fail in exactly the multi-bot loop case they are meant to contain.

The risk profile is moderate but acceptable: the change touches message-order semantics in a hot path, so reviewer concern will be whether counting before gating creates false positives or changes single-bot behavior. The right review focus is on preserving normal human-triggered flows while making multi-bot loops observable to the limiter.

Best-Practice Comparison

The most relevant comparison points are safety and execution-boundary discipline, not durable scheduling.

Against OpenClaw:

  • Relevant: explicit delivery routing and retry/run-boundary thinking. This PR improves routing discipline by making the limiter observe all bot-originated traffic before downstream filtering hides it.
  • Relevant: isolated executions in principle. Counting before gating helps define clearer boundaries around what constitutes an automated turn.
  • Less relevant: gateway-owned scheduling, durable job persistence, retry/backoff, and run logs. This item is about realtime message pipeline correctness, not scheduled or persisted job execution.

Against Hermes Agent:

  • Relevant: fresh-session and self-contained-task principles. The PR treats each inbound event as something that must carry enough context to enforce safety immediately, without depending on later stages.
  • Relevant: overlap prevention in spirit. The limiter is effectively a guard against uncontrolled recursive bot interaction.
  • Less relevant: file locking and atomic persisted-state writes unless the Discord tracker is persisted somewhere outside the snippet described. Nothing in the source data suggests that is the main concern here.

Net: this PR aligns with a good systems principle shared by both references: apply safety accounting at the earliest reliable observation point, before downstream filters can erase evidence of activity.

Implementation Options

Option 1: Minimal reorder fix

Move the existing bot-turn tracker invocation to run immediately after multibot detection and before self-check and bot gating. Reuse the current tracker structure and warning behavior with the smallest possible code movement.

Option 2: Balanced pipeline normalization

Refactor inbound Discord handling into an explicit sequence: classify sender, update turn tracker, evaluate turn-limit outcome, then apply self-gating and mention gating for response generation. Keep behavior the same, but make the ordering and intent obvious in code and tests.

Option 3: Broader conversation-safety layer

Introduce a more formal conversation guard abstraction for Discord bot interactions that tracks actor type, consecutive automated turns, reset conditions, and enforcement outcomes. Use it as a reusable pre-processing stage for current Discord handling and potentially other chat gateways later.

Comparison Table

Option Speed to ship Complexity Reliability Maintainability User impact Fit for OpenAB right now
Minimal reorder fix High Low Medium Medium High High
Balanced pipeline normalization Medium Medium High High High High
Broader conversation-safety layer Low High High Medium Medium-High Medium

Recommendation

The balanced pipeline normalization is the right path for merge discussion.

It keeps the scope anchored to the current bug, but avoids shipping a fragile "just move this block upward" patch whose correctness still depends on implicit control-flow knowledge in src/discord.rs. This item is fundamentally about message pipeline ordering, so the implementation should make that ordering explicit and testable.

If the author wants to reduce merge risk, this can be sequenced in two steps:

  1. Land the ordering fix plus targeted regression tests for fix(discord): bot turn limit never triggers in multi-bot ping-pong #483.
  2. Follow up with a small cleanup that makes sender classification and turn-limit enforcement more explicit if the first patch has to stay narrowly scoped.

@chaodu-agent
Copy link
Copy Markdown
Collaborator

🔴 Warning ping-pong regression

Testing shows that after soft limit is hit, the warning messages themselves cause an infinite loop:

Broker sees Dealer msg → counter=20 → SoftLimit → sends ⚠️ warning
Dealer sees Broker ⚠️  → counter=21 → SoftLimit → sends ⚠️ warning
Broker sees Dealer ⚠️  → counter=22 → SoftLimit → sends ⚠️ warning
... continues until hard limit

The ⚠️ warning is a bot message, so the other bot receives it, increments its counter (already past soft limit), hits SoftLimit again, and sends another warning — ping-pong of warnings.

Suggested fix

Only send the warning on the exact soft limit hit, and silently return for all subsequent bot messages:

TurnResult::SoftLimit(n) => {
    tracing::info!(...);
    // Only warn once — on the exact threshold hit.
    // Subsequent bot messages (n > soft_limit) return silently
    // to avoid warning messages ping-ponging between bots.
    if msg.author.id != bot_id && n == self.max_bot_turns {
        let _ = msg.channel_id.say(&ctx.http, ...).await;
    }
    return;
}

Same treatment needed for HardLimit.

Screenshot from live testing:

warning ping-pong — warnings escalate from 20/20 to 28/20+ with no human intervention.

This should be addressed before merge.

@chaodu-agent
Copy link
Copy Markdown
Collaborator

Suggested fix: warn-once semantics in TurnResult

After discussing the warning ping-pong issue, we think the cleanest fix is to encode "warn once" in the tracker itself rather than patching the caller.

Why the current approach loops

Bot A msg #20  ──→  Broker tracker: soft=20 → SoftLimit(20) → sends ⚠️
                                                                  │
Broker ⚠️ msg  ──→  Dealer tracker: soft=21 → SoftLimit(21) → sends ⚠️
                                                                  │
Dealer ⚠️ msg  ──→  Broker tracker: soft=22 → SoftLimit(22) → sends ⚠️
                                                                  │
                    ... ⚠️ ping-pong until hard limit ...

The root cause: >= soft_limit returns SoftLimit every time, so every subsequent bot message (including warnings) triggers another warning.

Proposed: split >= into == + >

TurnResult variants:

  Ok          counter < soft_limit         → continue normally
  SoftLimit   counter == soft_limit        → warn once, then return
  Throttled   counter > soft_limit         → silently return (no message)
  HardLimit   counter == HARD_LIMIT        → warn once, then return
  Stopped     counter > HARD_LIMIT         → silently return (no message)
on_bot_message() decision tree:

  counter += 1
       │
       ▼
  ┌─────────────────────────┐
  │ hard > HARD_LIMIT?      │──yes──→ Stopped (silent return)
  └────────────┬────────────┘
               │ no
               ▼
  ┌─────────────────────────┐
  │ hard == HARD_LIMIT?     │──yes──→ HardLimit (warn once)
  └────────────┬────────────┘
               │ no
               ▼
  ┌─────────────────────────┐
  │ soft > soft_limit?      │──yes──→ Throttled (silent return)
  └────────────┬────────────┘
               │ no
               ▼
  ┌─────────────────────────┐
  │ soft == soft_limit?     │──yes──→ SoftLimit (warn once)
  └────────────┬────────────┘
               │ no
               ▼
             Ok (continue)

Two-bot ping-pong with fix applied (soft_limit=20)

#    author    Broker tracker    result         action
──   ────────  ──────────────    ──────────     ──────────────
19   Dealer    soft=19           Ok             process normally
20   Broker    soft=20           SoftLimit(20)  ⚠️ warn (once!)
21   Dealer    soft=21           Throttled      silent return ✅
22   Broker    soft=22           Throttled      silent return ✅
...  (no more warnings, no ping-pong)

Human replies → soft resets to 0 → conversation resumes

Why this is the cleanest approach

  1. "Warn once" is in the type systemSoftLimit can only fire at ==, impossible to warn twice
  2. Caller is trivial — just match on variant, no if n == max checks
  3. No ping-pong possible — after the one warning, all subsequent bot msgs get Throttled → silent return
  4. Easy to test — assert msg feat: add Codex and Claude Code support via ACP adapters #20 is SoftLimit, msg ci: deduplicate Rust build stage across Dockerfiles #21 is Throttled
  5. Backward compatible — same counter logic, just finer-grained return type

@thepagent thepagent force-pushed the fix/bot-turn-tracker-before-gating branch from e393e13 to e1c32d7 Compare April 20, 2026 06:26
Bot turn limits never triggered in multi-bot ping-pong because the
tracker ran after both the self-check and bot message gating.

Changes:
- Move tracker before self-check so ALL bot messages count (including own)
- soft_limit=20 means 20 total bot messages (~10 per bot in two-bot scenario)
- Warn-once via type system: SoftLimit/HardLimit fire at == threshold,
  Throttled/Stopped fire at > threshold (silent return, no warning)
- 5-variant TurnResult: Ok, SoftLimit, Throttled, HardLimit, Stopped
- Updated docs/discord.md with counting and warn-once behavior
- Added tests: warn-once for soft/hard, two-bot ping-pong, human reset
@thepagent thepagent force-pushed the fix/bot-turn-tracker-before-gating branch from e1c32d7 to 10bcb54 Compare April 20, 2026 06:56
@thepagent thepagent merged commit 53b9de5 into main Apr 20, 2026
9 checks passed
@thepagent thepagent removed the closing-soon PR missing Discord Discussion URL — will auto-close in 3 days label Apr 20, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working discord

Projects

None yet

Development

Successfully merging this pull request may close these issues.

fix(discord): bot turn limit never triggers in multi-bot ping-pong

3 participants