Skip to content

fix(discord): keep multibot threads responsive#1439

Open
chaodu-obk[bot] wants to merge 17 commits into
mainfrom
fix/1436-discord-multibot-clean-v2
Open

fix(discord): keep multibot threads responsive#1439
chaodu-obk[bot] wants to merge 17 commits into
mainfrom
fix/1436-discord-multibot-clean-v2

Conversation

@chaodu-obk

@chaodu-obk chaodu-obk Bot commented Jul 22, 2026

Copy link
Copy Markdown
Contributor

Summary

  • keep Discord multibot threads visibly responsive with sanitized presentation streaming
  • separate human-facing presentation from structured bot-to-bot handoffs
  • classify only bot-authored, target-header JSON envelopes as handoff candidates
  • authorize outbound targets against the configured trusted bot allowlist
  • validate schema, trust, exact target, live channel provenance, expiry, hop count, replay, rate, and payload bounds before ACP dispatch
  • preserve the completed presentation after a successful handoff
  • preserve raw prompt text inside the structured control payload while Discord allowlists only the target mention
  • reject handoff envelopes that exceed Discord 2,000-character message limit
  • apply a per-source limit of 10 accepted handoffs per 60 seconds
  • cap the process-local replay cache at 4,096 events and fail closed when full
  • preserve normal Discord reply notifications while disabling automatic presentation mentions

Review fixes

  • clarify directive, adapter, Discord multibot streaming, expiry, and provenance documentation
  • trace invalid handoff directives and narrow candidate detection to avoid schema text collisions and schema-version bypass
  • bind envelope provenance to the receiving channel
  • authorize outbound handoff targets against trusted_bot_ids
  • preserve/finalize presentation output after handoff delivery
  • add regression coverage for multibot streaming, channel provenance, candidate collision, raw payload mentions, rate limiting, replay-cache capacity, schema validation, target authorization, and 1,800-character payload size

Operational limitations

  • replay deduplication is process-local; multi-replica deployments require a shared deduplication store before relying on cross-replica exactly-once behavior
  • accepted handoffs are subject to both the per-source rate limit and the existing per-thread bot-turn cap
  • trusted bot IDs remain startup configuration and require a restart to change; they also define the outbound handoff target allowlist
  • handoff protocol is currently first-hop only; non-zero hop_count values are rejected
  • the handoff control message remains visible in Discord because Discord has no hidden bot-only message channel
  • ordinary Discord presentation messages intentionally disable automatic user/role/everyone mentions; explicit handoff target mentions are the only control-plane mentions

Validation

  • Rust LSP diagnostics: no diagnostics for adapter.rs, discord.rs, config.rs, and main.rs
  • git diff --check
  • Full Cargo validation could not run because the authoring environment lacks cargo, rustc, rustfmt, and rust-analyzer

Closes #1436

Review Contract

Goal

