fix: handle string thread IDs in queue drain for Slack#4749
fix: handle string thread IDs in queue drain for Slack#4749nvonpentz wants to merge 1 commit intoopenclaw:mainfrom
Conversation
Slack thread timestamps are strings (e.g., '1769015989.090599'), not numbers. The type check `typeof threadId === 'number'` caused thread IDs to be dropped when collecting queued messages, resulting in replies going to the main channel instead of the thread. Changed to use null checks (`!= null`) which correctly handles both string and number thread IDs across different providers.
| (i) => i.originatingAccountId, | ||
| )?.originatingAccountId; | ||
| const originatingThreadId = items.find( | ||
| (i) => typeof i.originatingThreadId === "number", | ||
| (i) => i.originatingThreadId != null, | ||
| )?.originatingThreadId; |
There was a problem hiding this comment.
[P2] Collect-routing test type mismatch
FollowupRun.originatingThreadId is string | number (src/auto-reply/reply/queue/types.ts:40-41), and this PR explicitly targets Slack’s string thread timestamps, but createRun in src/auto-reply/reply/queue.collect-routing.test.ts currently types originatingThreadId?: number (line 13). This makes it easy to miss regressions involving string thread IDs in collect routing/drain behavior.
Consider widening the test helper type and adding at least one case that sets a string thread id.
Also appears in: src/auto-reply/reply/queue.collect-routing.test.ts:7-14
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/auto-reply/reply/queue/drain.ts
Line: 72:76
Comment:
[P2] Collect-routing test type mismatch
`FollowupRun.originatingThreadId` is `string | number` (`src/auto-reply/reply/queue/types.ts:40-41`), and this PR explicitly targets Slack’s string thread timestamps, but `createRun` in `src/auto-reply/reply/queue.collect-routing.test.ts` currently types `originatingThreadId?: number` (line 13). This makes it easy to miss regressions involving string thread IDs in collect routing/drain behavior.
Consider widening the test helper type and adding at least one case that sets a string thread id.
Also appears in: `src/auto-reply/reply/queue.collect-routing.test.ts:7-14`
How can I resolve this? If you propose a fix, please make it concise.|
Hey! I ran into the same issue and submitted #11194 which builds on this approach but adds empty-string edge case protection ( |
bfc1ccb to
f92900f
Compare
|
AI-assisted stale triage closure (2026-02-24). Closing this PR because the fix is already in Why:
This is AI-closed housekeeping, not a rejection of the report. If you still see Slack thread-id type issues, open a new focused PR from latest |
Problem
When multiple messages arrive while the agent is busy (queued messages), replies to Slack threads were incorrectly being sent to the main channel instead of the thread.
Root Cause
The code in
src/auto-reply/reply/queue/drain.tschecks for thread IDs using:However, Slack thread timestamps are strings (e.g.,
"1769015989.090599"), not numbers. This caused:originatingThreadIdto beundefinedwhen collecting queued messagesFix
Changed type checks from
typeof threadId === "number"tothreadId != nullwhich correctly handles both string (Slack) and number thread IDs across different providers.Testing
Tested locally by:
Greptile Overview
Greptile Summary
This PR fixes followup queue drain routing when draining queued messages for Slack threads. The prior logic treated
originatingThreadIdas numeric-only (typeof ... === "number"), but Slack thread timestamps are strings, so the thread component was dropped during cross-channel key generation and while selecting anoriginatingThreadIdfor collect mode.The change switches thread detection to null checks (
threadId != null) so both string and number thread IDs are preserved in the routing key and in the collected followup run, keeping replies routed to the original thread instead of the main channel.Confidence Score: 4/5
numberand doesn’t exercise the Slack case.(2/5) Greptile learns from your feedback when you react with thumbs up/down!