feishu: prevent duplicate streaming cards for multi-payload final replies#36909
Open
Mickey0811 wants to merge 1 commit intoopenclaw:mainfrom
Open
feishu: prevent duplicate streaming cards for multi-payload final replies#36909Mickey0811 wants to merge 1 commit intoopenclaw:mainfrom
Mickey0811 wants to merge 1 commit intoopenclaw:mainfrom
Conversation
Contributor
Greptile SummaryThis PR fixes a duplicate message delivery bug in the Feishu channel where multiple final The fix introduces a
Confidence Score: 4/5
Last reviewed commit: 6711cc7 |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Comment on lines
+147
to
+150
| // Track whether streaming was already closed for a final payload in this dispatch. | ||
| // When true, subsequent final payloads skip streaming and send as standalone cards | ||
| // to avoid creating duplicate streaming cards (#XXXX). | ||
| let streamingClosedForFinal = false; |
Contributor
There was a problem hiding this comment.
Placeholder issue number in comment
The inline comment references #XXXX as the linked issue number. This should be replaced with the actual issue/ticket number before merging to keep the codebase traceable.
Suggested change
| // Track whether streaming was already closed for a final payload in this dispatch. | |
| // When true, subsequent final payloads skip streaming and send as standalone cards | |
| // to avoid creating duplicate streaming cards (#XXXX). | |
| let streamingClosedForFinal = false; | |
| // Track whether streaming was already closed for a final payload in this dispatch. | |
| // When true, subsequent final payloads skip streaming and send as standalone cards | |
| // to avoid creating duplicate streaming cards. | |
| let streamingClosedForFinal = false; |
Prompt To Fix With AI
This is a comment left during a code review.
Path: extensions/feishu/src/reply-dispatcher.ts
Line: 147-150
Comment:
**Placeholder issue number in comment**
The inline comment references `#XXXX` as the linked issue number. This should be replaced with the actual issue/ticket number before merging to keep the codebase traceable.
```suggestion
// Track whether streaming was already closed for a final payload in this dispatch.
// When true, subsequent final payloads skip streaming and send as standalone cards
// to avoid creating duplicate streaming cards.
let streamingClosedForFinal = false;
```
How can I resolve this? If you propose a fix, please make it concise.| expect(streamingInstances[0].close).toHaveBeenCalledWith("```md\n完整回复第一段\n```"); | ||
| expect(streamingInstances[1].close).toHaveBeenCalledTimes(1); | ||
| expect(streamingInstances[1].close).toHaveBeenCalledWith("```md\n完整回复第一段 + 第二段\n```"); | ||
| expect(sendMarkdownCardFeishuMock).toHaveBeenCalledTimes(1); |
Contributor
There was a problem hiding this comment.
Test doesn't assert the content of the standalone card
The assertion confirms sendMarkdownCardFeishu was called once, but doesn't verify it was called with the second payload's text ("```md\n完整回复第一段 + 第二段\n```"). A malformed or empty card body would still pass this assertion. Adding a content check would make the test more robust:
Suggested change
| expect(sendMarkdownCardFeishuMock).toHaveBeenCalledTimes(1); | |
| expect(sendMarkdownCardFeishuMock).toHaveBeenCalledTimes(1); | |
| expect(sendMarkdownCardFeishuMock).toHaveBeenCalledWith( | |
| expect.objectContaining({ text: "```md\n完整回复第一段 + 第二段\n```" }), | |
| ); |
Prompt To Fix With AI
This is a comment left during a code review.
Path: extensions/feishu/src/reply-dispatcher.test.ts
Line: 281
Comment:
**Test doesn't assert the content of the standalone card**
The assertion confirms `sendMarkdownCardFeishu` was called once, but doesn't verify it was called with the second payload's text (`"```md\n完整回复第一段 + 第二段\n```"`). A malformed or empty card body would still pass this assertion. Adding a content check would make the test more robust:
```suggestion
expect(sendMarkdownCardFeishuMock).toHaveBeenCalledTimes(1);
expect(sendMarkdownCardFeishuMock).toHaveBeenCalledWith(
expect.objectContaining({ text: "```md\n完整回复第一段 + 第二段\n```" }),
);
```
How can I resolve this? If you propose a fix, please make it concise.
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
1 participant
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
Problem
When the agent returns multiple
ReplyPayloaditems in a single dispatch (common with auto-compaction notices, verbose mode, model fallback notices, or usage lines), each payload triggered a new streaming card creation in Feishu. This caused users to see duplicate messages — the main reply in one card and a short notice in another.Root cause chain:
agent-runner.tsprepends verbose notices to final payloads → returns arraydispatch-from-config.tsiterates the array, callingsendFinalReply()for eachreply-dispatcher.tscreates a streaming card for the first final, closes it, then creates a new streaming card for the second final (becausestreamingwas set tonullafter close)Gateway log pattern:
Fix
Added a
streamingClosedForFinalflag that prevents re-creating streaming sessions after a final payload has already been delivered via streaming. Subsequent final payloads fall through to the standard card/message send path instead.Changes:
extensions/feishu/src/reply-dispatcher.ts: AddstreamingClosedForFinalguard; reset ononReplyStartextensions/feishu/src/reply-dispatcher.test.ts: Update test to verify new behavior (second final goes to standalone card)Testing
media.test.tsfailures (4 tests, unrelatedtimeoutparameter issue) unchanged