Skip to content

Commit e84a0dd

Browse files
velanir-ai-managerYash Inaniclaudesteipete
authored
fix(msteams): surface quoted message body in Teams quote replies (#101856)
* fix(msteams): surface quoted message body in Teams quote replies Teams sends the quoted text of a 1:1 DM quote-reply in <p itemprop="preview"> (a truncated snippet), not <p itemprop="copy"> which the parser matched. The match failed, so quoteInfo was undefined and nothing was surfaced to the agent. Fix 1 (primary): extractMSTeamsQuoteInfo now accepts copy OR preview (prefers copy, falls back to preview) and captures the blockquote itemid as the quoted message id. Fix 2 (enhancement): when the quoted message id is known, fetch the full text via the app-only Graph endpoint GET /chats/{chatId}/messages/{id} (permitted with Chat.Read.All, unlike the delegated /me/chats listing) and use it as the quote body. Restricted to chats (DM + group) whose Graph chat id is a 19: id; any failure degrades to the truncated preview so message handling never breaks. Co-Authored-By: Claude Opus 4.8 <[email protected]> * fix(msteams): address review — DM-only full-text fetch, drop unsupported $select, fix mocks Addresses ClawSweeper review on the quote-reply PR: - Security (P1): restrict the app-only Graph full-text quote fetch to 1:1 DMs. Previously it ran for any non-channel chat, so in a group an allowlisted sender could quote a non-allowlisted member and the fetched full body would bypass the supplemental-quote visibility allowlist. Group/channel quotes keep the (now-surfaced) truncated preview from fix 1. - Graph contract (P2): the get-chatMessage endpoint does not support OData query params; drop the ?$select=id,body that tenants enforcing the contract would reject (which would silently fall back to the preview). - Tests (P1): add fetchChatMessageText to the two vi.mock(../graph-thread.js) factories prod now touches, and add a group-chat regression test proving the Graph full-text fetch does not fire for group quotes. Co-Authored-By: Claude Opus 4.8 <[email protected]> * docs(msteams): add redacted real Teams quote-reply proof screenshots Real-behavior evidence for the quote-reply fix (personal names + avatars redacted): a 1:1 DM where the bot now reads back the full quoted message. Co-Authored-By: Claude Opus 4.8 <[email protected]> * docs(msteams): remove committed proof screenshots from branch Proof media should live as external PR artifacts, not in the product tree. Co-Authored-By: Claude Opus 4.8 <[email protected]> * test(msteams): bound and prove quote enrichment * test(msteams): use valid open DM fixture --------- Co-authored-by: Yash Inani <[email protected]> Co-authored-by: Claude Opus 4.8 <[email protected]> Co-authored-by: Peter Steinberger <[email protected]>
1 parent ac71074 commit e84a0dd

9 files changed

Lines changed: 297 additions & 10 deletions

File tree

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

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ import { beforeEach, describe, expect, it, vi } from "vitest";
33
import {
44
_teamGroupIdCacheForTest,
55
fetchChannelMessage,
6+
fetchChatMessageText,
67
fetchThreadReplies,
78
formatThreadContext,
89
resolveTeamGroupId,
@@ -161,6 +162,53 @@ describe("fetchChannelMessage", () => {
161162
});
162163
});
163164

