@codex take a look bro

Codex Review: Didn't find any major issues. Another round soon, please!

Greptile Summary

This PR introduces two boolean flags — streamingClosedForReply and streamingCloseErroredForReply — to detect when a streaming card has already closed for the current reply and suppress duplicate late final text deliveries while still forwarding media attachments. The logic correctly resets both flags in onReplyStart, clears streamingClosedForReply on onError, and guards markClosedForReply to prevent the errored-close path from incorrectly setting the suppression flag. The main happy-path fix and the error-recovery tests are well-structured.

One edge case: streamingCloseErroredForReply is only reset by onReplyStart, so after an error-and-recovery within the same reply cycle, closeStreaming can no longer set streamingClosedForReply = true, leaving late finals after the recovery card unfiltered.

Confidence Score: 5/5

Safe to merge; the primary fix is correct and the identified edge case requires a rare multi-error-recovery sequence within one reply.

No P0 or P1 issues. The single P2 observation concerns a rare compound scenario (error + recovery + additional late final) that isn't addressed by existing tests but doesn't affect the core fix described in the PR.

extensions/feishu/src/reply-dispatcher.ts around the closeStreaming guard on line 383.

This is a comment left during a code review.
Path: extensions/feishu/src/reply-dispatcher.ts
Line: 383-385

Comment:
**`streamingCloseErroredForReply` persists after recovery, blocking re-suppression**

`streamingCloseErroredForReply` is only reset in `onReplyStart`; it is never cleared after a successful recovery card closes within the same reply cycle. Because `closeStreaming` guards `streamingClosedForReply = true` behind `!streamingCloseErroredForReply`, a recovery final that completes successfully via `onIdle` will leave `streamingClosedForReply = false`. Any late finals arriving after the recovery card closes are therefore not suppressed — potentially producing duplicate cards for the recovery content.

Concretely, the sequence: error → `onError` (sets `streamingCloseErroredForReply = true`, clears `streamingClosedForReply`) → recovery final delivered → `onIdle` → `closeStreaming()` *(skips setting `streamingClosedForReply` due to the guard)* → additional late final → delivered unfiltered as a new card.

If recovery within a reply should re-enable suppression, consider clearing `streamingCloseErroredForReply` when `onIdle` determines the streaming session closed successfully (i.e. when `streamText` is non-empty and no error flag was re-set in the interim).

How can I resolve this? If you propose a fix, please make it concise.

_{Reviews (4): Last reviewed commit: "fix(feishu): keep finals after streaming..." | Re-trigger Greptile}

@hxy91819 PTAL

Codex review: keeping this open for maintainer follow-up; there is still a little grit to resolve.

Keep this PR open. Current main still lacks the PR's distinct late-final suppression for Feishu streaming cards, and the latest PR head is the active implementation candidate for the closed linked bug, but the patch still needs maintainer review around lifecycle scoping and content-integrity tradeoffs before merge.

Best possible solution:

Keep this PR open as the active implementation candidate for #71977. Maintainers should review whether card-mode distinct late finals should be dropped, ensure the closed-stream suppression is scoped to the current reply lifecycle rather than a stale async close, and then land with the focused Feishu dispatcher regression tests plus the changed gate if the behavior is accepted.

What I checked:

• current-main-exact-dedupe-only: Current main suppresses only exact duplicate final text with deliveredFinalTexts.has(text); there is no streamingClosedForReply, streamingCloseErroredForReply, or skipTextForClosedStreamingFinal guard in the checked-out dispatcher. (extensions/feishu/src/reply-dispatcher.ts:464, 0015d34fda79)
• current-main-distinct-late-final-can-restart-streaming: After a prior onIdle close clears streaming, a later distinct final in streaming card mode still enters the final path and calls startStreaming() based on info.kind, streamingEnabled, and useCard. (extensions/feishu/src/reply-dispatcher.ts:487, 0015d34fda79)
• current-tests-do-not-cover-pr-case: Current main covers distinct finals before idle and exact duplicate final text after idle, but not the PR's distinct-late-final-after-close case. (extensions/feishu/src/reply-dispatcher.test.ts:334, 0015d34fda79)
• pr-head-implements-new-guard-and-regressions: The latest PR patch adds streamingClosedForReply and streamingCloseErroredForReply, suppresses closed-stream final text while still sending media, and adds regression tests named "skips distinct late final text after streaming card close", "does not suppress a later final after error closeout", and "does not suppress a recovery final after late media failure". (extensions/feishu/src/reply-dispatcher.ts, b96302c35585)
• lifecycle-risk-from-shared-dispatcher: The shared reply dispatcher invokes options.onError and options.onIdle without awaiting their async work, which is relevant because this PR's state is a mutable Feishu dispatcher flag written around async closeStreaming work. (src/auto-reply/reply/reply-dispatcher.ts:268, 0015d34fda79)
• security-review-pass: The PR patch changes only extensions/feishu/src/reply-dispatcher.ts and extensions/feishu/src/reply-dispatcher.test.ts. It does not touch CI workflows, action refs, dependency manifests, lockfiles, install/build/release scripts, package publishing metadata, generated/vendor files, or credential storage. Dummy app_secret values remain test fixtures. (b96302c35585)

Likely related people:

