Skip to content

Commit 0872d85

Browse files
LiuwqGitclaude
LiuwqGit
and
claude
committed
fix(msteams): paginate thread replies to include newest context (#98870)
fetchThreadReplies now uses fetchAllGraphPages to walk all reply pages via @odata.nextLink, then selects the newest limit replies by createdDateTime. Bounded at MAX_REPLY_PAGES=50 (up to 2500 replies). Threads with ≤50 replies are unaffected. Closes #98870 Co-Authored-By: Claude Haiku 4.5 <[email protected]>
1 parent 8b84512 commit 0872d85

3 files changed

Lines changed: 874 additions & 0 deletions

File tree

‎extensions/msteams/src/graph-thread.test.ts‎

Lines changed: 374 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,374 @@
1+
// Msteams tests cover graph thread plugin behavior.
2+
import { beforeEach, describe, expect, it, vi } from "vitest";
3+
import {
4+
_teamGroupIdCacheForTest,
5+
fetchChannelMessage,
6+
fetchThreadReplies,
7+
formatThreadContext,
8+
resolveTeamGroupId,
9+
stripHtmlFromTeamsMessage,
10+
} from "./graph-thread.js";
11+
import { fetchGraphJson } from "./graph.js";
12+
13+
// Mock fetchAllGraphPages follows @odata.nextLink across pages, calling fetchGraphJson.
14+
vi.mock("./graph.js", () => {
15+
const mockFetch = vi.fn();
16+
return {
17+
fetchGraphJson: mockFetch,
18+
fetchAllGraphPages: vi.fn(async <T>(params: {
19+
token: string;
20+
path: string;
21+
headers?: Record<string, string>;
22+
maxPages?: number;
23+
}) => {
24+
const items: T[] = [];
25+
const maxPages = params.maxPages ?? 50;
26+
let nextPath: string | undefined = params.path;
27+
const callArgs: Record<string, unknown> = { token: params.token, path: nextPath };
28+
if (params.headers) callArgs.headers = params.headers;
29+
for (let page = 0; page < maxPages && nextPath; page++) {
30+
const res = await mockFetch(callArgs) as { value?: T[]; "@odata.nextLink"?: string };
31+
const pageItems = res?.value ?? [];
32+
items.push(...pageItems);
33+
const rawNext = res?.["@odata.nextLink"];
34+
if (rawNext) {
35+
nextPath = rawNext
36+
.replace("https://graph.microsoft.com/v1.0", "")
37+
.replace("https://graph.microsoft.com/beta", "");
38+
callArgs.path = nextPath;
39+
} else {
40+
nextPath = undefined;
41+
}
42+
}
43+
return { items, truncated: false };
44+
}),
45+
};
46+
});
47+
48+
const firstGraphPath = () => {
49+
const [call] = vi.mocked(fetchGraphJson).mock.calls;
50+
if (!call) {
51+
throw new Error("expected Graph fetch call");
52+
}
53+
return call[0].path;
54+
};
55+
56+
describe("stripHtmlFromTeamsMessage", () => {
57+
it("preserves @mention display names from <at> tags", () => {
58+
expect(stripHtmlFromTeamsMessage("<at>Alice</at> hello")).toBe("@Alice hello");
59+
});
60+
61+
it("strips other HTML tags", () => {
62+
expect(stripHtmlFromTeamsMessage("<p>Hello <b>world</b></p>")).toBe("Hello world");
63+
});
64+
65+
it("decodes common HTML entities", () => {
66+
expect(stripHtmlFromTeamsMessage("&amp; &lt;b&gt; &quot;x&quot; &#39;y&#39; &nbsp;z")).toBe(
67+
"& <b> \"x\" 'y' z",
68+
);
69+
});
70+
71+
it("does not double-decode escaped entities (decodes &amp; last)", () => {
72+
// Graph encodes literally-typed entity text by escaping its '&' to '&amp;'.
73+
// Decoding '&amp;' first would re-decode the now-bare '&lt;'/'&gt;' into
74+
// angle brackets, corrupting the user's literal text.
75+
expect(stripHtmlFromTeamsMessage("The token is &amp;lt;APIKEY&amp;gt;")).toBe(
76+
"The token is &lt;APIKEY&gt;",
77+
);
78+
});
79+
80+
it("normalizes multiple whitespace to single space", () => {
81+
expect(stripHtmlFromTeamsMessage("hello world")).toBe("hello world");
82+
});
83+
84+
it("handles <at> tags with attributes", () => {
85+
expect(stripHtmlFromTeamsMessage('<at id="123">Bob</at> please review')).toBe(
86+
"@Bob please review",
87+
);
88+
});
89+
90+
it("returns empty string for empty input", () => {
91+
expect(stripHtmlFromTeamsMessage("")).toBe("");
92+
});
93+
});
94+
95+
describe("resolveTeamGroupId", () => {
96+
beforeEach(() => {
97+
vi.mocked(fetchGraphJson).mockReset();
98+
_teamGroupIdCacheForTest.clear();
99+
});
100+
101+
it("fetches team id from Graph and caches it", async () => {
102+
vi.mocked(fetchGraphJson).mockResolvedValueOnce({ id: "group-guid-1" } as never);
103+
104+
const result = await resolveTeamGroupId("tok", "team-123");
105+
expect(result).toBe("group-guid-1");
106+
expect(fetchGraphJson).toHaveBeenCalledWith({
107+
token: "tok",
108+
path: "/teams/team-123?$select=id",
109+
});
110+
});
111+
112+
it("returns cached value without calling Graph again", async () => {
113+
vi.mocked(fetchGraphJson).mockResolvedValueOnce({ id: "group-guid-2" } as never);
114+
115+
await resolveTeamGroupId("tok", "team-456");
116+
await resolveTeamGroupId("tok", "team-456");
117+
118+
expect(fetchGraphJson).toHaveBeenCalledTimes(1);
119+
});
120+
121+
it("does not cache team ids when the expiry would exceed a valid Date", async () => {
122+
vi.useFakeTimers();
123+
vi.setSystemTime(new Date(8_640_000_000_000_000));
124+
try {
125+
vi.mocked(fetchGraphJson).mockResolvedValue({ id: "group-guid-boundary" } as never);
126+
127+
await resolveTeamGroupId("tok", "team-boundary");
128+
await resolveTeamGroupId("tok", "team-boundary");
129+
130+
expect(fetchGraphJson).toHaveBeenCalledTimes(2);
131+
} finally {
132+
vi.useRealTimers();
133+
}
134+
});
135+
136+
it("evicts cached team ids when the current clock is invalid", async () => {
137+
vi.mocked(fetchGraphJson).mockResolvedValue({ id: "group-guid-invalid-clock" } as never);
138+
139+
await resolveTeamGroupId("tok", "team-invalid-clock");
140+
const dateNow = vi.spyOn(Date, "now").mockReturnValue(Number.NaN);
141+
try {
142+
await resolveTeamGroupId("tok", "team-invalid-clock");
143+
} finally {
144+
dateNow.mockRestore();
145+
}
146+
147+
expect(fetchGraphJson).toHaveBeenCalledTimes(2);
148+
});
149+
150+
it("falls back to conversationTeamId when Graph returns no id", async () => {
151+
vi.mocked(fetchGraphJson).mockResolvedValueOnce({} as never);
152+
153+
const result = await resolveTeamGroupId("tok", "team-fallback");
154+
expect(result).toBe("team-fallback");
155+
});
156+
});
157+
158+
describe("fetchChannelMessage", () => {
159+
beforeEach(() => {
160+
vi.mocked(fetchGraphJson).mockReset();
161+
});
162+
163+
it("fetches the parent message with correct path", async () => {
164+
const mockMsg = { id: "msg-1", body: { content: "hello", contentType: "text" } };
165+
vi.mocked(fetchGraphJson).mockResolvedValueOnce(mockMsg as never);
166+
167+
const result = await fetchChannelMessage("tok", "group-1", "channel-1", "msg-1");
168+
169+
expect(result).toEqual(mockMsg);
170+
expect(fetchGraphJson).toHaveBeenCalledWith({
171+
token: "tok",
172+
path: "/teams/group-1/channels/channel-1/messages/msg-1?$select=id,from,body,createdDateTime",
173+
});
174+
});
175+
176+
it("returns undefined on fetch error", async () => {
177+
vi.mocked(fetchGraphJson).mockRejectedValueOnce(new Error("forbidden") as never);
178+
179+
const result = await fetchChannelMessage("tok", "group-1", "channel-1", "msg-1");
180+
expect(result).toBeUndefined();
181+
});
182+
183+
it("URL-encodes group, channel, and message IDs", async () => {
184+
vi.mocked(fetchGraphJson).mockResolvedValueOnce({} as never);
185+
186+
await fetchChannelMessage("tok", "g/1", "c/2", "m/3");
187+
188+
expect(fetchGraphJson).toHaveBeenCalledWith({
189+
token: "tok",
190+
path: "/teams/g%2F1/channels/c%2F2/messages/m%2F3?$select=id,from,body,createdDateTime",
191+
});
192+
});
193+
});
194+
195+
describe("fetchThreadReplies", () => {
196+
beforeEach(() => {
197+
vi.mocked(fetchGraphJson).mockReset();
198+
});
199+
200+
it("fetches replies with correct path and default limit", async () => {
201+
vi.mocked(fetchGraphJson).mockResolvedValueOnce({
202+
value: [{ id: "reply-1" }, { id: "reply-2" }],
203+
} as never);
204+
205+
const result = await fetchThreadReplies("tok", "group-1", "channel-1", "msg-1");
206+
207+
expect(result).toHaveLength(2);
208+
expect(fetchGraphJson).toHaveBeenCalledWith({
209+
token: "tok",
210+
path: "/teams/group-1/channels/channel-1/messages/msg-1/replies?$top=50&$select=id,from,body,createdDateTime",
211+
});
212+
});
213+
214+
it("clamps limit to 50 maximum", async () => {
215+
vi.mocked(fetchGraphJson).mockResolvedValueOnce({ value: [] } as never);
216+
217+
await fetchThreadReplies("tok", "g", "c", "m", 200);
218+
219+
expect(firstGraphPath()).toContain("$top=50");
220+
});
221+
222+
it("clamps limit to 1 minimum", async () => {
223+
vi.mocked(fetchGraphJson).mockResolvedValueOnce({ value: [] } as never);
224+
225+
await fetchThreadReplies("tok", "g", "c", "m", 0);
226+
227+
expect(firstGraphPath()).toContain("$top=1");
228+
});
229+
230+
it("returns empty array when value is missing", async () => {
231+
vi.mocked(fetchGraphJson).mockResolvedValueOnce({} as never);
232+
233+
const result = await fetchThreadReplies("tok", "g", "c", "m");
234+
expect(result).toStrictEqual([]);
235+
});
236+
237+
it("paginates through @odata.nextLink and returns newest 50 replies from 60", async () => {
238+
// First page: replies 1-50 (oldest), with nextLink
239+
const page1 = Array.from({ length: 50 }, (_, i) => ({
240+
id: `reply-${i + 1}`,
241+
from: { user: { displayName: `User${i + 1}` } },
242+
body: { content: `msg ${i + 1}`, contentType: "text" },
243+
createdDateTime: `2026-06-01T00:${String(i).padStart(2, "0")}:00Z`,
244+
}));
245+
// Second page: replies 51-60 (newest), no nextLink
246+
const page2 = Array.from({ length: 10 }, (_, i) => ({
247+
id: `reply-${51 + i}`,
248+
from: { user: { displayName: `User${51 + i}` } },
249+
body: { content: `msg ${51 + i}`, contentType: "text" },
250+
createdDateTime: `2026-06-01T00:${String(50 + i).padStart(2, "0")}:00Z`,
251+
}));
252+
253+
vi.mocked(fetchGraphJson)
254+
.mockResolvedValueOnce({
255+
value: page1,
256+
"@odata.nextLink": "https://graph.microsoft.com/v1.0/teams/g/channels/c/messages/m/replies?$skip=50&$top=50",
257+
} as never)
258+
.mockResolvedValueOnce({
259+
value: page2,
260+
} as never);
261+
262+
const result = await fetchThreadReplies("tok", "g", "c", "m");
263+
264+
expect(result).toHaveLength(50);
265+
// The newest 50 should be replies 11-60 (oldest 10 dropped)
266+
expect(result[0].id).toBe("reply-11");
267+
expect(result[49].id).toBe("reply-60");
268+
expect(fetchGraphJson).toHaveBeenCalledTimes(2);
269+
});
270+
271+
it("returns all replies when total is within limit (no pagination needed)", async () => {
272+
const replies = Array.from({ length: 30 }, (_, i) => ({
273+
id: `reply-${i + 1}`,
274+
from: { user: { displayName: `User${i + 1}` } },
275+
body: { content: `msg ${i + 1}`, contentType: "text" },
276+
createdDateTime: `2026-06-01T00:${String(i).padStart(2, "0")}:00Z`,
277+
}));
278+
279+
vi.mocked(fetchGraphJson).mockResolvedValueOnce({ value: replies } as never);
280+
281+
const result = await fetchThreadReplies("tok", "g", "c", "m");
282+
283+
expect(result).toHaveLength(30);
284+
// Items returned in chronological order (unchanged)
285+
expect(result[0].id).toBe("reply-1");
286+
expect(result[29].id).toBe("reply-30");
287+
});
288+
});
289+
290+
describe("formatThreadContext", () => {
291+
it("formats messages as sender: content lines", () => {
292+
const messages = [
293+
{
294+
id: "m1",
295+
from: { user: { displayName: "Alice" } },
296+
body: { content: "Hello!", contentType: "text" },
297+
},
298+
{
299+
id: "m2",
300+
from: { user: { displayName: "Bob" } },
301+
body: { content: "World!", contentType: "text" },
302+
},
303+
];
304+
expect(formatThreadContext(messages)).toBe("Alice: Hello!\nBob: World!");
305+
});
306+
307+
it("skips the current message by id", () => {
308+
const messages = [
309+
{
310+
id: "m1",
311+
from: { user: { displayName: "Alice" } },
312+
body: { content: "Hello!", contentType: "text" },
313+
},
314+
{
315+
id: "m2",
316+
from: { user: { displayName: "Bob" } },
317+
body: { content: "Current", contentType: "text" },
318+
},
319+
];
320+
expect(formatThreadContext(messages, "m2")).toBe("Alice: Hello!");
321+
});
322+
323+
it("strips HTML from html contentType messages", () => {
324+
const messages = [
325+
{
326+
id: "m1",
327+
from: { user: { displayName: "Carol" } },
328+
body: { content: "<p>Hello <b>world</b></p>", contentType: "html" },
329+
},
330+
];
331+
expect(formatThreadContext(messages)).toBe("Carol: Hello world");
332+
});
333+
334+
it("uses application displayName when user is absent", () => {
335+
const messages = [
336+
{
337+
id: "m1",
338+
from: { application: { displayName: "BotApp" } },
339+
body: { content: "automated msg", contentType: "text" },
340+
},
341+
];
342+
expect(formatThreadContext(messages)).toBe("BotApp: automated msg");
343+
});
344+
345+
it("skips messages with empty content", () => {
346+
const messages = [
347+
{
348+
id: "m1",
349+
from: { user: { displayName: "Alice" } },
350+
body: { content: "", contentType: "text" },
351+
},
352+
{
353+
id: "m2",
354+
from: { user: { displayName: "Bob" } },
355+
body: { content: "actual content", contentType: "text" },
356+
},
357+
];
358+
expect(formatThreadContext(messages)).toBe("Bob: actual content");
359+
});
360+
361+
it("falls back to 'unknown' sender when from is missing", () => {
362+
const messages = [
363+
{
364+
id: "m1",
365+
body: { content: "orphan msg", contentType: "text" },
366+
},
367+
];
368+
expect(formatThreadContext(messages)).toBe("unknown: orphan msg");
369+
});
370+
371+
it("returns empty string for empty messages array", () => {
372+
expect(formatThreadContext([])).toBe("");
373+
});
374+
});

0 commit comments

Comments
 (0)