fix(discord): allow marked room_event final fallback delivery#86672
fix(discord): allow marked room_event final fallback delivery#86672anyech wants to merge 2 commits into
Conversation
|
Codex review: needs real behavior proof before merge. Reviewed May 30, 2026, 4:15 AM ET / 08:15 UTC. Summary PR surface: Source +17, Tests +219. Total +236 across 2 files. Reproducibility: yes. source inspection shows current main suppresses marked non-command Review metrics: 1 noteworthy metric.
Merge readiness Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch. Rank-up moves:
Proof guidance:
Mantis proof suggestion Risk before merge
Maintainer options:
Next step before merge
Security Review findings
Review detailsBest possible solution: Rebase onto current main, fix the sanitizer import, merge the Discord marked-fallback logic with the current explicit-command guard, then provide focused regression coverage plus redacted real Discord/Gateway proof before merge. Do we have a high-confidence way to reproduce the issue? Yes, source inspection shows current main suppresses marked non-command Is this the best way to solve the issue? No, the submitted branch is not yet the best merge shape because it is dirty against current main and imports a non-existent sanitizer path. The right solution is the narrow Discord marked-fallback exception after a rebase that preserves current explicit-command behavior. Full review comments:
Overall correctness: patch is incorrect AGENTS.md: found and applied where relevant. Codex review notes: model gpt-5.5, reasoning high; reviewed against 7d71c5d0c62f. Label changesLabel changes:
Label justifications:
Evidence reviewedPR surface: Source +17, Tests +219. Total +236 across 2 files. View PR surface stats
What I checked:
Likely related people:
What the crustacean ranks mean
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 PR egg 🎁 Pass real behavior proof to wake the egg and unlock a hatchable treat. Where did the egg go?
|
|
I added the strongest proof I can provide from this contributor side without touching live Gateway/Discord state. Non-live dispatch-path validationCheckout: Command run locally in the patched checkout: ./node_modules/.bin/vitest run src/auto-reply/reply/dispatch-from-config.test.ts \
-t "marked room-event|sendPolicy denies delivery|marked empty room-event|marked NO_REPLY room-event|ambient room-event" \
--reporter=verboseResult: Covered cases through the actual
Caveat: this is isolated source-checkout validation through the real dispatch path, not live Discord/Gateway proof. It therefore should be treated as a non-live harness result, not as evidence that a live Discord transport received exactly one visible message. Live proof request@openclaw-mantis Could you run or prepare a live Discord/Gateway
I did not run this against my live Gateway/Discord state because that would use private production runtime/config rather than a maintainer-controlled public QA surface. Secondary note: I also see |
9dec540 to
aa2066a
Compare
|
I pushed a follow-up update to narrow the exception to Discord Updated commit: What changed:
Validation note: the original focused dispatch validation passed before this scope-narrowing update. After rebasing onto latest This still does not claim live Discord/Gateway proof; the Mantis/maintainer live-proof request remains the next proof step. |
This comment was marked as spam.
This comment was marked as spam.
|
Closing this PR, but keeping #84022 open. This patch is no longer the right shape for the remaining Discord room-event/thread-bound delivery problem. The branch is stale, imports a helper path that does not exist on current The next fix should be a fresh, narrower patch for plugin-owned/bound thread replies and ACP completion delivery, with #84022 as the canonical issue. Thanks for the work here; the issue itself is still real. |
Summary
Fixes #84022 by allowing Discord
room_eventfinal-visible fallback/failure payloads to be delivered when runtime code explicitly marks them withdeliverDespiteSourceReplySuppression.The change keeps the ambient room-event privacy guard intact:
room_eventfinal payloads still remain private under source-reply suppression;sendPolicy: denystill suppresses marked fallback delivery;NO_REPLY/ silent-envelope payloads stay private;Why
room_eventdispatch currently has an absolute suppression check, so even runtime-marked final fallback/failure notices cannot reachsendFinalReply. That means a user-visible final can be lost on Discord room-event/message-tool-only surfaces when the runtime deliberately prepared a visible fallback.#84328 is only partial/unmerged for this case because it does not cover the room-event dispatch suppressor.
Real behavior proof
Behavior or issue addressed: Discord
room_eventfinal-visible fallback/failure payloads that runtime code explicitly marks withdeliverDespiteSourceReplySuppressionshould reach final reply dispatch, while ordinary ambientroom_eventcontent,sendPolicy: deny, empty payloads, exactNO_REPLY, and silent envelopes stay suppressed.Real environment tested: Disposable OpenClaw source checkout using this PR branch on Linux with Node 22. The proof exercised the actual
dispatchReplyFromConfigdelivery gate with a fake dispatcher and synthetic Discordroom_eventinputs. No production Gateway, live runtime state, live config, or real Discord channel was used.Exact steps or command run after this patch:
Evidence after fix: Copied terminal output from the disposable checkout showed:
Observed result after fix: The patched dispatch path delivered the explicitly marked Discord
room_eventfallback exactly once whensendPolicyallowed it, while unmarked ambientroom_eventfinals, empty marked fallbacks, exactNO_REPLYmarked fallbacks, andsendPolicy: denyremained private.What was not tested: No production Gateway, live runtime state, live config, or real Discord transport was exercised. This is targeted non-live source-level proof for the final-delivery gate changed by this PR; maintainer/Mantis live proof would still be stronger if required for real transport evidence.
Reviewer / AI review map
This PR now intentionally supersedes #86634 so reviewers only need to evaluate one
dispatch-from-configbranch for this overlapping seam.room_eventfallback/failure finals to bypass source-reply suppression whensendPolicyallows it.src/auto-reply/reply/dispatch-from-config.tsandsrc/auto-reply/reply/dispatch-from-config.test.tsonly.room_eventchange is a later final-delivery suppression exception guarded by provider, marker, non-silent content, andsendPolicy.NO_REPLYand silent envelopes remain suppressed;sendPolicy: denystill wins; unmarked ambientroom_eventfinals remain private.Combined validation update
After combining #86634 into this branch in a disposable checkout, targeted non-live validation passed:
This validation is intentionally source-level and non-production; no live Gateway, runtime state, live config, or real Discord transport was exercised.
Risk / notes
dispatch-from-configfinal delivery gating plus focused unit coverage.Related