Keep Discord multibot threads visibly responsive during long ACP turns while separating sanitized human-facing presentation from structured, fail-closed bot-to-bot handoffs (closes #1436).

Non-goals

  • Cross-replica exactly-once handoff delivery (replay deduplication remains process-local)
  • Hiding the handoff control message from Discord (Discord has no bot-only message channel)
  • Runtime reconfiguration of trusted bot IDs (startup configuration only)
  • Multi-hop handoff routing (protocol is first-hop only; non-zero hop_count rejected)

Accepted Residual Risks

  • Replay deduplication is process-local; multi-replica deployments need a shared deduplication store before relying on cross-replica exactly-once behavior
  • The handoff control message remains visible in the Discord thread
  • Trusted bot ID changes require a bot restart to take effect
  • Accepted handoffs remain subject to the per-source rate limit and the existing per-thread bot-turn cap

Acceptance Criteria

  • Only bot-authored, target-header JSON envelopes are classified as handoff candidates
  • Envelopes are validated for schema, trust, exact target, live channel provenance, expiry, hop count, replay, rate, and payload bounds before ACP dispatch
  • Outbound handoff targets are authorized against the configured trusted bot allowlist; self-targeting is rejected
  • Presentation output is preserved and finalized after a successful handoff
  • Handoff envelopes exceeding the Discord 2,000-character limit are rejected; per-source limit of 10 accepted handoffs per 60 seconds; replay cache capped at 4,096 events and fails closed when full
  • Regression tests cover multibot streaming, channel provenance, candidate collision, raw payload mentions, rate limiting, replay-cache capacity, schema validation, target authorization, and payload size bounds; CI (fmt, clippy, tests, smoke tests) is green

Follow-ups

  • Introduce a shared deduplication store to support exactly-once handoff semantics across replicas
  • Consider a hidden control-plane transport if Discord ever exposes bot-only messaging
  • Evaluate hot-reload of trusted bot IDs without restart

@chaodu-obk
chaodu-obk Bot requested a review from thepagent as a code owner July 22, 2026 21:59
@chaodu-obk

This comment has been minimized.

@chaodu-obk chaodu-obk Bot left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

LGTM � � exact-SHA review at 8774c9a confirms outbound target authorization and self-target rejection, validated schema/prompt gating, bounded replay/rate state, inert presentation streaming, handoff size bounds, and synchronized documentation. Canonical review: #1439 (comment)

@github-actions github-actions Bot added needs-rebase closing-soon PR missing Discord Discussion URL — will auto-close in 24 hours. labels Jul 24, 2026
@github-actions

Copy link
Copy Markdown

Caution

This PR has been waiting on the author for more than 2 days (labeled pending-contributor since 2026-07-22).
It will be automatically closed in 24 hours if there is no update.

@chaodu-obk[bot] — You must add a new comment on this PR to remove the closing-soon label and keep it open. Pushing commits alone is not sufficient. Feel free to reopen a new PR later if it gets closed and you want to pick it back up.

@chaodu-obk

chaodu-obk Bot commented Jul 25, 2026

Copy link
Copy Markdown
Contributor Author

Note

LGTM ✅ - The exact-SHA follow-up is safe to merge; the final delta tightens canonical source and target ID validation and changes only an equivalent iterator predicate.

What This PR Does

This PR keeps Discord multibot threads responsive with sanitized presentation streaming while separating human-facing output from explicit, structured bot-to-bot handoffs. Handoffs are allowlisted, provenance-bound, bounded, replay-protected, rate-limited, and fail-closed.

How It Works

Discord presentation sends and edits disable automatic mentions and sanitize mention-like text. A complete ACP turn may emit a [[handoff:TARGET_BOT_ID]] directive, which becomes a JSON control envelope addressed only to an allowlisted target. The receiver requires the canonical decimal source and target IDs, exact target mention, trusted source, live channel provenance, expiry, first-hop semantics, UUID event ID, replay capacity, rate, and payload bounds before dispatching the raw payload to ACP. The final presentation remains visible independently of handoff delivery.

Findings

# Severity Finding Location
1 🟢 Canonical source and target IDs are now enforced before a handoff can pass trust and routing checks; the is_none_or change preserves the prior optional-provenance behavior while satisfying current Rust style. crates/openab-core/src/discord.rs:3526-3546
Finding Details

🟢 F1: Canonical handoff identity validation

The follow-up rejects padded numeric forms such as 020 and 00010 instead of allowing alternate textual representations of the same Discord IDs. This prevents an envelope from passing numeric equality checks while carrying non-canonical wire identities. The iterator-predicate rewrite is behaviorally equivalent for the optional thread provenance check.

Baseline Check
  • PR title: fix(discord): keep multibot threads responsive
  • PR author: chaodu-obk[bot]
  • PR opened: 2026-07-22
  • Declared base: main
  • Merge-base: 685973307bf79d6848c21b33a5bd55b6aea1c433
  • Reviewed head: 82a79f7b8727ae23c959ed66d103014d989afea3
  • Scope: 9 files, 769 additions, 204 deletions
  • Net-new value: explicit structured handoff routing, bounded validation, inert multibot presentation streaming, and synchronized documentation; the final three-commit delta further canonicalizes identity fields.
Validation
  • Local diff review was performed from the isolated worktree pinned to 82a79f7b8727ae23c959ed66d103014d989afea3.
  • git diff --check passed for main...82a79f7b8727ae23c959ed66d103014d989afea3.
  • GitHub checks were green for validation, check, and all reported smoke-test jobs; only the operator job was skipped.
  • Rust unit tests and formatting could not be run locally because cargo, rustc, rustfmt, and rust-analyzer are unavailable in this environment.
What's Good (🟢)
  • The presentation and control planes are separated instead of treating partial model output as executable routing input.
  • Source and target trust, canonical identity, channel provenance, expiry, hop count, replay, rate, and size checks are layered and fail-closed.
  • The added regression cases cover padded IDs and preserve the established handoff safeguards.

Addressing External Reviewer Feedback

No substantive external reviewer feedback was present for this exact-SHA round. The prior review state was rechecked against the new three-commit delta, and no unresolved finding was identified.

5. Three Reasons We Might Not Need This PR

  1. The protocol adds complexity - Schema, trust, replay, rate, and size controls are unnecessary for deployments that never use bot-to-bot handoffs.
  2. Multibot streaming is a product choice - Deployments that prefer send-once behavior may not need live presentation edits.
  3. Replay protection is process-local - Multi-replica deployments still need a shared deduplication store for cross-replica exactly-once semantics.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

closing-soon PR missing Discord Discussion URL — will auto-close in 24 hours.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

bug(multibot): keep Discord threads responsive during long-running tasks

1 participant