← Back to PRs

#19083: Slack: preserve per-thread context and consistent thread replies

by jkimbo open 2026-02-17 10:36 View on GitHub →
channel: slack channel: telegram size: S
## Summary This PR fixes two Slack threading/session regressions in the monitor path: 1. New Slack thread sessions were created, but the first thread turn could lose the original thread-starter context. 2. Some replies that should be threaded were sent to the main channel when chat-type reply-mode overrides were configured. It also includes a small Telegram test stabilization change so full CI/test runs remain deterministic. Note: this is my first PR on openclaw so I might be missing lots of context here. ## Problem We Were Seeing When Slack is configured to always reply in threads, the expected behavior is: - each Slack thread gets its own session (to keep the main channel uncluttered), and - the new thread session still includes the parent/original message context. In practice we observed: - first messages in new threads sometimes started without the thread starter/history context, and - some messages that should have stayed in-thread were posted to channel root. ## Root Cause ### 1) Premature thread session creation suppressed first-turn thread context `prepareSlackMessage(...)` recorded inbound session metadata before reply generation, including thread session entries. `get-reply-run` only adds thread-starter/history context when it considers the turn a **new session**. Because the thread session entry already existed by then, that first turn was treated as non-new and context injection was skipped. Result: thread session existed, but starter/history context could be missing on first thread turn. ### 2) Reply mode source mismatch between inbound and outbound flow Inbound monitor processing effectively used a monitor-level reply mode while downstream delivery/tool-result logic resolved threading behavior in a chat-type-aware way. When `replyToModeByChatType` overrides were present, this mismatch could cause decisions to diverge, and replies that should have been threaded could go to main channel. ## What Changed ### Slack behavior fixes - Resolve Slack `replyToMode` per message/chat type in prepare flow. - Carry resolved `replyToMode` through prepared payload and use it in dispatch for both: - thread target resolution, - reply delivery planning. - Do **not** pre-create thread session entries during prepare for thread turns (`createIfMissing: !isThreadReply`), so first reply generation can still detect a new thread session and include starter/history context. - Align provider default mode resolution with `resolveSlackReplyToMode(...)` for consistency. ### Test coverage Added targeted regressions for: - no pre-creation of new thread session entry before reply generation, - direct-message chat-type threading override behavior, - channel chat-type threading override behavior. ### Test stability Updated two timing-sensitive Telegram tests to wait on reply-call conditions (`vi.waitFor`) instead of fixed sleeps to avoid intermittent race failures in full-suite runs. ## Why This Fix Should Address the Issues - By deferring thread-session creation until reply-generation path for first thread turns, we preserve the “new session” signal needed to include thread starter/history context. - By resolving and carrying one authoritative chat-type-aware `replyToMode` through prepare -> dispatch, we remove inconsistent threading decisions and keep replies in the intended thread. Together these changes preserve the desired UX: - one session per Slack thread, - first thread message includes original context, - replies consistently stay in thread when configured to do so. ## Validation Ran locally: - `pnpm check` - `pnpm build` - `pnpm test` All passed. <!-- greptile_comment --> <h3>Greptile Summary</h3> This PR fixes two Slack threading regressions: (1) thread session entries were pre-created during `prepareSlackMessage`, causing the first thread turn to lose starter/history context because the reply generation path treated it as a non-new session; (2) `replyToMode` was resolved at the provider level (without chat-type awareness) but dispatch used a different resolution path, causing mismatched threading decisions when `replyToModeByChatType` overrides were configured. - `prepare.ts`: Resolves `replyToMode` per message using `resolveSlackReplyToMode(account, chatType)` and carries it through the `PreparedSlackMessage`. Defers thread session creation via `createIfMissing: !isThreadReply`. - `dispatch.ts`: Switches from `ctx.replyToMode` to `prepared.replyToMode` for both thread target resolution and reply delivery planning, ensuring consistent threading decisions. - `provider.ts`: Aligns context-level default to use `resolveSlackReplyToMode(account, undefined)` instead of raw config fallback. - `types.ts`: Adds `replyToMode` field to `PreparedSlackMessage` type. - Tests: Adds regression tests for chat-type threading overrides and session non-pre-creation. Stabilizes two Telegram tests by replacing `sleep` with `vi.waitFor`. <h3>Confidence Score: 4/5</h3> - This PR is safe to merge with low risk — changes are well-scoped, logically sound, and backed by targeted regression tests. - Score of 4 reflects well-structured fixes to two clear regressions, with good test coverage for the new behavior. The core logic changes (per-message replyToMode resolution, deferred thread session creation) are correct and consistent with the existing architecture. One minor observation: the createThreadAccount test helper doesn't propagate replyToMode to the top-level account fields (unlike the updated createSlackAccount helper), but this is a pre-existing inconsistency that doesn't affect test correctness. No security concerns identified. - No files require special attention — all changes are consistent and well-tested. <sub>Last reviewed commit: 5299d8c</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