Skip to content

fix(discord): harden reply delivery accounting#86716

Closed
Steady-ai wants to merge 1 commit into
openclaw:mainfrom
Steady-ai:fix/discord-delivery-accounting
Closed

fix(discord): harden reply delivery accounting#86716
Steady-ai wants to merge 1 commit into
openclaw:mainfrom
Steady-ai:fix/discord-delivery-accounting

Conversation

@Steady-ai

@Steady-ai Steady-ai commented May 26, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Scope

  • Return an explicit delivery result from Discord interaction replies so expired interactions are not counted as delivered.
  • Fail final Discord delivery when sanitizer removes all visible/sendable payload content.
  • Normalize Discord message send extraction for both send and sendMessage actions.

Non-goals

  • No changes to Discord transport connection, auth, or gateway startup.
  • No changes to deliberate silent NO_REPLY handling before final delivery.

Risk

Low. The change is scoped to Discord reply accounting and message-tool send target extraction. The main behavioral change is surfacing empty final replies as delivery failures instead of silent success.

Rollback

Revert commit 64f2dc4b2d.

Real behavior proof

Behavior or issue addressed: Discord final reply delivery after applying the equivalent local runtime hotfix no longer silently completes for normal visible replies; the live channel continued receiving final assistant replies and the Discord transport stayed healthy.

Real environment tested: Richard's real OpenClaw gateway using Discord channel #starlord-infra, installed OpenClaw 2026.5.22 with the equivalent local Discord delivery-accounting hotfix applied before this PR was ported back to source.

Exact steps or command run after the patch:

  1. Applied the equivalent hotfix to the installed OpenClaw runtime bundles.
  2. Restarted/verified the live gateway loaded the patched runtime.
  3. Sent live Discord turns through the active #starlord-infra channel.
  4. Ran openclaw channels status --probe && openclaw gateway stability after the visible Discord replies landed.

Evidence after fix:

Checking channel status (probe)...
Gateway reachable.
- Discord default: enabled, configured, running, connected, in:20m ago, out:39m ago, transport:just now, bot:@Starlord, token:config, intents:content=limited, works, audit ok
Gateway Stability
Recent:
  2026-05-26T01:49:44.186Z #3701 diagnostic.heartbeat queued=0

Observed result after fix: The same real Discord setup delivered visible final replies after the hotfix, while the gateway reported Discord works and queue depth 0. This PR ports that runtime hotfix into source and adds regression coverage for the expired-interaction accounting path, sanitizer-empty final path, and send/sendMessage extraction mismatch.

What was not tested: I did not force a real expired Discord slash-interaction token in production; that path is covered by unit regression because manufacturing an expired live token would be brittle and user-visible.

Verification

  • node scripts/run-vitest.mjs run --config test/vitest/vitest.extension-discord.config.ts extensions/discord/src/monitor/native-command-reply.test.ts extensions/discord/src/monitor/reply-delivery.test.ts extensions/discord/src/channel-actions.test.ts -> 3 files / 38 tests passed
  • node scripts/run-oxlint.mjs --tsconfig config/tsconfig/oxlint.extensions.json extensions/discord/src/monitor/native-command-reply.ts extensions/discord/src/monitor/native-command-agent-reply.ts extensions/discord/src/monitor/reply-delivery.ts extensions/discord/src/channel-actions.ts extensions/discord/src/monitor/native-command-reply.test.ts extensions/discord/src/monitor/reply-delivery.test.ts extensions/discord/src/channel-actions.test.ts -> passed
  • corepack pnpm exec oxfmt --check extensions/discord/src/monitor/native-command-reply.ts extensions/discord/src/monitor/native-command-agent-reply.ts extensions/discord/src/monitor/reply-delivery.ts extensions/discord/src/channel-actions.ts extensions/discord/src/monitor/native-command-reply.test.ts extensions/discord/src/monitor/reply-delivery.test.ts extensions/discord/src/channel-actions.test.ts -> passed
  • node scripts/run-tsgo.mjs -p tsconfig.extensions.json --noEmit -> passed

