Skip to content

Commit a56237a

Browse files
teconomixteconomix
committed
fix(mattermost): inherit thread context in message tool send action
Add buildToolContext to supply currentThreadTs/replyToMode/hasRepliedRef to the message tool, and use those in handleAction to inherit the active thread when sending without an explicit replyTo. Two fixes in one: 1. buildToolContext implementation for the Mattermost plugin. Populates: - currentChannelId: from context.To (platform prefix stripped defensively) - currentThreadTs: from MessageThreadId or ReplyToId - replyToMode: resolved from account config; off promoted to all when inside an existing thread (mirrors Slack threading-tool-context.ts) - hasRepliedRef: passed through 2. handleAction send path: falls back to toolContext.currentThreadTs when no explicit replyTo/replyToId param is provided, with guards: - Same channel check: normalized to/currentChannelId must match - replyToMode gate: all=always inherit, first=once (hasRepliedRef gates), off=never - hasRepliedRef flipped after any threaded send (implicit or explicit) under replyToMode=first 9 unit tests cover all guard branches and edge cases. Closes #45082
1 parent f7ced43 commit a56237a

2 files changed

Lines changed: 300 additions & 2 deletions

File tree

‎extensions/mattermost/src/channel.test.ts‎

Lines changed: 227 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -399,6 +399,233 @@ describe("mattermostPlugin", () => {
399399
}),
400400
);
401401
});
402+
403+
describe("thread context inheritance", () => {
404+
it("falls back to toolContext.currentThreadTs when no replyTo param is provided", async () => {
405+
const cfg = createMattermostTestConfig();
406+
407+
await mattermostPlugin.actions?.handleAction?.({
408+
channel: "mattermost",
409+
action: "send",
410+
params: { to: "channel:CHAN1", message: "hello" },
411+
cfg,
412+
accountId: "default",
413+
toolContext: {
414+
currentThreadTs: "thread-root-id",
415+
currentChannelId: "channel:CHAN1",
416+
replyToMode: "all",
417+
},
418+
} as any);
419+
420+
expect(sendMessageMattermostMock).toHaveBeenCalledWith(
421+
"channel:CHAN1",
422+
"hello",
423+
expect.objectContaining({ replyToId: "thread-root-id" }),
424+
);
425+
});
426+
427+
it("explicit replyTo param wins over toolContext.currentThreadTs", async () => {
428+
const cfg = createMattermostTestConfig();
429+
430+
await mattermostPlugin.actions?.handleAction?.({
431+
channel: "mattermost",
432+
action: "send",
433+
params: { to: "channel:CHAN1", message: "hello", replyTo: "explicit-root" },
434+
cfg,
435+
accountId: "default",
436+
toolContext: {
437+
currentThreadTs: "session-thread-id",
438+
currentChannelId: "channel:CHAN1",
439+
replyToMode: "all",
440+
},
441+
} as any);
442+
443+
expect(sendMessageMattermostMock).toHaveBeenCalledWith(
444+
"channel:CHAN1",
445+
"hello",
446+
expect.objectContaining({ replyToId: "explicit-root" }),
447+
);
448+
});
449+
450+
it("does not inherit thread context when sending to a different channel", async () => {
451+
const cfg = createMattermostTestConfig();
452+
453+
await mattermostPlugin.actions?.handleAction?.({
454+
channel: "mattermost",
455+
action: "send",
456+
params: { to: "channel:OTHER", message: "hello" },
457+
cfg,
458+
accountId: "default",
459+
toolContext: {
460+
currentThreadTs: "thread-root-id",
461+
currentChannelId: "channel:CHAN1",
462+
replyToMode: "all",
463+
},
464+
} as any);
465+
466+
expect(sendMessageMattermostMock).toHaveBeenCalledWith(
467+
"channel:OTHER",
468+
"hello",
469+
expect.objectContaining({ replyToId: undefined }),
470+
);
471+
});
472+
473+
it("inherits thread context for replyToMode=first before first reply", async () => {
474+
const cfg = createMattermostTestConfig();
475+
const hasRepliedRef = { value: false };
476+
477+
await mattermostPlugin.actions?.handleAction?.({
478+
channel: "mattermost",
479+
action: "send",
480+
params: { to: "channel:CHAN1", message: "hello" },
481+
cfg,
482+
accountId: "default",
483+
toolContext: {
484+
currentThreadTs: "thread-root-id",
485+
currentChannelId: "channel:CHAN1",
486+
replyToMode: "first",
487+
hasRepliedRef,
488+
},
489+
} as any);
490+
491+
expect(sendMessageMattermostMock).toHaveBeenCalledWith(
492+
"channel:CHAN1",
493+
"hello",
494+
expect.objectContaining({ replyToId: "thread-root-id" }),
495+
);
496+
// hasRepliedRef must be flipped so the next tool send goes to channel root
497+
expect(hasRepliedRef.value).toBe(true);
498+
});
499+
500+
it("does not inherit thread context for replyToMode=first after first reply", async () => {
501+
const cfg = createMattermostTestConfig();
502+
const hasRepliedRef = { value: true };
503+
504+
await mattermostPlugin.actions?.handleAction?.({
505+
channel: "mattermost",
506+
action: "send",
507+
params: { to: "channel:CHAN1", message: "hello" },
508+
cfg,
509+
accountId: "default",
510+
toolContext: {
511+
currentThreadTs: "thread-root-id",
512+
currentChannelId: "channel:CHAN1",
513+
replyToMode: "first",
514+
hasRepliedRef,
515+
},
516+
} as any);
517+
518+
expect(sendMessageMattermostMock).toHaveBeenCalledWith(
519+
"channel:CHAN1",
520+
"hello",
521+
expect.objectContaining({ replyToId: undefined }),
522+
);
523+
});
524+
525+
it("does not inherit thread context when replyToMode is off", async () => {
526+
const cfg = createMattermostTestConfig();
527+
528+
await mattermostPlugin.actions?.handleAction?.({
529+
channel: "mattermost",
530+
action: "send",
531+
params: { to: "channel:CHAN1", message: "hello" },
532+
cfg,
533+
accountId: "default",
534+
toolContext: {
535+
currentThreadTs: "thread-root-id",
536+
currentChannelId: "channel:CHAN1",
537+
replyToMode: "off",
538+
},
539+
} as any);
540+
541+
expect(sendMessageMattermostMock).toHaveBeenCalledWith(
542+
"channel:CHAN1",
543+
"hello",
544+
expect.objectContaining({ replyToId: undefined }),
545+
);
546+
});
547+
548+
it("inherits thread context when replyToMode=off is promoted to all by existing thread", async () => {
549+
// buildToolContext promotes off→all when an existing thread is detected
550+
// (MessageThreadId set on the inbound post). handleAction receives the
551+
// already-promoted toolContext from the agent runner.
552+
const cfg = createMattermostTestConfig();
553+
554+
await mattermostPlugin.actions?.handleAction?.({
555+
channel: "mattermost",
556+
action: "send",
557+
params: { to: "channel:CHAN1", message: "hello" },
558+
cfg,
559+
accountId: "default",
560+
toolContext: {
561+
currentThreadTs: "thread-root-id",
562+
currentChannelId: "channel:CHAN1",
563+
replyToMode: "all", // promoted from off by buildToolContext
564+
},
565+
} as any);
566+
567+
expect(sendMessageMattermostMock).toHaveBeenCalledWith(
568+
"channel:CHAN1",
569+
"hello",
570+
expect.objectContaining({ replyToId: "thread-root-id" }),
571+
);
572+
});
573+
574+
it("preserves replyToMode=first when thread exists (does not promote to all)", async () => {
575+
// replyToMode=first must NOT be upgraded to all — hasRepliedRef gates further sends.
576+
const cfg = createMattermostTestConfig();
577+
const hasRepliedRef = { value: true }; // first reply already done
578+
579+
await mattermostPlugin.actions?.handleAction?.({
580+
channel: "mattermost",
581+
action: "send",
582+
params: { to: "channel:CHAN1", message: "hello" },
583+
cfg,
584+
accountId: "default",
585+
toolContext: {
586+
currentThreadTs: "thread-root-id",
587+
currentChannelId: "channel:CHAN1",
588+
replyToMode: "first",
589+
hasRepliedRef,
590+
},
591+
} as any);
592+
593+
// After first reply, subsequent sends go to channel root, not thread
594+
expect(sendMessageMattermostMock).toHaveBeenCalledWith(
595+
"channel:CHAN1",
596+
"hello",
597+
expect.objectContaining({ replyToId: undefined }),
598+
);
599+
});
600+
601+
it("flips hasRepliedRef when explicit replyTo is used under replyToMode=first", async () => {
602+
// Explicit replyTo should also consume the first-reply slot
603+
const cfg = createMattermostTestConfig();
604+
const hasRepliedRef = { value: false };
605+
606+
await mattermostPlugin.actions?.handleAction?.({
607+
channel: "mattermost",
608+
action: "send",
609+
params: { to: "channel:CHAN1", message: "hello", replyTo: "explicit-root" },
610+
cfg,
611+
accountId: "default",
612+
toolContext: {
613+
currentThreadTs: "thread-root-id",
614+
currentChannelId: "channel:CHAN1",
615+
replyToMode: "first",
616+
hasRepliedRef,
617+
},
618+
} as any);
619+
620+
expect(sendMessageMattermostMock).toHaveBeenCalledWith(
621+
"channel:CHAN1",
622+
"hello",
623+
expect.objectContaining({ replyToId: "explicit-root" }),
624+
);
625+
// Explicit threaded send must also flip hasRepliedRef
626+
expect(hasRepliedRef.value).toBe(true);
627+
});
628+
});
402629
});
403630

