fix(mattermost): carry thread context to non-inbound reply paths

Fixes a bug where replies triggered from TUI/WebUI or by agent-initiated
sends (tool callbacks, subagent responses, message tool) land in the
Mattermost channel root instead of the originating thread.

Root cause: three gaps in the outbound routing path for turns not
directly triggered by an inbound Mattermost message:

1. dispatch-from-config.ts: sendPayloadAsync passed ctx.MessageThreadId,
   which is undefined for webchat/TUI turns. Now falls back to the
   session entry's deliveryContext.threadId (the lastThreadId stored when
   the session was first created from an inbound Mattermost message).

2. route-reply.ts: threadId was only forwarded as replyToId for Slack.
   Mattermost uses the same root_id mechanic, so the same fallback now
   applies to Mattermost too.

3. channel.ts (Mattermost outbound): sendText/sendMedia only consumed
   replyToId, ignoring threadId. Added threadId as a defense-in-depth
   fallback for any path that sets threadId but not replyToId.

All three gaps in a single PR. Tests added for each fix path.

Fixes #39759
This commit is contained in:
teconomix
2026-03-12 17:54:56 +00:00
committed by Muhammed Mukhthar CM
parent 17cb60080a
commit 62b6d6fca7
6 changed files with 145 additions and 12 deletions

View File

@@ -41,6 +41,12 @@ const acpMocks = vi.hoisted(() => ({
const sessionBindingMocks = vi.hoisted(() => ({
listBySession: vi.fn<(targetSessionKey: string) => SessionBindingRecord[]>(() => []),
}));
const sessionStoreMocks = vi.hoisted(() => ({
currentEntry: undefined as Record<string, unknown> | undefined,
loadSessionStore: vi.fn(() => ({})),
resolveStorePath: vi.fn(() => "/tmp/mock-sessions.json"),
resolveSessionStoreEntry: vi.fn(() => ({ existing: sessionStoreMocks.currentEntry })),
}));
const ttsMocks = vi.hoisted(() => {
const state = {
synthesizeFinalAudio: false,
@@ -77,9 +83,16 @@ vi.mock("./route-reply.js", () => ({
isRoutableChannel: (channel: string | undefined) =>
Boolean(
channel &&
["telegram", "slack", "discord", "signal", "imessage", "whatsapp", "feishu"].includes(
channel,
),
[
"telegram",
"slack",
"discord",
"signal",
"imessage",
"whatsapp",
"feishu",
"mattermost",
].includes(channel),
),
routeReply: mocks.routeReply,
}));
@@ -100,6 +113,11 @@ vi.mock("../../logging/diagnostic.js", () => ({
logMessageProcessed: diagnosticMocks.logMessageProcessed,
logSessionStateChange: diagnosticMocks.logSessionStateChange,
}));
vi.mock("../../config/sessions.js", () => ({
loadSessionStore: sessionStoreMocks.loadSessionStore,
resolveStorePath: sessionStoreMocks.resolveStorePath,
resolveSessionStoreEntry: sessionStoreMocks.resolveSessionStoreEntry,
}));
vi.mock("../../plugins/hook-runner-global.js", () => ({
getGlobalHookRunner: () => hookMocks.runner,
@@ -228,6 +246,10 @@ describe("dispatchReplyFromConfig", () => {
acpMocks.requireAcpRuntimeBackend.mockReset();
sessionBindingMocks.listBySession.mockReset();
sessionBindingMocks.listBySession.mockReturnValue([]);
sessionStoreMocks.currentEntry = undefined;
sessionStoreMocks.loadSessionStore.mockClear();
sessionStoreMocks.resolveStorePath.mockClear();
sessionStoreMocks.resolveSessionStoreEntry.mockClear();
ttsMocks.state.synthesizeFinalAudio = false;
ttsMocks.maybeApplyTtsToPayload.mockClear();
ttsMocks.normalizeTtsAutoMode.mockClear();
@@ -293,6 +315,43 @@ describe("dispatchReplyFromConfig", () => {
);
});
it("falls back to session deliveryContext threadId when current ctx has no MessageThreadId", async () => {
setNoAbort();
mocks.routeReply.mockClear();
sessionStoreMocks.currentEntry = {
deliveryContext: {
channel: "mattermost",
to: "channel:CHAN1",
accountId: "default",
threadId: "post-root",
},
lastThreadId: "post-root",
};
const cfg = emptyConfig;
const dispatcher = createDispatcher();
const ctx = buildTestCtx({
Provider: "webchat",
Surface: "webchat",
SessionKey: "agent:main:mattermost:channel:CHAN1:thread:post-root",
AccountId: "default",
MessageThreadId: undefined,
OriginatingChannel: "mattermost",
OriginatingTo: "channel:CHAN1",
ExplicitDeliverRoute: true,
});
const replyResolver = async () => ({ text: "hi" }) satisfies ReplyPayload;
await dispatchReplyFromConfig({ ctx, cfg, dispatcher, replyResolver });
expect(mocks.routeReply).toHaveBeenCalledWith(
expect.objectContaining({
channel: "mattermost",
to: "channel:CHAN1",
threadId: "post-root",
}),
);
});
it("forces suppressTyping when routing to a different originating channel", async () => {
setNoAbort();
const cfg = emptyConfig;