fix: handle string thread_ts from Slack properly#4389
fix: handle string thread_ts from Slack properly#4389Chloe-VP wants to merge 1 commit intoopenclaw:mainfrom
Conversation
Issue #4380: The queue drain logic was using typeof === 'number' to check for thread IDs, which fails for Slack where thread_ts is a string like '1737766124.329349'. Changed three occurrences: - Line 43: typeof threadId !== 'number' → threadId == null - Line 49: typeof threadId === 'number' → threadId != null - Line 75: typeof originatingThreadId === 'number' → originatingThreadId != null This allows Slack string thread IDs to be properly recognized while still filtering out undefined/null values.
| const originatingThreadId = items.find( | ||
| (i) => typeof i.originatingThreadId === "number", | ||
| (i) => i.originatingThreadId != null, | ||
| )?.originatingThreadId; |
There was a problem hiding this comment.
[P1] originatingThreadId selection now picks the first non-null value, which includes "" (empty string). That can change routing/collect behavior by treating empty thread IDs as a real thread (vs previously only numeric IDs were considered). If empty-string thread IDs are possible from any provider normalization, this could cause messages to be grouped under a distinct thread key or passed through as originatingThreadId: "".
Consider mirroring the normalization behavior used elsewhere (e.g., treat "" as absent) by checking i.originatingThreadId != null && i.originatingThreadId !== "".
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/auto-reply/reply/queue/drain.ts
Line: 74:76
Comment:
[P1] `originatingThreadId` selection now picks the first *non-null* value, which includes `""` (empty string). That can change routing/collect behavior by treating empty thread IDs as a real thread (vs previously only numeric IDs were considered). If empty-string thread IDs are possible from any provider normalization, this could cause messages to be grouped under a distinct thread key or passed through as `originatingThreadId: ""`.
Consider mirroring the normalization behavior used elsewhere (e.g., treat `""` as absent) by checking `i.originatingThreadId != null && i.originatingThreadId !== ""`.
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 ( |
|
Closing as duplicate of #4749. If this is incorrect, comment and we can reopen. |
Summary
Fixes #4380 - Slack thread_ts messages were being dropped because the queue drain logic used
typeof === 'number'to check for thread IDs.Problem
Slack thread_ts values are strings (e.g.,
'1737766124.329349'), not numbers. The original code:This caused valid Slack thread messages to be filtered out incorrectly.
Solution
Changed three occurrences in
src/auto-reply/reply/queue/drain.ts:typeof threadId !== 'number'→threadId == nulltypeof threadId === 'number'→threadId != nulltypeof originatingThreadId === 'number'→originatingThreadId != nullTesting
Notes
This is a straightforward type check fix - no behavioral changes for existing numeric thread IDs.
Greptile Overview
Greptile Summary
This PR updates follow-up queue drain routing so Slack
thread_tsvalues (strings like"1737766124.329349") aren’t dropped bytypeof === "number"checks. Insrc/auto-reply/reply/queue/drain.ts, the cross-channel key derivation and collected-runoriginatingThreadIdselection now treat any non-null thread ID (string or number) as present and stringify it into the routing key, aligning with theoriginatingThreadId?: string | numbertype used across the queue pipeline.Confidence Score: 4/5
thread_tsand the existingstring | numbertype. Main risk is minor behavior change if empty-string thread IDs can occur, since the new null-check treats""as present in one selection path.(3/5) Reply to the agent's comments like "Can you suggest a fix for this @greptileai?" or ask follow-up questions!
Context used:
dashboard- CLAUDE.md (source)dashboard- AGENTS.md (source)