← Back to PRs

#22440: fix(slack): prevent duplicate final replies from draft previews

by Solvely-Colin open 2026-02-21 05:25 View on GitHub →
channel: slack size: M
## Summary - Problem: Slack replies could show duplicate/stale first output when a pending draft preview flush happened after final delivery. - Why it matters: users see confusing double responses and noisy system events while interacting in Slack. - What changed: final delivery now flushes pending draft state first, clears stale preview before fallback send when preview-edit is not possible, and filters bot-authored `message_changed` events. - What did NOT change (scope boundary): no Block Kit feature expansion, no routing contract changes, no non-Slack channel behavior changes. ## Change Type (select all) - [x] Bug fix - [ ] Feature - [ ] Refactor - [ ] Docs - [ ] Security hardening - [ ] Chore/infra ## Scope (select all touched areas) - [ ] Gateway / orchestration - [ ] Skills / tool execution - [ ] Auth / tokens - [ ] Memory / storage - [x] Integrations - [ ] API / contracts - [ ] UI / DX - [ ] CI/CD / infra ## Linked Issue/PR - Closes #22254 - Related #18555 ## User-visible / Behavior Changes - Slack: prevents draft preview + final reply duplication in the final send path. - Slack: avoids stale draft preview text persisting when final payload cannot use preview-edit finalization. - Slack: suppresses bot self-edit `message_changed` notifications that were surfacing as noisy system events. ## Security Impact (required) - New permissions/capabilities? (`No`) - Secrets/tokens handling changed? (`No`) - New/changed network calls? (`No`) - Command/tool execution surface changed? (`No`) - Data access scope changed? (`No`) - If any `Yes`, explain risk + mitigation: ## Repro + Verification ### Environment - OS: macOS - Runtime/container: Node + pnpm workspace - Model/provider: N/A (channel pipeline behavior) - Integration/channel (if any): Slack monitor + reply dispatch - Relevant config (redacted): Slack account with streaming preview path enabled ### Steps 1. Trigger a Slack request that emits partial draft text before final output. 2. Let the final payload resolve through `dispatchPreparedSlackMessage`. 3. Observe message timeline and system events. ### Expected - One coherent final user-facing response. - No stale draft preview left behind on fallback paths. - No bot self-edit message_changed system-event noise. ### Actual - Implemented behavior now matches expected. ## Evidence - [x] Failing test/log before + passing after - [x] Trace/log snippets - [ ] Screenshot/recording - [ ] Perf numbers (if relevant) ## Human Verification (required) - Verified scenarios: - `pnpm build` passes. - `pnpm check` passes. - `pnpm vitest run src/slack/monitor/events/messages.test.ts src/slack/monitor/message-handler/dispatch.test.ts` passes (4 tests). - Edge cases checked: - Final preview edit path after pending draft flush. - Fallback send path when final payload cannot use preview edit. - Bot-authored edit event suppression via `message.user`, `previous_message.user`, and `message.edited.user`. - What you did **not** verify: - End-to-end Slack socket/manual click-through in this run. ## Compatibility / Migration - Backward compatible? (`Yes`) - Config/env changes? (`No`) - Migration needed? (`No`) - If yes, exact upgrade steps: ## Failure Recovery (if this breaks) - How to disable/revert this change quickly: - Revert commit `7c93bf12a`. - Files/config to restore: - `src/slack/monitor/message-handler/dispatch.ts` - `src/slack/monitor/events/messages.ts` - `src/slack/monitor/types.ts` - Known bad symptoms reviewers should watch for: - Reappearance of duplicate first responses or bot self-edit notification spam. ## Risks and Mitigations - Risk: filtering `message_changed` could suppress legitimate bot-originated edits a maintainer wants surfaced. - Mitigation: filtering is limited to self-authored bot edits and still allows non-bot edits through. - Risk: clearing preview on fallback could remove useful interim text in rare error flows. - Mitigation: clear is only applied when preview-edit finalization is unavailable, preventing stale + duplicate output. ## AI Assistance - AI-assisted: yes (Codex). - Testing level: lightly tested locally (`build`, `check`, focused Slack regression tests) with known unrelated baseline failures in full `pnpm test`. <!-- greptile_comment --> <h3>Greptile Summary</h3> This PR fixes a race condition in the Slack message dispatch pipeline where draft preview messages could appear alongside or after final responses, causing duplicate content and confusing bot self-edit notifications. **Key changes:** - Added `await draftStream.flush()` before final delivery strategy decision in `dispatch.ts:233` to drain pending preview updates and prevent race conditions - Implemented `isSelfAuthoredSlackMessageChange()` filter in `messages.ts:14-32` to suppress bot-authored `message_changed` system events by checking `message.user`, `previous_message.user`, and `message.edited.user` - Added fallback clear logic in `dispatch.ts:280-288` to remove stale draft previews when final payload cannot use preview-edit finalization (e.g., error payloads) - Enhanced `SlackMessageChangedEvent` type definition to include user fields needed for self-authorship detection **Implementation approach:** The fix addresses the duplication at two points in the pipeline: (1) preventing the draft flush from racing with final delivery by explicitly draining it first, and (2) clearing stale preview text before fallback sends when the final payload characteristics (error flag, media presence) prevent editing the preview message in place. <h3>Confidence Score: 4/5</h3> - This PR is safe to merge with minimal risk - it fixes a clear race condition with well-tested changes - The implementation correctly addresses the stated problem through explicit synchronization (`flush()` before decision points) and proper cleanup (`clear()` for stale drafts). The logic is straightforward with defensive checks. Tests cover both the happy path (preview-edit finalization) and error path (fallback with clear). Minor deduction because end-to-end manual Slack verification was not performed, and the self-authorship check relies on optional fields that could theoretically all be missing in edge cases. - No files require special attention - the changes are focused and well-tested <sub>Last reviewed commit: 7c93bf1</sub> <!-- greptile_other_comments_section --> <!-- /greptile_comment -->

Most Similar PRs