Skip to content

Commit ab6587c

Browse files
steipetesteipete
committed
fix(mattermost): preserve explicit reply targets
1 parent 8719980 commit ab6587c

8 files changed

Lines changed: 97 additions & 17 deletions

File tree

‎docs/.generated/plugin-sdk-api-baseline.sha256‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,2 +1,2 @@
1-
5b43ccf4683723ba723c029f78307ed247c7e1f92e6b5d9c30d21854aa430286 plugin-sdk-api-baseline.json
2-
7b4ee75584778d35ef23e6799d5d84be424bc3e515045bd3802dcc0d75365d42 plugin-sdk-api-baseline.jsonl
1+
b121079a0912b3051a9fc319a675ef920da9db23364ca0c0ccd3c9f0a05a3a49 plugin-sdk-api-baseline.json
2+
61a0108da670e0f44ba4b861c002eb6eaa5cf63e392d4e7e7de42044cbe7d115 plugin-sdk-api-baseline.jsonl

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

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -283,6 +283,21 @@ describe("mattermostPlugin", () => {
283283
replyToId: "root-post",
284284
threadId: "root-post",
285285
});
286+
expect(
287+
resolveReplyTransport({
288+
cfg: {},
289+
replyToId: "other-root",
290+
replyToIsExplicit: true,
291+
threadId: "ambient-root",
292+
replyDelivery: {
293+
chatType: "channel",
294+
replyToMode: "all",
295+
},
296+
}),
297+
).toEqual({
298+
replyToId: "other-root",
299+
threadId: "other-root",
300+
});
286301
expect(
287302
resolveReplyTransport({
288303
cfg: {},

‎extensions/mattermost/src/channel.ts‎

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -896,15 +896,16 @@ export const mattermostPlugin: ChannelPlugin<ResolvedMattermostAccount> = create
896896
},
897897
resolveAutoThreadId: ({ to, replyToId, toolContext }) =>
898898
resolveMattermostAutoThreadId({ to, replyToId, toolContext }),
899-
resolveReplyTransport: ({ threadId, replyToId, replyDelivery }) => {
899+
resolveReplyTransport: ({ threadId, replyToId, replyToIsExplicit, replyDelivery }) => {
900+
const ambientThreadId = threadId != null ? String(threadId) : undefined;
900901
const resolvedThreadId =
901902
replyDelivery?.chatType === "direct"
902903
? undefined
903-
: replyDelivery
904-
? threadId != null
905-
? String(threadId)
906-
: (replyToId ?? undefined)
907-
: (replyToId ?? (threadId != null ? String(threadId) : undefined));
904+
: replyToIsExplicit
905+
? (replyToId ?? ambientThreadId)
906+
: replyDelivery
907+
? (ambientThreadId ?? replyToId ?? undefined)
908+
: (replyToId ?? ambientThreadId);
908909
return {
909910
replyToId: replyDelivery?.chatType === "direct" ? null : resolvedThreadId,
910911
threadId: resolvedThreadId ?? null,

‎src/auto-reply/reply/agent-runner-payloads.test.ts‎

Lines changed: 35 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -86,16 +86,18 @@ describe("buildReplyPayloads media filter integration", () => {
8686
resolveReplyTransport: ({
8787
threadId,
8888
replyToId,
89+
replyToIsExplicit,
8990
replyDelivery,
9091
}: ResolveReplyTransportParams) => {
92+
const ambientThreadId = threadId != null ? String(threadId) : undefined;
9193
const resolvedThreadId =
9294
replyDelivery?.chatType === "direct"
9395
? undefined
94-
: replyDelivery
95-
? threadId != null
96-
? String(threadId)
97-
: (replyToId ?? undefined)
98-
: (replyToId ?? (threadId != null ? String(threadId) : undefined));
96+
: replyToIsExplicit
97+
? (replyToId ?? ambientThreadId)
98+
: replyDelivery
99+
? (ambientThreadId ?? replyToId ?? undefined)
100+
: (replyToId ?? ambientThreadId);
99101
return {
100102
replyToId: resolvedThreadId,
101103
threadId: resolvedThreadId ?? null,
@@ -699,11 +701,11 @@ describe("buildReplyPayloads media filter integration", () => {
699701
expect(replyPayloads).toHaveLength(0);
700702
});
701703

702-
it("dedupes an explicit Mattermost reply against the existing thread root", async () => {
704+
it("does not dedupe an explicit Mattermost reply to another thread root", async () => {
703705
const { replyPayloads } = await buildReplyPayloads({
704706
...baseParams,
705707
config: {},
706-
payloads: [{ text: "same reply", replyToId: "child-post", replyToTag: true }],
708+
payloads: [{ text: "same reply", replyToId: "other-root", replyToTag: true }],
707709
replyToMode: "all",
708710
replyToChannel: "mattermost",
709711
messageProvider: "mattermost",
@@ -722,6 +724,32 @@ describe("buildReplyPayloads media filter integration", () => {
722724
],
723725
});
724726

727+
expect(replyPayloads).toHaveLength(1);
728+
});
729+
730+
it("dedupes an explicit Mattermost reply to the same thread root", async () => {
731+
const { replyPayloads } = await buildReplyPayloads({
732+
...baseParams,
733+
config: {},
734+
payloads: [{ text: "same reply", replyToId: "root-post", replyToTag: true }],
735+
replyToMode: "all",
736+
replyToChannel: "mattermost",
737+
messageProvider: "mattermost",
738+
originatingChatType: "channel",
739+
originatingTo: "channel:C1",
740+
originatingThreadId: "ambient-root",
741+
messagingToolSentTexts: ["same reply"],
742+
messagingToolSentTargets: [
743+
{
744+
tool: "mattermost",
745+
provider: "mattermost",
746+
to: "channel:C1",
747+
threadId: "root-post",
748+
text: "same reply",
749+
},
750+
],
751+
});
752+
725753
expect(replyPayloads).toHaveLength(0);
726754
});
727755

‎src/auto-reply/reply/reply-payloads-dedupe.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -223,6 +223,7 @@ function resolveOriginThreadIdForPayload(params: {
223223
accountId: params.accountId,
224224
threadId: originThreadId,
225225
replyToId,
226+
replyToIsExplicit: params.replyToIsExplicit,
226227
replyDelivery: params.replyDelivery,
227228
});
228229
if (transport?.threadId != null) {

‎src/auto-reply/reply/route-reply.test.ts‎

Lines changed: 32 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -104,11 +104,16 @@ function resolveSlackThreadTsCandidate(value?: string | number | null): string |
104104
}
105105

106106
const mattermostThreading: ChannelThreadingAdapter = {
107-
resolveReplyTransport: ({ threadId, replyToId, replyDelivery }) => {
107+
resolveReplyTransport: ({ threadId, replyToId, replyToIsExplicit, replyDelivery }) => {
108+
const ambientThreadId = threadId != null && threadId !== "" ? String(threadId) : undefined;
108109
const resolvedThreadId =
109110
replyDelivery?.chatType === "direct"
110111
? undefined
111-
: (replyToId ?? (threadId != null && threadId !== "" ? String(threadId) : undefined));
112+
: replyToIsExplicit
113+
? (replyToId ?? ambientThreadId)
114+
: replyDelivery
115+
? (ambientThreadId ?? replyToId ?? undefined)
116+
: (replyToId ?? ambientThreadId);
112117
return {
113118
replyToId: replyDelivery?.chatType === "direct" ? null : resolvedThreadId,
114119
threadId: resolvedThreadId ?? null,
@@ -475,6 +480,31 @@ describe("routeReply", () => {
475480
expect(lastDeliveryPayload().replyToId).toBeUndefined();
476481
});
477482

483+
it("preserves explicit Mattermost reply targets over the ambient thread", async () => {
484+
const res = await routeReply({
485+
payload: {
486+
text: "hello",
487+
replyToId: "other-root",
488+
replyToTag: true,
489+
},
490+
channel: "mattermost",
491+
to: "channel:C123",
492+
threadId: "ambient-root",
493+
replyDelivery: {
494+
chatType: "channel",
495+
replyToMode: "all",
496+
},
497+
cfg: {} as never,
498+
});
499+
500+
expect(res.ok).toBe(true);
501+
expectLastDeliveryFields({
502+
replyToId: "other-root",
503+
threadId: "other-root",
504+
});
505+
expect(lastDeliveryPayload().replyToId).toBe("other-root");
506+
});
507+
478508
it("preserves reply targets when an adapter returns undefined", async () => {
479509
const res = await routeReply({
480510
payload: { text: "hello", replyToId: "msg-internal-1" },

‎src/auto-reply/reply/route-reply.ts‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -240,6 +240,9 @@ export async function routeReply(params: RouteReplyParams): Promise<RouteReplyRe
240240
accountId,
241241
threadId,
242242
replyToId,
243+
replyToIsExplicit: Boolean(
244+
payloadMetadata?.replyToIdExplicit || normalized.replyToTag || normalized.replyToCurrent,
245+
),
243246
replyDelivery,
244247
}) ?? null;
245248
const resolvedReplyToId =

‎src/channels/plugins/types.core.ts‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -440,6 +440,8 @@ export type ChannelThreadingAdapter = {
440440
accountId?: string | null;
441441
threadId?: string | number | null;
442442
replyToId?: string | null;
443+
/** True when replyToId came from an explicit payload target or reply tag. */
444+
replyToIsExplicit?: boolean;
443445
replyDelivery?: ReplyDeliveryContext;
444446
}) => ChannelReplyTransport | null;
445447
resolveFocusedBinding?: (params: {

0 commit comments

Comments
 (0)