Skip to content

Commit fce81fc

Browse files
BradGrouxBradGroux
authored
msteams: add typingIndicator config and prevent duplicate DM typing indicator (#60771)
* msteams: add typingIndicator config and avoid duplicate DM typing * fix(msteams): validate typingIndicator config * fix(msteams): stop streaming before Teams timeout * fix(msteams): classify expired streams correctly * fix(msteams): handle link text from html attachments --------- Co-authored-by: Brad Groux <[email protected]>
1 parent af4e9d1 commit fce81fc

9 files changed

Lines changed: 193 additions & 7 deletions

File tree

‎extensions/msteams/src/errors.test.ts‎

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,25 @@ describe("msteams errors", () => {
1818
expect(classifyMSTeamsSendError({ statusCode: 403 }).kind).toBe("auth");
1919
});
2020

21+
it("classifies ContentStreamNotAllowed as permanent instead of auth", () => {
22+
expect(
23+
classifyMSTeamsSendError({
24+
statusCode: 403,
25+
response: {
26+
body: {
27+
error: {
28+
code: "ContentStreamNotAllowed",
29+
},
30+
},
31+
},
32+
}),
33+
).toMatchObject({
34+
kind: "permanent",
35+
statusCode: 403,
36+
errorCode: "ContentStreamNotAllowed",
37+
});
38+
});
39+
2140
it("classifies throttling errors and parses retry-after", () => {
2241
expect(classifyMSTeamsSendError({ statusCode: 429, retryAfter: "1.5" })).toMatchObject({
2342
kind: "throttled",
@@ -43,6 +62,12 @@ describe("msteams errors", () => {
4362
it("provides actionable hints for common cases", () => {
4463
expect(formatMSTeamsSendErrorHint({ kind: "auth" })).toContain("msteams");
4564
expect(formatMSTeamsSendErrorHint({ kind: "throttled" })).toContain("throttled");
65+
expect(
66+
formatMSTeamsSendErrorHint({
67+
kind: "permanent",
68+
errorCode: "ContentStreamNotAllowed",
69+
}),
70+
).toContain("expired the content stream");
4671
});
4772

4873
describe("isRevokedProxyError", () => {

‎extensions/msteams/src/errors.ts‎

Lines changed: 44 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -63,6 +63,32 @@ function extractStatusCode(err: unknown): number | null {
6363
return null;
6464
}
6565

66+
function extractErrorCode(err: unknown): string | null {
67+
if (!isRecord(err)) {
68+
return null;
69+
}
70+
71+
const direct = err.code;
72+
if (typeof direct === "string" && direct.trim()) {
73+
return direct;
74+
}
75+
76+
const response = err.response;
77+
if (!isRecord(response)) {
78+
return null;
79+
}
80+
81+
const body = response.body;
82+
if (isRecord(body)) {
83+
const error = body.error;
84+
if (isRecord(error) && typeof error.code === "string" && error.code.trim()) {
85+
return error.code;
86+
}
87+
}
88+
89+
return null;
90+
}
91+
6692
function extractRetryAfterMs(err: unknown): number | null {
6793
if (!isRecord(err)) {
6894
return null;
@@ -129,6 +155,7 @@ export type MSTeamsSendErrorClassification = {
129155
kind: MSTeamsSendErrorKind;
130156
statusCode?: number;
131157
retryAfterMs?: number;
158+
errorCode?: string;
132159
};
133160

134161
/**
@@ -142,16 +169,25 @@ export type MSTeamsSendErrorClassification = {
142169
export function classifyMSTeamsSendError(err: unknown): MSTeamsSendErrorClassification {
143170
const statusCode = extractStatusCode(err);
144171
const retryAfterMs = extractRetryAfterMs(err);
172+
const errorCode = extractErrorCode(err) ?? undefined;
145173

146-
if (statusCode === 401 || statusCode === 403) {
147-
return { kind: "auth", statusCode };
174+
if (statusCode === 401) {
175+
return { kind: "auth", statusCode, errorCode };
176+
}
177+
178+
if (statusCode === 403) {
179+
if (errorCode === "ContentStreamNotAllowed") {
180+
return { kind: "permanent", statusCode, errorCode };
181+
}
182+
return { kind: "auth", statusCode, errorCode };
148183
}
149184

150185
if (statusCode === 429) {
151186
return {
152187
kind: "throttled",
153188
statusCode,
154189
retryAfterMs: retryAfterMs ?? undefined,
190+
errorCode,
155191
};
156192
}
157193

@@ -160,17 +196,19 @@ export function classifyMSTeamsSendError(err: unknown): MSTeamsSendErrorClassifi
160196
kind: "transient",
161197
statusCode,
162198
retryAfterMs: retryAfterMs ?? undefined,
199+
errorCode,
163200
};
164201
}
165202

166203
if (statusCode != null && statusCode >= 400) {
167-
return { kind: "permanent", statusCode };
204+
return { kind: "permanent", statusCode, errorCode };
168205
}
169206

170207
return {
171208
kind: "unknown",
172209
statusCode: statusCode ?? undefined,
173210
retryAfterMs: retryAfterMs ?? undefined,
211+
errorCode,
174212
};
175213
}
176214

@@ -195,6 +233,9 @@ export function formatMSTeamsSendErrorHint(
195233
if (classification.kind === "auth") {
196234
return "check msteams appId/appPassword/tenantId (or env vars MSTEAMS_APP_ID/MSTEAMS_APP_PASSWORD/MSTEAMS_TENANT_ID)";
197235
}
236+
if (classification.errorCode === "ContentStreamNotAllowed") {
237+
return "Teams expired the content stream; stop streaming earlier and fall back to normal message delivery";
238+
}
198239
if (classification.kind === "throttled") {
199240
return "Teams throttled the bot; backing off may help";
200241
}

‎extensions/msteams/src/monitor-handler/message-handler.ts‎

Lines changed: 36 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,39 @@ import {
3838
translateMSTeamsDmConversationIdForGraph,
3939
wasMSTeamsBotMentioned,
4040
} from "../inbound.js";
41+
42+
function extractTextFromHtmlAttachments(attachments: MSTeamsAttachmentLike[]): string {
43+
for (const attachment of attachments) {
44+
if (attachment.contentType !== "text/html") {
45+
continue;
46+
}
47+
const raw =
48+
typeof attachment.content === "string"
49+
? attachment.content
50+
: typeof attachment.content?.text === "string"
51+
? attachment.content.text
52+
: typeof attachment.content?.body === "string"
53+
? attachment.content.body
54+
: "";
55+
if (!raw) {
56+
continue;
57+
}
58+
const text = raw
59+
.replace(/<at[^>]*>.*?<\/at>/gis, " ")
60+
.replace(/<a\b[^>]*href=["']([^"']+)["'][^>]*>(.*?)<\/a>/gis, "$2 $1")
61+
.replace(/<br\s*\/?>/gi, "\n")
62+
.replace(/<\/p>/gi, "\n")
63+
.replace(/<[^>]+>/g, " ")
64+
.replace(/&nbsp;/gi, " ")
65+
.replace(/&amp;/gi, "&")
66+
.replace(/\s+/g, " ")
67+
.trim();
68+
if (text) {
69+
return text;
70+
}
71+
}
72+
return "";
73+
}
4174
import type { MSTeamsMessageHandlerDeps } from "../monitor-handler.js";
4275
import {
4376
isMSTeamsGroupAllowed,
@@ -778,11 +811,12 @@ export function createMSTeamsMessageHandler(deps: MSTeamsMessageHandlerDeps) {
778811

779812
return async function handleTeamsMessage(context: MSTeamsTurnContext) {
780813
const activity = context.activity;
781-
const rawText = activity.text?.trim() ?? "";
782-
const text = stripMSTeamsMentionTags(rawText);
783814
const attachments = Array.isArray(activity.attachments)
784815
? (activity.attachments as unknown as MSTeamsAttachmentLike[])
785816
: [];
817+
const rawText = activity.text?.trim() ?? "";
818+
const htmlText = extractTextFromHtmlAttachments(attachments);
819+
const text = stripMSTeamsMentionTags(rawText || htmlText);
786820
const wasMentioned = wasMSTeamsBotMentioned(activity);
787821
const conversationId = normalizeMSTeamsConversationId(activity.conversation?.id ?? "");
788822
const replyToId = activity.replyToId ?? undefined;

‎extensions/msteams/src/reply-dispatcher.test.ts‎

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -145,9 +145,28 @@ describe("createMSTeamsReplyDispatcher", () => {
145145

146146
expect(streamInstances).toHaveLength(1);
147147
expect(streamInstances[0]?.sendInformativeUpdate).toHaveBeenCalledTimes(1);
148+
expect(typingCallbacks.onReplyStart).not.toHaveBeenCalled();
149+
});
150+
151+
it("sends native typing indicator for channel conversations by default", async () => {
152+
createDispatcher("channel");
153+
const options = createReplyDispatcherWithTypingMock.mock.calls[0]?.[0];
154+
155+
await options.onReplyStart?.();
156+
157+
expect(streamInstances).toHaveLength(0);
148158
expect(typingCallbacks.onReplyStart).toHaveBeenCalledTimes(1);
149159
});
150160

161+
it("skips native typing indicator when typingIndicator=false", async () => {
162+
createDispatcher("channel", { typingIndicator: false });
163+
const options = createReplyDispatcherWithTypingMock.mock.calls[0]?.[0];
164+
165+
await options.onReplyStart?.();
166+
167+
expect(typingCallbacks.onReplyStart).not.toHaveBeenCalled();
168+
});
169+
151170
it("only sends the informative status update once", async () => {
152171
createDispatcher("personal");
153172
const options = createReplyDispatcherWithTypingMock.mock.calls[0]?.[0];

‎extensions/msteams/src/reply-dispatcher.ts‎

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -110,6 +110,8 @@ export function createMSTeamsReplyDispatcher(params: {
110110

111111
const blockStreamingEnabled =
112112
typeof msteamsCfg?.blockStreaming === "boolean" ? msteamsCfg.blockStreaming : false;
113+
const typingIndicatorEnabled =
114+
typeof msteamsCfg?.typingIndicator === "boolean" ? msteamsCfg.typingIndicator : true;
113115

114116
const pendingMessages: MSTeamsRenderedMessage[] = [];
115117

@@ -210,7 +212,10 @@ export function createMSTeamsReplyDispatcher(params: {
210212
humanDelay: core.channel.reply.resolveHumanDelayConfig(params.cfg, params.agentId),
211213
onReplyStart: async () => {
212214
await streamController.onReplyStart();
213-
await typingCallbacks?.onReplyStart?.();
215+
// Avoid duplicate typing UX in DMs: stream status already shows progress.
216+
if (typingIndicatorEnabled && !streamController.hasStream()) {
217+
await typingCallbacks?.onReplyStart?.();
218+
}
214219
},
215220
typingCallbacks,
216221
deliver: async (payload) => {

‎extensions/msteams/src/streaming-message.test.ts‎

Lines changed: 41 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,11 @@
1-
import { describe, expect, it, vi } from "vitest";
1+
import { afterEach, describe, expect, it, vi } from "vitest";
22
import { TeamsHttpStream } from "./streaming-message.js";
33

44
describe("TeamsHttpStream", () => {
5+
afterEach(() => {
6+
vi.useRealTimers();
7+
});
8+
59
it("sends first chunk as typing activity with streaminfo", async () => {
610
const sent: unknown[] = [];
711
const stream = new TeamsHttpStream({
@@ -203,4 +207,40 @@ describe("TeamsHttpStream", () => {
203207
await stream.finalize();
204208
expect(sendActivity.mock.calls.length).toBe(callCount);
205209
});
210+
211+
it("stops streaming before stream age timeout and finalizes with last good text", async () => {
212+
vi.useFakeTimers();
213+
214+
const sent: unknown[] = [];
215+
const sendActivity = vi.fn(async (activity) => {
216+
sent.push(activity);
217+
return { id: "stream-1" };
218+
});
219+
const stream = new TeamsHttpStream({ sendActivity, throttleMs: 1 });
220+
221+
stream.update("Hello, this is a long enough response for streaming to begin.");
222+
await vi.advanceTimersByTimeAsync(1);
223+
224+
stream.update(
225+
"Hello, this is a long enough response for streaming to begin. More text before timeout.",
226+
);
227+
await vi.advanceTimersByTimeAsync(1);
228+
229+
vi.setSystemTime(new Date(Date.now() + 45_001));
230+
stream.update(
231+
"Hello, this is a long enough response for streaming to begin. More text before timeout. Even more text after timeout.",
232+
);
233+
await vi.advanceTimersByTimeAsync(1);
234+
235+
expect(stream.isFailed).toBe(true);
236+
237+
const finalActivity = sent.find((a) => (a as Record<string, unknown>).type === "message") as
238+
| Record<string, unknown>
239+
| undefined;
240+
241+
expect(finalActivity).toBeDefined();
242+
expect(finalActivity!.text).toBe(
243+
"Hello, this is a long enough response for streaming to begin. More text before timeout.",
244+
);
245+
});
206246
});

‎extensions/msteams/src/streaming-message.ts‎

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,12 @@ const MIN_INITIAL_CHARS = 20;
2121
/** Teams message text limit. */
2222
const TEAMS_MAX_CHARS = 4000;
2323

24+
/**
25+
* Stop streaming before Teams expires the content stream server-side.
26+
* The exact service limit is opaque, so stay comfortably under it.
27+
*/
28+
const MAX_STREAM_AGE_MS = 45_000;
29+
2430
type StreamSendFn = (activity: Record<string, unknown>) => Promise<{ id?: string } | unknown>;
2531

2632
export type TeamsStreamOptions = {
@@ -77,6 +83,7 @@ export class TeamsHttpStream {
7783
private finalized = false;
7884
private streamFailed = false;
7985
private lastStreamedText = "";
86+
private streamStartedAt: number | undefined = undefined;
8087
private loop: DraftStreamLoop;
8188

8289
constructor(options: TeamsStreamOptions) {
@@ -142,6 +149,15 @@ export class TeamsHttpStream {
142149
return;
143150
}
144151

152+
// Stop early before Teams expires the stream server-side. finalize() will
153+
// close the stream with the last good content, and reply-stream-controller
154+
// will deliver any remaining suffix via normal fallback delivery.
155+
if (this.streamStartedAt && Date.now() - this.streamStartedAt >= MAX_STREAM_AGE_MS) {
156+
this.streamFailed = true;
157+
void this.finalize();
158+
return;
159+
}
160+
145161
// Don't append cursor — Teams requires each chunk to be a prefix of subsequent chunks.
146162
// The cursor character would cause "content should contain previously streamed content" errors.
147163
this.loop.update(this.accumulatedText);
@@ -256,6 +272,9 @@ export class TeamsHttpStream {
256272

257273
try {
258274
const response = await this.sendActivity(activity);
275+
if (!this.streamStartedAt) {
276+
this.streamStartedAt = Date.now();
277+
}
259278
if (!this.streamId) {
260279
this.streamId = extractId(response);
261280
}

‎src/config/types.msteams.ts‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -90,6 +90,8 @@ export type MSTeamsConfig = {
9090
textChunkLimit?: number;
9191
/** Chunking mode: "length" (default) splits by size; "newline" splits on every newline. */
9292
chunkMode?: "length" | "newline";
93+
/** Send native Teams typing indicator before replies. Default: true for groups/channels; DMs use informative stream status. */
94+
typingIndicator?: boolean;
9395
/** Enable progressive block-by-block message delivery instead of a single reply. */
9496
blockStreaming?: boolean;
9597
/** Merge streamed block replies before sending. */

‎src/config/zod-schema.providers-core.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1534,6 +1534,7 @@ export const MSTeamsConfigSchema = z
15341534
contextVisibility: ContextVisibilityModeSchema.optional(),
15351535
textChunkLimit: z.number().int().positive().optional(),
15361536
chunkMode: z.enum(["length", "newline"]).optional(),
1537+
typingIndicator: z.boolean().optional(),
15371538
blockStreaming: z.boolean().optional(),
15381539
blockStreamingCoalesce: BlockStreamingCoalesceSchema.optional(),
15391540
mediaAllowHosts: z.array(z.string()).optional(),

0 commit comments

Comments
 (0)