← Back to PRs

#22982: fix: prevent stale threadId from routing subagent announces to wrong Slack threads

by unboxed-ai open 2026-02-21 22:00 View on GitHub →
docs channel: slack gateway commands agents size: S
## Summary Fixes subagent announce replies landing in stale Slack threads instead of DMs/channels. Companion to #22944. ### Problem Non-thread sessions (DMs, channels) accumulated stale `threadId` values in their `deliveryContext` from previous thread interactions. When a subagent announced back to the requester session, this stale `threadId` flowed through the delivery chain to Slack's `thread_ts` parameter, causing the reply to land in whatever the most recent thread was. ### Root Cause Three independent code paths leaked `threadId` into announce delivery: 1. **`subagent-announce.ts`**: `resolveAnnounceOrigin` merged the session entry's `deliveryContext.threadId` into the announce origin 2. **`server-methods/agent.ts`**: `resolveAgentDeliveryPlan` read the session entry's threadId into `deliveryPlan.resolvedThreadId` 3. **`commands/agent/delivery.ts`**: A second call to `resolveAgentDeliveryPlan` read the same stale threadId for Slack `replyToId` Additionally, the agent method's `nextEntry` was dropping `sessionFile`, `chatType`, `displayName`, and `origin` fields, causing subagent announces to write transcripts to the wrong file. ### Fix - **Gate threadId in `subagent-announce.ts`** on `isThreadSessionKey()` — only thread/topic sessions pass threadId to the agent method - **Gate `resolvedThreadId` in `agent.ts`** — block stale session-derived threadId for non-thread sessions, but allow explicit caller-provided threadId through - **Gate `resolvedThreadId` in `delivery.ts`** — same pattern: block stale, allow explicit - **Prevent non-thread sessions from storing threadId** in `session.ts` — both `lastThreadId` and `deliveryContext.threadId` are forced to `undefined` for non-thread sessions, breaking the circular pollution loop - **Preserve session fields** in the agent method's `nextEntry` — `sessionFile`, `chatType`, `displayName`, `origin` ### Testing - All 966 test files pass (57 directly affected tests pass) - Verified on production: - DM-spawned subagent announces land in DM ✅ - Channel-spawned subagent announces land in channel ✅ - Thread-spawned subagent announces land in correct thread ✅ ### Code Review Discussion **Finding 1 (Medium): Explicit `threadId` overrides initially suppressed.** The first implementation blocked ALL threadId for non-thread sessions, including intentionally caller-provided ones. This conflicted with the override contract in `AgentCommandOpts.threadId`. *Resolution:* Changed gating to only block `deliveryPlan.resolvedThreadId` (stale, session-derived) while allowing `explicitThreadId` (caller-provided) through. The announce flow already gates threadId at the source (`subagent-announce.ts`), so the announce bug stays fixed while preserving the explicit override path. **Finding 2 (Medium): `isThreadSessionKey` is substring-based.** The function checks for `:thread:` or `:topic:` anywhere in the session key. A key with those substrings in a user/peer ID could be misclassified. *Decision: Accepted as low-risk.* Session keys are constructed by OpenClaw from known patterns. User IDs don't contain `:thread:` in practice. This is pre-existing logic reused from `config/sessions/reset.ts`, not something introduced by this PR. ### Related Issues - Fixes openclaw/openclaw#17731 - Fixes openclaw/openclaw#22273 - Depends on #22944 <!-- greptile_comment --> <h3>Greptile Summary</h3> Fixes subagent announce replies landing in stale Slack threads by gating `threadId` propagation for non-thread sessions (DMs, channels). The fix prevents `deliveryContext.threadId` from leaking across interactions by blocking it at three key points: announce origin resolution (`subagent-announce.ts:156-158`), agent method delivery plan (`agent.ts:533-535`), and delivery execution (`delivery.ts:114-116`). Session state now forces `threadId` to `undefined` for non-thread sessions (`session.ts:286`, `session.ts:313-317`), breaking the circular pollution loop. **Key Changes:** - Gate `threadId` in subagent announce flow using `isThreadSessionKey()` check - Block stale session-derived `resolvedThreadId` while preserving explicit caller-provided `threadId` - Preserve `sessionFile`, `chatType`, `displayName`, `origin` in agent method's `nextEntry` to fix transcript routing - Change Slack default `replyToMode` from `"all"` to `"off"` and disable `allowExplicitReplyTagsWhenOff` - Update `statusThreadTs` logic to match `replyThreadTs` (both undefined when threading is off) <h3>Confidence Score: 4/5</h3> - Safe to merge with minor risk from Slack default behavior change - The core threadId gating logic is sound and consistently applied across all three delivery paths. Tests validate the fix and all 966 test files pass. The consistent pattern of `isThreadSessionKey()` checks prevents stale threadId leakage while preserving explicit overrides. Score reflects the Slack default change from `replyToMode: "all"` to `"off"` which alters user-facing behavior for existing installations without explicit config. - Pay close attention to `src/slack/monitor/provider.ts:120` - the default replyToMode change may affect existing Slack users who haven't explicitly configured this setting <sub>Last reviewed commit: 2273b5f</sub> <!-- greptile_other_comments_section --> <sub>(3/5) Reply to the agent's comments like "Can you suggest a fix for this @greptileai?" or ask follow-up questions!</sub> <!-- /greptile_comment -->

Most Similar PRs