fix(slack): per-thread session keys to prevent context mixing#17908
Closed
tonydehnke wants to merge 5 commits intoopenclaw:mainfrom
Closed
fix(slack): per-thread session keys to prevent context mixing#17908tonydehnke wants to merge 5 commits intoopenclaw:mainfrom
tonydehnke wants to merge 5 commits intoopenclaw:mainfrom
Conversation
… mixing
## Problem
All messages in a Slack channel share a single session, causing context from
different threads to mix together. When users have multiple conversations in
different threads of the same channel, the agent sees combined context from
all threads, leading to confused responses.
Session key was: `slack:channel:${channelId}` (no thread identifier)
## Solution
1. **Thread-level session keys**: Each message in channels/groups now gets
its own session based on thread_ts:
- Thread replies: use the parent thread's ts
- New messages: use the message's own ts (becomes thread root)
- DMs: unchanged (no thread-level sessions needed)
New session key format: `slack:channel:${channelId}:thread:${threadTs}`
2. **Increased thread cache TTL**: Changed from 60 seconds to 6 hours.
Users often pause conversations, and the short TTL caused unnecessary
API calls and thread resolution failures.
3. **Increased cache size**: Changed from 500 to 10,000 entries to support
busy workspaces with many active threads.
## Testing
1. Create two threads in the same Slack channel
2. In Thread A: tell the bot your name is "Alice" and ask about "billing"
3. In Thread B: tell the bot your name is "Bob" and ask about "API"
4. Reply in Thread A and ask "what's my name?" - should say "Alice"
5. Check sessions: each thread should have a unique session key with :thread: suffix
Fixes context bleed issues related to openclaw#758
…level sessions The context.ts file has a separate function for resolving session keys for system events (reactions, file uploads, etc.). This also needs to support thread-level sessions to ensure all Slack events route to the correct thread-specific session. Added threadTs and messageTs parameters to resolveSlackSystemEventSessionKey and updated the implementation to use thread-level keys for channels/groups.
The previous change broke thread-level sessions for DMs that have threads. DMs with parent_user_id should still get thread-level sessions. - For channels/groups: always use thread-level sessions - For DMs: use thread-level sessions only when isThreadReply is true
Contributor
Additional Comments (1)
Prompt To Fix With AIThis is a comment left during a code review.
Path: src/slack/monitor/context.ts
Line: 177:209
Comment:
added `threadTs`/`messageTs` for forward-compatible thread isolation - current callers in `events/` (reactions, edits, pins, members) don't pass thread info yet, so system events still use base channel sessions
How can I resolve this? If you propose a fix, please make it concise. |
Member
|
Closing as duplicate of #10686. If this is incorrect, please contact us. |
Contributor
|
I have added all the changes and fixes @tonydehnke requested in PR #10686. Also, I added @tonydehnke as a co-author. Thank you for reviewing and improving this @tonydehnke. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
5 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Rebased and extended version of #10686 by @pablohrcarvalho, updated to current
mainwith thepreviousTimestampfix from Greptile's review.resolveSlackSystemEventSessionKeynow acceptsthreadTs/messageTsfor forward-compatible thread isolation of system eventspreviousTimestampfix (Greptile review) — reads from the resolved thread-levelsessionKeyinstead ofroute.sessionKey, so the elapsed-time envelope header uses the correct per-thread timestampChanges
src/slack/monitor/message-handler/prepare.tscanonicalThreadIdfor thread-level session routing +previousTimestampfixsrc/slack/monitor/thread-resolution.tssrc/slack/monitor/context.tsresolveSlackSystemEventSessionKeyacceptsthreadTs/messageTssrc/slack/monitor/message-handler/prepare.thread-session-key.test.tspreviousTimestamp(3 cases: channel, thread reply, DM)Production evidence
We've been running these fixes as local bundle patches since v2026.2.9 (~2 weeks). Before the fix, ~10% of channel threads experienced session splitting or cross-contamination. After the fix: zero incidents.
Test plan
pnpm buildcleanpnpm check(oxlint + oxfmt) cleanreadSessionUpdatedAtuses thread-level key (fails without fix, passes with)Co-authored-by: pablohrcarvalho pablohrcarvalho@users.noreply.github.com
Supersedes #10686
Closes #758
Related: #12389, #4927, #7610
Greptile Summary
This PR implements per-thread session isolation for Slack to prevent context mixing between threads. The implementation correctly distinguishes between channels/groups (where every message gets its own thread-level session) and DMs (where only actual thread replies get thread-level sessions).
Key changes:
prepare.ts:207-211introducescanonicalThreadIdlogic that usesmessage.tsas the thread ID for top-level channel messages, ensuring each channel message starts its own session contextprepare.ts:429fixes thepreviousTimestampbug by reading from the resolved thread-levelsessionKeyinstead ofroute.sessionKey, ensuring elapsed-time envelope headers reflect per-thread timestampsthread-resolution.ts:13-14increases cache TTL from 60s to 6 hours and max entries from 500 to 10k to better support real-world usage patternscontext.ts:177-209adds optionalthreadTs/messageTsparameters toresolveSlackSystemEventSessionKeyfor forward-compatible thread isolation of system events (currently not used by existing callers)Test coverage:
The new test file validates the core fix with three scenarios: channel messages, thread replies, and DMs. The tests spy on
readSessionUpdatedAtcalls to verify the correct session key is used for readingpreviousTimestamp.Production validation:
Per the PR description, these fixes have been running in production as bundle patches for ~2 weeks with zero incidents after previously experiencing ~10% thread contamination.
Confidence Score: 5/5
Last reviewed commit: d9bc772