• vincentkoc: Authored recent Feishu streaming-card closeout and final coalescing commits that directly shape the current late-final behavior and tests. (role: recent Feishu streaming maintainer; confidence: high; commits: 84a22a64bef0, 455eba7f94f0, d609f71c9b74; files: extensions/feishu/src/reply-dispatcher.ts, extensions/feishu/src/reply-dispatcher.test.ts, extensions/feishu/src/streaming-card.ts)
• MoerAI: Authored the merged exact duplicate-after-streaming-close fix that this PR extends from exact-text dedupe to distinct late-final suppression. (role: adjacent fix author; confidence: medium; commits: 2aa313cd905c; files: extensions/feishu/src/reply-dispatcher.ts, extensions/feishu/src/reply-dispatcher.test.ts)
• rexl2018: Authored earlier Feishu streaming merge/final dedupe work that introduced the deliveredFinalTexts-style exact dedupe semantics and explicitly preserved distinct final payloads. (role: introduced related final dedupe behavior; confidence: medium; commits: 3bf6ed181e03; files: extensions/feishu/src/reply-dispatcher.ts, extensions/feishu/src/reply-dispatcher.test.ts, extensions/feishu/src/streaming-card.ts)
• steipete: Has recent Feishu dispatcher refactors and maintainer touches in the same reply-dispatcher area, including chunked reply delivery and runtime/secret-resolution refactors. (role: recent maintainer/refactor owner; confidence: medium; commits: 143ae5a5b070, ba95d43e3ce1, b0c55eb65948; files: extensions/feishu/src/reply-dispatcher.ts, extensions/feishu/src/reply-dispatcher.test.ts)

Remaining risk / open question:

• The PR intentionally suppresses distinct late final text while still sending media, which can separate attachments from captions, warnings, or other context unless maintainers accept that card-mode product behavior.
• The proposed closed/error state is still shared mutable dispatcher state rather than a reply token; because shared onError/onIdle hooks are fire-and-forget, slow closeout or back-to-back replies need careful lifecycle review before merge.
• Auto render mode short late finals are explicitly out of scope in the discussion, so duplicate plain messages may still be possible outside forced card mode.
• Aisle's security check had just started on the latest head in the PR discussion; the local security pass found no supply-chain-sensitive file changes, but that pending review should still be allowed to complete.

Codex review notes: model gpt-5.5, reasoning high; reviewed against 0015d34fda79.

Codex review: keeping this open for maintainer follow-up; there is still a little grit to resolve.

Keep this PR open. Current main still only suppresses exact duplicate Feishu final text after a streaming card closes; it does not implement the distinct late-final suppression this PR is trying to add. The PR is focused, but the current discussion surfaces real lifecycle and content-integrity questions that need maintainer review before merge.

Best possible solution:

Keep this PR open as the active implementation candidate for #71977. The best path is maintainer review of the focused Feishu dispatcher/test change, with explicit resolution of the late-text/media content-integrity question and a lifecycle-safe way to scope closed-stream suppression to the current reply before landing.

What I checked:

• current-main-exact-dedupe-only: Current main tracks final text in deliveredFinalTexts and suppresses only when deliveredFinalTexts.has(text) matches exactly; there is no streamingClosedForReply or skipTextForClosedStreamingFinal guard in main. (extensions/feishu/src/reply-dispatcher.ts:464, 7120f5b25487)
• current-main-can-still-start-late-final-card: After onIdle closes and clears the streaming session, a later distinct final in streaming card mode can still call startStreaming() again because the final-card path is keyed only on info.kind, streamingEnabled, and useCard. (extensions/feishu/src/reply-dispatcher.ts:487, 7120f5b25487)
• current-tests-adjacent-not-target: Main covers distinct final payloads before idle and exact duplicate final text after streaming close, but not the PR's distinct-late-final-after-close case. (extensions/feishu/src/reply-dispatcher.test.ts:334, 7120f5b25487)
• pr-head-implements-new-guard-and-regression: The current PR head adds streamingClosedForReply/streamingCloseErroredForReply, suppresses final text when the stream has closed for the reply, and adds a regression test named "skips distinct late final text after streaming card close". (extensions/feishu/src/reply-dispatcher.ts:475, 5e0e1a2df48d)
• pr-live-state-and-scope: Live API data shows the PR is open, unmerged, contributor-authored, based on current main, and changes only extensions/feishu/src/reply-dispatcher.ts plus its colocated test. (5e0e1a2df48d)
• linked-bug-and-pr-purpose-align: The linked bug reports Feishu streaming card mode creating one correct card plus additional distinct final cards after tool-heavy replies; the PR body uses closing syntax for that bug and directly targets that behavior.

Remaining risk / open question:

• The proposed suppression can drop distinct late final text while still delivering media, which can separate attachments from captions, warnings, attribution, or other user-visible context.
• The proposed mutable close-state flags interact with fire-and-forget onError/onIdle callbacks, so slow closeout, error recovery, or back-to-back replies need lifecycle-safe scoping before merge.
• No tests were executed because this was a read-only cleanup review.

Codex review notes: model gpt-5.5, reasoning high; reviewed against 7120f5b25487.

Resolved

Merged via squash.

• Prepared head SHA: d18a9ff4c30f29cd00af394662656d22dca3ba1e
• Merge commit: 8c8f3969856f219318ca30b7a6cecd3e2af7af25

Thanks @MonkeyLeeT!