mirror of
https://github.com/openclaw/openclaw.git
synced 2026-03-12 07:20:45 +00:00
fix(msteams): use General channel conversation ID as team key for Bot Framework compatibility (#41838)
* fix(msteams): use General channel conversation ID as team key for Bot Framework compatibility Bot Framework sends `activity.channelData.team.id` as the General channel's conversation ID (e.g. `19:abc@thread.tacv2`), not the Graph API group GUID (e.g. `fa101332-cf00-431b-b0ea-f701a85fde81`). The startup resolver was storing the Graph GUID as the team config key, so runtime matching always failed and every channel message was silently dropped. Fix: always call `listChannelsForTeam` during resolution to find the General channel, then use its conversation ID as the stored `teamId`. When a specific channel is also configured, reuse the same channel list rather than issuing a second API call. Falls back to the Graph GUID if the General channel cannot be found (renamed/deleted edge case). Fixes #41390 * fix(msteams): handle listChannelsForTeam failure gracefully * fix(msteams): trim General channel ID and guard against empty string * fix: document MS Teams allowlist team-key fix (#41838) (thanks @BradGroux) --------- Co-authored-by: bradgroux <bradgroux@users.noreply.github.com> Co-authored-by: Onur <onur@textcortex.com>
This commit is contained in:
@@ -54,10 +54,12 @@ describe("resolveMSTeamsUserAllowlist", () => {
|
||||
|
||||
describe("resolveMSTeamsChannelAllowlist", () => {
|
||||
it("resolves team/channel by team name + channel display name", async () => {
|
||||
listTeamsByName.mockResolvedValueOnce([{ id: "team-1", displayName: "Product Team" }]);
|
||||
// After the fix, listChannelsForTeam is called once and reused for both
|
||||
// General channel resolution and channel matching.
|
||||
listTeamsByName.mockResolvedValueOnce([{ id: "team-guid-1", displayName: "Product Team" }]);
|
||||
listChannelsForTeam.mockResolvedValueOnce([
|
||||
{ id: "channel-1", displayName: "General" },
|
||||
{ id: "channel-2", displayName: "Roadmap" },
|
||||
{ id: "19:general-conv-id@thread.tacv2", displayName: "General" },
|
||||
{ id: "19:roadmap-conv-id@thread.tacv2", displayName: "Roadmap" },
|
||||
]);
|
||||
|
||||
const [result] = await resolveMSTeamsChannelAllowlist({
|
||||
@@ -65,14 +67,80 @@ describe("resolveMSTeamsChannelAllowlist", () => {
|
||||
entries: ["Product Team/Roadmap"],
|
||||
});
|
||||
|
||||
// teamId is now the General channel's conversation ID — not the Graph GUID —
|
||||
// because that's what Bot Framework sends as channelData.team.id at runtime.
|
||||
expect(result).toEqual({
|
||||
input: "Product Team/Roadmap",
|
||||
resolved: true,
|
||||
teamId: "team-1",
|
||||
teamId: "19:general-conv-id@thread.tacv2",
|
||||
teamName: "Product Team",
|
||||
channelId: "channel-2",
|
||||
channelId: "19:roadmap-conv-id@thread.tacv2",
|
||||
channelName: "Roadmap",
|
||||
note: "multiple channels; chose first",
|
||||
});
|
||||
});
|
||||
|
||||
it("uses General channel conversation ID as team key for team-only entry", async () => {
|
||||
// When no channel is specified we still resolve the General channel so the
|
||||
// stored key matches what Bot Framework sends as channelData.team.id.
|
||||
listTeamsByName.mockResolvedValueOnce([{ id: "guid-engineering", displayName: "Engineering" }]);
|
||||
listChannelsForTeam.mockResolvedValueOnce([
|
||||
{ id: "19:eng-general@thread.tacv2", displayName: "General" },
|
||||
{ id: "19:eng-standups@thread.tacv2", displayName: "Standups" },
|
||||
]);
|
||||
|
||||
const [result] = await resolveMSTeamsChannelAllowlist({
|
||||
cfg: {},
|
||||
entries: ["Engineering"],
|
||||
});
|
||||
|
||||
expect(result).toEqual({
|
||||
input: "Engineering",
|
||||
resolved: true,
|
||||
teamId: "19:eng-general@thread.tacv2",
|
||||
teamName: "Engineering",
|
||||
});
|
||||
});
|
||||
|
||||
it("falls back to Graph GUID when listChannelsForTeam throws", async () => {
|
||||
// Edge case: API call fails (rate limit, network error). We fall back to
|
||||
// the Graph GUID as the team key — the pre-fix behavior — so resolution
|
||||
// still succeeds instead of propagating the error.
|
||||
listTeamsByName.mockResolvedValueOnce([{ id: "guid-flaky", displayName: "Flaky Team" }]);
|
||||
listChannelsForTeam.mockRejectedValueOnce(new Error("429 Too Many Requests"));
|
||||
|
||||
const [result] = await resolveMSTeamsChannelAllowlist({
|
||||
cfg: {},
|
||||
entries: ["Flaky Team"],
|
||||
});
|
||||
|
||||
expect(result).toEqual({
|
||||
input: "Flaky Team",
|
||||
resolved: true,
|
||||
teamId: "guid-flaky",
|
||||
teamName: "Flaky Team",
|
||||
});
|
||||
});
|
||||
|
||||
it("falls back to Graph GUID when General channel is not found", async () => {
|
||||
// Edge case: General channel was renamed or deleted. We fall back to the
|
||||
// Graph GUID so resolution still succeeds rather than silently breaking.
|
||||
listTeamsByName.mockResolvedValueOnce([{ id: "guid-ops", displayName: "Operations" }]);
|
||||
listChannelsForTeam.mockResolvedValueOnce([
|
||||
{ id: "19:ops-announce@thread.tacv2", displayName: "Announcements" },
|
||||
{ id: "19:ops-random@thread.tacv2", displayName: "Random" },
|
||||
]);
|
||||
|
||||
const [result] = await resolveMSTeamsChannelAllowlist({
|
||||
cfg: {},
|
||||
entries: ["Operations"],
|
||||
});
|
||||
|
||||
expect(result).toEqual({
|
||||
input: "Operations",
|
||||
resolved: true,
|
||||
teamId: "guid-ops",
|
||||
teamName: "Operations",
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
@@ -120,11 +120,26 @@ export async function resolveMSTeamsChannelAllowlist(params: {
|
||||
return { input, resolved: false, note: "team not found" };
|
||||
}
|
||||
const teamMatch = teams[0];
|
||||
const teamId = teamMatch.id?.trim();
|
||||
const graphTeamId = teamMatch.id?.trim();
|
||||
const teamName = teamMatch.displayName?.trim() || team;
|
||||
if (!teamId) {
|
||||
if (!graphTeamId) {
|
||||
return { input, resolved: false, note: "team id missing" };
|
||||
}
|
||||
// Bot Framework sends the General channel's conversation ID as
|
||||
// channelData.team.id at runtime, NOT the Graph API group GUID.
|
||||
// Fetch channels upfront so we can resolve the correct key format for
|
||||
// runtime matching and reuse the list for channel lookups.
|
||||
let teamChannels: Awaited<ReturnType<typeof listChannelsForTeam>> = [];
|
||||
try {
|
||||
teamChannels = await listChannelsForTeam(token, graphTeamId);
|
||||
} catch {
|
||||
// API failure (rate limit, network error) — fall back to Graph GUID as team key
|
||||
}
|
||||
const generalChannel = teamChannels.find((ch) => ch.displayName?.toLowerCase() === "general");
|
||||
// Use the General channel's conversation ID as the team key — this
|
||||
// matches what Bot Framework sends at runtime. Fall back to the Graph
|
||||
// GUID if the General channel isn't found (renamed or deleted).
|
||||
const teamId = generalChannel?.id?.trim() || graphTeamId;
|
||||
if (!channel) {
|
||||
return {
|
||||
input,
|
||||
@@ -134,11 +149,11 @@ export async function resolveMSTeamsChannelAllowlist(params: {
|
||||
note: teams.length > 1 ? "multiple teams; chose first" : undefined,
|
||||
};
|
||||
}
|
||||
const channels = await listChannelsForTeam(token, teamId);
|
||||
// Reuse teamChannels — already fetched above
|
||||
const channelMatch =
|
||||
channels.find((item) => item.id === channel) ??
|
||||
channels.find((item) => item.displayName?.toLowerCase() === channel.toLowerCase()) ??
|
||||
channels.find((item) =>
|
||||
teamChannels.find((item) => item.id === channel) ??
|
||||
teamChannels.find((item) => item.displayName?.toLowerCase() === channel.toLowerCase()) ??
|
||||
teamChannels.find((item) =>
|
||||
item.displayName?.toLowerCase().includes(channel.toLowerCase() ?? ""),
|
||||
);
|
||||
if (!channelMatch?.id) {
|
||||
@@ -151,7 +166,7 @@ export async function resolveMSTeamsChannelAllowlist(params: {
|
||||
teamName,
|
||||
channelId: channelMatch.id,
|
||||
channelName: channelMatch.displayName ?? channel,
|
||||
note: channels.length > 1 ? "multiple channels; chose first" : undefined,
|
||||
note: teamChannels.length > 1 ? "multiple channels; chose first" : undefined,
|
||||
};
|
||||
},
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user