From 6a1eedf10b1b1cbdd25a2d6c8cc24b06b303ebd1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E4=B8=8D=E5=81=9A=E4=BA=86=E7=9D=A1=E5=A4=A7=E8=A7=89?= <64798754+stakeswky@users.noreply.github.com> Date: Mon, 2 Mar 2026 09:11:08 +0800 Subject: [PATCH] fix: deliver subagent completion announces to Slack without invalid thread_ts (#31105) * fix(subagent): avoid invalid Slack thread_ts for bound completion announces * build: regenerate host env security policy swift --------- Co-authored-by: User Co-authored-by: Peter Steinberger --- src/agents/subagent-announce.format.test.ts | 61 +++++++++++++++++++++ src/agents/subagent-announce.ts | 9 ++- 2 files changed, 69 insertions(+), 1 deletion(-) diff --git a/src/agents/subagent-announce.format.test.ts b/src/agents/subagent-announce.format.test.ts index 5f1394c7787..dc0d492f02d 100644 --- a/src/agents/subagent-announce.format.test.ts +++ b/src/agents/subagent-announce.format.test.ts @@ -849,6 +849,67 @@ describe("subagent announce formatting", () => { } }); + it("does not force Slack threadId from bound conversation id", async () => { + sendSpy.mockClear(); + agentSpy.mockClear(); + sessionStore = { + "agent:main:subagent:test": { + sessionId: "child-session-slack-bound", + }, + "agent:main:main": { + sessionId: "requester-session-slack-bound", + }, + }; + chatHistoryMock.mockResolvedValueOnce({ + messages: [{ role: "assistant", content: [{ type: "text", text: "done" }] }], + }); + registerSessionBindingAdapter({ + channel: "slack", + accountId: "acct-1", + listBySession: (targetSessionKey: string) => + targetSessionKey === "agent:main:subagent:test" + ? [ + { + bindingId: "slack:acct-1:C123", + targetSessionKey, + targetKind: "subagent", + conversation: { + channel: "slack", + accountId: "acct-1", + conversationId: "C123", + }, + status: "active", + boundAt: Date.now(), + }, + ] + : [], + resolveByConversation: () => null, + }); + + const didAnnounce = await runSubagentAnnounceFlow({ + childSessionKey: "agent:main:subagent:test", + childRunId: "run-direct-slack-bound", + requesterSessionKey: "agent:main:main", + requesterDisplayKey: "main", + requesterOrigin: { + channel: "slack", + to: "channel:C123", + accountId: "acct-1", + }, + ...defaultOutcomeAnnounce, + expectsCompletionMessage: true, + spawnMode: "session", + }); + + expect(didAnnounce).toBe(true); + expect(sendSpy).toHaveBeenCalledTimes(1); + expect(agentSpy).not.toHaveBeenCalled(); + const call = sendSpy.mock.calls[0]?.[0] as { params?: Record }; + expect(call?.params?.channel).toBe("slack"); + expect(call?.params?.to).toBe("channel:C123"); + expect(call?.params?.threadId).toBeUndefined(); + }); + it("routes manual completion direct-send for telegram forum topics", async () => { sendSpy.mockClear(); agentSpy.mockClear(); diff --git a/src/agents/subagent-announce.ts b/src/agents/subagent-announce.ts index c6cbfc09c23..5932594d301 100644 --- a/src/agents/subagent-announce.ts +++ b/src/agents/subagent-announce.ts @@ -517,7 +517,14 @@ async function resolveSubagentCompletionOrigin(params: { channel: route.binding.conversation.channel, accountId: route.binding.conversation.accountId, to: `channel:${route.binding.conversation.conversationId}`, - threadId: route.binding.conversation.conversationId, + // `conversationId` identifies the target conversation (channel/DM/thread), + // but it is not always a thread identifier. Passing it as `threadId` breaks + // Slack DM/top-level delivery by forcing an invalid thread_ts. Preserve only + // explicit requester thread hints for channels that actually use threading. + threadId: + requesterOrigin?.threadId != null && requesterOrigin.threadId !== "" + ? String(requesterOrigin.threadId) + : undefined, }; return { // Bound target is authoritative; requester hints fill only missing fields.