Skip to content

Fix Discord native empty reply fallback for compact commands#90079

Draft
AliAbuelkheir wants to merge 1 commit into
openclaw:mainfrom
AliAbuelkheir:fix/89950-discord-compact-visible-reply
Draft

Fix Discord native empty reply fallback for compact commands#90079
AliAbuelkheir wants to merge 1 commit into
openclaw:mainfrom
AliAbuelkheir:fix/89950-discord-compact-visible-reply

Conversation

@AliAbuelkheir

Copy link
Copy Markdown

Summary

  • Gate the Discord native empty-visible-reply fallback on the shared dispatcher noVisibleReplyFallbackEligible signal.
  • Keep the warning for genuinely fallback-eligible empty command dispatches.
  • Add a Discord native /compact regression covering successful zero-counter dispatch results that should not emit the empty-reply warning.

Linked context

Fixes #89950

AI-assisted: yes. I used Codex to inspect the issue, make the patch, and run focused validation. I reviewed the diff and understand the change.

Real behavior proof

  • Behavior addressed: Discord native command dispatch could send the empty visible reply warning even when shared reply dispatch completed with zero visible counters but did not mark the result as fallback-eligible. This covers the /compact report where compaction can succeed while the user-facing interaction still receives the empty warning.
  • Real environment tested: Not tested against a live Discord bot in this workspace.
  • Exact steps or command run after this patch:
    • node --no-maglev node_modules\vitest\vitest.mjs run --config test/vitest/vitest.extension-discord.config.ts extensions/discord/src/monitor/native-command.plugin-dispatch.test.ts --reporter=dot
  • Evidence after fix:
    • Test Files 1 passed (1)
    • Tests 23 passed (23)
  • Observed result after fix: the focused Discord native command dispatch suite passes, including the new /compact zero-counter non-fallback-eligible regression.
  • What was not tested: live Discord slash command execution and live Codex compaction against a real Discord interaction.
  • Proof limitations or environment constraints: this Windows Codex workspace had to run Vitest directly through the local CLI because scripts/run-vitest.mjs no-oped under Node v22.14.0; dependency install also timed out on a few unrelated package downloads after hydrating enough dependencies for the focused test.
  • Before evidence: issue [Bug]: Discord native /compact slash command produces no visible reply despite successful compaction #89950 reports the native Discord /compact command completed compaction but produced the empty visible reply warning instead of a visible confirmation.

Tests and validation

  • Passed: node --no-maglev node_modules\vitest\vitest.mjs run --config test/vitest/vitest.extension-discord.config.ts extensions/discord/src/monitor/native-command.plugin-dispatch.test.ts --reporter=dot
  • Passed: git diff --check

Risk checklist

  • Small, scoped change
  • Tests added or updated
  • No unrelated refactors
  • No CHANGELOG.md edits
  • Live behavior proof limitations are stated

Current review state

Draft for maintainer and Ali review because live Discord proof was not available in this workspace.

@openclaw-barnacle openclaw-barnacle Bot added channel: discord Channel integration: discord size: XS triage: mock-only-proof Candidate: PR proof only shows tests, mocks, snapshots, lint, typecheck, or CI. labels Jun 4, 2026
@clawsweeper

clawsweeper Bot commented Jun 4, 2026

Copy link
Copy Markdown
Contributor

Codex review: needs real behavior proof before merge. Reviewed June 3, 2026, 8:58 PM ET / 00:58 UTC.

Summary
The branch gates Discord native empty-reply warnings on the shared dispatcher fallback-eligibility signal and adds a /compact zero-counter regression test.

PR surface: Source +19, Tests +22. Total +41 across 2 files.

Reproducibility: no. high-confidence live reproduction was established in this read-only review. The linked report gives concrete Discord native /compact steps, and source inspection shows the current zero-count fallback path, but live Discord was not run here or in the PR proof.

Review metrics: none identified.

Merge readiness
Overall: 🦪 silver shellfish
Proof: 🦪 silver shellfish
Patch quality: 🐚 platinum hermit
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:

  • [P1] Add redacted live Discord native /compact proof showing the expected confirmation reply and no empty-visible-reply warning.
  • Update the PR body with that proof so ClawSweeper can re-review automatically, or ask a maintainer to comment @clawsweeper re-review if it does not.

