Skip to content

Commit 4f08dcc

Browse files
authored
Mattermost: add interactive model picker (#38767)
Merged via squash. Prepared head SHA: 0883654 Co-authored-by: mukhtharcm <[email protected]> Co-authored-by: mukhtharcm <[email protected]> Reviewed-by: @mukhtharcm
1 parent 33e7394 commit 4f08dcc

23 files changed

Lines changed: 1867 additions & 290 deletions

CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@ Docs: https://docs.openclaw.ai
3030
- Docs/Web search: remove outdated Brave free-tier wording and replace prescriptive AI ToS guidance with neutral compliance language in Brave setup docs. (#26860) Thanks @HenryLoenwind.
3131
- Config/Compaction safeguard tuning: expose `agents.defaults.compaction.recentTurnsPreserve` and quality-guard retry knobs through the validated config surface and embedded-runner wiring, with regression coverage for real config loading and schema metadata. (#25557) thanks @rodrigouroz.
3232
- iOS/App Store Connect release prep: align iOS bundle identifiers under `ai.openclaw.client`, refresh Watch app icons, add Fastlane metadata/screenshot automation, and support Keychain-backed ASC auth for uploads. (#38936) Thanks @ngutman.
33+
- Mattermost/model picker: add Telegram-style interactive provider/model browsing for `/oc_model` and `/oc_models`, fix picker callback updates, and emit a normal confirmation reply when a model is selected. (#38767) thanks @mukhtharcm.
3334

3435
### Breaking
3536

extensions/mattermost/src/channel.ts

Lines changed: 3 additions & 57 deletions
Original file line numberDiff line numberDiff line change
@@ -26,21 +26,14 @@ import {
2626
listMattermostDirectoryGroups,
2727
listMattermostDirectoryPeers,
2828
} from "./mattermost/directory.js";
29-
import {
30-
buildButtonAttachments,
31-
resolveInteractionCallbackUrl,
32-
setInteractionSecret,
33-
} from "./mattermost/interactions.js";
3429
import { monitorMattermostProvider } from "./mattermost/monitor.js";
3530
import { probeMattermost } from "./mattermost/probe.js";
3631
import { addMattermostReaction, removeMattermostReaction } from "./mattermost/reactions.js";
37-
import { resolveMattermostSendChannelId, sendMessageMattermost } from "./mattermost/send.js";
32+
import { sendMessageMattermost } from "./mattermost/send.js";
3833
import { looksLikeMattermostTargetId, normalizeMattermostMessagingTarget } from "./normalize.js";
3934
import { mattermostOnboardingAdapter } from "./onboarding.js";
4035
import { getMattermostRuntime } from "./runtime.js";
4136

42-
const SIGNED_CHANNEL_ID_CONTEXT_KEY = "__openclaw_channel_id";
43-
4437
const mattermostMessageActions: ChannelMessageActionAdapter = {
4538
listActions: ({ cfg }) => {
4639
const enabledAccounts = listMattermostAccountIds(cfg)
@@ -162,61 +155,14 @@ const mattermostMessageActions: ChannelMessageActionAdapter = {
162155
const replyToId = typeof params.replyToId === "string" ? params.replyToId : undefined;
163156
const resolvedAccountId = accountId || undefined;
164157

165-
// Build props with button attachments if buttons are provided
166-
let props: Record<string, unknown> | undefined;
167-
if (params.buttons && Array.isArray(params.buttons)) {
168-
const account = resolveMattermostAccount({ cfg, accountId: resolvedAccountId });
169-
if (account.botToken) setInteractionSecret(account.accountId, account.botToken);
170-
const channelId = await resolveMattermostSendChannelId(to, {
171-
cfg,
172-
accountId: account.accountId,
173-
});
174-
const callbackUrl = resolveInteractionCallbackUrl(account.accountId, {
175-
gateway: cfg.gateway,
176-
interactions: account.config.interactions,
177-
});
178-
179-
// Flatten 2D array (rows of buttons) to 1D — core schema sends Array<Array<Button>>
180-
// but Mattermost doesn't have row layout, so we flatten all rows into a single list.
181-
// Also supports 1D arrays for backward compatibility.
182-
const rawButtons = (params.buttons as Array<unknown>).flatMap((item) =>
183-
Array.isArray(item) ? item : [item],
184-
) as Array<Record<string, unknown>>;
185-
186-
const buttons = rawButtons
187-
.map((btn) => ({
188-
id: String(btn.id ?? btn.callback_data ?? ""),
189-
name: String(btn.text ?? btn.name ?? btn.label ?? ""),
190-
style: (btn.style as "default" | "primary" | "danger") ?? "default",
191-
context:
192-
typeof btn.context === "object" && btn.context !== null
193-
? {
194-
...(btn.context as Record<string, unknown>),
195-
[SIGNED_CHANNEL_ID_CONTEXT_KEY]: channelId,
196-
}
197-
: { [SIGNED_CHANNEL_ID_CONTEXT_KEY]: channelId },
198-
}))
199-
.filter((btn) => btn.id && btn.name);
200-
201-
const attachmentText =
202-
typeof params.attachmentText === "string" ? params.attachmentText : undefined;
203-
props = {
204-
attachments: buildButtonAttachments({
205-
callbackUrl,
206-
accountId: account.accountId,
207-
buttons,
208-
text: attachmentText,
209-
}),
210-
};
211-
}
212-
213158
const mediaUrl =
214159
typeof params.media === "string" ? params.media.trim() || undefined : undefined;
215160

216161
const result = await sendMessageMattermost(to, message, {
217162
accountId: resolvedAccountId,
218163
replyToId,
219-
props,
164+
buttons: Array.isArray(params.buttons) ? params.buttons : undefined,
165+
attachmentText: typeof params.attachmentText === "string" ? params.attachmentText : undefined,
220166
mediaUrl,
221167
});
222168

extensions/mattermost/src/config-schema.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,7 @@ const MattermostAccountSchemaBase = z
5353
interactions: z
5454
.object({
5555
callbackBaseUrl: z.string().optional(),
56+
allowedSourceIps: z.array(z.string()).optional(),
5657
})
5758
.optional(),
5859
})

extensions/mattermost/src/mattermost/interactions.test.ts

Lines changed: 184 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
import { type IncomingMessage, type ServerResponse } from "node:http";
2-
import { describe, expect, it, beforeEach, afterEach } from "vitest";
2+
import { describe, expect, it, beforeEach, afterEach, vi } from "vitest";
33
import { setMattermostRuntime } from "../runtime.js";
44
import { resolveMattermostAccount } from "./accounts.js";
55
import type { MattermostClient } from "./client.js";
@@ -109,6 +109,53 @@ describe("generateInteractionToken / verifyInteractionToken", () => {
109109
expect(verifyInteractionToken(reorderedContext, token)).toBe(true);
110110
});
111111

112+
it("verifies nested context regardless of nested key order", () => {
113+
const originalContext = {
114+
action_id: "nested",
115+
payload: {
116+
model: "gpt-5",
117+
meta: {
118+
provider: "openai",
119+
page: 2,
120+
},
121+
},
122+
};
123+
const token = generateInteractionToken(originalContext);
124+
125+
const reorderedContext = {
126+
payload: {
127+
meta: {
128+
page: 2,
129+
provider: "openai",
130+
},
131+
model: "gpt-5",
132+
},
133+
action_id: "nested",
134+
};
135+
136+
expect(verifyInteractionToken(reorderedContext, token)).toBe(true);
137+
});
138+
139+
it("rejects nested context tampering", () => {
140+
const originalContext = {
141+
action_id: "nested",
142+
payload: {
143+
provider: "openai",
144+
model: "gpt-5",
145+
},
146+
};
147+
const token = generateInteractionToken(originalContext);
148+
const tamperedContext = {
149+
action_id: "nested",
150+
payload: {
151+
provider: "anthropic",
152+
model: "gpt-5",
153+
},
154+
};
155+
156+
expect(verifyInteractionToken(tamperedContext, token)).toBe(false);
157+
});
158+
112159
it("scopes tokens per account when account secrets differ", () => {
113160
setInteractionSecret("acct-a", "bot-token-a");
114161
setInteractionSecret("acct-b", "bot-token-b");
@@ -400,12 +447,14 @@ describe("createMattermostInteractionHandler", () => {
400447
method?: string;
401448
body?: unknown;
402449
remoteAddress?: string;
450+
headers?: Record<string, string>;
403451
}): IncomingMessage {
404452
const body = params.body === undefined ? "" : JSON.stringify(params.body);
405453
const listeners = new Map<string, Array<(...args: unknown[]) => void>>();
406454

407455
const req = {
408456
method: params.method ?? "POST",
457+
headers: params.headers ?? {},
409458
socket: { remoteAddress: params.remoteAddress ?? "203.0.113.10" },
410459
on(event: string, handler: (...args: unknown[]) => void) {
411460
const existing = listeners.get(event) ?? [];
@@ -447,7 +496,7 @@ describe("createMattermostInteractionHandler", () => {
447496
return res as unknown as ServerResponse & { headers: Record<string, string>; body: string };
448497
}
449498

450-
it("accepts non-localhost requests when the interaction token is valid", async () => {
499+
it("accepts callback requests from an allowlisted source IP", async () => {
451500
const context = { action_id: "approve", __openclaw_channel_id: "chan-1" };
452501
const token = generateInteractionToken(context, "acct");
453502
const requestLog: Array<{ path: string; method?: string }> = [];
@@ -469,6 +518,7 @@ describe("createMattermostInteractionHandler", () => {
469518
} as unknown as MattermostClient,
470519
botUserId: "bot",
471520
accountId: "acct",
521+
allowedSourceIps: ["198.51.100.8"],
472522
});
473523

474524
const req = createReq({
@@ -493,6 +543,80 @@ describe("createMattermostInteractionHandler", () => {
493543
]);
494544
});
495545

546+
it("accepts forwarded Mattermost source IPs from a trusted proxy", async () => {
547+
const context = { action_id: "approve", __openclaw_channel_id: "chan-1" };
548+
const token = generateInteractionToken(context, "acct");
549+
const handler = createMattermostInteractionHandler({
550+
client: {
551+
request: async (_path: string, init?: { method?: string }) => {
552+
if (init?.method === "PUT") {
553+
return { id: "post-1" };
554+
}
555+
return {
556+
channel_id: "chan-1",
557+
message: "Choose",
558+
props: {
559+
attachments: [{ actions: [{ id: "approve", name: "Approve" }] }],
560+
},
561+
};
562+
},
563+
} as unknown as MattermostClient,
564+
botUserId: "bot",
565+
accountId: "acct",
566+
allowedSourceIps: ["198.51.100.8"],
567+
trustedProxies: ["127.0.0.1"],
568+
});
569+
570+
const req = createReq({
571+
remoteAddress: "127.0.0.1",
572+
headers: { "x-forwarded-for": "198.51.100.8" },
573+
body: {
574+
user_id: "user-1",
575+
user_name: "alice",
576+
channel_id: "chan-1",
577+
post_id: "post-1",
578+
context: { ...context, _token: token },
579+
},
580+
});
581+
const res = createRes();
582+
583+
await handler(req, res);
584+
585+
expect(res.statusCode).toBe(200);
586+
expect(res.body).toBe("{}");
587+
});
588+
589+
it("rejects callback requests from non-allowlisted source IPs", async () => {
590+
const context = { action_id: "approve", __openclaw_channel_id: "chan-1" };
591+
const token = generateInteractionToken(context, "acct");
592+
const handler = createMattermostInteractionHandler({
593+
client: {
594+
request: async () => {
595+
throw new Error("should not fetch post for rejected origins");
596+
},
597+
} as unknown as MattermostClient,
598+
botUserId: "bot",
599+
accountId: "acct",
600+
allowedSourceIps: ["127.0.0.1"],
601+
});
602+
603+
const req = createReq({
604+
remoteAddress: "198.51.100.8",
605+
body: {
606+
user_id: "user-1",
607+
channel_id: "chan-1",
608+
post_id: "post-1",
609+
context: { ...context, _token: token },
610+
},
611+
});
612+
const res = createRes();
613+
614+
await handler(req, res);
615+
616+
expect(res.statusCode).toBe(403);
617+
expect(res.body).toContain("Forbidden origin");
618+
});
619+
496620
it("rejects requests with an invalid interaction token", async () => {
497621
const handler = createMattermostInteractionHandler({
498622
client: {
@@ -610,4 +734,62 @@ describe("createMattermostInteractionHandler", () => {
610734
expect(res.statusCode).toBe(403);
611735
expect(res.body).toContain("Unknown action");
612736
});
737+
738+
it("lets a custom interaction handler short-circuit generic completion updates", async () => {
739+
const context = { action_id: "mdlprov", __openclaw_channel_id: "chan-1" };
740+
const token = generateInteractionToken(context, "acct");
741+
const requestLog: Array<{ path: string; method?: string }> = [];
742+
const handleInteraction = vi.fn().mockResolvedValue({
743+
ephemeral_text: "Only the original requester can use this picker.",
744+
});
745+
const dispatchButtonClick = vi.fn();
746+
const handler = createMattermostInteractionHandler({
747+
client: {
748+
request: async (path: string, init?: { method?: string }) => {
749+
requestLog.push({ path, method: init?.method });
750+
return {
751+
channel_id: "chan-1",
752+
message: "Choose",
753+
props: {
754+
attachments: [{ actions: [{ id: "mdlprov", name: "Browse providers" }] }],
755+
},
756+
};
757+
},
758+
} as unknown as MattermostClient,
759+
botUserId: "bot",
760+
accountId: "acct",
761+
handleInteraction,
762+
dispatchButtonClick,
763+
});
764+
765+
const req = createReq({
766+
body: {
767+
user_id: "user-2",
768+
user_name: "alice",
769+
channel_id: "chan-1",
770+
post_id: "post-1",
771+
context: { ...context, _token: token },
772+
},
773+
});
774+
const res = createRes();
775+
776+
await handler(req, res);
777+
778+
expect(res.statusCode).toBe(200);
779+
expect(res.body).toBe(
780+
JSON.stringify({
781+
ephemeral_text: "Only the original requester can use this picker.",
782+
}),
783+
);
784+
expect(requestLog).toEqual([{ path: "/posts/post-1", method: undefined }]);
785+
expect(handleInteraction).toHaveBeenCalledWith(
786+
expect.objectContaining({
787+
actionId: "mdlprov",
788+
actionName: "Browse providers",
789+
originalMessage: "Choose",
790+
userName: "alice",
791+
}),
792+
);
793+
expect(dispatchButtonClick).not.toHaveBeenCalled();
794+
});
613795
});

0 commit comments

Comments
 (0)