From 0bf97f6681e4cf8cc42f7cfe36cb90c58c32e59a Mon Sep 17 00:00:00 2001 From: Codex CLI Audit Date: Sun, 8 Mar 2026 18:11:58 -0400 Subject: [PATCH] fix(googlechat): preserve replyToId across multi-payload sends for thread routing Google Chat uses replyToId as persistent thread context (threadName), similar to Slack (thread_ts) and Mattermost (rootId). The outbound delivery core was consuming inherited replyToId after the first successful send, orphaning subsequent payloads to the top level. Add googlechat to the isThreadBasedChannel check so replyToId survives across all payloads in a multi-payload response. Co-Authored-By: Claude Opus 4.6 --- .../outbound/deliver.greptile-fixes.test.ts | 84 +++++++++++++++++++ src/infra/outbound/deliver.ts | 10 ++- 2 files changed, 90 insertions(+), 4 deletions(-) diff --git a/src/infra/outbound/deliver.greptile-fixes.test.ts b/src/infra/outbound/deliver.greptile-fixes.test.ts index 49ba7ff1327..9b169570ffb 100644 --- a/src/infra/outbound/deliver.greptile-fixes.test.ts +++ b/src/infra/outbound/deliver.greptile-fixes.test.ts @@ -272,6 +272,90 @@ describe("deliverOutboundPayloads Greptile fixes", () => { ]); }); + it("preserves inherited replyToId across all googlechat sendPayload payloads (thread routing)", async () => { + const sendPayload = vi + .fn() + .mockResolvedValueOnce({ channel: "googlechat", messageId: "gc-1" }) + .mockResolvedValueOnce({ channel: "googlechat", messageId: "gc-2" }) + .mockResolvedValueOnce({ channel: "googlechat", messageId: "gc-3" }); + const sendText = vi.fn(); + const sendMedia = vi.fn(); + + setActivePluginRegistry( + createTestRegistry([ + { + pluginId: "googlechat", + source: "test", + plugin: createOutboundTestPlugin({ + id: "googlechat", + outbound: { deliveryMode: "direct", sendPayload, sendText, sendMedia }, + }), + }, + ]), + ); + + const results = await deliverOutboundPayloads({ + cfg: {}, + channel: "googlechat", + to: "spaces/AAAA", + payloads: [ + { text: "first", channelData: { mode: "custom" } }, + { text: "second", channelData: { mode: "custom" } }, + { text: "third", channelData: { mode: "custom" } }, + ], + replyToId: "spaces/AAAA/threads/BBBB", + skipQueue: true, + }); + + expect(sendPayload).toHaveBeenCalledTimes(3); + // All payloads must retain the thread identifier — consuming it after the + // first send would orphan subsequent payloads to the top level. + expect(sendPayload.mock.calls[0]?.[0]?.replyToId).toBe("spaces/AAAA/threads/BBBB"); + expect(sendPayload.mock.calls[1]?.[0]?.replyToId).toBe("spaces/AAAA/threads/BBBB"); + expect(sendPayload.mock.calls[2]?.[0]?.replyToId).toBe("spaces/AAAA/threads/BBBB"); + expect(results).toEqual([ + { channel: "googlechat", messageId: "gc-1" }, + { channel: "googlechat", messageId: "gc-2" }, + { channel: "googlechat", messageId: "gc-3" }, + ]); + }); + + it("preserves inherited replyToId across googlechat text payloads (sendText path)", async () => { + const sendText = vi + .fn() + .mockResolvedValueOnce({ channel: "googlechat", messageId: "gc-t1", chatId: "spaces/X" }) + .mockResolvedValueOnce({ channel: "googlechat", messageId: "gc-t2", chatId: "spaces/X" }); + const sendMedia = vi.fn(); + + setActivePluginRegistry( + createTestRegistry([ + { + pluginId: "googlechat", + source: "test", + plugin: createOutboundTestPlugin({ + id: "googlechat", + outbound: { deliveryMode: "direct", sendText, sendMedia }, + }), + }, + ]), + ); + + const results = await deliverOutboundPayloads({ + cfg: {}, + channel: "googlechat", + to: "spaces/X", + payloads: [{ text: "chunk one" }, { text: "chunk two" }], + replyToId: "spaces/X/threads/T1", + skipQueue: true, + }); + + expect(sendText).toHaveBeenCalledTimes(2); + // Both text sends should receive replyToId for thread routing + expect(sendText.mock.calls[0]?.[0]?.replyToId).toBe("spaces/X/threads/T1"); + expect(sendText.mock.calls[1]?.[0]?.replyToId).toBe("spaces/X/threads/T1"); + expect(results).toHaveLength(2); + }); + it("retries replyToId on later non-signal media payloads after a best-effort failure", async () => { const sendText = vi.fn(); const sendMedia = vi diff --git a/src/infra/outbound/deliver.ts b/src/infra/outbound/deliver.ts index 5aa309e6b73..ad63e589aa2 100644 --- a/src/infra/outbound/deliver.ts +++ b/src/infra/outbound/deliver.ts @@ -738,10 +738,12 @@ async function deliverOutboundPayloadsCore( ); } let replyConsumed = false; - // Slack and Mattermost use replyToId as persistent thread context (thread_ts - // and rootId respectively) that must survive across all payloads. Never - // consume inherited reply state for thread-based channels. - const isThreadBasedChannel = channel === "slack" || channel === "mattermost"; + // Slack, Mattermost, and Google Chat use replyToId as persistent thread + // context (thread_ts, rootId, and threadName respectively) that must survive + // across all payloads. Never consume inherited reply state for thread-based + // channels. + const isThreadBasedChannel = + channel === "slack" || channel === "mattermost" || channel === "googlechat"; const shouldConsumeReplyAfterSend = (replyTo: string | undefined) => { if (!replyTo) { return false;