Proof guidance:

  • [P1] Needs real behavior proof before merge: The PR body provides focused Vitest output and explicitly says live Discord slash command execution was not tested, so contributor-visible real behavior proof is still needed with private details redacted. 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 visible live Discord slash-command proof would materially reduce the remaining message-delivery risk, and no narrower Mantis Discord lane matches this command path. 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: verify Discord native /compact returns a compaction confirmation and no empty visible reply warning.

Risk before merge

  • [P2] Live Discord native slash-command delivery is still unproven; because the patch suppresses a fallback warning based on dispatcher metadata, a bad interpretation could leave /compact with no visible interaction reply instead of the expected confirmation.

Maintainer options:

  1. Require live Discord proof (recommended)
    Ask for a redacted live Discord native /compact run showing the confirmation reply and absence of the empty-visible-reply warning before merge.
  2. Accept unit-only delivery risk
    Maintainers can intentionally merge on the focused dispatcher regression alone, accepting that the real Discord interaction path was not exercised.

Next step before merge

  • [P1] The remaining blocker is live Discord proof and maintainer review for message-delivery risk, not a narrow automated code repair.

Security
Cleared: The diff touches only Discord TypeScript runtime/test code and introduces no dependency, workflow, credential, or supply-chain surface.

Review details

Best possible solution:

Land only after redacted live Discord native /compact proof shows the compaction confirmation reaches the interaction and the empty-visible-reply warning is absent.

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

No high-confidence live reproduction was established in this read-only review. The linked report gives concrete Discord native /compact steps, and source inspection shows the current zero-count fallback path, but live Discord was not run here or in the PR proof.

Is this the best way to solve the issue?

Unclear until live proof: gating Discord on the shared dispatcher eligibility signal matches the Feishu sibling pattern and is a narrow fix shape, but the proof must show the expected compact confirmation is delivered rather than only hiding the warning.

AGENTS.md: found and applied where relevant.

Codex review notes: model gpt-5.5, reasoning high; reviewed against 392af2e61201.

Label changes

Label justifications:

  • P2: This is a normal-priority Discord message-delivery bugfix with limited channel-specific blast radius.
  • merge-risk: 🚨 message-delivery: Merging changes when Discord native slash commands suppress the empty-reply fallback, so the real interaction reply path needs proof before landing.
  • rating: 🦪 silver shellfish: Overall readiness is 🦪 silver shellfish; proof is 🦪 silver shellfish and patch quality is 🐚 platinum hermit.
  • 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 provides focused Vitest output and explicitly says live Discord slash command execution was not tested, so contributor-visible real behavior proof is still needed with private details redacted. 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 +19, Tests +22. Total +41 across 2 files.

View PR surface stats
Area Files Added Removed Net
Source 1 20 1 +19
Tests 1 22 0 +22
Docs 0 0 0 0
Config 0 0 0 0
Generated 0 0 0 0
Other 0 0 0 0
Total 2 42 1 +41

What I checked:

Likely related people:

  • Peter Steinberger: Current-main blame and feature-history commands point to commit 190fd03 for the Discord fallback, dispatcher eligibility signal, and Feishu sibling helper. (role: recent area contributor; confidence: high; commits: 190fd034d59c; files: extensions/discord/src/monitor/native-command-agent-reply.ts, extensions/feishu/src/bot.ts, src/auto-reply/reply/dispatch-from-config.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: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. P2 Normal backlog priority with limited blast radius. merge-risk: 🚨 message-delivery 🚨 May drop, duplicate, misroute, suppress, or wrongly target messages. labels Jun 4, 2026
@AliAbuelkheir

Copy link
Copy Markdown
Author

I do not currently have a live Discord bot setup available for this PR branch. Could a maintainer run the suggested Mantis proof or apply proof override if the source-level review is sufficient?

Mantis prompt:
visual proof: verify Discord native /compact shows the compaction confirmation and not the empty visible reply warning.

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

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. rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. size: XS status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. triage: mock-only-proof Candidate: PR proof only shows tests, mocks, snapshots, lint, typecheck, or CI.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: Discord native /compact slash command produces no visible reply despite successful compaction

1 participant