fix: restore Slack thread_ts when agent drops it from channel reply#44
Merged
yourbuddyconner merged 4 commits intomainfrom Apr 30, 2026
Merged
fix: restore Slack thread_ts when agent drops it from channel reply#44yourbuddyconner merged 4 commits intomainfrom
yourbuddyconner merged 4 commits intomainfrom
Conversation
figitaki
reviewed
Apr 30, 2026
| 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; |
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.
e48c857 to
18f7845
Compare
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
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
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
channelIdlikeD123: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 callingchannel_reply.SlackTransport.parseTarget()then splits it into{ channelId, threadId }and setsthread_tsin thechat.postMessagecall.If the agent sends a bare channel ID (e.g. just
D123without the:thread_tssuffix),parseTarget()returns nothreadId, and Slack posts a new top-level message instead of replying in-thread.Root Cause
handleChannelReply()insession-agent.tspasses the agent-providedchannelIddirectly tochannelRouter.sendReply()with no fallback. The agent's LLM sometimes strips the:thread_tssuffix from the composite ID.Fix
Added a defensive fallback in
handleChannelReply()that checks whether the agent sent a bare SlackchannelId. If so, it looks up the currently-processing prompt's stored reply context viaPromptQueue.getProcessingChannelContext()(which preserves the full compositechannelIdincludingthread_ts). The fallback only fires when:channelTypeis'slack'channelIddoes NOT contain:channelIdwith::) matches the agent's barechannelIdThis avoids unintended cross-channel overrides and leaves Telegram unaffected.
Tests Added
6 new tests in
session-agent.test.ts:All 49 session-agent tests pass. 378/449 worker tests pass (71 pre-existing
better-sqlite3binding failures unrelated to this change).