Discord: fix chunk delivery order for long messages#32819
Closed
haosenwang1018 wants to merge 1 commit intoopenclaw:mainfrom
Closed
Discord: fix chunk delivery order for long messages#32819haosenwang1018 wants to merge 1 commit intoopenclaw:mainfrom
haosenwang1018 wants to merge 1 commit intoopenclaw:mainfrom
Conversation
When delivering chunked Discord replies, bypass sendMessageDiscord for the bot-send path and call sendDiscordText directly with a pre-resolved channel ID and retry runner. This eliminates per-chunk overhead (channel-type GET request, recipient resolution, re-chunking, client creation) that caused ordering issues under RequestClient queue contention or rate limiting. Fixes openclaw#32743
openclaw-barnacle
bot
added
channel: discord
Contributor
Greptile SummaryThis PR fixes Discord chunk delivery ordering by pre-resolving the channel ID and retry runner once in Key observations:
Confidence Score: 4/5
Last reviewed commit: 3f58840 |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Comment on lines
+200
to
+202
| const request: RetryRunner | undefined = channelId | ||
| ? createDiscordRetryRunner({}) | ||
| : undefined; |
Contributor
There was a problem hiding this comment.
The retry runner is created whenever channelId resolves, but it is only consumed in sendDiscordChunkWithFallback when both params.request and params.rest are provided (line 117). When params.rest is undefined, the runner is allocated but never used. Consider checking params.rest to avoid unnecessary allocation:
Suggested change
| const request: RetryRunner | undefined = channelId | |
| ? createDiscordRetryRunner({}) | |
| : undefined; | |
| const request: RetryRunner | undefined = channelId && params.rest | |
| ? createDiscordRetryRunner({}) | |
| : undefined; |
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/discord/monitor/reply-delivery.ts
Line: 200-202
Comment:
The retry runner is created whenever `channelId` resolves, but it is only consumed in `sendDiscordChunkWithFallback` when both `params.request` and `params.rest` are provided (line 117). When `params.rest` is undefined, the runner is allocated but never used. Consider checking `params.rest` to avoid unnecessary allocation:
```suggestion
const request: RetryRunner | undefined = channelId && params.rest
? createDiscordRetryRunner({})
: undefined;
```
How can I resolve this? If you propose a fix, please make it concise.
18 tasks
Member
|
Superseded by #33226 (combined Discord chunk ordering + retry fixes). |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
deliverDiscordReplynow pre-resolves the channel ID and retry runner once, then sends each text chunk directly viasendDiscordTextinstead of routing through the fullsendMessageDiscordpipeline per chunk. This eliminates per-chunk overhead (channel-type GET, recipient re-resolution, re-chunking, client creation) that caused ordering issues under RequestClient queue contention or rate limiting.sendMessageDiscordfunction itself, and thesendDiscordTextinternal implementation are all untouched. The fallback tosendMessageDiscordremains for cases whererestis not provided (e.g., CLI/outbound sends).Change Type (select all)
Scope (select all touched areas)
Linked Issue/PR
User-visible / Behavior Changes
Discord message chunks now arrive in the correct sequential order when a long reply is split into multiple messages.
Security Impact (required)
Repro + Verification
Environment
textChunkLimit: 2000,maxLinesPerMessage: 17)Steps
Expected
Actual
Evidence
sends text chunks in order via sendDiscordText when rest is providedverifies chunk orderingfalls back to sendMessageDiscord when rest is not providedverifies fallback pathHuman Verification (required)
Compatibility / Migration
Failure Recovery (if this breaks)
sendMessageDiscordis still present whenrestis not providedsrc/discord/monitor/reply-delivery.tssendDiscordTextmock path has a type mismatch)Risks and Mitigations
sendDiscordTextdoes not perform forum/media channel detection (whichsendMessageDiscorddoes viaresolveDiscordChannelType)sendMessageDiscordfor outbound sends.