@openclaw-barnacle openclaw-barnacle Bot added channel: discord Channel integration: discord size: S triage: needs-real-behavior-proof Candidate: external PR needs after-fix proof from a real setup. triage: refactor-only Candidate: refactor/cleanup-only PR without maintainer context. 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 26, 2026
@github-actions github-actions Bot mentioned this pull request May 26, 2026
Open
@clawsweeper

clawsweeper Bot commented May 26, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Thanks for the contribution. This PR still needs the requested real-behavior proof, and the branch has been idle since that ask.

Close as stalled and unproven: this external Discord delivery PR is idle on the same May 26, 2026 head, has carried a needs-proof ask since June 15, 2026, the supplied proof does not exercise the changed PR-head failure paths, and source review still finds a dispatcher-accounting blocker.

So I’m closing this for now to keep the review queue honest. Please reopen or open a fresh PR with real-behavior proof (a live run, logs, or a reproducible validation transcript) and it will be reviewed again.

Review details

Best possible solution:

Close this stale PR now; if the fix is still needed, reopen or send a fresh branch that routes false Discord interaction delivery into dispatcher failure or cancellation accounting and includes redacted real Discord proof for one changed path.

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

Yes, from source inspection of the PR head: deliverDiscordInteractionReply can return false, and the delivery callback then resolves normally while the shared dispatcher only records failed delivery on rejection. I did not run tests because this review is read-only.

Is this the best way to solve the issue?

No as submitted. Returning an explicit delivery result is a reasonable direction, but the best fix must convert a false interaction delivery into dispatcher-visible failed or cancelled accounting and prove at least one changed Discord path in a real setup.

Security review:

Security review cleared: Security review cleared: the diff is limited to Discord TypeScript source and tests and does not add dependencies, workflows, secrets handling, package-resolution changes, or generated code.

AGENTS.md: found and applied where relevant.

What I checked:

  • Repository policy applied: Root AGENTS.md and scoped extensions/AGENTS.md were read; their PR-review, proof, plugin-boundary, and message-delivery merge-risk guidance applies to this Discord plugin PR. (AGENTS.md:1, b4e0c5306cd0)
  • PR-head accounting blocker: At PR head, deliverDiscordInteractionReply returning false makes the native delivery callback return normally instead of failing or cancelling the queued delivery. (extensions/discord/src/monitor/native-command-agent-reply.ts:81, 64f2dc4b2df4)
  • Dispatcher contract: The shared dispatcher only increments failed counts when options.deliver rejects; a resolved false delivery is therefore still treated as settled successfully. (src/auto-reply/reply/reply-dispatcher.ts:268, b4e0c5306cd0)
  • Proof and stalled state: Live GitHub data shows no assignees, no requested reviewers, no human reviews, head commit 64f2dc4b2df40fc4790e4982161dc0e0f8c4bde3 from May 26, 2026, and the status: 📣 needs proof label applied on June 15, 2026. (64f2dc4b2df4)
  • Real behavior proof gap: The PR body supplies copied live output from an installed runtime with an equivalent local hotfix, but it does not show an after-fix observation of the expired-interaction, sanitizer-empty final, or send extraction paths at this PR head. (64f2dc4b2df4)
  • History and routing: Local history and blame point to recent Discord delivery/shared-dispatch work by Peter Steinberger and Vincent Koc, plus older Discord action extraction work by Gustavo Madeira Santana and adjacent final-delivery work by Patrick Erichsen. (extensions/discord/src/monitor/native-command-agent-reply.ts:54, b4e0c5306cd0)