165+
describe("fetchChatMessageText", () => {
166+
beforeEach(() => {
167+
vi.mocked(fetchGraphJson).mockReset();
168+
});
169+
170+
it("fetches the chat message and strips HTML body to plain text", async () => {
171+
vi.mocked(fetchGraphJson).mockResolvedValueOnce({
172+
id: "1783379480258",
173+
body: {
174+
content: "<p>San Francisco right now: <at>Bot</at> full text</p>",
175+
contentType: "html",
176+
},
177+
} as never);
178+
179+
const result = await fetchChatMessageText("tok", "19:[email protected]", "1783379480258");
180+
181+
expect(result).toBe("San Francisco right now: @Bot full text");
182+
expect(fetchGraphJson).toHaveBeenCalledWith({
183+
token: "tok",
184+
path: "/chats/19%3Achat%40thread.v2/messages/1783379480258",
185+
});
186+
});
187+
188+
it("returns trimmed plain text when body is not HTML", async () => {
189+
vi.mocked(fetchGraphJson).mockResolvedValueOnce({
190+
body: { content: " plain body ", contentType: "text" },
191+
} as never);
192+
193+
const result = await fetchChatMessageText("tok", "19:chat", "m-1");
194+
expect(result).toBe("plain body");
195+
});
196+
197+
it("returns undefined on fetch error", async () => {
198+
vi.mocked(fetchGraphJson).mockRejectedValueOnce(new Error("not found") as never);
199+
200+
const result = await fetchChatMessageText("tok", "19:chat", "m-1");
201+
expect(result).toBeUndefined();
202+
});
203+
204+
it("returns undefined when the message has no body", async () => {
205+
vi.mocked(fetchGraphJson).mockResolvedValueOnce({} as never);
206+
207+
const result = await fetchChatMessageText("tok", "19:chat", "m-1");
208+
expect(result).toBeUndefined();
209+
});
210+
});
211+
164212
describe("fetchThreadReplies", () => {
165213
beforeEach(() => {
166214
vi.mocked(fetchGraphJson).mockReset();

extensions/msteams/src/graph-thread.ts

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -112,6 +112,36 @@ export async function fetchChannelMessage(
112112
}
113113
}
114114

115+
/**
116+
* Fetch a single chat message's full text via Graph and return plain text.
117+
*
118+
* Used to recover the complete quoted message for Teams quote replies: the
119+
* inbound blockquote only carries a Teams-truncated `preview` snippet. The
120+
* app-only `GET /chats/{chatId}/messages/{messageId}` endpoint IS permitted
121+
* with the `Chat.Read.All` application permission (unlike the delegated
122+
* `/me/chats` listing used by `resolveGraphChatId`, which 400s app-only).
123+
*
124+
* Returns undefined on any failure so callers degrade to the truncated preview.
125+
*/
126+
export async function fetchChatMessageText(
127+
token: string,
128+
chatId: string,
129+
messageId: string,
130+
): Promise<string | undefined> {
131+
// The get-chatMessage endpoint does not support OData query params (e.g.
132+
// `$select`); tenants that enforce the documented contract reject the request,
133+
// which would silently fall back to the truncated preview. Request it plainly.
134+
const path = `/chats/${encodeURIComponent(chatId)}/messages/${encodeURIComponent(messageId)}`;
135+
try {
136+
const msg = await fetchGraphJson<GraphThreadMessage>({ token, path });
137+
const raw = msg.body?.content ?? "";
138+
const text = msg.body?.contentType === "html" ? stripHtmlFromTeamsMessage(raw) : raw.trim();
139+
return text || undefined;
140+
} catch {
141+
return undefined;
142+
}
143+
}
144+
115145
/**
116146
* Fetch thread replies for a channel message, ordered chronologically.
117147
*

extensions/msteams/src/graph.test.ts

Lines changed: 27 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -4,12 +4,20 @@ import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
44
const {
55
loadMSTeamsSdkWithAuthMock,
66
createMSTeamsTokenProviderMock,
7+
fetchWithSsrFGuardMock,
78
readAccessTokenMock,
89
resolveMSTeamsCredentialsMock,
910
} = vi.hoisted(() => {
1011
return {
1112
loadMSTeamsSdkWithAuthMock: vi.fn(),
1213
createMSTeamsTokenProviderMock: vi.fn(),
14+
fetchWithSsrFGuardMock: vi.fn(
15+
async (params: { url: string; init?: RequestInit; timeoutMs?: number }) => ({
16+
response: await globalThis.fetch(params.url, params.init),
17+
finalUrl: params.url,
18+
release: async () => undefined,
19+
}),
20+
),
1321
readAccessTokenMock: vi.fn(),
1422
resolveMSTeamsCredentialsMock: vi.fn(),
1523
};
@@ -32,11 +40,7 @@ vi.mock("../runtime-api.js", async (importOriginal) => {
3240
const original = await importOriginal<typeof import("../runtime-api.js")>();
3341
return {
3442
...original,
35-
fetchWithSsrFGuard: async (params: { url: string; init?: RequestInit }) => ({
36-
response: await globalThis.fetch(params.url, params.init),
37-
finalUrl: params.url,
38-
release: async () => undefined,
39-
}),
43+
fetchWithSsrFGuard: fetchWithSsrFGuardMock,
4044
};
4145
});
4246

@@ -45,6 +49,7 @@ import {
4549
deleteGraphRequest,
4650
escapeOData,
4751
fetchAllGraphPages,
52+
fetchGraphAbsoluteUrl,
4853
fetchGraphJson,
4954
listChannelsForTeam,
5055
listTeamsByName,
@@ -231,6 +236,10 @@ describe("msteams graph helpers", () => {
231236
expect(fetchCallUrl(0)).toBe("https://graph.microsoft.com/v1.0/groups?$select=id");
232237
expect(fetchCallHeader(0, "Authorization")).toBe(`Bearer ${graphToken}`);
233238
expect(fetchCallHeader(0, "ConsistencyLevel")).toBe("eventual");
239+
expect(fetchWithSsrFGuardMock).toHaveBeenNthCalledWith(
240+
1,
241+
expect.objectContaining({ timeoutMs: 30_000 }),
242+
);
234243

235244
mockTextFetchResponse("forbidden", { status: 403 });
236245

@@ -270,6 +279,19 @@ describe("msteams graph helpers", () => {
270279
expect(arrayBuffer).not.toHaveBeenCalled();
271280
});
272281

282+
it("bounds absolute Graph pagination requests", async () => {
283+
mockGraphCollection(groupOne);
284+
285+
await fetchGraphAbsoluteUrl({
286+
token: graphToken,
287+
url: "https://graph.microsoft.com/v1.0/groups?$skiptoken=next",
288+
});
289+
290+
expect(fetchWithSsrFGuardMock).toHaveBeenCalledWith(
291+
expect.objectContaining({ timeoutMs: 30_000 }),
292+
);
293+
});
294+
273295
it("posts Graph JSON to v1 and beta roots and treats empty mutation responses as undefined", async () => {
274296
mockFetch(async (input) => {
275297
if (requestUrl(input).startsWith("https://graph.microsoft.com/beta")) {

extensions/msteams/src/graph.ts

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ import { resolveDelegatedAccessToken, resolveMSTeamsCredentials } from "./token.
1111
import { buildUserAgent } from "./user-agent.js";
1212

1313
const GRAPH_BETA = "https://graph.microsoft.com/beta";
14+
const GRAPH_REQUEST_TIMEOUT_MS = 30_000;
1415

1516
export type GraphUser = {
1617
id?: string;
@@ -65,6 +66,7 @@ async function requestGraph(params: {
6566
body: hasBody ? JSON.stringify(params.body) : undefined,
6667
},
6768
auditContext: "msteams.graph",
69+
timeoutMs: GRAPH_REQUEST_TIMEOUT_MS,
6870
});
6971
let releaseInFinally = true;
7072
try {
@@ -130,6 +132,7 @@ export async function fetchGraphAbsoluteUrl<T>(params: {
130132
},
131133
},
132134
auditContext: "msteams.graph.absolute",
135+
timeoutMs: GRAPH_REQUEST_TIMEOUT_MS,
133136
});
134137
try {
135138
if (!response.ok) {

extensions/msteams/src/inbound.test.ts

Lines changed: 67 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -218,5 +218,72 @@ describe("msteams inbound", () => {
218218
]);
219219
expect(result).toEqual({ sender: "Alice", body: "Hello world" });
220220
});
221+
222+
it("parses body from itemprop='preview' when 'copy' is absent", () => {
223+
const result = extractMSTeamsQuoteInfo([
224+
{
225+
contentType: "text/html",
226+
content:
227+
'<blockquote itemtype="http://schema.skype.com/Reply" itemscope>' +
228+
'<strong itemprop="mri">Frank</strong>' +
229+
'<p itemprop="preview">truncated snippet…</p>' +
230+
"</blockquote>",
231+
},
232+
]);
233+
expect(result?.body).toBe("truncated snippet…");
234+
expect(result?.sender).toBe("Frank");
235+
});
236+
237+
it("prefers 'copy' over 'preview' when both are present", () => {
238+
const result = extractMSTeamsQuoteInfo([
239+
{
240+
contentType: "text/html",
241+
content:
242+
'<blockquote itemtype="http://schema.skype.com/Reply" itemscope>' +
243+
'<strong itemprop="mri">Grace</strong>' +
244+
'<p itemprop="preview">short…</p>' +
245+
'<p itemprop="copy">the full text</p>' +
246+
"</blockquote>",
247+
},
248+
]);
249+
expect(result?.body).toBe("the full text");
250+
});
251+
252+
it("captures the blockquote itemid as the quoted message id", () => {
253+
const result = extractMSTeamsQuoteInfo([
254+
{
255+
contentType: "text/html",
256+
content:
257+
'<blockquote itemscope itemtype="http://schema.skype.com/Reply" itemid="1783379480258">' +
258+
'<strong itemprop="mri">Heidi</strong>' +
259+
'<p itemprop="preview">San Francisco right now…</p>' +
260+
"</blockquote>",
261+
},
262+
]);
263+
expect(result).toEqual({
264+
sender: "Heidi",
265+
body: "San Francisco right now…",
266+
id: "1783379480258",
267+
});
268+
});
269+
270+
it("parses a real Teams quote-reply payload (preview + itemid)", () => {
271+
const result = extractMSTeamsQuoteInfo([
272+
{
273+
contentType: "text/html",
274+
content:
275+
'<blockquote itemscope itemtype="http://schema.skype.com/Reply" itemid="1783379480258">' +
276+
'<strong itemprop="mri" itemid="28:abc">Display Name</strong>' +
277+
'<span itemprop="time" itemid="1783379480258"></span>' +
278+
'<p itemprop="preview">San Francisco right now ... Today\'s range: 54-64 °F (avg…</p>' +
279+
"</blockquote>\n<p>what abt not?</p>",
280+
},
281+
]);
282+
expect(result).toEqual({
283+
sender: "Display Name",
284+
body: "San Francisco right now ... Today's range: 54-64 °F (avg…",
285+
id: "1783379480258",
286+
});
287+
});
221288
});
222289
});

extensions/msteams/src/inbound.ts

Lines changed: 19 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,12 @@
22
type MSTeamsQuoteInfo = {
33
sender: string;
44
body: string;
5+
/**
6+
* The quoted message's Teams id (the blockquote `itemid`). Present when Teams
7+
* includes it; used to fetch the complete message text via Graph because the
8+
* inbound blockquote only carries a truncated `preview` snippet.
9+
*/
10+
id?: string;
511
};
612

713
/**
@@ -64,12 +70,22 @@ export function extractMSTeamsQuoteInfo(
6470
const senderMatch = /<strong[^>]*itemprop=["']mri["'][^>]*>(.*?)<\/strong>/i.exec(content);
6571
const sender = senderMatch?.[1] ? htmlToPlainText(senderMatch[1]) : undefined;
6672

67-
// Extract body from <p itemprop="copy">.
68-
const bodyMatch = /<p[^>]*itemprop=["']copy["'][^>]*>(.*?)<\/p>/is.exec(content);
73+
// Extract body from <p itemprop="copy"> (full quoted text) and fall back to
74+
// <p itemprop="preview"> — the truncated snippet Teams actually sends for
75+
// quote replies. Prefer `copy` when both are present.
76+
const copyMatch = /<p[^>]*itemprop=["']copy["'][^>]*>(.*?)<\/p>/is.exec(content);
77+
const bodyMatch =
78+
copyMatch ?? /<p[^>]*itemprop=["']preview["'][^>]*>(.*?)<\/p>/is.exec(content);
6979
const body = bodyMatch?.[1] ? htmlToPlainText(bodyMatch[1]) : undefined;
7080

81+
// Capture the blockquote `itemid` (the quoted message's Teams id) so callers
82+
// can fetch the complete message text via Graph when only a preview snippet
83+
// is available.
84+
const idMatch = /<blockquote[^>]*\bitemid=["']([^"']+)["'][^>]*>/is.exec(content);
85+
const id = idMatch?.[1]?.trim() || undefined;
86+
7187
if (body) {
72-
return { sender: sender ?? "unknown", body };
88+
return { sender: sender ?? "unknown", body, ...(id ? { id } : {}) };
7389
}
7490
}
7591
return undefined;

0 commit comments

Comments
 (0)