← Back to PRs

#22433: Slack: fix thread context loss after session reset

by stgarrity open 2026-02-21 05:02 View on GitHub →
When replying to a Slack thread after the session has gone stale (daily reset or idle timeout), the bot loses all thread context — responding with no history or, worse, loading irrelevant old conversation context from a previous session. **Three interrelated issues:** 1. **Thread history never re-fetched.** `readSessionUpdatedAt` returns the old timestamp (the store entry persists after staleness), so the `!threadSessionPreviousTimestamp` guard skips the Slack API fetch entirely. 2. **Fetched history dropped downstream.** `recordInboundSession()` at the end of `prepare.ts` updates `updatedAt` to `Date.now()` *before* `session.ts` runs. Session evaluation then sees a fresh entry → `isNewSession=false` → `get-reply-run.ts` gates thread history on `isNewSession` and drops it. 3. **Old session JSONL loaded.** For threads with existing store entries, the stale `sessionId` is preserved, so `session.ts` loads the old JSONL (e.g. 48 messages from days ago) alongside the freshly-fetched thread history. **Fixes:** - **`prepare.ts`**: Evaluate session freshness before skipping thread history fetch. When stale, re-fetch from Slack API and reset the store entry (`sessionId` → new UUID, `updatedAt` → 0) so `session.ts` creates a fresh session. - **`get-reply-run.ts`**: Use `IsFirstThreadTurn` (already set by prepare) as an additional signal to include thread history, decoupling it from the `isNewSession` timing dependency. Tested on a live instance for a week — old threads now correctly hydrate context on reply. <!-- greptile_comment --> <h3>Greptile Summary</h3> Fixes thread context loss when replying to a Slack thread after a session goes stale (daily reset or idle timeout). Previously, three interrelated issues caused the bot to either respond with no history or load irrelevant old conversation context. - **`prepare.ts`**: Evaluates session freshness before deciding whether to skip thread history re-fetch. When stale, re-fetches from the Slack API and resets the store entry (`sessionId` → new UUID, `updatedAt` → 0) so `session.ts` creates a fresh session instead of resuming the old JSONL. Also extends the `IsFirstThreadTurn` flag to fire on stale sessions. - **`get-reply-run.ts`**: Introduces `includeThreadHistory` which combines `isNewSession` with `IsFirstThreadTurn`, decoupling thread history injection from session evaluation timing. This prevents thread history from being dropped when `recordInboundSession` updates `updatedAt` before `session.ts` runs. - **`prepare.test.ts`**: Adds a dedicated test for the stale session re-fetch scenario, verifying that `IsFirstThreadTurn` is set, thread history is populated, and the Slack API is called the expected number of times. <h3>Confidence Score: 4/5</h3> - This PR is safe to merge — it fixes a real user-facing bug with a well-tested, narrowly scoped change. - The fix correctly addresses three interrelated timing issues in thread session handling. The approach is sound: decoupling thread history injection from isNewSession via IsFirstThreadTurn is the right architectural fix. The fire-and-forget store update introduces a theoretical race, but the IsFirstThreadTurn flag makes the outcome deterministic regardless. Code is well-commented, tested, and validated on a live instance. No critical issues found. - No files require special attention. The changes in `src/slack/monitor/message-handler/prepare.ts` are the most complex but are well-documented with comments explaining the timing considerations. <sub>Last reviewed commit: 8b6d4f8</sub> <!-- greptile_other_comments_section --> <sub>(2/5) Greptile learns from your feedback when you react with thumbs up/down!</sub> <!-- /greptile_comment -->

Most Similar PRs