Likely related people:

  • steipete: Current-main blame for the native Discord reply and dispatcher paths points to Peter Steinberger, and nearby history includes Discord trimmed-reader/refactor work plus the prior final-delivery fix merge path. (role: recent Discord delivery and shared-dispatch contributor; confidence: high; commits: d54fc627bd53, c7347a492ee2, 8c7dd66a7b9a; files: extensions/discord/src/monitor/native-command-agent-reply.ts, extensions/discord/src/monitor/native-command-reply.ts, extensions/discord/src/monitor/reply-delivery.ts)
  • vincentkoc: Recent history touches the same Discord delivery files, shared dispatcher file, and message-tool extraction surface around the current release snapshot. (role: recent Discord and dispatcher area contributor; confidence: medium; commits: e085fa1a3ffd, 6964e4acf7c1, 29ad211e7647; files: extensions/discord/src/monitor/native-command-agent-reply.ts, extensions/discord/src/monitor/native-command-reply.ts, extensions/discord/src/monitor/reply-delivery.ts)
  • gumadeiras: Older history shows Gustavo Madeira Santana shaping the Discord channel action and plugin-boundary surface affected by the extractToolSend part of this PR. (role: historical Discord action and plugin-boundary contributor; confidence: medium; commits: b48194a07eec, 1c6676cd5745, 9df3e9b617b2; files: extensions/discord/src/channel-actions.ts)
  • Patrick-Erichsen: Patrick Erichsen authored the prior merged Discord final-reply delivery failure fix that overlaps the durable delivery accounting part of this PR. (role: adjacent final-delivery behavior contributor; confidence: medium; commits: 9324a756410c, 9e97cdb21389; files: extensions/discord/src/monitor/reply-delivery.ts, extensions/discord/src/monitor/reply-delivery.test.ts)

Codex review notes: model internal, reasoning high; reviewed against b4e0c5306cd0.

@clawsweeper clawsweeper Bot added proof: sufficient ClawSweeper judged the real behavior proof convincing. rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. status: ⏳ waiting on author ClawSweeper has contributor-facing work open and is waiting for author action. P2 Normal backlog priority with limited blast radius. labels May 26, 2026
@clawsweeper

clawsweeper Bot commented May 26, 2026

Copy link
Copy Markdown
Contributor

ClawSweeper PR egg

🔥 Warming up: real-behavior proof passed; findings, security review, or rank-up moves are still in progress.

Hatch command

Comment @clawsweeper hatch when this PR is hatchable.

Hatchability rules:

  • Merged PRs are hatchable.
  • Open PRs are hatchable when they are status: 👀 ready for maintainer look, status: 🚀 automerge armed, or labeled clawsweeper:automerge.
  • Closed unmerged PRs are hatchable only when one of those hatchable labels is still present in the durable record.
What is this egg doing here?
  • Eggs appear after the PR passes real-behavior proof. It is here for vibes, not verdicts: it does not change labels, ratings, merge decisions, or automation.
  • The shell reacts to review momentum: open follow-up work warms it up, re-review makes it wobble, and a clean final review lets it hatch.
  • Hatchability usually comes from sufficient real-behavior proof, no blocking P0/P1/P2 findings, no security attention needed, and clean correctness. A merged PR is already final, so merge makes the egg hatchable independently.
  • The hatch is seeded from this repository and PR number, so the same PR keeps the same creature; the reviewed head SHA can only change safe visual details.
  • Rarity is just collectible sparkle: 🥚 common, 🌱 uncommon, 💎 rare, ✨ glimmer, and 🌈 legendary.

@Steady-ai Steady-ai mentioned this pull request May 27, 2026
Closed
@clawsweeper clawsweeper Bot added the merge-risk: 🚨 message-delivery 🚨 May drop, duplicate, misroute, suppress, or wrongly target messages. label May 29, 2026
@clawsweeper clawsweeper Bot added rating: 🌊 off-meta tidepool PR readiness rating does not apply to this item. and removed proof: sufficient ClawSweeper judged the real behavior proof convincing. rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. status: ⏳ waiting on author ClawSweeper has contributor-facing work open and is waiting for author action. labels May 29, 2026
@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. and removed rating: 🌊 off-meta tidepool PR readiness rating does not apply to this item. labels Jun 15, 2026
@clawsweeper

clawsweeper Bot commented Jul 5, 2026

Copy link
Copy Markdown
Contributor

ClawSweeper applied the proposed close for this PR.

@clawsweeper clawsweeper Bot closed this Jul 5, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

channel: discord Channel integration: discord merge-risk: 🚨 message-delivery 🚨 May drop, duplicate, misroute, suppress, or wrongly target messages. P2 Normal backlog priority with limited blast radius. 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: S status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. triage: refactor-only Candidate: refactor/cleanup-only PR without maintainer context.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@Steady-ai @RomneyDa