← Back to PRs

#11194: fix(slack): queue drain drops string thread_ts — replies leak to main channel instead of thread

by Lukavyi open 2026-02-07 14:42 View on GitHub →
stale
#### Summary Queue drain uses `typeof threadId === "number"` which silently drops Slack's string `thread_ts`, causing queued replies to leak to the main channel instead of the thread. lobster-biscuit #### Repro Steps 1. Configure Slack with `replyToMode: all` 2. Send 2+ messages while agent is processing 3. Queued response appears in channel, not thread #### Root Cause `src/auto-reply/reply/queue/drain.ts` uses `typeof threadId === "number"` in 3 places. Telegram uses numeric topic IDs (passes check), Slack uses string `thread_ts` like `"1770474140.187459"` (fails check). The type in `queue/types.ts` was updated to `string | number` when Slack threading was added, but `drain.ts` was not updated. #### Behavior Changes - String thread IDs (Slack `thread_ts`) now preserved during queue drain - Empty string `""` treated as absent (prevents spurious thread grouping if any provider normalizes missing values to `""`) - No change for numeric thread IDs (Telegram) #### Codebase and GitHub Search - Searched: `originatingThreadId`, `typeof.*number.*threadId`, `thread_ts.*queue` - Found 3 prior PRs with same fix but missing empty-string edge case (#4389, #4749, #5245) - Reviewer on #4389 (Greptile) flagged the empty-string gap — this PR addresses it #### Tests 3 new cases in `queue.collect-routing.test.ts`: - ✅ Collects messages with matching string `thread_ts` (same Slack thread) - ✅ Separates messages with different string `thread_ts` (different threads) - ✅ Treats empty string threadId same as absent ``` pnpm test src/auto-reply/reply/queue.collect-routing.test.ts ✓ 9 tests passed ``` **Sign-Off** - Models used: Claude Opus 4 (via OpenClaw) - Submitter effort: AI-assisted, fully tested, code reviewed - Agent notes: Fix addresses empty-string edge case missed by #4389, #4749, #5245. Verified no other `typeof === "number"` checks on threadId exist in codebase. Fixes #11195 Related PRs (same fix, but without empty-string edge case protection): - #4389 — uses `threadId == null` without `!== ""` check. Greptile flagged this gap at Confidence 4\/5. - #4749 — identical diff to #4389, same gap. - #5245 — adds comments but same `!= null` without empty-string guard. This PR adds `\&\& threadId !== ""` to all 3 locations, addressing the edge case Greptile identified. Also includes 3 new tests (string thread_ts collection, different threads separation, empty-string handling) which the other PRs lack. Related issues: #4380, #4424, #5615 --- <!-- greptile_comment --> <h2>Greptile Overview</h2> <h3>Greptile Summary</h3> This PR fixes follow-up queue drain routing so Slack thread replies don't lose their `thread_ts` context during "collect" drains. The drain logic previously treated `originatingThreadId` as present only when it was a `number`, which works for Telegram topic IDs but drops Slack's string `thread_ts` values. The change updates the routing key computation and the collected-batch `originatingThreadId` selection to treat any non-empty `string | number` as a valid thread id, while treating `""` as absent to avoid creating spurious thread groups. New tests in `queue.collect-routing.test.ts` cover same-thread collection, different-thread separation, and empty-string behavior. <h3>Confidence Score: 5/5</h3> - This PR is safe to merge with minimal risk. - Changes are narrowly scoped to queue drain routing, align with existing `FollowupRun.originatingThreadId: string | number` typing, and are covered by targeted unit tests for Slack thread_ts string handling and empty-string edge cases. - No files require special attention <sub>Greptile review restored manually after accidental body overwrite.</sub> <!-- /greptile_comment -->

Most Similar PRs