From a2bab7d92372764b0c1c6a2d193184c0f1f33d0a Mon Sep 17 00:00:00 2001 From: Onur Date: Thu, 8 Jan 2026 09:47:01 +0300 Subject: [PATCH] MS Teams: refactor provider + replyStyle + reliability --- src/config/config.test.ts | 37 +++ src/msteams/conversation-store-fs.test.ts | 76 ++++++ src/msteams/conversation-store-fs.ts | 268 +++++++++++++++++++ src/msteams/conversation-store-memory.ts | 45 ++++ src/msteams/conversation-store.ts | 110 +------- src/msteams/errors.test.ts | 50 ++++ src/msteams/errors.ts | 171 +++++++++++++ src/msteams/inbound.test.ts | 64 +++++ src/msteams/inbound.ts | 35 +++ src/msteams/messenger.test.ts | 209 +++++++++++++++ src/msteams/messenger.ts | 294 +++++++++++++++++++++ src/msteams/monitor.ts | 297 +++++++--------------- src/msteams/policy.test.ts | 99 ++++++++ src/msteams/policy.ts | 58 +++++ src/msteams/probe.test.ts | 57 +++++ src/msteams/probe.ts | 23 +- src/msteams/sdk-types.ts | 19 ++ src/msteams/send.ts | 155 +++++------ tmp/msteams-refactor-plan.md | 156 ++++++++++++ 19 files changed, 1834 insertions(+), 389 deletions(-) create mode 100644 src/msteams/conversation-store-fs.test.ts create mode 100644 src/msteams/conversation-store-fs.ts create mode 100644 src/msteams/conversation-store-memory.ts create mode 100644 src/msteams/errors.test.ts create mode 100644 src/msteams/errors.ts create mode 100644 src/msteams/inbound.test.ts create mode 100644 src/msteams/inbound.ts create mode 100644 src/msteams/messenger.test.ts create mode 100644 src/msteams/messenger.ts create mode 100644 src/msteams/policy.test.ts create mode 100644 src/msteams/policy.ts create mode 100644 src/msteams/probe.test.ts create mode 100644 src/msteams/sdk-types.ts create mode 100644 tmp/msteams-refactor-plan.md diff --git a/src/config/config.test.ts b/src/config/config.test.ts index 6c05c889b..787857a7d 100644 --- a/src/config/config.test.ts +++ b/src/config/config.test.ts @@ -500,6 +500,43 @@ describe("config discord", () => { }); }); +describe("config msteams", () => { + it("accepts replyStyle at global/team/channel levels", async () => { + vi.resetModules(); + const { validateConfigObject } = await import("./config.js"); + const res = validateConfigObject({ + msteams: { + replyStyle: "top-level", + teams: { + team123: { + replyStyle: "thread", + channels: { + chan456: { replyStyle: "top-level" }, + }, + }, + }, + }, + }); + expect(res.ok).toBe(true); + if (res.ok) { + expect(res.config.msteams?.replyStyle).toBe("top-level"); + expect(res.config.msteams?.teams?.team123?.replyStyle).toBe("thread"); + expect( + res.config.msteams?.teams?.team123?.channels?.chan456?.replyStyle, + ).toBe("top-level"); + } + }); + + it("rejects invalid replyStyle", async () => { + vi.resetModules(); + const { validateConfigObject } = await import("./config.js"); + const res = validateConfigObject({ + msteams: { replyStyle: "nope" }, + }); + expect(res.ok).toBe(false); + }); +}); + describe("Nix integration (U3, U5, U9)", () => { describe("U3: isNixMode env var detection", () => { it("isNixMode is false when CLAWDBOT_NIX_MODE is not set", async () => { diff --git a/src/msteams/conversation-store-fs.test.ts b/src/msteams/conversation-store-fs.test.ts new file mode 100644 index 000000000..ee1618dc1 --- /dev/null +++ b/src/msteams/conversation-store-fs.test.ts @@ -0,0 +1,76 @@ +import fs from "node:fs"; +import os from "node:os"; +import path from "node:path"; + +import { describe, expect, it } from "vitest"; + +import type { StoredConversationReference } from "./conversation-store.js"; +import { createMSTeamsConversationStoreFs } from "./conversation-store-fs.js"; + +describe("msteams conversation store (fs)", () => { + it("filters and prunes expired entries (but keeps legacy ones)", async () => { + const stateDir = await fs.promises.mkdtemp( + path.join(os.tmpdir(), "clawdbot-msteams-store-"), + ); + + const env: NodeJS.ProcessEnv = { + ...process.env, + CLAWDBOT_STATE_DIR: stateDir, + }; + + const store = createMSTeamsConversationStoreFs({ env, ttlMs: 1_000 }); + + const ref: StoredConversationReference = { + conversation: { id: "19:active@thread.tacv2" }, + channelId: "msteams", + serviceUrl: "https://service.example.com", + user: { id: "u1", aadObjectId: "aad1" }, + }; + + await store.upsert("19:active@thread.tacv2", ref); + + const filePath = path.join(stateDir, "msteams-conversations.json"); + const raw = await fs.promises.readFile(filePath, "utf-8"); + const json = JSON.parse(raw) as { + version: number; + conversations: Record< + string, + StoredConversationReference & { lastSeenAt?: string } + >; + }; + + json.conversations["19:old@thread.tacv2"] = { + ...ref, + conversation: { id: "19:old@thread.tacv2" }, + lastSeenAt: new Date(Date.now() - 60_000).toISOString(), + }; + + // Legacy entry without lastSeenAt should be preserved. + json.conversations["19:legacy@thread.tacv2"] = { + ...ref, + conversation: { id: "19:legacy@thread.tacv2" }, + }; + + await fs.promises.writeFile(filePath, `${JSON.stringify(json, null, 2)}\n`); + + const list = await store.list(); + const ids = list.map((e) => e.conversationId).sort(); + expect(ids).toEqual(["19:active@thread.tacv2", "19:legacy@thread.tacv2"]); + + expect(await store.get("19:old@thread.tacv2")).toBeNull(); + expect(await store.get("19:legacy@thread.tacv2")).not.toBeNull(); + + await store.upsert("19:new@thread.tacv2", { + ...ref, + conversation: { id: "19:new@thread.tacv2" }, + }); + + const rawAfter = await fs.promises.readFile(filePath, "utf-8"); + const jsonAfter = JSON.parse(rawAfter) as typeof json; + expect(Object.keys(jsonAfter.conversations).sort()).toEqual([ + "19:active@thread.tacv2", + "19:legacy@thread.tacv2", + "19:new@thread.tacv2", + ]); + }); +}); diff --git a/src/msteams/conversation-store-fs.ts b/src/msteams/conversation-store-fs.ts new file mode 100644 index 000000000..f1891fa3a --- /dev/null +++ b/src/msteams/conversation-store-fs.ts @@ -0,0 +1,268 @@ +import crypto from "node:crypto"; +import fs from "node:fs"; +import os from "node:os"; +import path from "node:path"; + +import lockfile from "proper-lockfile"; + +import { resolveStateDir } from "../config/paths.js"; +import type { + MSTeamsConversationStore, + MSTeamsConversationStoreEntry, + StoredConversationReference, +} from "./conversation-store.js"; + +type ConversationStoreData = { + version: 1; + conversations: Record< + string, + StoredConversationReference & { lastSeenAt?: string } + >; +}; + +const STORE_FILENAME = "msteams-conversations.json"; +const MAX_CONVERSATIONS = 1000; +const CONVERSATION_TTL_MS = 365 * 24 * 60 * 60 * 1000; +const STORE_LOCK_OPTIONS = { + retries: { + retries: 10, + factor: 2, + minTimeout: 100, + maxTimeout: 10_000, + randomize: true, + }, + stale: 30_000, +} as const; + +function resolveStorePath( + env: NodeJS.ProcessEnv = process.env, + homedir?: () => string, +): string { + const stateDir = homedir + ? resolveStateDir(env, homedir) + : resolveStateDir(env); + return path.join(stateDir, STORE_FILENAME); +} + +function safeParseJson(raw: string): T | null { + try { + return JSON.parse(raw) as T; + } catch { + return null; + } +} + +async function readJsonFile( + filePath: string, + fallback: T, +): Promise<{ value: T; exists: boolean }> { + try { + const raw = await fs.promises.readFile(filePath, "utf-8"); + const parsed = safeParseJson(raw); + if (parsed == null) return { value: fallback, exists: true }; + return { value: parsed, exists: true }; + } catch (err) { + const code = (err as { code?: string }).code; + if (code === "ENOENT") return { value: fallback, exists: false }; + return { value: fallback, exists: false }; + } +} + +async function writeJsonFile(filePath: string, value: unknown): Promise { + const dir = path.dirname(filePath); + await fs.promises.mkdir(dir, { recursive: true, mode: 0o700 }); + const tmp = path.join( + dir, + `${path.basename(filePath)}.${crypto.randomUUID()}.tmp`, + ); + await fs.promises.writeFile(tmp, `${JSON.stringify(value, null, 2)}\n`, { + encoding: "utf-8", + }); + await fs.promises.chmod(tmp, 0o600); + await fs.promises.rename(tmp, filePath); +} + +async function ensureJsonFile(filePath: string, fallback: unknown) { + try { + await fs.promises.access(filePath); + } catch { + await writeJsonFile(filePath, fallback); + } +} + +async function withFileLock( + filePath: string, + fallback: unknown, + fn: () => Promise, +): Promise { + await ensureJsonFile(filePath, fallback); + let release: (() => Promise) | undefined; + try { + release = await lockfile.lock(filePath, STORE_LOCK_OPTIONS); + return await fn(); + } finally { + if (release) { + try { + await release(); + } catch { + // ignore unlock errors + } + } + } +} + +function parseTimestamp(value: string | undefined): number | null { + if (!value) return null; + const parsed = Date.parse(value); + if (!Number.isFinite(parsed)) return null; + return parsed; +} + +function pruneToLimit( + conversations: Record< + string, + StoredConversationReference & { lastSeenAt?: string } + >, +) { + const entries = Object.entries(conversations); + if (entries.length <= MAX_CONVERSATIONS) return conversations; + + entries.sort((a, b) => { + const aTs = parseTimestamp(a[1].lastSeenAt) ?? 0; + const bTs = parseTimestamp(b[1].lastSeenAt) ?? 0; + return aTs - bTs; + }); + + const keep = entries.slice(entries.length - MAX_CONVERSATIONS); + return Object.fromEntries(keep); +} + +function pruneExpired( + conversations: Record< + string, + StoredConversationReference & { lastSeenAt?: string } + >, + nowMs: number, + ttlMs: number, +) { + let removed = false; + const kept: typeof conversations = {}; + for (const [conversationId, reference] of Object.entries(conversations)) { + const lastSeenAt = parseTimestamp(reference.lastSeenAt); + // Preserve legacy entries that have no lastSeenAt until they're seen again. + if (lastSeenAt != null && nowMs - lastSeenAt > ttlMs) { + removed = true; + continue; + } + kept[conversationId] = reference; + } + return { conversations: kept, removed }; +} + +function normalizeConversationId(raw: string): string { + return raw.split(";")[0] ?? raw; +} + +export function createMSTeamsConversationStoreFs(params?: { + env?: NodeJS.ProcessEnv; + homedir?: () => string; + ttlMs?: number; +}): MSTeamsConversationStore { + const env = params?.env ?? process.env; + const homedir = params?.homedir ?? os.homedir; + const ttlMs = params?.ttlMs ?? CONVERSATION_TTL_MS; + const filePath = resolveStorePath(env, homedir); + + const empty: ConversationStoreData = { version: 1, conversations: {} }; + + const readStore = async (): Promise => { + const { value } = await readJsonFile( + filePath, + empty, + ); + if ( + value.version !== 1 || + !value.conversations || + typeof value.conversations !== "object" || + Array.isArray(value.conversations) + ) { + return empty; + } + const nowMs = Date.now(); + const pruned = pruneExpired( + value.conversations, + nowMs, + ttlMs, + ).conversations; + return { version: 1, conversations: pruneToLimit(pruned) }; + }; + + const list = async (): Promise => { + const store = await readStore(); + return Object.entries(store.conversations).map( + ([conversationId, reference]) => ({ + conversationId, + reference, + }), + ); + }; + + const get = async ( + conversationId: string, + ): Promise => { + const store = await readStore(); + return store.conversations[normalizeConversationId(conversationId)] ?? null; + }; + + const findByUserId = async ( + id: string, + ): Promise => { + const target = id.trim(); + if (!target) return null; + for (const entry of await list()) { + const { conversationId, reference } = entry; + if (reference.user?.aadObjectId === target) { + return { conversationId, reference }; + } + if (reference.user?.id === target) { + return { conversationId, reference }; + } + } + return null; + }; + + const upsert = async ( + conversationId: string, + reference: StoredConversationReference, + ): Promise => { + const normalizedId = normalizeConversationId(conversationId); + await withFileLock(filePath, empty, async () => { + const store = await readStore(); + store.conversations[normalizedId] = { + ...reference, + lastSeenAt: new Date().toISOString(), + }; + const nowMs = Date.now(); + store.conversations = pruneExpired( + store.conversations, + nowMs, + ttlMs, + ).conversations; + store.conversations = pruneToLimit(store.conversations); + await writeJsonFile(filePath, store); + }); + }; + + const remove = async (conversationId: string): Promise => { + const normalizedId = normalizeConversationId(conversationId); + return await withFileLock(filePath, empty, async () => { + const store = await readStore(); + if (!(normalizedId in store.conversations)) return false; + delete store.conversations[normalizedId]; + await writeJsonFile(filePath, store); + return true; + }); + }; + + return { upsert, get, list, remove, findByUserId }; +} diff --git a/src/msteams/conversation-store-memory.ts b/src/msteams/conversation-store-memory.ts new file mode 100644 index 000000000..098f09bb6 --- /dev/null +++ b/src/msteams/conversation-store-memory.ts @@ -0,0 +1,45 @@ +import type { + MSTeamsConversationStore, + MSTeamsConversationStoreEntry, + StoredConversationReference, +} from "./conversation-store.js"; + +export function createMSTeamsConversationStoreMemory( + initial: MSTeamsConversationStoreEntry[] = [], +): MSTeamsConversationStore { + const map = new Map(); + for (const { conversationId, reference } of initial) { + map.set(conversationId, reference); + } + + return { + upsert: async (conversationId, reference) => { + map.set(conversationId, reference); + }, + get: async (conversationId) => { + return map.get(conversationId) ?? null; + }, + list: async () => { + return Array.from(map.entries()).map(([conversationId, reference]) => ({ + conversationId, + reference, + })); + }, + remove: async (conversationId) => { + return map.delete(conversationId); + }, + findByUserId: async (id) => { + const target = id.trim(); + if (!target) return null; + for (const [conversationId, reference] of map.entries()) { + if (reference.user?.aadObjectId === target) { + return { conversationId, reference }; + } + if (reference.user?.id === target) { + return { conversationId, reference }; + } + } + return null; + }, + }; +} diff --git a/src/msteams/conversation-store.ts b/src/msteams/conversation-store.ts index d1463d521..75bd63c92 100644 --- a/src/msteams/conversation-store.ts +++ b/src/msteams/conversation-store.ts @@ -1,15 +1,10 @@ /** * Conversation store for MS Teams proactive messaging. * - * Stores ConversationReference objects keyed by conversation ID so we can + * Stores ConversationReference-like objects keyed by conversation ID so we can * send proactive messages later (after the webhook turn has completed). */ -import fs from "node:fs"; -import path from "node:path"; - -import { resolveStateDir } from "../config/paths.js"; - /** Minimal ConversationReference shape for proactive messaging */ export type StoredConversationReference = { /** Activity ID from the last message */ @@ -28,95 +23,18 @@ export type StoredConversationReference = { locale?: string; }; -type ConversationStoreData = { - version: 1; - conversations: Record; +export type MSTeamsConversationStoreEntry = { + conversationId: string; + reference: StoredConversationReference; }; -const STORE_FILENAME = "msteams-conversations.json"; -const MAX_CONVERSATIONS = 1000; - -function resolveStorePath(): string { - const stateDir = resolveStateDir(process.env); - return path.join(stateDir, STORE_FILENAME); -} - -async function readStore(): Promise { - try { - const raw = await fs.promises.readFile(resolveStorePath(), "utf-8"); - const data = JSON.parse(raw) as ConversationStoreData; - if (data.version !== 1) { - return { version: 1, conversations: {} }; - } - return data; - } catch { - return { version: 1, conversations: {} }; - } -} - -async function writeStore(data: ConversationStoreData): Promise { - const filePath = resolveStorePath(); - const dir = path.dirname(filePath); - await fs.promises.mkdir(dir, { recursive: true, mode: 0o700 }); - await fs.promises.writeFile(filePath, JSON.stringify(data, null, 2), "utf-8"); -} - -/** - * Save a conversation reference for later proactive messaging. - */ -export async function saveConversationReference( - conversationId: string, - reference: StoredConversationReference, -): Promise { - const store = await readStore(); - - // Prune if over limit (keep most recent) - const keys = Object.keys(store.conversations); - if (keys.length >= MAX_CONVERSATIONS) { - const toRemove = keys.slice(0, keys.length - MAX_CONVERSATIONS + 1); - for (const key of toRemove) { - delete store.conversations[key]; - } - } - - store.conversations[conversationId] = reference; - await writeStore(store); -} - -/** - * Get a stored conversation reference. - */ -export async function getConversationReference( - conversationId: string, -): Promise { - const store = await readStore(); - return store.conversations[conversationId] ?? null; -} - -/** - * List all stored conversation references. - */ -export async function listConversationReferences(): Promise< - Array<{ conversationId: string; reference: StoredConversationReference }> -> { - const store = await readStore(); - return Object.entries(store.conversations).map( - ([conversationId, reference]) => ({ - conversationId, - reference, - }), - ); -} - -/** - * Remove a conversation reference. - */ -export async function removeConversationReference( - conversationId: string, -): Promise { - const store = await readStore(); - if (!(conversationId in store.conversations)) return false; - delete store.conversations[conversationId]; - await writeStore(store); - return true; -} +export type MSTeamsConversationStore = { + upsert: ( + conversationId: string, + reference: StoredConversationReference, + ) => Promise; + get: (conversationId: string) => Promise; + list: () => Promise; + remove: (conversationId: string) => Promise; + findByUserId: (id: string) => Promise; +}; diff --git a/src/msteams/errors.test.ts b/src/msteams/errors.test.ts new file mode 100644 index 000000000..554305988 --- /dev/null +++ b/src/msteams/errors.test.ts @@ -0,0 +1,50 @@ +import { describe, expect, it } from "vitest"; + +import { + classifyMSTeamsSendError, + formatMSTeamsSendErrorHint, + formatUnknownError, +} from "./errors.js"; + +describe("msteams errors", () => { + it("formats unknown errors", () => { + expect(formatUnknownError("oops")).toBe("oops"); + expect(formatUnknownError(null)).toBe("null"); + }); + + it("classifies auth errors", () => { + expect(classifyMSTeamsSendError({ statusCode: 401 }).kind).toBe("auth"); + expect(classifyMSTeamsSendError({ statusCode: 403 }).kind).toBe("auth"); + }); + + it("classifies throttling errors and parses retry-after", () => { + expect( + classifyMSTeamsSendError({ statusCode: 429, retryAfter: "1.5" }), + ).toMatchObject({ + kind: "throttled", + statusCode: 429, + retryAfterMs: 1500, + }); + }); + + it("classifies transient errors", () => { + expect(classifyMSTeamsSendError({ statusCode: 503 })).toMatchObject({ + kind: "transient", + statusCode: 503, + }); + }); + + it("classifies permanent 4xx errors", () => { + expect(classifyMSTeamsSendError({ statusCode: 400 })).toMatchObject({ + kind: "permanent", + statusCode: 400, + }); + }); + + it("provides actionable hints for common cases", () => { + expect(formatMSTeamsSendErrorHint({ kind: "auth" })).toContain("msteams"); + expect(formatMSTeamsSendErrorHint({ kind: "throttled" })).toContain( + "throttled", + ); + }); +}); diff --git a/src/msteams/errors.ts b/src/msteams/errors.ts new file mode 100644 index 000000000..8dd4800c9 --- /dev/null +++ b/src/msteams/errors.ts @@ -0,0 +1,171 @@ +export function formatUnknownError(err: unknown): string { + if (err instanceof Error) return err.message; + if (typeof err === "string") return err; + if (err === null) return "null"; + if (err === undefined) return "undefined"; + if ( + typeof err === "number" || + typeof err === "boolean" || + typeof err === "bigint" + ) { + return String(err); + } + if (typeof err === "symbol") return err.description ?? err.toString(); + if (typeof err === "function") { + return err.name ? `[function ${err.name}]` : "[function]"; + } + try { + return JSON.stringify(err) ?? "unknown error"; + } catch { + return "unknown error"; + } +} + +function isRecord(value: unknown): value is Record { + return typeof value === "object" && value !== null && !Array.isArray(value); +} + +function extractStatusCode(err: unknown): number | null { + if (!isRecord(err)) return null; + const direct = err.statusCode ?? err.status; + if (typeof direct === "number" && Number.isFinite(direct)) return direct; + if (typeof direct === "string") { + const parsed = Number.parseInt(direct, 10); + if (Number.isFinite(parsed)) return parsed; + } + + const response = err.response; + if (isRecord(response)) { + const status = response.status; + if (typeof status === "number" && Number.isFinite(status)) return status; + if (typeof status === "string") { + const parsed = Number.parseInt(status, 10); + if (Number.isFinite(parsed)) return parsed; + } + } + + return null; +} + +function extractRetryAfterMs(err: unknown): number | null { + if (!isRecord(err)) return null; + + const direct = err.retryAfterMs ?? err.retry_after_ms; + if (typeof direct === "number" && Number.isFinite(direct) && direct >= 0) { + return direct; + } + + const retryAfter = err.retryAfter ?? err.retry_after; + if (typeof retryAfter === "number" && Number.isFinite(retryAfter)) { + return retryAfter >= 0 ? retryAfter * 1000 : null; + } + if (typeof retryAfter === "string") { + const parsed = Number.parseFloat(retryAfter); + if (Number.isFinite(parsed) && parsed >= 0) return parsed * 1000; + } + + const response = err.response; + if (!isRecord(response)) return null; + + const headers = response.headers; + if (!headers) return null; + + if (isRecord(headers)) { + const raw = headers["retry-after"] ?? headers["Retry-After"]; + if (typeof raw === "string") { + const parsed = Number.parseFloat(raw); + if (Number.isFinite(parsed) && parsed >= 0) return parsed * 1000; + } + } + + // Fetch Headers-like interface + if ( + typeof headers === "object" && + headers !== null && + "get" in headers && + typeof (headers as { get?: unknown }).get === "function" + ) { + const raw = (headers as { get: (name: string) => string | null }).get( + "retry-after", + ); + if (raw) { + const parsed = Number.parseFloat(raw); + if (Number.isFinite(parsed) && parsed >= 0) return parsed * 1000; + } + } + + return null; +} + +export type MSTeamsSendErrorKind = + | "auth" + | "throttled" + | "transient" + | "permanent" + | "unknown"; + +export type MSTeamsSendErrorClassification = { + kind: MSTeamsSendErrorKind; + statusCode?: number; + retryAfterMs?: number; +}; + +/** + * Classify outbound send errors for safe retries and actionable logs. + * + * Important: We only mark errors as retryable when we have an explicit HTTP + * status code that indicates the message was not accepted (e.g. 429, 5xx). + * For transport-level errors where delivery is ambiguous, we prefer to avoid + * retries to reduce the chance of duplicate posts. + */ +export function classifyMSTeamsSendError( + err: unknown, +): MSTeamsSendErrorClassification { + const statusCode = extractStatusCode(err); + const retryAfterMs = extractRetryAfterMs(err); + + if (statusCode === 401 || statusCode === 403) { + return { kind: "auth", statusCode }; + } + + if (statusCode === 429) { + return { + kind: "throttled", + statusCode, + retryAfterMs: retryAfterMs ?? undefined, + }; + } + + if (statusCode === 408 || (statusCode != null && statusCode >= 500)) { + return { + kind: "transient", + statusCode, + retryAfterMs: retryAfterMs ?? undefined, + }; + } + + if (statusCode != null && statusCode >= 400) { + return { kind: "permanent", statusCode }; + } + + return { + kind: "unknown", + statusCode: statusCode ?? undefined, + retryAfterMs: retryAfterMs ?? undefined, + }; +} + +export function formatMSTeamsSendErrorHint( + classification: MSTeamsSendErrorClassification, +): string | undefined { + if (classification.kind === "auth") { + return "check msteams appId/appPassword/tenantId (or env vars MSTEAMS_APP_ID/MSTEAMS_APP_PASSWORD/MSTEAMS_TENANT_ID)"; + } + if (classification.kind === "throttled") { + return "Teams throttled the bot; backing off may help"; + } + if (classification.kind === "transient") { + return "transient Teams/Bot Framework error; retry may succeed"; + } + return undefined; +} diff --git a/src/msteams/inbound.test.ts b/src/msteams/inbound.test.ts new file mode 100644 index 000000000..98c9b2df4 --- /dev/null +++ b/src/msteams/inbound.test.ts @@ -0,0 +1,64 @@ +import { describe, expect, it } from "vitest"; + +import { + normalizeMSTeamsConversationId, + parseMSTeamsActivityTimestamp, + stripMSTeamsMentionTags, + wasMSTeamsBotMentioned, +} from "./inbound.js"; + +describe("msteams inbound", () => { + describe("stripMSTeamsMentionTags", () => { + it("removes ... tags and trims", () => { + expect(stripMSTeamsMentionTags("Bot hi")).toBe("hi"); + expect(stripMSTeamsMentionTags("hi Bot")).toBe("hi"); + }); + }); + + describe("normalizeMSTeamsConversationId", () => { + it("strips the ;messageid suffix", () => { + expect( + normalizeMSTeamsConversationId( + "19:abc@thread.tacv2;messageid=deadbeef", + ), + ).toBe("19:abc@thread.tacv2"); + }); + }); + + describe("parseMSTeamsActivityTimestamp", () => { + it("returns undefined for empty/invalid values", () => { + expect(parseMSTeamsActivityTimestamp(undefined)).toBeUndefined(); + expect(parseMSTeamsActivityTimestamp("not-a-date")).toBeUndefined(); + }); + + it("parses string timestamps", () => { + const ts = parseMSTeamsActivityTimestamp("2024-01-01T00:00:00.000Z"); + expect(ts?.toISOString()).toBe("2024-01-01T00:00:00.000Z"); + }); + + it("passes through Date instances", () => { + const d = new Date("2024-01-01T00:00:00.000Z"); + expect(parseMSTeamsActivityTimestamp(d)).toBe(d); + }); + }); + + describe("wasMSTeamsBotMentioned", () => { + it("returns true when a mention entity matches recipient.id", () => { + expect( + wasMSTeamsBotMentioned({ + recipient: { id: "bot" }, + entities: [{ type: "mention", mentioned: { id: "bot" } }], + }), + ).toBe(true); + }); + + it("returns false when there is no matching mention", () => { + expect( + wasMSTeamsBotMentioned({ + recipient: { id: "bot" }, + entities: [{ type: "mention", mentioned: { id: "other" } }], + }), + ).toBe(false); + }); + }); +}); diff --git a/src/msteams/inbound.ts b/src/msteams/inbound.ts new file mode 100644 index 000000000..5c37c68db --- /dev/null +++ b/src/msteams/inbound.ts @@ -0,0 +1,35 @@ +export type MentionableActivity = { + recipient?: { id?: string } | null; + entities?: Array<{ + type?: string; + mentioned?: { id?: string }; + }> | null; +}; + +export function normalizeMSTeamsConversationId(raw: string): string { + return raw.split(";")[0] ?? raw; +} + +export function parseMSTeamsActivityTimestamp( + value: unknown, +): Date | undefined { + if (!value) return undefined; + if (value instanceof Date) return value; + if (typeof value !== "string") return undefined; + const date = new Date(value); + return Number.isNaN(date.getTime()) ? undefined : date; +} + +export function stripMSTeamsMentionTags(text: string): string { + // Teams wraps mentions in ... tags + return text.replace(/.*?<\/at>/gi, "").trim(); +} + +export function wasMSTeamsBotMentioned(activity: MentionableActivity): boolean { + const botId = activity.recipient?.id; + if (!botId) return false; + const entities = activity.entities ?? []; + return entities.some( + (e) => e.type === "mention" && e.mentioned?.id === botId, + ); +} diff --git a/src/msteams/messenger.test.ts b/src/msteams/messenger.test.ts new file mode 100644 index 000000000..0fbbdb764 --- /dev/null +++ b/src/msteams/messenger.test.ts @@ -0,0 +1,209 @@ +import { describe, expect, it } from "vitest"; + +import { SILENT_REPLY_TOKEN } from "../auto-reply/tokens.js"; +import type { StoredConversationReference } from "./conversation-store.js"; +import { + type MSTeamsAdapter, + renderReplyPayloadsToMessages, + sendMSTeamsMessages, +} from "./messenger.js"; + +describe("msteams messenger", () => { + describe("renderReplyPayloadsToMessages", () => { + it("filters silent replies", () => { + const messages = renderReplyPayloadsToMessages( + [{ text: SILENT_REPLY_TOKEN }], + { textChunkLimit: 4000 }, + ); + expect(messages).toEqual([]); + }); + + it("splits media into separate messages by default", () => { + const messages = renderReplyPayloadsToMessages( + [{ text: "hi", mediaUrl: "https://example.com/a.png" }], + { textChunkLimit: 4000 }, + ); + expect(messages).toEqual(["hi", "https://example.com/a.png"]); + }); + + it("supports inline media mode", () => { + const messages = renderReplyPayloadsToMessages( + [{ text: "hi", mediaUrl: "https://example.com/a.png" }], + { textChunkLimit: 4000, mediaMode: "inline" }, + ); + expect(messages).toEqual(["hi\n\nhttps://example.com/a.png"]); + }); + + it("chunks long text when enabled", () => { + const long = "hello ".repeat(200); + const messages = renderReplyPayloadsToMessages([{ text: long }], { + textChunkLimit: 50, + }); + expect(messages.length).toBeGreaterThan(1); + }); + }); + + describe("sendMSTeamsMessages", () => { + const baseRef: StoredConversationReference = { + activityId: "activity123", + conversation: { id: "19:abc@thread.tacv2;messageid=deadbeef" }, + channelId: "msteams", + serviceUrl: "https://service.example.com", + }; + + it("sends thread messages via the provided context", async () => { + const sent: string[] = []; + const ctx = { + sendActivity: async (activity: unknown) => { + const { text } = activity as { text?: string }; + sent.push(text ?? ""); + return { id: `id:${text ?? ""}` }; + }, + }; + + const adapter: MSTeamsAdapter = { + continueConversation: async () => {}, + }; + + const ids = await sendMSTeamsMessages({ + replyStyle: "thread", + adapter, + appId: "app123", + conversationRef: baseRef, + context: ctx, + messages: ["one", "two"], + }); + + expect(sent).toEqual(["one", "two"]); + expect(ids).toEqual(["id:one", "id:two"]); + }); + + it("sends top-level messages via continueConversation and strips activityId", async () => { + const seen: { reference?: unknown; texts: string[] } = { texts: [] }; + + const adapter: MSTeamsAdapter = { + continueConversation: async (_appId, reference, logic) => { + seen.reference = reference; + await logic({ + sendActivity: async (activity: unknown) => { + const { text } = activity as { text?: string }; + seen.texts.push(text ?? ""); + return { id: `id:${text ?? ""}` }; + }, + }); + }, + }; + + const ids = await sendMSTeamsMessages({ + replyStyle: "top-level", + adapter, + appId: "app123", + conversationRef: baseRef, + messages: ["hello"], + }); + + expect(seen.texts).toEqual(["hello"]); + expect(ids).toEqual(["id:hello"]); + + const ref = seen.reference as { + activityId?: string; + conversation?: { id?: string }; + }; + expect(ref.activityId).toBeUndefined(); + expect(ref.conversation?.id).toBe("19:abc@thread.tacv2"); + }); + + it("retries thread sends on throttling (429)", async () => { + const attempts: string[] = []; + const retryEvents: Array<{ nextAttempt: number; delayMs: number }> = []; + + const ctx = { + sendActivity: async (activity: unknown) => { + const { text } = activity as { text?: string }; + attempts.push(text ?? ""); + if (attempts.length === 1) { + throw Object.assign(new Error("throttled"), { statusCode: 429 }); + } + return { id: `id:${text ?? ""}` }; + }, + }; + + const adapter: MSTeamsAdapter = { + continueConversation: async () => {}, + }; + + const ids = await sendMSTeamsMessages({ + replyStyle: "thread", + adapter, + appId: "app123", + conversationRef: baseRef, + context: ctx, + messages: ["one"], + retry: { maxAttempts: 2, baseDelayMs: 0, maxDelayMs: 0 }, + onRetry: (e) => + retryEvents.push({ nextAttempt: e.nextAttempt, delayMs: e.delayMs }), + }); + + expect(attempts).toEqual(["one", "one"]); + expect(ids).toEqual(["id:one"]); + expect(retryEvents).toEqual([{ nextAttempt: 2, delayMs: 0 }]); + }); + + it("does not retry thread sends on client errors (4xx)", async () => { + const ctx = { + sendActivity: async () => { + throw Object.assign(new Error("bad request"), { statusCode: 400 }); + }, + }; + + const adapter: MSTeamsAdapter = { + continueConversation: async () => {}, + }; + + await expect( + sendMSTeamsMessages({ + replyStyle: "thread", + adapter, + appId: "app123", + conversationRef: baseRef, + context: ctx, + messages: ["one"], + retry: { maxAttempts: 3, baseDelayMs: 0, maxDelayMs: 0 }, + }), + ).rejects.toMatchObject({ statusCode: 400 }); + }); + + it("retries top-level sends on transient (5xx)", async () => { + const attempts: string[] = []; + + const adapter: MSTeamsAdapter = { + continueConversation: async (_appId, _reference, logic) => { + await logic({ + sendActivity: async (activity: unknown) => { + const { text } = activity as { text?: string }; + attempts.push(text ?? ""); + if (attempts.length === 1) { + throw Object.assign(new Error("server error"), { + statusCode: 503, + }); + } + return { id: `id:${text ?? ""}` }; + }, + }); + }, + }; + + const ids = await sendMSTeamsMessages({ + replyStyle: "top-level", + adapter, + appId: "app123", + conversationRef: baseRef, + messages: ["hello"], + retry: { maxAttempts: 2, baseDelayMs: 0, maxDelayMs: 0 }, + }); + + expect(attempts).toEqual(["hello", "hello"]); + expect(ids).toEqual(["id:hello"]); + }); + }); +}); diff --git a/src/msteams/messenger.ts b/src/msteams/messenger.ts new file mode 100644 index 000000000..aa21be60a --- /dev/null +++ b/src/msteams/messenger.ts @@ -0,0 +1,294 @@ +import { chunkMarkdownText } from "../auto-reply/chunk.js"; +import { SILENT_REPLY_TOKEN } from "../auto-reply/tokens.js"; +import type { ReplyPayload } from "../auto-reply/types.js"; +import type { MSTeamsReplyStyle } from "../config/types.js"; +import type { StoredConversationReference } from "./conversation-store.js"; +import { classifyMSTeamsSendError } from "./errors.js"; + +type SendContext = { + sendActivity: (textOrActivity: string | object) => Promise; +}; + +type ConversationReference = { + activityId?: string; + user?: { id?: string; name?: string; aadObjectId?: string }; + bot?: { id?: string; name?: string }; + conversation: { id: string; conversationType?: string; tenantId?: string }; + channelId: string; + serviceUrl?: string; + locale?: string; +}; + +export type MSTeamsAdapter = { + continueConversation: ( + appId: string, + reference: ConversationReference, + logic: (context: SendContext) => Promise, + ) => Promise; +}; + +export type MSTeamsReplyRenderOptions = { + textChunkLimit: number; + chunkText?: boolean; + mediaMode?: "split" | "inline"; +}; + +export type MSTeamsSendRetryOptions = { + maxAttempts?: number; + baseDelayMs?: number; + maxDelayMs?: number; +}; + +export type MSTeamsSendRetryEvent = { + messageIndex: number; + messageCount: number; + nextAttempt: number; + maxAttempts: number; + delayMs: number; + classification: ReturnType; +}; + +function normalizeConversationId(rawId: string): string { + return rawId.split(";")[0] ?? rawId; +} + +function buildConversationReference( + ref: StoredConversationReference, +): ConversationReference { + const conversationId = ref.conversation?.id?.trim(); + if (!conversationId) { + throw new Error("Invalid stored reference: missing conversation.id"); + } + return { + activityId: ref.activityId, + user: ref.user, + bot: ref.bot, + conversation: { + id: normalizeConversationId(conversationId), + conversationType: ref.conversation?.conversationType, + tenantId: ref.conversation?.tenantId, + }, + channelId: ref.channelId ?? "msteams", + serviceUrl: ref.serviceUrl, + locale: ref.locale, + }; +} + +function extractMessageId(response: unknown): string | null { + if (!response || typeof response !== "object") return null; + if (!("id" in response)) return null; + const { id } = response as { id?: unknown }; + if (typeof id !== "string" || !id) return null; + return id; +} + +function pushTextMessages( + out: string[], + text: string, + opts: { + chunkText: boolean; + chunkLimit: number; + }, +) { + if (!text) return; + if (opts.chunkText) { + for (const chunk of chunkMarkdownText(text, opts.chunkLimit)) { + const trimmed = chunk.trim(); + if (!trimmed || trimmed === SILENT_REPLY_TOKEN) continue; + out.push(trimmed); + } + return; + } + + const trimmed = text.trim(); + if (!trimmed || trimmed === SILENT_REPLY_TOKEN) return; + out.push(trimmed); +} + +function clampMs(value: number, maxMs: number): number { + if (!Number.isFinite(value) || value < 0) return 0; + return Math.min(value, maxMs); +} + +async function sleep(ms: number): Promise { + const delay = Math.max(0, ms); + if (delay === 0) return; + await new Promise((resolve) => { + setTimeout(resolve, delay); + }); +} + +function resolveRetryOptions( + retry: false | MSTeamsSendRetryOptions | undefined, +): Required & { enabled: boolean } { + if (!retry) { + return { enabled: false, maxAttempts: 1, baseDelayMs: 0, maxDelayMs: 0 }; + } + return { + enabled: true, + maxAttempts: Math.max(1, retry?.maxAttempts ?? 3), + baseDelayMs: Math.max(0, retry?.baseDelayMs ?? 250), + maxDelayMs: Math.max(0, retry?.maxDelayMs ?? 10_000), + }; +} + +function computeRetryDelayMs( + attempt: number, + classification: ReturnType, + opts: Required, +): number { + if (classification.retryAfterMs != null) { + return clampMs(classification.retryAfterMs, opts.maxDelayMs); + } + const exponential = opts.baseDelayMs * 2 ** Math.max(0, attempt - 1); + return clampMs(exponential, opts.maxDelayMs); +} + +function shouldRetry( + classification: ReturnType, +): boolean { + return ( + classification.kind === "throttled" || classification.kind === "transient" + ); +} + +export function renderReplyPayloadsToMessages( + replies: ReplyPayload[], + options: MSTeamsReplyRenderOptions, +): string[] { + const out: string[] = []; + const chunkLimit = Math.min(options.textChunkLimit, 4000); + const chunkText = options.chunkText !== false; + const mediaMode = options.mediaMode ?? "split"; + + for (const payload of replies) { + const mediaList = + payload.mediaUrls ?? (payload.mediaUrl ? [payload.mediaUrl] : []); + const text = payload.text ?? ""; + + if (!text && mediaList.length === 0) continue; + + if (mediaList.length === 0) { + pushTextMessages(out, text, { chunkText, chunkLimit }); + continue; + } + + if (mediaMode === "inline") { + const combined = text + ? `${text}\n\n${mediaList.join("\n")}` + : mediaList.join("\n"); + pushTextMessages(out, combined, { chunkText, chunkLimit }); + continue; + } + + // mediaMode === "split" + pushTextMessages(out, text, { chunkText, chunkLimit }); + for (const mediaUrl of mediaList) { + if (!mediaUrl) continue; + out.push(mediaUrl); + } + } + + return out; +} + +export async function sendMSTeamsMessages(params: { + replyStyle: MSTeamsReplyStyle; + adapter: MSTeamsAdapter; + appId: string; + conversationRef: StoredConversationReference; + context?: SendContext; + messages: string[]; + retry?: false | MSTeamsSendRetryOptions; + onRetry?: (event: MSTeamsSendRetryEvent) => void; +}): Promise { + const messages = params.messages + .map((m) => (typeof m === "string" ? m : String(m))) + .filter((m) => m.trim().length > 0); + if (messages.length === 0) return []; + + const retryOptions = resolveRetryOptions(params.retry); + + const sendWithRetry = async ( + sendOnce: () => Promise, + meta: { messageIndex: number; messageCount: number }, + ): Promise => { + if (!retryOptions.enabled) return await sendOnce(); + + let attempt = 1; + while (true) { + try { + return await sendOnce(); + } catch (err) { + const classification = classifyMSTeamsSendError(err); + const canRetry = + attempt < retryOptions.maxAttempts && shouldRetry(classification); + if (!canRetry) throw err; + + const delayMs = computeRetryDelayMs( + attempt, + classification, + retryOptions, + ); + const nextAttempt = attempt + 1; + params.onRetry?.({ + messageIndex: meta.messageIndex, + messageCount: meta.messageCount, + nextAttempt, + maxAttempts: retryOptions.maxAttempts, + delayMs, + classification, + }); + + await sleep(delayMs); + attempt = nextAttempt; + } + } + }; + + if (params.replyStyle === "thread") { + const ctx = params.context; + if (!ctx) { + throw new Error("Missing context for replyStyle=thread"); + } + const messageIds: string[] = []; + for (const [idx, message] of messages.entries()) { + const response = await sendWithRetry( + async () => + await ctx.sendActivity({ + type: "message", + text: message, + }), + { messageIndex: idx, messageCount: messages.length }, + ); + messageIds.push(extractMessageId(response) ?? "unknown"); + } + return messageIds; + } + + const baseRef = buildConversationReference(params.conversationRef); + const proactiveRef: ConversationReference = { + ...baseRef, + activityId: undefined, + }; + + const messageIds: string[] = []; + await params.adapter.continueConversation( + params.appId, + proactiveRef, + async (ctx) => { + for (const [idx, message] of messages.entries()) { + const response = await sendWithRetry( + async () => + await ctx.sendActivity({ + type: "message", + text: message, + }), + { messageIndex: idx, messageCount: messages.length }, + ); + messageIds.push(extractMessageId(response) ?? "unknown"); + } + }, + ); + return messageIds; +} diff --git a/src/msteams/monitor.ts b/src/msteams/monitor.ts index 620be86a6..5b9cebe5f 100644 --- a/src/msteams/monitor.ts +++ b/src/msteams/monitor.ts @@ -1,12 +1,8 @@ -import { - chunkMarkdownText, - resolveTextChunkLimit, -} from "../auto-reply/chunk.js"; +import type { Request, Response } from "express"; +import { resolveTextChunkLimit } from "../auto-reply/chunk.js"; import { formatAgentEnvelope } from "../auto-reply/envelope.js"; import { dispatchReplyFromConfig } from "../auto-reply/reply/dispatch-from-config.js"; import { createReplyDispatcherWithTyping } from "../auto-reply/reply/reply-dispatcher.js"; -import { SILENT_REPLY_TOKEN } from "../auto-reply/tokens.js"; -import type { ReplyPayload } from "../auto-reply/types.js"; import type { ClawdbotConfig } from "../config/types.js"; import { danger, logVerbose, shouldLogVerbose } from "../globals.js"; import { enqueueSystemEvent } from "../infra/system-events.js"; @@ -17,10 +13,32 @@ import { } from "../pairing/pairing-store.js"; import { resolveAgentRoute } from "../routing/resolve-route.js"; import type { RuntimeEnv } from "../runtime.js"; -import { - saveConversationReference, - type StoredConversationReference, +import type { + MSTeamsConversationStore, + StoredConversationReference, } from "./conversation-store.js"; +import { createMSTeamsConversationStoreFs } from "./conversation-store-fs.js"; +import { + classifyMSTeamsSendError, + formatMSTeamsSendErrorHint, + formatUnknownError, +} from "./errors.js"; +import { + normalizeMSTeamsConversationId, + parseMSTeamsActivityTimestamp, + stripMSTeamsMentionTags, + wasMSTeamsBotMentioned, +} from "./inbound.js"; +import { + type MSTeamsAdapter, + renderReplyPayloadsToMessages, + sendMSTeamsMessages, +} from "./messenger.js"; +import { + resolveMSTeamsReplyPolicy, + resolveMSTeamsRouteConfig, +} from "./policy.js"; +import type { MSTeamsTurnContext } from "./sdk-types.js"; import { resolveMSTeamsCredentials } from "./token.js"; const log = getChildLogger({ name: "msteams" }); @@ -29,6 +47,7 @@ export type MonitorMSTeamsOpts = { cfg: ClawdbotConfig; runtime?: RuntimeEnv; abortSignal?: AbortSignal; + conversationStore?: MSTeamsConversationStore; }; export type MonitorMSTeamsResult = { @@ -36,51 +55,6 @@ export type MonitorMSTeamsResult = { shutdown: () => Promise; }; -type TeamsActivity = { - id?: string; - type?: string; - timestamp?: string | Date; - text?: string; - from?: { id?: string; name?: string; aadObjectId?: string }; - recipient?: { id?: string; name?: string }; - conversation?: { - id?: string; - conversationType?: string; - tenantId?: string; - isGroup?: boolean; - }; - channelId?: string; - serviceUrl?: string; - membersAdded?: Array<{ id?: string; name?: string }>; - /** Entities including mentions */ - entities?: Array<{ - type?: string; - mentioned?: { id?: string; name?: string }; - }>; - /** Teams-specific channel data including team info */ - channelData?: { - team?: { id?: string; name?: string }; - channel?: { id?: string; name?: string }; - tenant?: { id?: string }; - }; -}; - -type TeamsTurnContext = { - activity: TeamsActivity; - sendActivity: (textOrActivity: string | object) => Promise; - sendActivities?: ( - activities: Array<{ type: string } & Record>, - ) => Promise; -}; - -// Helper to convert timestamp to Date -function parseTimestamp(ts?: string | Date): Date | undefined { - if (!ts) return undefined; - if (ts instanceof Date) return ts; - const date = new Date(ts); - return Number.isNaN(date.getTime()) ? undefined : date; -} - export async function monitorMSTeamsProvider( opts: MonitorMSTeamsOpts, ): Promise { @@ -108,6 +82,8 @@ export async function monitorMSTeamsProvider( const port = msteamsCfg.webhook?.port ?? 3978; const textLimit = resolveTextChunkLimit(cfg, "msteams"); + const conversationStore = + opts.conversationStore ?? createMSTeamsConversationStoreFs(); log.info(`starting provider (port ${port})`); @@ -115,8 +91,12 @@ export async function monitorMSTeamsProvider( const agentsHosting = await import("@microsoft/agents-hosting"); const express = await import("express"); - const { ActivityHandler, CloudAdapter, authorizeJWT, getAuthConfigWithDefaults } = - agentsHosting; + const { + ActivityHandler, + CloudAdapter, + authorizeJWT, + getAuthConfigWithDefaults, + } = agentsHosting; // Auth configuration - create early so adapter is available for deliverReplies const authConfig = getAuthConfigWithDefaults({ @@ -126,100 +106,11 @@ export async function monitorMSTeamsProvider( }); const adapter = new CloudAdapter(authConfig); - // Helper to deliver replies with configurable reply style - // - "thread": reply to the original message (for Posts layout channels) - // - "top-level": post as a new message (for Threads layout channels) - async function deliverReplies(params: { - replies: ReplyPayload[]; - // eslint-disable-next-line @typescript-eslint/no-explicit-any - context: any; // TurnContext from SDK - has activity.getConversationReference() - adapter: InstanceType; - appId: string; - replyStyle: "thread" | "top-level"; - }) { - const chunkLimit = Math.min(textLimit, 4000); - - // For "thread" style, use context.sendActivity directly (replies to original message) - // For "top-level" style, use proactive messaging without activityId - const sendMessage = - params.replyStyle === "thread" - ? async (message: string) => { - await params.context.sendActivity({ type: "message", text: message }); - } - : async (message: string) => { - // Get conversation reference from SDK's activity (includes proper bot info) - // Then remove activityId to avoid threading - // eslint-disable-next-line @typescript-eslint/no-explicit-any - const fullRef = params.context.activity.getConversationReference() as any; - const conversationRef = { - ...fullRef, - activityId: undefined, // Remove to post as top-level message - }; - // Also strip the messageid suffix from conversation.id if present - if (conversationRef.conversation?.id) { - conversationRef.conversation = { - ...conversationRef.conversation, - id: conversationRef.conversation.id.split(";")[0], - }; - } - // eslint-disable-next-line @typescript-eslint/no-explicit-any - await (params.adapter as any).continueConversation( - params.appId, - conversationRef, - // eslint-disable-next-line @typescript-eslint/no-explicit-any - async (ctx: any) => { - await ctx.sendActivity({ type: "message", text: message }); - }, - ); - }; - - for (const payload of params.replies) { - const mediaList = - payload.mediaUrls ?? (payload.mediaUrl ? [payload.mediaUrl] : []); - const text = payload.text ?? ""; - if (!text && mediaList.length === 0) continue; - - if (mediaList.length === 0) { - for (const chunk of chunkMarkdownText(text, chunkLimit)) { - const trimmed = chunk.trim(); - if (!trimmed || trimmed === SILENT_REPLY_TOKEN) continue; - await sendMessage(trimmed); - } - } else { - // For media, send text first then media URLs as separate messages - if (text.trim() && text.trim() !== SILENT_REPLY_TOKEN) { - for (const chunk of chunkMarkdownText(text, chunkLimit)) { - await sendMessage(chunk); - } - } - for (const mediaUrl of mediaList) { - await sendMessage(mediaUrl); - } - } - } - } - - // Strip Teams @mention HTML tags from message text - function stripMentionTags(text: string): string { - // Teams wraps mentions in ... tags - return text.replace(/.*?<\/at>/gi, "").trim(); - } - - // Check if the bot was mentioned in the activity - function wasBotMentioned(activity: TeamsActivity): boolean { - const botId = activity.recipient?.id; - if (!botId) return false; - const entities = activity.entities ?? []; - return entities.some( - (e) => e.type === "mention" && e.mentioned?.id === botId, - ); - } - // Handler for incoming messages - async function handleTeamsMessage(context: TeamsTurnContext) { + async function handleTeamsMessage(context: MSTeamsTurnContext) { const activity = context.activity; const rawText = activity.text?.trim() ?? ""; - const text = stripMentionTags(rawText); + const text = stripMSTeamsMentionTags(rawText); const from = activity.from; const conversation = activity.conversation; @@ -241,7 +132,7 @@ export async function monitorMSTeamsProvider( // Teams conversation.id may include ";messageid=..." suffix - strip it for session key const rawConversationId = conversation?.id ?? ""; - const conversationId = rawConversationId.split(";")[0]; + const conversationId = normalizeMSTeamsConversationId(rawConversationId); const conversationType = conversation?.conversationType ?? "personal"; const isGroupChat = conversationType === "groupChat" || conversation?.isGroup === true; @@ -266,8 +157,10 @@ export async function monitorMSTeamsProvider( channelId: activity.channelId, serviceUrl: activity.serviceUrl, }; - saveConversationReference(conversationId, conversationRef).catch((err) => { - log.debug("failed to save conversation reference", { error: String(err) }); + conversationStore.upsert(conversationId, conversationRef).catch((err) => { + log.debug("failed to save conversation reference", { + error: formatUnknownError(err), + }); }); // Build Teams-specific identifiers @@ -346,19 +239,21 @@ export async function monitorMSTeamsProvider( // Resolve team/channel config for channels and group chats const teamId = activity.channelData?.team?.id; const channelId = conversationId; - const teamConfig = teamId ? msteamsCfg?.teams?.[teamId] : undefined; - const channelConfig = teamConfig?.channels?.[channelId]; + const { teamConfig, channelConfig } = resolveMSTeamsRouteConfig({ + cfg: msteamsCfg, + teamId, + conversationId: channelId, + }); + const { requireMention, replyStyle } = resolveMSTeamsReplyPolicy({ + isDirectMessage, + globalConfig: msteamsCfg, + teamConfig, + channelConfig, + }); // Check requireMention for channels and group chats if (!isDirectMessage) { - // Resolution order: channel config > team config > global config > default (true) - const requireMention = - channelConfig?.requireMention ?? - teamConfig?.requireMention ?? - msteamsCfg?.requireMention ?? - true; - - const mentioned = wasBotMentioned(activity); + const mentioned = wasMSTeamsBotMentioned(activity); if (requireMention && !mentioned) { log.debug("skipping message (mention required)", { @@ -371,26 +266,8 @@ export async function monitorMSTeamsProvider( } } - // Resolve reply style for channels/groups - // Resolution order: channel config > team config > global config > default based on requireMention - // If requireMention is false (Threads layout), default to "top-level" - // If requireMention is true (Posts layout), default to "thread" - const explicitReplyStyle = - channelConfig?.replyStyle ?? - teamConfig?.replyStyle ?? - msteamsCfg?.replyStyle; - const effectiveRequireMention = - channelConfig?.requireMention ?? - teamConfig?.requireMention ?? - msteamsCfg?.requireMention ?? - true; - // For DMs, always use "thread" style (direct reply) - const replyStyle: "thread" | "top-level" = isDirectMessage - ? "thread" - : explicitReplyStyle ?? (effectiveRequireMention ? "thread" : "top-level"); - // Format the message body with envelope - const timestamp = parseTimestamp(activity.timestamp); + const timestamp = parseMSTeamsActivityTimestamp(activity.timestamp); const body = formatAgentEnvelope({ provider: "Teams", from: senderName, @@ -413,7 +290,7 @@ export async function monitorMSTeamsProvider( Surface: "msteams" as const, MessageSid: activity.id, Timestamp: timestamp?.getTime() ?? Date.now(), - WasMentioned: isDirectMessage || wasBotMentioned(activity), + WasMentioned: isDirectMessage || wasMSTeamsBotMentioned(activity), CommandAuthorized: true, OriginatingChannel: "msteams" as const, OriginatingTo: teamsTo, @@ -428,9 +305,7 @@ export async function monitorMSTeamsProvider( // Send typing indicator const sendTypingIndicator = async () => { try { - if (context.sendActivities) { - await context.sendActivities([{ type: "typing" }]); - } + await context.sendActivities([{ type: "typing" }]); } catch { // Typing indicator is best-effort } @@ -441,25 +316,43 @@ export async function monitorMSTeamsProvider( createReplyDispatcherWithTyping({ responsePrefix: cfg.messages?.responsePrefix, deliver: async (payload) => { - await deliverReplies({ - replies: [payload], - context, - adapter, - appId, + const messages = renderReplyPayloadsToMessages([payload], { + textChunkLimit: textLimit, + chunkText: true, + mediaMode: "split", + }); + await sendMSTeamsMessages({ replyStyle, + adapter: adapter as unknown as MSTeamsAdapter, + appId, + conversationRef, + context, + messages, + // Enable default retry/backoff for throttling/transient failures. + retry: {}, + onRetry: (event) => { + log.debug("retrying send", { + replyStyle, + ...event, + }); + }, }); }, onError: (err, info) => { - const errMsg = - err instanceof Error - ? err.message - : typeof err === "object" - ? JSON.stringify(err) - : String(err); + const errMsg = formatUnknownError(err); + const classification = classifyMSTeamsSendError(err); + const hint = formatMSTeamsSendErrorHint(classification); runtime.error?.( - danger(`msteams ${info.kind} reply failed: ${errMsg}`), + danger( + `msteams ${info.kind} reply failed: ${errMsg}${hint ? ` (${hint})` : ""}`, + ), ); - log.error("reply failed", { kind: info.kind, error: err }); + log.error("reply failed", { + kind: info.kind, + error: errMsg, + classification, + hint, + }); }, onReplyStart: sendTypingIndicator, }); @@ -499,11 +392,10 @@ export async function monitorMSTeamsProvider( } // Create activity handler using fluent API - // The SDK's TurnContext is compatible with our TeamsTurnContext const handler = new ActivityHandler() .onMessage(async (context, next) => { try { - await handleTeamsMessage(context as unknown as TeamsTurnContext); + await handleTeamsMessage(context as unknown as MSTeamsTurnContext); } catch (err) { runtime.error?.(danger(`msteams handler failed: ${String(err)}`)); } @@ -527,9 +419,12 @@ export async function monitorMSTeamsProvider( // Set up the messages endpoint - use configured path and /api/messages as fallback const configuredPath = msteamsCfg.webhook?.path ?? "/api/messages"; - // eslint-disable-next-line @typescript-eslint/no-explicit-any - const messageHandler = (req: any, res: any) => { - adapter.process(req, res, (context) => handler.run(context)); + const messageHandler = (req: Request, res: Response) => { + void adapter + .process(req, res, (context) => handler.run(context)) + .catch((err) => { + log.error("msteams webhook failed", { error: formatUnknownError(err) }); + }); }; // Listen on configured path and /api/messages (standard Bot Framework path) diff --git a/src/msteams/policy.test.ts b/src/msteams/policy.test.ts new file mode 100644 index 000000000..c0900ceb2 --- /dev/null +++ b/src/msteams/policy.test.ts @@ -0,0 +1,99 @@ +import { describe, expect, it } from "vitest"; + +import type { MSTeamsConfig } from "../config/types.js"; +import { + resolveMSTeamsReplyPolicy, + resolveMSTeamsRouteConfig, +} from "./policy.js"; + +describe("msteams policy", () => { + describe("resolveMSTeamsRouteConfig", () => { + it("returns team and channel config when present", () => { + const cfg: MSTeamsConfig = { + teams: { + team123: { + requireMention: false, + channels: { + chan456: { requireMention: true }, + }, + }, + }, + }; + + const res = resolveMSTeamsRouteConfig({ + cfg, + teamId: "team123", + conversationId: "chan456", + }); + + expect(res.teamConfig?.requireMention).toBe(false); + expect(res.channelConfig?.requireMention).toBe(true); + }); + + it("returns undefined configs when teamId is missing", () => { + const cfg: MSTeamsConfig = { + teams: { team123: { requireMention: false } }, + }; + + const res = resolveMSTeamsRouteConfig({ + cfg, + teamId: undefined, + conversationId: "chan", + }); + expect(res.teamConfig).toBeUndefined(); + expect(res.channelConfig).toBeUndefined(); + }); + }); + + describe("resolveMSTeamsReplyPolicy", () => { + it("forces thread replies for direct messages", () => { + const policy = resolveMSTeamsReplyPolicy({ + isDirectMessage: true, + globalConfig: { replyStyle: "top-level", requireMention: false }, + }); + expect(policy).toEqual({ requireMention: false, replyStyle: "thread" }); + }); + + it("defaults to requireMention=true and replyStyle=thread", () => { + const policy = resolveMSTeamsReplyPolicy({ + isDirectMessage: false, + globalConfig: {}, + }); + expect(policy).toEqual({ requireMention: true, replyStyle: "thread" }); + }); + + it("defaults replyStyle to top-level when requireMention=false", () => { + const policy = resolveMSTeamsReplyPolicy({ + isDirectMessage: false, + globalConfig: { requireMention: false }, + }); + expect(policy).toEqual({ + requireMention: false, + replyStyle: "top-level", + }); + }); + + it("prefers channel overrides over team and global defaults", () => { + const policy = resolveMSTeamsReplyPolicy({ + isDirectMessage: false, + globalConfig: { requireMention: true }, + teamConfig: { requireMention: true }, + channelConfig: { requireMention: false }, + }); + + // requireMention from channel -> false, and replyStyle defaults from requireMention -> top-level + expect(policy).toEqual({ + requireMention: false, + replyStyle: "top-level", + }); + }); + + it("uses explicit replyStyle even when requireMention defaults would differ", () => { + const policy = resolveMSTeamsReplyPolicy({ + isDirectMessage: false, + globalConfig: { requireMention: false, replyStyle: "thread" }, + }); + expect(policy).toEqual({ requireMention: false, replyStyle: "thread" }); + }); + }); +}); diff --git a/src/msteams/policy.ts b/src/msteams/policy.ts new file mode 100644 index 000000000..b96a83205 --- /dev/null +++ b/src/msteams/policy.ts @@ -0,0 +1,58 @@ +import type { + MSTeamsChannelConfig, + MSTeamsConfig, + MSTeamsReplyStyle, + MSTeamsTeamConfig, +} from "../config/types.js"; + +export type MSTeamsResolvedRouteConfig = { + teamConfig?: MSTeamsTeamConfig; + channelConfig?: MSTeamsChannelConfig; +}; + +export function resolveMSTeamsRouteConfig(params: { + cfg?: MSTeamsConfig; + teamId?: string | null | undefined; + conversationId?: string | null | undefined; +}): MSTeamsResolvedRouteConfig { + const teamId = params.teamId?.trim(); + const conversationId = params.conversationId?.trim(); + const teamConfig = teamId ? params.cfg?.teams?.[teamId] : undefined; + const channelConfig = + teamConfig && conversationId + ? teamConfig.channels?.[conversationId] + : undefined; + return { teamConfig, channelConfig }; +} + +export type MSTeamsReplyPolicy = { + requireMention: boolean; + replyStyle: MSTeamsReplyStyle; +}; + +export function resolveMSTeamsReplyPolicy(params: { + isDirectMessage: boolean; + globalConfig?: MSTeamsConfig; + teamConfig?: MSTeamsTeamConfig; + channelConfig?: MSTeamsChannelConfig; +}): MSTeamsReplyPolicy { + if (params.isDirectMessage) { + return { requireMention: false, replyStyle: "thread" }; + } + + const requireMention = + params.channelConfig?.requireMention ?? + params.teamConfig?.requireMention ?? + params.globalConfig?.requireMention ?? + true; + + const explicitReplyStyle = + params.channelConfig?.replyStyle ?? + params.teamConfig?.replyStyle ?? + params.globalConfig?.replyStyle; + + const replyStyle: MSTeamsReplyStyle = + explicitReplyStyle ?? (requireMention ? "thread" : "top-level"); + + return { requireMention, replyStyle }; +} diff --git a/src/msteams/probe.test.ts b/src/msteams/probe.test.ts new file mode 100644 index 000000000..1e22a42cf --- /dev/null +++ b/src/msteams/probe.test.ts @@ -0,0 +1,57 @@ +import { describe, expect, it, vi } from "vitest"; + +import type { MSTeamsConfig } from "../config/types.js"; + +const hostMockState = vi.hoisted(() => ({ + tokenError: null as Error | null, +})); + +vi.mock("@microsoft/agents-hosting", () => ({ + getAuthConfigWithDefaults: (cfg: unknown) => cfg, + MsalTokenProvider: class { + async getAccessToken() { + if (hostMockState.tokenError) throw hostMockState.tokenError; + return "token"; + } + }, +})); + +import { probeMSTeams } from "./probe.js"; + +describe("msteams probe", () => { + it("returns an error when credentials are missing", async () => { + const cfg = { enabled: true } as unknown as MSTeamsConfig; + await expect(probeMSTeams(cfg)).resolves.toMatchObject({ + ok: false, + }); + }); + + it("validates credentials by acquiring a token", async () => { + hostMockState.tokenError = null; + const cfg = { + enabled: true, + appId: "app", + appPassword: "pw", + tenantId: "tenant", + } as unknown as MSTeamsConfig; + await expect(probeMSTeams(cfg)).resolves.toMatchObject({ + ok: true, + appId: "app", + }); + }); + + it("returns a helpful error when token acquisition fails", async () => { + hostMockState.tokenError = new Error("bad creds"); + const cfg = { + enabled: true, + appId: "app", + appPassword: "pw", + tenantId: "tenant", + } as unknown as MSTeamsConfig; + await expect(probeMSTeams(cfg)).resolves.toMatchObject({ + ok: false, + appId: "app", + error: "bad creds", + }); + }); +}); diff --git a/src/msteams/probe.ts b/src/msteams/probe.ts index ecb4ecae1..44c36287a 100644 --- a/src/msteams/probe.ts +++ b/src/msteams/probe.ts @@ -1,4 +1,5 @@ import type { MSTeamsConfig } from "../config/types.js"; +import { formatUnknownError } from "./errors.js"; import { resolveMSTeamsCredentials } from "./token.js"; export type ProbeMSTeamsResult = { @@ -18,6 +19,24 @@ export async function probeMSTeams( }; } - // TODO: Validate credentials by attempting to get a token - return { ok: true, appId: creds.appId }; + try { + const { MsalTokenProvider, getAuthConfigWithDefaults } = await import( + "@microsoft/agents-hosting" + ); + const authConfig = getAuthConfigWithDefaults({ + clientId: creds.appId, + clientSecret: creds.appPassword, + tenantId: creds.tenantId, + }); + + const tokenProvider = new MsalTokenProvider(authConfig); + await tokenProvider.getAccessToken("https://api.botframework.com/.default"); + return { ok: true, appId: creds.appId }; + } catch (err) { + return { + ok: false, + appId: creds.appId, + error: formatUnknownError(err), + }; + } } diff --git a/src/msteams/sdk-types.ts b/src/msteams/sdk-types.ts new file mode 100644 index 000000000..0901848a3 --- /dev/null +++ b/src/msteams/sdk-types.ts @@ -0,0 +1,19 @@ +import type { TurnContext } from "@microsoft/agents-hosting"; + +/** + * Minimal public surface we depend on from the Microsoft SDK types. + * + * Note: we intentionally avoid coupling to SDK classes with private members + * (like TurnContext) in our own public signatures. The SDK's TS surface is also + * stricter than what the runtime accepts (e.g. it allows plain activity-like + * objects), so we model the minimal structural shape we rely on. + */ +export type MSTeamsActivity = TurnContext["activity"]; + +export type MSTeamsTurnContext = { + activity: MSTeamsActivity; + sendActivity: (textOrActivity: string | object) => Promise; + sendActivities: ( + activities: Array<{ type: string } & Record>, + ) => Promise; +}; diff --git a/src/msteams/send.ts b/src/msteams/send.ts index 0daf2a7c1..46192d913 100644 --- a/src/msteams/send.ts +++ b/src/msteams/send.ts @@ -1,23 +1,23 @@ import type { ClawdbotConfig } from "../config/types.js"; import type { getChildLogger as getChildLoggerFn } from "../logging.js"; -import { - getConversationReference, - listConversationReferences, - type StoredConversationReference, +import type { + MSTeamsConversationStore, + StoredConversationReference, } from "./conversation-store.js"; +import { createMSTeamsConversationStoreFs } from "./conversation-store-fs.js"; +import { + classifyMSTeamsSendError, + formatMSTeamsSendErrorHint, + formatUnknownError, +} from "./errors.js"; +import { type MSTeamsAdapter, sendMSTeamsMessages } from "./messenger.js"; import { resolveMSTeamsCredentials } from "./token.js"; -// Lazy logger to avoid initialization order issues in tests let _log: ReturnType | undefined; -const getLog = (): ReturnType => { - if (!_log) { - // Dynamic import to defer initialization - // eslint-disable-next-line @typescript-eslint/no-require-imports - const { getChildLogger } = require("../logging.js") as { - getChildLogger: typeof getChildLoggerFn; - }; - _log = getChildLogger({ name: "msteams:send" }); - } +const getLog = async (): Promise> => { + if (_log) return _log; + const { getChildLogger } = await import("../logging.js"); + _log = getChildLogger({ name: "msteams:send" }); return _log; }; @@ -66,63 +66,23 @@ function parseRecipient(to: string): { /** * Find a stored conversation reference for the given recipient. */ -async function findConversationReference( - recipient: { type: "conversation" | "user"; id: string }, -): Promise<{ conversationId: string; ref: StoredConversationReference } | null> { +async function findConversationReference(recipient: { + type: "conversation" | "user"; + id: string; + store: MSTeamsConversationStore; +}): Promise<{ + conversationId: string; + ref: StoredConversationReference; +} | null> { if (recipient.type === "conversation") { - const ref = await getConversationReference(recipient.id); + const ref = await recipient.store.get(recipient.id); if (ref) return { conversationId: recipient.id, ref }; return null; } - // Search by user AAD object ID - const all = await listConversationReferences(); - for (const { conversationId, reference } of all) { - if (reference.user?.aadObjectId === recipient.id) { - return { conversationId, ref: reference }; - } - if (reference.user?.id === recipient.id) { - return { conversationId, ref: reference }; - } - } - return null; -} - -// Type matching @microsoft/agents-activity ConversationReference -type ConversationReferenceShape = { - activityId?: string; - user?: { id: string; name?: string }; - bot?: { id: string; name?: string }; - conversation: { id: string; conversationType?: string; tenantId?: string }; - channelId: string; - serviceUrl?: string; - locale?: string; -}; - -/** - * Build a Bot Framework ConversationReference from our stored format. - * Note: activityId is intentionally omitted so proactive messages post as - * top-level messages rather than replies/threads. - */ -function buildConversationReference( - ref: StoredConversationReference, -): ConversationReferenceShape { - if (!ref.conversation?.id) { - throw new Error("Invalid stored reference: missing conversation.id"); - } - return { - // activityId omitted to avoid creating reply threads - user: ref.user?.id ? { id: ref.user.id, name: ref.user.name } : undefined, - bot: ref.bot?.id ? { id: ref.bot.id, name: ref.bot.name } : undefined, - conversation: { - id: ref.conversation.id, - conversationType: ref.conversation.conversationType, - tenantId: ref.conversation.tenantId, - }, - channelId: ref.channelId ?? "msteams", - serviceUrl: ref.serviceUrl, - locale: ref.locale, - }; + const found = await recipient.store.findByUserId(recipient.id); + if (!found) return null; + return { conversationId: found.conversationId, ref: found.reference }; } /** @@ -147,9 +107,11 @@ export async function sendMessageMSTeams( throw new Error("msteams credentials not configured"); } + const store = createMSTeamsConversationStoreFs(); + // Parse recipient and find conversation reference const recipient = parseRecipient(to); - const found = await findConversationReference(recipient); + const found = await findConversationReference({ ...recipient, store }); if (!found) { throw new Error( @@ -159,9 +121,10 @@ export async function sendMessageMSTeams( } const { conversationId, ref } = found; - const conversationRef = buildConversationReference(ref); - getLog().debug("sending proactive message", { + const log = await getLog(); + + log.debug("sending proactive message", { conversationId, textLength: text.length, hasMedia: Boolean(mediaUrl), @@ -179,27 +142,38 @@ export async function sendMessageMSTeams( const adapter = new CloudAdapter(authConfig); - let messageId = "unknown"; + const message = mediaUrl + ? text + ? `${text}\n\n${mediaUrl}` + : mediaUrl + : text; + let messageIds: string[]; + try { + messageIds = await sendMSTeamsMessages({ + replyStyle: "top-level", + adapter: adapter as unknown as MSTeamsAdapter, + appId: creds.appId, + conversationRef: ref, + messages: [message], + // Enable default retry/backoff for throttling/transient failures. + retry: {}, + onRetry: (event) => { + log.debug("retrying send", { conversationId, ...event }); + }, + }); + } catch (err) { + const classification = classifyMSTeamsSendError(err); + const hint = formatMSTeamsSendErrorHint(classification); + const status = classification.statusCode + ? ` (HTTP ${classification.statusCode})` + : ""; + throw new Error( + `msteams send failed${status}: ${formatUnknownError(err)}${hint ? ` (${hint})` : ""}`, + ); + } + const messageId = messageIds[0] ?? "unknown"; - // eslint-disable-next-line @typescript-eslint/no-explicit-any - await (adapter as any).continueConversation( - creds.appId, - conversationRef, - // eslint-disable-next-line @typescript-eslint/no-explicit-any - async (context: any) => { - // Build the activity - const activity = { - type: "message", - text: mediaUrl ? (text ? `${text}\n\n${mediaUrl}` : mediaUrl) : text, - }; - const response = await context.sendActivity(activity); - if (response?.id) { - messageId = response.id; - } - }, - ); - - getLog().info("sent proactive message", { conversationId, messageId }); + log.info("sent proactive message", { conversationId, messageId }); return { messageId, @@ -217,7 +191,8 @@ export async function listMSTeamsConversations(): Promise< conversationType?: string; }> > { - const all = await listConversationReferences(); + const store = createMSTeamsConversationStoreFs(); + const all = await store.list(); return all.map(({ conversationId, reference }) => ({ conversationId, userName: reference.user?.name, diff --git a/tmp/msteams-refactor-plan.md b/tmp/msteams-refactor-plan.md new file mode 100644 index 000000000..4296da48b --- /dev/null +++ b/tmp/msteams-refactor-plan.md @@ -0,0 +1,156 @@ +# MS Teams provider refactor plan (production-ready) + +Goal: refactor the MS Teams provider code (`src/msteams/*`) for long-term maintainability and correctness **without changing user-facing behavior** (except incidental bug fixes discovered during refactor). + +Status (2026-01-08): implemented (Phases 1–3) with unit tests; `pnpm lint && pnpm build && pnpm test` pass. + +## Why refactor + +Current pain points in `src/msteams/monitor.ts` / `src/msteams/send.ts` / `src/msteams/conversation-store.ts`: + +- **Mixed concerns**: HTTP server wiring, SDK handler, routing, policy resolution, and outbound delivery live in one file. +- **Duplicated outbound logic**: proactive vs in-thread sending is implemented in multiple places (monitor + send). +- **Weak typing boundary**: custom “SDK-like” shapes + structural casts make it harder to evolve safely. +- **Conversation store is fragile**: JSON file writes are un-locked and non-atomic; no TTL; potential corruption under concurrency. +- **Hard to test**: key logic (policy precedence and delivery behavior) is not isolated/pure. + +## Non-goals + +- Rewriting the provider around a different SDK. +- Introducing new configuration knobs beyond what already exists (`msteams.replyStyle`, `requireMention`, etc.). +- Changing routing semantics, payload envelope format, or session key logic. +- Adding new CLI commands (unless needed for validation/testing). + +## Target architecture (module split) + +### 1) Policy resolution (pure + tested) + +Add `src/msteams/policy.ts` (and `src/msteams/policy.test.ts`) containing pure functions: + +- `resolveMSTeamsRouteConfig({ cfg, teamId, conversationId }): { teamConfig?, channelConfig? }` +- `resolveMSTeamsReplyPolicy({ isDirectMessage, cfg, teamConfig?, channelConfig? }): { requireMention: boolean; replyStyle: "thread" | "top-level" }` + +Acceptance: precedence is encoded and unit-tested: + +- Channel overrides > team defaults > global defaults > implicit defaults. +- DM behavior: `replyStyle` is forced to `"thread"`, mention-gating is bypassed. +- Defaulting behavior matches existing runtime logic (e.g. `requireMention -> default replyStyle` heuristic). + +### 2) Outbound delivery (single implementation) + +Add `src/msteams/messenger.ts` (and `src/msteams/messenger.test.ts`) to centralize: + +- chunking (`resolveTextChunkLimit`, `chunkMarkdownText`, `SILENT_REPLY_TOKEN`) +- send mode selection (`"thread"` vs `"top-level"`) +- media URL message splitting (same semantics as current) +- error formatting + consistent structured logs + +Surface (current implementation): + +- `renderReplyPayloadsToMessages(replies, { textChunkLimit, chunkText, mediaMode })` +- `sendMSTeamsMessages({ replyStyle, adapter, appId, conversationRef, context?, messages })` + - uses `context.sendActivity` for `"thread"` + - uses `adapter.continueConversation` for `"top-level"` + +Acceptance: `src/msteams/monitor.ts` and `src/msteams/send.ts` both use the messenger, so there’s exactly one “how do we send a message” implementation. + +### 3) SDK typing boundary (type-only imports; no eager runtime deps) + +Add `src/msteams/sdk-types.ts` exporting the minimal types we depend on: + +- Turn context type (`sendActivity`, `activity` with fields we read) +- Conversation reference type for `continueConversation` +- Adapter interface subset (`continueConversation`, `process`) + +Implementation note: + +- Use `import type …` from the Microsoft SDK packages (or fallback to minimal structural types if the SDK does not export them cleanly). +- Keep current dynamic runtime imports (`await import("@microsoft/agents-hosting")`) intact; type-only imports compile away. + +Acceptance: eliminate bespoke `TeamsTurnContext` / ad-hoc casts where possible, while preserving lazy-load behavior (some casting may remain if SDK typings are stricter than runtime behavior). + +### 4) Conversation store interface + hardened FS implementation + +Introduce a store interface (e.g. `src/msteams/conversation-store.ts`) and move the current file-backed store to `src/msteams/conversation-store-fs.ts`. + +Store interface: + +- `upsert(conversationId, reference)` +- `get(conversationId)` +- `findByUser({ aadObjectId?, userId? })` +- `list()` +- `remove(conversationId)` + +FS implementation hardening: + +- **Atomic writes**: write to `*.tmp` then `rename` (or equivalent). +- **Locking**: use `proper-lockfile` (already a dependency) to guard read-modify-write. +- **TTL + pruning**: + - persist `lastSeenAt` + - prune on every write and/or on a timer + - cap size (keep existing `MAX_CONVERSATIONS` behavior, but deterministic + documented) +- **Permissions**: + - dir is already `0700`; ensure file is written with `0600` + +Tests: + +- Use an in-memory store implementation for unit tests. +- Add FS store tests only where stable (avoid flaky timing issues). + +Acceptance: no store corruption under concurrent writes in-process; behavior preserved for CLI `send` lookup. + +### 5) Monitor wiring becomes “thin” + +Refactor `src/msteams/monitor.ts` so it: + +- loads config + credentials +- creates adapter + express routes +- routes inbound messages to a smaller `handleInboundMessage(...)` function +- delegates: + - policy decisions to `policy.ts` + - outbound sends to `messenger.ts` + - reference persistence to the store abstraction + +Acceptance: `monitor.ts` is mostly wiring and orchestration; logic-heavy parts are tested in isolation. + +## Implementation phases (incremental, safe) + +### Phase 1 (behavior-preserving extraction) + +1. Add `src/msteams/policy.ts` + `src/msteams/policy.test.ts`. +2. Add `src/msteams/messenger.ts` + `src/msteams/messenger.test.ts` (unit test chunking + send mode selection; mock context/adapter). +3. Refactor `src/msteams/monitor.ts` to use policy + messenger (no behavior change). +4. Refactor `src/msteams/send.ts` to use messenger (no behavior change). +5. Extract inbound helpers (`stripMentionTags`, mention detection, conversation ID normalization) into `src/msteams/inbound.ts` + tests. +6. Ensure `pnpm lint && pnpm build && pnpm test` pass. + +### Phase 2 (store hardening) + +1. Introduce store interface + in-memory test store. +2. Move FS store to its own module; add locking + atomic writes + TTL. +3. Update `monitor.ts` + `send.ts` to depend on the interface (inject FS store from wiring). +4. Add targeted tests. + +### Phase 3 (production reliability) + +1. Add retry/backoff around outbound sends (careful: avoid duplicate posts; only retry safe failures). +2. Error classification helpers (auth misconfig, transient network, throttling). +3. Improve `probeMSTeams` to validate credentials (optional; can be separate). + +## Done criteria / checkpoints + +- Phase 1 done: + - New policy tests cover precedence and DM behavior. + - `monitor.ts` + `send.ts` share outbound sending via messenger. + - No new runtime imports that break lazy-load behavior. +- Phase 2 done: + - Store is locked + atomic + bounded. + - Clear migration story (keep same file format/version or bump explicitly). +- Phase 3 done: + - Retries are safe and bounded; logs are structured and actionable. + +## Notes / edge cases to validate during refactor + +- “Channel config” keys: currently based on `conversation.id` (e.g. `19:…@thread.tacv2`). Preserve that. +- `replyStyle="top-level"` correctness: ensure the conversation reference normalization is centralized and tested. +- Mention-gating: preserve current detection behavior (`entities` mention matching `recipient.id`), but isolate it for future improvements.