#12414: fix(slack): do not cache API failures in thread_ts resolver
channel: slack
stale
Cluster:
Slack Thread Management Improvements
## Summary
- When `resolveThreadTsFromHistory()` fails (rate-limited, network error, etc.), the error was silently cached as `null` ("confirmed no thread_ts") for the full 60-second TTL
- Every subsequent resolve() for that message during the TTL window returns the wrong answer, causing thread replies to lose their thread context and land in the main channel
- Users reported ~10% of thread replies silently losing context (#12389)
## Root cause
`resolveThreadTsFromHistory()` returns `undefined` on API failure (line 40). The caller converts it via `resolved ?? null` and caches the `null` value. `getCached` returns `null`, and `cached !== undefined` is true, so the cached `null` is treated as a confirmed "no thread_ts" result.
## Fix
Return a `{ ok, threadTs }` result object from `resolveThreadTsFromHistory()` so callers can distinguish "API succeeded, no thread_ts found" (cache it) from "API call failed" (skip cache, retry next time).
## Test plan
- [x] Added regression test: API failure on first call does not pollute cache; second call retries and succeeds
- [x] Existing cache test still passes
- [x] All existing tests pass
Closes #12389
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
<!-- greptile_comment -->
<h2>Greptile Overview</h2>
<h3>Greptile Summary</h3>
This PR changes Slack thread timestamp resolution to avoid caching negative results when the Slack `conversations.history` API call fails (rate limits/network errors). `resolveThreadTsFromHistory` now returns an `{ ok, threadTs }` result so callers can distinguish a successful lookup with no `thread_ts` (cache `null`) from a failed API call (skip cache), preventing a 60s cache pollution window that can cause thread replies to post to the main channel.
A regression test was added to verify that a rejected API call does not populate the cache and that a subsequent call retries and succeeds, while the existing cache-hit behavior remains covered.
<h3>Confidence Score: 5/5</h3>
- This PR is safe to merge with minimal risk.
- The change is narrowly scoped to Slack thread_ts resolution and fixes a concrete cache-pollution bug by distinguishing API failure from a legitimate “no thread” result. Types and control flow remain straightforward, and the added regression test directly covers the failure-then-retry scenario that motivated the fix.
- No files require special attention
<!-- greptile_other_comments_section -->
<!-- /greptile_comment -->
Most Similar PRs
#22433: Slack: fix thread context loss after session reset
by stgarrity · 2026-02-21
81.5%
#20406: fix(slack): respect replyToMode when computing statusThreadTs in DMs
by QuinnYates · 2026-02-18
80.8%
#20389: fix(slack): inject thread history on first thread turn, not only on...
by lafawnduh1966 · 2026-02-18
80.5%
#19083: Slack: preserve per-thread context and consistent thread replies
by jkimbo · 2026-02-17
79.6%
#4749: fix: handle string thread IDs in queue drain for Slack
by nvonpentz · 2026-01-30
79.5%
#12954: feat(slack): Add channel name resolution with TTL cache
by trevorgordon981 · 2026-02-10
79.1%
#10686: fix(slack): use thread-level sessions for channels to prevent conte...
by pablohrcarvalho · 2026-02-06
78.3%
#22101: fix(slack): dedupe mentions by ts fallback for app_mention
by AIflow-Labs · 2026-02-20
78.2%
#14720: fix(slack): pass threadId in plugin read action (#14706)
by lailoo · 2026-02-12
77.9%
#7592: fix(slack): prevent unbounded memory growth in channel type cache
by namratabhaumik · 2026-02-03
77.9%