fix(discord): dedupe inbound messages by message id#41145
fix(discord): dedupe inbound messages by message id#41145Garyko0730 wants to merge 2 commits intoopenclaw:mainfrom
Conversation
openclaw-barnacle
bot
added
channel: discord
Greptile SummaryThis PR introduces a lightweight inbound dedupe guard in
Confidence Score: 3/5
Last reviewed commit: 6bc1c52 |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| const dedupeKey = buildDiscordInboundMessageDedupeKey({ | ||
| accountId: params.accountId, | ||
| data, | ||
| }); | ||
| if (dedupeKey && discordInboundMessageDedupe.peek(dedupeKey)) { | ||
| return; | ||
| } | ||
| if (dedupeKey) { | ||
| discordInboundMessageDedupe.check(dedupeKey); | ||
| } | ||
|
|
||
| await debouncer.enqueue({ data, client, abortSignal: options?.abortSignal }); |
There was a problem hiding this comment.
Dedupe key registered before enqueue — message silently dropped on enqueue failure
The dedupe key is inserted into the cache before debouncer.enqueue(...) is awaited. If debouncer.enqueue throws (or rejects), the catch block logs the error and the handler returns — but the key is already in the cache. Any subsequent delivery of the exact same message.id will be silently dropped for the full 2-minute TTL, even though the message was never successfully enqueued.
Before this PR, a failed enqueue would have allowed the same message to be retried on the next delivery. After this change, it is silently suppressed for up to 2 minutes.
To preserve retry-ability on enqueue failures, the key should only be committed to the cache after debouncer.enqueue succeeds — or it should be evicted from the cache in the error path. Note that DedupeCache does not currently expose a single-key delete method, so the cleanest fix is to move discordInboundMessageDedupe.check(dedupeKey) to after the await debouncer.enqueue(...) call succeeds.
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/discord/monitor/message-handler.ts
Line: 209-220
Comment:
**Dedupe key registered before enqueue — message silently dropped on enqueue failure**
The dedupe key is inserted into the cache _before_ `debouncer.enqueue(...)` is awaited. If `debouncer.enqueue` throws (or rejects), the `catch` block logs the error and the handler returns — but the key is already in the cache. Any subsequent delivery of the exact same `message.id` will be silently dropped for the full 2-minute TTL, even though the message was never successfully enqueued.
Before this PR, a failed enqueue would have allowed the same message to be retried on the next delivery. After this change, it is silently suppressed for up to 2 minutes.
To preserve retry-ability on enqueue failures, the key should only be committed to the cache after `debouncer.enqueue` succeeds — or it should be evicted from the cache in the error path. Note that `DedupeCache` does not currently expose a single-key delete method, so the cleanest fix is to move `discordInboundMessageDedupe.check(dedupeKey)` to after the `await debouncer.enqueue(...)` call succeeds.
How can I resolve this? If you propose a fix, please make it concise.
1 participant
Summary
This PR prevents duplicate processing of the same Discord inbound message by adding a short-TTL dedupe guard keyed by Discord
message.idbefore the message enters the debounce queue.Problem
OpenClaw already has duplicate protection in later stages (for example queued follow-up handling, and more general inbound dedupe at dispatch time), but a duplicate Discord event can still be admitted earlier in the pipeline.
In practice, this can happen when the same Discord message is replayed/re-injected into an active session path (observed around model switching in a live local deployment). When that happens, the same physical Discord message may enter processing twice.
The result can be:
Fix
Add a lightweight inbound dedupe cache in
src/discord/monitor/message-handler.tskeyed by:This guard runs before
debouncer.enqueue(...), so a duplicate Discord message never consumes debounce capacity and never re-enters the downstream processing path.Why this approach
accountId + channelId + authorId) for normal short-burst message coalescingNO_REPLY/ duplicate suppression behaviorTests
Added a test in
src/discord/monitor/message-handler.queue.test.tsto verify that two inbound Discord events with the samemessage.idare only processed once.Validated locally with:
Related
Notes
This PR intentionally keeps scope small and focused on Discord inbound idempotency. If maintainers want a second session-level guard as defense-in-depth for model-switch replay paths, that can be proposed separately.