Skip to content

fix(discord): unblock inbound queue after stuck run#88251

Closed
anyech wants to merge 1 commit into
openclaw:mainfrom
anyech:fix-discord-inbound-run-watchdog
Closed

fix(discord): unblock inbound queue after stuck run#88251
anyech wants to merge 1 commit into
openclaw:mainfrom
anyech:fix-discord-inbound-run-watchdog

Conversation

@anyech

@anyech anyech commented May 30, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Summary

This adds a Discord-specific watchdog around queued inbound message runs so one stuck run cannot indefinitely block later messages for the same Discord session key.

The watchdog mirrors the existing configured agent timeout contract via the typed SDK resolver resolveAgentTimeoutMs({ cfg }). It does not add a new Discord-specific 30-minute production SLA.

Problem

Discord inbound handling serializes work per route/session key. If one queued message run never settles after a stuck agent/tool path, later Discord messages for that same session key remain behind the unresolved queue tail. To users, later messages can look silently dropped because no new automatic agent turn starts.

Manual session injection can still work because it bypasses the Discord inbound run queue.

Root cause

createDiscordMessageRunQueue() delegates to createChannelRunQueue(), which serializes per key and intentionally imposes no per-task timeout. A non-settling Discord inbound job can therefore hold the queue tail indefinitely.

Fix

Wrap each Discord queued message run in a Discord-owned watchdog tied to the existing agent timeout resolver:

  • resolve the watchdog timeout with resolveAgentTimeoutMs({ cfg }) from openclaw/plugin-sdk/agent-runtime;
  • pass a merged abort signal into processDiscordMessage;
  • when that timeout fires, abort the signal;
  • commit replay keys for the timed-out job;
  • throw/log DiscordMessageRunTimeoutError;
  • allow the keyed queue to advance to later messages.

The focused test hook can still inject a short runTimeoutMs so the queue-tail behavior is testable without waiting for the real agent timeout.

Late completion of the original timed-out job is ignored so replay keys are not committed twice.

Compatibility

The shipped DISCORD_DEFAULT_INBOUND_WORKER_TIMEOUT_MS export is preserved as a deprecated compatibility export. Production watchdog behavior no longer uses that constant.

Real behavior proof

  • Behavior addressed: A stuck Discord inbound message run can hold the per-session queue tail forever, so a later same-session Discord message does not get processed. After this patch, the queued run receives an abort signal when the configured agent timeout fires, replay is committed for the timed-out delivery, and the next same-session message can proceed.

  • Real setup tested: Isolated source checkout for this PR head (1440ed950a0d) running the real Discord monitor queue implementation (createDiscordMessageRunQueue / createDiscordMessageHandler) under the repository's extension-discord Vitest project. This did not use the production Gateway or production Discord bot token.

  • Exact steps or command run after this patch:

openclaw-heavy-run --backend auto -- pnpm exec vitest run extensions/discord/src/monitor/message-handler.queue.test.ts
  • Evidence after fix: Redacted terminal output from the isolated PR checkout:
heavy-run: ok (heaviness=always backend=systemd reason=systemd guard passed)
RUN v4.1.7 <isolated-checkout>
✓ extension-discord ../../extensions/discord/src/monitor/message-handler.queue.test.ts (20 tests) 65ms

Test Files  1 passed (1)
Tests       20 passed (20)
Duration    7.81s

  • Observed result after fix: The regression suite demonstrates the queue-tail behavior directly: message m-1 starts a Discord run that never resolves, message m-2 is queued for the same session key, the injected watchdog timeout aborts m-1, and m-2 is processed instead of remaining stuck behind m-1. The suite also verifies replay protection for duplicate m-1, late resolution of the original m-1 without double-commit/logging, and the default timeout path using the configured agent timeout resolver rather than a new Discord-specific 30-minute production SLA.

  • What was not tested: I did not run this patch against a live production Gateway/runtime or production Discord bot. A maintainer/Mantis Discord live proof would still be valuable for the end-to-end visible-delivery path; I requested that separately in a PR comment.

Replay semantics

A watchdog timeout is treated like a non-retryable consumed inbound delivery. This favors avoiding duplicate processing over replaying a message whose processing state is unknown.

