Skip to content

fix: restore Slack thread_ts when agent drops it from channel reply#44

Merged
yourbuddyconner merged 4 commits intomainfrom
fix/slack-thread-routing-fallback
Apr 30, 2026
Merged

fix: restore Slack thread_ts when agent drops it from channel reply#44
yourbuddyconner merged 4 commits intomainfrom
fix/slack-thread-routing-fallback

Conversation

@valet-valet-turnkey-dev
Copy link
Copy Markdown

@valet-valet-turnkey-dev valet-valet-turnkey-dev Bot commented Apr 30, 2026

Summary

Fixes a bug where Slack replies sometimes start a new thread instead of replying in the same thread the user messaged from.

Bug

When a user messages valet in a Slack thread, the system encodes the thread context as a composite channelId like D123:1234567890.123456 (channelId:thread_ts). This is presented to the agent in the [via slack | chatId: D123:1234567890.123456] prefix. The agent is expected to echo this composite ID back when calling channel_reply. SlackTransport.parseTarget() then splits it into { channelId, threadId } and sets thread_ts in the chat.postMessage call.

If the agent sends a bare channel ID (e.g. just D123 without the :thread_ts suffix), parseTarget() returns no threadId, and Slack posts a new top-level message instead of replying in-thread.

Root Cause

handleChannelReply() in session-agent.ts passes the agent-provided channelId directly to channelRouter.sendReply() with no fallback. The agent's LLM sometimes strips the :thread_ts suffix from the composite ID.

Fix

Added a defensive fallback in handleChannelReply() that checks whether the agent sent a bare Slack channelId. If so, it looks up the currently-processing prompt's stored reply context via PromptQueue.getProcessingChannelContext() (which preserves the full composite channelId including thread_ts). The fallback only fires when:

  1. channelType is 'slack'
  2. The agent's channelId does NOT contain :
  3. The stored context has a composite channelId with :
  4. The base channel (before :) matches the agent's bare channelId

This avoids unintended cross-channel overrides and leaves Telegram unaffected.

Tests Added

6 new tests in session-agent.test.ts:

  1. Bare Slack channelId + matching composite stored context — thread_ts restored
  2. Bare Slack channelId + no stored context — passes through unchanged
  3. Bare Slack channelId + different base channel stored — no override (safety guard)
  4. Composite channelId already provided — not modified
  5. Telegram with bare channelId — unaffected by Slack-specific fallback
  6. Log output verification — confirms observability log fires

All 49 session-agent tests pass. 378/449 worker tests pass (71 pre-existing better-sqlite3 binding failures unrelated to this change).


Created on behalf of Conner Swann conner@turnkey.io

it('restores thread_ts when agent sends bare Slack channelId but prompt has composite', async () => {
const { agent } = await createTestAgent();
const sendReplySpy = vi.fn().mockResolvedValue({ success: true });
(agent as any).channelRouter.sendReply = sendReplySpy;
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

as any 👻

Place extracted file contents (text, PDF, audio transcription) after
the user's message text so the agent sees the request before the
file contents, improving its understanding of the relationship
between the message and attached files.
When a user messages valet in a Slack thread, the thread context is
encoded as a composite channelId (e.g. D123:1234567890.123456). The
agent is expected to echo this back via channel_reply, but sometimes
sends only the bare channel ID (D123), causing SlackTransport.parseTarget()
to omit thread_ts and start a new thread instead of replying in-thread.

Add a defensive fallback in handleChannelReply() that checks whether the
agent sent a bare Slack channelId and, if so, restores the composite
channelId from the currently-processing prompt's stored reply context
(via PromptQueue.getProcessingChannelContext()). The fallback only fires
when the base channel matches, avoiding unintended cross-channel overrides.

Includes 6 tests covering:
- Bare channelId + matching composite stored context (thread restored)
- Bare channelId + no stored context (pass through)
- Bare channelId + different base channel in stored context (no override)
- Composite channelId already present (pass through)
- Telegram channelId (unaffected)
- Log output when fallback fires

Reported by Keisha in Slack #C0AK0T8KPPY
…idance

Extract thread_ts restoration logic from handleChannelReply into an
exported pure function. Tests now assert input/output directly with no
mocks or private member access.

Add coding convention: prefer extracting pure functions over casting
to test private members.
@yourbuddyconner yourbuddyconner force-pushed the fix/slack-thread-routing-fallback branch from e48c857 to 18f7845 Compare April 30, 2026 23:52
@yourbuddyconner yourbuddyconner merged commit f927ff2 into main Apr 30, 2026
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants