← Back to PRs

#20248: fix(slack): flush draft stream before final reply to preserve messag ordering

by Utkarshbhimte open 2026-02-18 18:01 View on GitHub →
channel: slack size: XS
### Summary • Problem: When streaming is enabled, the draft stream loop runs independently of the dispatcher delivery chain. On final reply, throttled partial-reply text can flush after the final message, causing messages to appear out of order. • Why it matters: Users see the final answer first, then get a dump of intermediate "thinking" text after — confusing and unprofessional. • What changed: Added a draftStream.flush() + stop() call before delivering the final reply payload, ensuring all pending streamed text is sent before the final message. • What did NOT change: Non-streaming delivery path unchanged. No changes to block reply behavior or partial reply throttling logic. ### Change Type • [x] Bug fix ### Scope • Slack monitor (dispatch.ts) <!-- greptile_comment --> <h3>Greptile Summary</h3> This PR fixes a message ordering bug in Slack's non-streaming draft stream delivery path. When the final reply is dispatched, throttled partial-reply text from the independent draft stream loop could flush *after* the final message, causing confusing out-of-order delivery. The fix adds `draftStream.flush()` + `stop()` before delivering the final reply payload when `kind === "final"`. - The core fix is correct: flushing the draft stream before the final `deliverReplies` call ensures all pending partial text is sent first. - The `{ kind }` destructuring with a default value is safe but unnecessary — `createReplyDispatcherWithTyping` always passes `{ kind }` to the deliver callback. - There's an edge case in the `canFinalizeViaPreviewEdit` fallback path where `stop()` is called (line 243) before `flush()` would run, making the new flush block a no-op in that scenario. This is pre-existing behavior but worth noting. - No test coverage was added for the new flush-before-final behavior. <h3>Confidence Score: 4/5</h3> - This PR is safe to merge — the fix is targeted, the flush/stop pattern is well-established in the codebase, and the change only affects the non-streaming draft stream delivery path. - The fix correctly addresses the race condition described. The flush() and stop() calls are safe (idempotent, no-op when no pending data). The only concern is a minor edge case in the preview-edit fallback path where the flush becomes a no-op, but that's a pre-existing issue not introduced by this PR. No tests were added, but the change is minimal and low-risk. - No files require special attention beyond the single changed file `src/slack/monitor/message-handler/dispatch.ts`. <sub>Last reviewed commit: eb8cdc9</sub> <!-- greptile_other_comments_section --> <!-- /greptile_comment -->

Most Similar PRs