Scope / relation to existing work

This is intentionally narrower than #79562. It does not add queue caps, drop policy, persistence, or global channel queue behavior changes. It only prevents a stuck Discord message run from blocking later same-session Discord inbound messages indefinitely.

Related: #9238, #75346, #29497 / #29532, #79562.

A separate adjacent failure mode can still happen after a stuck run is aborted: session write locks may remain or block later writes, producing SessionWriteLockTimeoutError. That lock-cleanup/recovery seam is already covered by the session-lock issue/PR cluster (for example #21783, #31489, #86540, #86806) and is intentionally out of scope for this Discord queue-tail PR.

Risks / limitations

The watchdog aborts cooperatively. If downstream work ignores abort signals, it may still complete later, so a timed-out run can overlap with a newer same-session run. This is preferred over an indefinite queue stall but is not a hard cancellation mechanism.

The timeout ceiling follows the existing agent timeout resolver. It does not add a new 30-minute production timeout.

Full root tsc -p tsconfig.json --noEmit was attempted in an isolated checkout before this revision but hit a Node heap OOM before type diagnostics. The focused Discord queue regression suite and diff check passed on current origin/main.

Tests

git diff --check

openclaw-heavy-run --backend auto -- pnpm exec vitest run extensions/discord/src/monitor/message-handler.queue.test.ts

Targeted result:

1 file passed, 20 tests passed

@openclaw-barnacle openclaw-barnacle Bot added channel: discord Channel integration: discord size: M triage: needs-real-behavior-proof Candidate: external PR needs after-fix proof from a real setup. labels May 30, 2026
@clawsweeper

clawsweeper Bot commented May 30, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Codex review: needs real behavior proof before merge. Reviewed May 30, 2026, 6:06 AM ET / 10:06 UTC.

Summary
The PR adds a Discord inbound run watchdog that aborts a stuck per-session queued message run, commits replay keys on watchdog timeout, and adds focused queue regression coverage.

PR surface: Source +78, Tests +167. Total +245 across 4 files.

Reproducibility: yes. from source, but not by live repro in this review. Current createChannelRunQueue serializes per key without a task timeout, so a non-settling Discord message task can hold that key's queue tail indefinitely.

Review metrics: none identified.

Merge readiness
Overall: 🧂 unranked krab
Proof: 🦪 silver shellfish
Patch quality: 🧂 unranked krab
Result: blocked until real behavior proof from a real setup is added.

Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch.

Rank-up moves:

  • [P2] Replace the timeout resolver import with a supported runtime/SDK contract and add the matching contract/type validation.
  • [P1] Add redacted live Gateway/Discord proof or a Mantis artifact showing the later same-session message proceeds after a stuck run.
  • Make the timeout-as-consumed replay behavior and late-overlap tradeoff explicit for maintainer acceptance.

Proof guidance:

  • [P1] Needs real behavior proof before merge: The PR body now has structured redacted terminal output, but it is focused Vitest proof and the author explicitly did not run a live Gateway/Discord bot path. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.

Mantis proof suggestion
A real Discord transport artifact would materially reduce risk because the change is about visible same-session delivery after a stuck run. A maintainer can ask Mantis to capture proof by posting a new PR comment that starts with the OpenClaw Mantis account mention, followed by:

visual task: prove a stuck Discord same-session run times out and a later message gets one reply without duplicate or misordered output.

Risk before merge

  • [P1] Merging as-is can break Discord typechecking or runtime loading because the new import uses a public SDK subpath that does not export resolveAgentTimeoutMs.
  • [P1] The watchdog treats a timed-out inbound delivery as consumed and can let a later same-session run overlap with a late completion from the original run, so maintainers need to own the message-ordering and session-state tradeoff.
  • [P1] The supplied proof is focused Vitest output only; it does not show a live Gateway/Discord path where the later same-session message is actually delivered after a stuck run.

Maintainer options:

  1. Repair the SDK Boundary First (recommended)
    Replace the unsupported openclaw/plugin-sdk/agent-runtime import with a supported runtime seam, or intentionally add and document the public SDK export with contract coverage before merge.
  2. Accept Timeout-As-Consumed Semantics
    Maintainers can intentionally accept that a watchdog timeout commits replay keys and may overlap with late work, but that decision should be explicit before landing.
  3. Pause for Live Delivery Proof
    If maintainers want end-to-end confidence before the semantic choice, pause merge until a live Discord/Gateway or Mantis artifact shows later same-session delivery without duplicate or misordered output.

Next step before merge

  • [P1] Human review is needed because the PR still has a blocking SDK import defect, mock-only proof, and maintainer-owned replay/session overlap semantics.

Security
Cleared: The diff does not change dependencies, lockfiles, secrets handling, CI, install scripts, or permissions; no concrete security or supply-chain issue was found.

Review findings

  • [P1] Use a supported agent timeout contract — extensions/discord/src/monitor/message-run-queue.ts:1
Review details

Best possible solution:

Land a Discord-owned queue watchdog only after it uses a supported timeout resolver path, the replay/overlap semantics are explicitly accepted, and live Discord/Gateway or Mantis proof shows the visible delivery path.

Do we have a high-confidence way to reproduce the issue?

Yes from source, but not by live repro in this review. Current createChannelRunQueue serializes per key without a task timeout, so a non-settling Discord message task can hold that key's queue tail indefinitely.

Is this the best way to solve the issue?

No, not as currently implemented. The watchdog direction is plausible, but it must use a supported timeout resolver contract and get maintainer acceptance for consumed-replay and late-overlap semantics.

Full review comments:

  • [P1] Use a supported agent timeout contract — extensions/discord/src/monitor/message-run-queue.ts:1
    openclaw/plugin-sdk/agent-runtime is the package export for src/plugin-sdk/agent-runtime.ts, and that file does not export resolveAgentTimeoutMs. This will fail typechecking or runtime loading for the Discord plugin, so the watchdog needs to get the resolver through the supported runtime seam, or the SDK export must be intentionally added with matching docs/contracts.
    Confidence: 0.95

Overall correctness: patch is incorrect
Overall confidence: 0.9

AGENTS.md: found and applied where relevant.

Codex review notes: model gpt-5.5, reasoning high; reviewed against 6804b7cb716b.

Label changes

Label justifications:

  • P1: The item targets a real Discord inbound queue stall that can block same-session replies, and the current patch has a merge-blocking SDK/runtime issue.
  • merge-risk: 🚨 compatibility: The PR changes a plugin runtime path and imports a helper from a public SDK subpath that does not currently export it.
  • merge-risk: 🚨 message-delivery: The new timeout path commits replay keys for a delivery whose processing state may be unknown, changing whether Discord retries or suppresses later duplicate delivery.
  • merge-risk: 🚨 session-state: After timeout, the prior run may still complete late while a newer same-session message is already running, creating session ordering and state overlap risk.
  • rating: 🧂 unranked krab: Overall readiness is 🧂 unranked krab; proof is 🦪 silver shellfish and patch quality is 🧂 unranked krab.
  • status: 📣 needs proof: The PR needs real behavior proof before ClawSweeper can clear the contributor ask. Needs real behavior proof before merge: The PR body now has structured redacted terminal output, but it is focused Vitest proof and the author explicitly did not run a live Gateway/Discord bot path. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.
Evidence reviewed

PR surface:

Source +78, Tests +167. Total +245 across 4 files.

View PR surface stats
Area Files Added Removed Net
Source 3 92 14 +78
Tests 1 170 3 +167
Docs 0 0 0 0
Config 0 0 0 0
Generated 0 0 0 0
Other 0 0 0 0
Total 4 262 17 +245

What I checked:

Likely related people:

  • Vincent Koc: Current blame/log history for the Discord queue, Discord timeout constants, and shared channel queue lifecycle points to commit 7054aa5. (role: recent area contributor; confidence: high; commits: 7054aa562e09; files: extensions/discord/src/monitor/message-run-queue.ts, extensions/discord/src/monitor/timeouts.ts, src/plugin-sdk/channel-lifecycle.core.ts)
  • Peter Steinberger: Search history for the Discord timeout export and plugin SDK facade surface includes release/facade commits near the compatibility boundary this PR touches. (role: adjacent SDK/export history; confidence: medium; commits: 27ae826f6525, 2bdbb189bdfe; files: extensions/discord/src/monitor/timeouts.ts, extensions/discord/runtime-api.ts, src/plugin-sdk/agent-runtime.ts)
  • scoootscooob: History for the Discord timeout export includes the commit that moved the Discord channel implementation into extensions/, which is relevant to the plugin boundary touched here. (role: Discord extension migration contributor; confidence: medium; commits: 5682ec37fada; files: extensions/discord/src/monitor/timeouts.ts, extensions/discord/runtime-api.ts)
What the crustacean ranks mean
  • 🦀 challenger crab: rare, exceptional readiness with strong proof, clean implementation, and convincing validation.
  • 🦞 diamond lobster: very strong readiness with only minor maintainer review expected.
  • 🐚 platinum hermit: good normal PR, likely mergeable with ordinary maintainer review.
  • 🦐 gold shrimp: useful signal, but proof or patch confidence is still limited.
  • 🦪 silver shellfish: thin signal; proof, validation, or implementation needs work.
  • 🧂 unranked krab: not merge-ready because proof is missing/unusable or there are serious correctness or safety concerns.
  • 🌊 off-meta tidepool: rating does not apply to this item.

Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics.

How this review workflow works
  • ClawSweeper keeps one durable marker-backed review comment per issue or PR.
  • Re-runs edit this comment so the latest verdict, findings, and automation markers stay together instead of adding duplicate bot comments.
  • A fresh review can be triggered by eligible @clawsweeper re-review comments, exact-item GitHub events, scheduled/background review runs, or manual workflow dispatch.
  • PR/issue authors and users with repository write access can comment @clawsweeper re-review or @clawsweeper re-run on an open PR or issue to request a fresh review only.
  • Maintainers can also comment @clawsweeper review to request a fresh review only.
  • Fresh-review commands do not start repair, autofix, rebase, CI repair, or automerge.
  • Maintainer-only repair and merge flows require explicit commands such as @clawsweeper autofix, @clawsweeper automerge, @clawsweeper fix ci, or @clawsweeper address review.
  • Maintainers can comment @clawsweeper explain to ask for more context, or @clawsweeper stop to stop active automation.

@clawsweeper clawsweeper Bot added rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. P1 High-priority user-facing bug, regression, or broken workflow. merge-risk: 🚨 compatibility 🚨 May break existing users, config, migrations, defaults, or upgrade paths. merge-risk: 🚨 message-delivery 🚨 May drop, duplicate, misroute, suppress, or wrongly target messages. merge-risk: 🚨 session-state 🚨 May lose, corrupt, stale, or mis-associate session, agent, or context state. labels May 30, 2026
@anyech
anyech force-pushed the fix-discord-inbound-run-watchdog branch from 3b51b0d to ae42c4f Compare May 30, 2026 08:20
@anyech

anyech commented May 30, 2026

Copy link
Copy Markdown
Contributor Author

@clawsweeper re-review

Updated in ae42c4f6f679 to address the previous review blockers:

  • The watchdog now uses the existing configured agent timeout contract via runtime.agent.resolveAgentTimeoutMs({ cfg }) instead of adding a new 30-minute Discord production SLA.
  • Removed the new exported 30-minute inbound worker timeout constant.
  • Added focused coverage that the default watchdog path calls the configured agent timeout resolver.
  • Targeted Discord queue regression now passes with 20 tests.
  • PR body includes a public-safe Real behavior proof section and calls out the adjacent session write-lock cleanup seam as out of scope, linked to the existing lock-path issue/PR cluster.

@clawsweeper

clawsweeper Bot commented May 30, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

🦞🧹
ClawSweeper re-review requested.

I asked ClawSweeper to review this item again.
Action: item re-review queued (workflow sweep.yml, event repository_dispatch).
Result: the existing ClawSweeper review comment will be edited in place when the review finishes.

Re-review progress:

@anyech
anyech force-pushed the fix-discord-inbound-run-watchdog branch from ae42c4f to 1440ed9 Compare May 30, 2026 08:50
@anyech

anyech commented May 30, 2026

Copy link
Copy Markdown
Contributor Author

@clawsweeper re-review

Updated in 1440ed950a0d to address the latest code blockers before proof review:

  • Preserved the shipped DISCORD_DEFAULT_INBOUND_WORKER_TIMEOUT_MS export as deprecated compatibility API.
  • Replaced the unsupported RuntimeEnv.agent access with the typed SDK resolver import resolveAgentTimeoutMs from openclaw/plugin-sdk/agent-runtime.
  • Kept production watchdog behavior aligned with the existing configured agent timeout contract.
  • Targeted Discord queue regression passes: message-handler.queue.test.ts 20/20.
  • PR body updated to describe compatibility, typed timeout resolver, current non-production proof, and the remaining request for maintainer/Mantis live Discord/Gateway proof.

I do not have a safe author-side way to run this patch against a live production Gateway/Discord bot. If maintainers want the requested end-to-end visible proof, please run or trigger a Mantis/maintainer proof lane for:

visual task: prove a stuck Discord same-session run times out and a later message gets a reply without duplicate or misordered output.

@clawsweeper

clawsweeper Bot commented May 30, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

🦞🧹
ClawSweeper re-review requested.

I asked ClawSweeper to review this item again.
Action: item re-review queued (workflow sweep.yml, event repository_dispatch).
Result: the existing ClawSweeper review comment will be edited in place when the review finishes.

@openclaw-barnacle openclaw-barnacle Bot added proof: supplied External PR includes structured after-fix real behavior proof. and removed triage: needs-real-behavior-proof Candidate: external PR needs after-fix proof from a real setup. labels May 30, 2026
@anyech

anyech commented May 30, 2026

Copy link
Copy Markdown
Contributor Author

@clawsweeper re-review

Updated the PR body at head 1440ed950a0d to put the Real behavior proof into the repository policy's required field format:

  • Behavior addressed
  • Real setup tested
  • Exact steps or command run after this patch
  • Evidence after fix
  • Observed result after fix
  • What was not tested

I also locally ran the repository proof-policy evaluator against the updated body; it returns passed for evaluateRealBehaviorProof. Sanitization validation passed after redacting the local checkout path.

@clawsweeper

clawsweeper Bot commented May 30, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

🦞👀
ClawSweeper picked this up.

Command router queued. I will update this comment with the next step.

Re-review progress:

@steipete steipete self-assigned this May 31, 2026
@steipete

steipete commented May 31, 2026

Copy link
Copy Markdown
Contributor

Thanks for working on this. I am closing this shape because it reintroduces a Discord-owned full-run timeout in the inbound queue, which conflicts with the current channel lifecycle contract: src/plugin-sdk/channel-lifecycle.core.ts:41 says channel queues do not impose run timeouts, and extensions/discord/src/monitor/timeouts.ts:3 says Discord no longer enforces channel-owned inbound run timeouts.

The default also comes from the global agent timeout, which is 48h at src/agents/timeout.ts:7, so it does not meaningfully unblock normal stuck Discord queues. At the same time, the Promise.race approach lets the original run continue in the background while later same-session messages can proceed, which creates session-state and duplicate-side-effect risk. This needs a broader session/runtime cancellation or terminal-state fix, not a Discord-local watchdog.

@steipete steipete closed this May 31, 2026
@anyech anyech mentioned this pull request Jun 2, 2026
Merged
This was referenced Jun 11, 2026
Closed
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

@steipete steipete

Labels

channel: discord Channel integration: discord merge-risk: 🚨 compatibility 🚨 May break existing users, config, migrations, defaults, or upgrade paths. merge-risk: 🚨 message-delivery 🚨 May drop, duplicate, misroute, suppress, or wrongly target messages. merge-risk: 🚨 session-state 🚨 May lose, corrupt, stale, or mis-associate session, agent, or context state. P1 High-priority user-facing bug, regression, or broken workflow. proof: supplied External PR includes structured after-fix real behavior proof. rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. size: M status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@anyech @steipete