404631
describe("outbound", () => {

‎extensions/mattermost/src/channel.ts‎

Lines changed: 73 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -112,7 +112,7 @@ const mattermostMessageActions: ChannelMessageActionAdapter = {
112112
supportsAction: ({ action }) => {
113113
return action === "send" || action === "react";
114114
},
115-
handleAction: async ({ action, params, cfg, accountId }) => {
115+
handleAction: async ({ action, params, cfg, accountId, toolContext }) => {
116116
if (action === "react") {
117117
const resolvedAccountId = accountId ?? resolveDefaultMattermostAccountId(cfg);
118118
const mattermostConfig = cfg.channels?.mattermost as MattermostConfig | undefined;
@@ -176,7 +176,41 @@ const mattermostMessageActions: ChannelMessageActionAdapter = {
176176
const message = typeof params.message === "string" ? params.message : "";
177177
// Match the shared runner semantics: trim empty reply IDs away before
178178
// falling back from replyToId to replyTo on direct plugin calls.
179-
const replyToId = readMattermostReplyToId(params);
179+
// When no explicit reply target is set, inherit the thread context from
180+
// the active session — but only when:
181+
// 1. The send target is the same channel as the active session (same channelId),
182+
// to avoid injecting a foreign root_id into a different Mattermost channel.
183+
// 2. replyToMode is "all", or "first" and the first reply has not yet been sent.
184+
const sessionThreadTs = toolContext?.currentThreadTs?.trim() || undefined;
185+
const normalizedTo = normalizeMattermostMessagingTarget(to) ?? to;
186+
const normalizedCurrentChannelId = toolContext?.currentChannelId
187+
? (normalizeMattermostMessagingTarget(toolContext.currentChannelId) ??
188+
toolContext.currentChannelId)
189+
: undefined;
190+
const isSameChannel =
191+
sessionThreadTs !== undefined &&
192+
normalizedCurrentChannelId !== undefined &&
193+
normalizedTo === normalizedCurrentChannelId;
194+
// For replyToMode=first, only inherit if a hasRepliedRef exists AND it hasn't fired yet.
195+
// Fail-closed: absent ref → don't inherit (avoids every send threading like replyToMode=all).
196+
const replyToModeAllowsThread =
197+
toolContext?.replyToMode === "all" ||
198+
(toolContext?.replyToMode === "first" &&
199+
toolContext.hasRepliedRef !== undefined &&
200+
toolContext.hasRepliedRef.value !== true);
201+
const implicitReplyToId =
202+
isSameChannel && replyToModeAllowsThread ? sessionThreadTs : undefined;
203+
const replyToId = readMattermostReplyToId(params) ?? implicitReplyToId;
204+
// For replyToMode=first: flip hasRepliedRef after any threaded send (implicit or
205+
// explicit) so subsequent tool sends in the same turn go to the channel root.
206+
// This covers both implicit fallback and explicit replyTo/replyToId params.
207+
if (
208+
replyToId !== undefined &&
209+
toolContext?.replyToMode === "first" &&
210+
toolContext.hasRepliedRef !== undefined
211+
) {
212+
toolContext.hasRepliedRef.value = true;
213+
}
180214
const resolvedAccountId = accountId || undefined;
181215

182216
const mediaUrl =
@@ -435,6 +469,43 @@ export const mattermostPlugin: ChannelPlugin<ResolvedMattermostAccount> = create
435469
: "channel",
436470
),
437471
},
472+
buildToolContext: ({ cfg, accountId, context, hasRepliedRef }) => {
473+
// Resolve replyToMode from account config so the message tool can respect it.
474+
const account = resolveMattermostAccount({ cfg, accountId: accountId ?? "default" });
475+
const chatType =
476+
context.ChatType === "direct" ||
477+
context.ChatType === "group" ||
478+
context.ChatType === "channel"
479+
? context.ChatType
480+
: "channel";
481+
const configuredReplyToMode = resolveMattermostReplyToMode(account, chatType);
482+
// currentThreadTs is the Mattermost root post ID of the active thread.
483+
// MessageThreadId is set when the inbound message is already part of a thread.
484+
const threadTs =
485+
typeof context.MessageThreadId === "string" && context.MessageThreadId.trim()
486+
? context.MessageThreadId.trim()
487+
: typeof context.ReplyToId === "string" && context.ReplyToId.trim()
488+
? context.ReplyToId.trim()
489+
: undefined;
490+
// When the session is already inside a thread and the configured mode is
491+
// "off", promote to "all" so that tool sends stay in the thread (mirrors
492+
// Slack's threading-tool-context.ts effective-mode promotion).
493+
// For "first", preserve it so hasRepliedRef can gate subsequent sends.
494+
// For "all", it stays "all" naturally.
495+
const effectiveReplyToMode =
496+
configuredReplyToMode === "off" && threadTs != null ? "all" : configuredReplyToMode;
497+
return {
498+
// context.To is "channel:<id>" or "user:<id>" (no mattermost: prefix).
499+
// Strip any platform prefix defensively to ensure it matches the
500+
// tool's `to` param format used in normalizeMattermostMessagingTarget.
501+
currentChannelId: context.To?.trim().replace(/^mattermost:/i, "") || undefined,
502+
currentChannelProvider: "mattermost",
503+
currentThreadTs: threadTs,
504+
currentMessageId: context.CurrentMessageId,
505+
replyToMode: effectiveReplyToMode,
506+
hasRepliedRef,
507+
};
508+
},
438509
},
439510
security: mattermostSecurityAdapter,
440511
outbound: {

0 commit comments

Comments
 (0)