From 1bf8d69d950deecea49646f4f8104bf95e9309a5 Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Sat, 28 Mar 2026 02:26:41 +0000 Subject: [PATCH] refactor(msteams): share conversation store helpers --- .../msteams/src/conversation-store-fs.test.ts | 314 +----------------- .../msteams/src/conversation-store-fs.ts | 50 +-- ...ction.ts => conversation-store-helpers.ts} | 34 +- .../msteams/src/conversation-store-memory.ts | 60 ++-- .../src/conversation-store.shared.test.ts | 201 +++++++++++ extensions/msteams/src/conversation-store.ts | 3 + extensions/msteams/src/graph-messages.test.ts | 22 +- extensions/msteams/src/graph-messages.ts | 2 +- .../src/monitor-handler.test-helpers.ts | 1 + extensions/msteams/src/send-context.ts | 2 +- 10 files changed, 311 insertions(+), 378 deletions(-) rename extensions/msteams/src/{conversation-store-selection.ts => conversation-store-helpers.ts} (55%) create mode 100644 extensions/msteams/src/conversation-store.shared.test.ts diff --git a/extensions/msteams/src/conversation-store-fs.test.ts b/extensions/msteams/src/conversation-store-fs.test.ts index a813cce1dd7..d7d92071cad 100644 --- a/extensions/msteams/src/conversation-store-fs.test.ts +++ b/extensions/msteams/src/conversation-store-fs.test.ts @@ -1,19 +1,18 @@ import fs from "node:fs"; import os from "node:os"; import path from "node:path"; -import { beforeEach, describe, expect, it, vi } from "vitest"; +import { beforeEach, describe, expect, it } from "vitest"; import { createMSTeamsConversationStoreFs } from "./conversation-store-fs.js"; -import { createMSTeamsConversationStoreMemory } from "./conversation-store-memory.js"; import type { StoredConversationReference } from "./conversation-store.js"; import { setMSTeamsRuntime } from "./runtime.js"; import { msteamsRuntimeStub } from "./test-runtime.js"; -describe("msteams conversation store (fs)", () => { +describe("msteams conversation store (fs-only)", () => { beforeEach(() => { setMSTeamsRuntime(msteamsRuntimeStub); }); - it("filters and prunes expired entries (but keeps legacy ones)", async () => { + it("filters and prunes expired entries while preserving legacy entries without lastSeenAt", async () => { const stateDir = await fs.promises.mkdtemp(path.join(os.tmpdir(), "openclaw-msteams-store-")); const env: NodeJS.ProcessEnv = { @@ -45,7 +44,6 @@ describe("msteams conversation store (fs)", () => { 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" }, @@ -54,7 +52,7 @@ describe("msteams conversation store (fs)", () => { await fs.promises.writeFile(filePath, `${JSON.stringify(json, null, 2)}\n`); const list = await store.list(); - const ids = list.map((e) => e.conversationId).toSorted(); + const ids = list.map((entry) => entry.conversationId).toSorted(); expect(ids).toEqual(["19:active@thread.tacv2", "19:legacy@thread.tacv2"]); expect(await store.get("19:old@thread.tacv2")).toBeNull(); @@ -73,308 +71,4 @@ describe("msteams conversation store (fs)", () => { "19:new@thread.tacv2", ]); }); - - it("stores and retrieves timezone from conversation reference", async () => { - const stateDir = await fs.promises.mkdtemp(path.join(os.tmpdir(), "openclaw-msteams-store-")); - const store = createMSTeamsConversationStoreFs({ - env: { ...process.env, OPENCLAW_STATE_DIR: stateDir }, - ttlMs: 60_000, - }); - - const ref: StoredConversationReference = { - conversation: { id: "19:tz-test@thread.tacv2" }, - channelId: "msteams", - serviceUrl: "https://service.example.com", - user: { id: "u1", aadObjectId: "aad1" }, - timezone: "America/Los_Angeles", - }; - - await store.upsert("19:tz-test@thread.tacv2", ref); - - const retrieved = await store.get("19:tz-test@thread.tacv2"); - expect(retrieved).not.toBeNull(); - expect(retrieved!.timezone).toBe("America/Los_Angeles"); - }); - - it("preserves existing timezone when upsert omits timezone", async () => { - const stateDir = await fs.promises.mkdtemp(path.join(os.tmpdir(), "openclaw-msteams-store-")); - const store = createMSTeamsConversationStoreFs({ - env: { ...process.env, OPENCLAW_STATE_DIR: stateDir }, - ttlMs: 60_000, - }); - - await store.upsert("19:tz-keep@thread.tacv2", { - conversation: { id: "19:tz-keep@thread.tacv2" }, - channelId: "msteams", - serviceUrl: "https://service.example.com", - user: { id: "u1" }, - timezone: "Europe/London", - }); - - // Second upsert without timezone field - await store.upsert("19:tz-keep@thread.tacv2", { - conversation: { id: "19:tz-keep@thread.tacv2" }, - channelId: "msteams", - serviceUrl: "https://service.example.com", - user: { id: "u1" }, - }); - - const retrieved = await store.get("19:tz-keep@thread.tacv2"); - expect(retrieved).not.toBeNull(); - expect(retrieved!.timezone).toBe("Europe/London"); - }); - - it("prefers the freshest personal conversation when a user has multiple references", async () => { - vi.useFakeTimers(); - try { - const stateDir = await fs.promises.mkdtemp(path.join(os.tmpdir(), "openclaw-msteams-store-")); - const store = createMSTeamsConversationStoreFs({ - env: { ...process.env, OPENCLAW_STATE_DIR: stateDir }, - ttlMs: 60_000, - }); - - vi.setSystemTime(new Date("2026-03-25T20:00:00.000Z")); - await store.upsert("a:old-personal", { - conversation: { id: "a:old-personal", conversationType: "personal" }, - channelId: "msteams", - serviceUrl: "https://service.example.com", - user: { id: "old-user", aadObjectId: "shared-aad" }, - }); - - vi.setSystemTime(new Date("2026-03-25T20:30:00.000Z")); - await store.upsert("19:group-chat", { - conversation: { id: "19:group-chat", conversationType: "groupChat" }, - channelId: "msteams", - serviceUrl: "https://service.example.com", - user: { id: "group-user", aadObjectId: "shared-aad" }, - }); - - vi.setSystemTime(new Date("2026-03-25T21:00:00.000Z")); - await store.upsert("a:new-personal", { - conversation: { id: "a:new-personal", conversationType: "personal" }, - channelId: "msteams", - serviceUrl: "https://service.example.com", - user: { id: "new-user", aadObjectId: "shared-aad" }, - }); - - await expect(store.findByUserId("shared-aad")).resolves.toEqual({ - conversationId: "a:new-personal", - reference: expect.objectContaining({ - conversation: expect.objectContaining({ - id: "a:new-personal", - conversationType: "personal", - }), - user: expect.objectContaining({ id: "new-user", aadObjectId: "shared-aad" }), - lastSeenAt: "2026-03-25T21:00:00.000Z", - }), - }); - } finally { - vi.useRealTimers(); - } - }); -}); - -describe("msteams conversation store (memory)", () => { - it("normalizes conversation ids the same way as the fs store", async () => { - const store = createMSTeamsConversationStoreMemory(); - - await store.upsert("conv-norm;messageid=123", { - conversation: { id: "conv-norm" }, - channelId: "msteams", - serviceUrl: "https://service.example.com", - user: { id: "u1" }, - }); - - await expect(store.get("conv-norm")).resolves.toEqual( - expect.objectContaining({ - conversation: { id: "conv-norm" }, - }), - ); - await expect(store.remove("conv-norm")).resolves.toBe(true); - await expect(store.get("conv-norm;messageid=123")).resolves.toBeNull(); - }); - - it("upserts, lists, removes, and resolves users by both AAD and Bot Framework ids", async () => { - const store = createMSTeamsConversationStoreMemory([ - { - conversationId: "conv-a", - reference: { - conversation: { id: "conv-a" }, - user: { id: "user-a", aadObjectId: "aad-a", name: "Alice" }, - }, - }, - { - conversationId: "dm-old", - reference: { - conversation: { id: "dm-old", conversationType: "personal" }, - user: { id: "user-shared-old", aadObjectId: "aad-shared", name: "Old DM" }, - lastSeenAt: "2026-03-25T20:00:00.000Z", - }, - }, - { - conversationId: "group-shared", - reference: { - conversation: { id: "group-shared", conversationType: "groupChat" }, - user: { id: "user-shared-group", aadObjectId: "aad-shared", name: "Group" }, - lastSeenAt: "2026-03-25T20:30:00.000Z", - }, - }, - { - conversationId: "dm-new", - reference: { - conversation: { id: "dm-new", conversationType: "personal" }, - user: { id: "user-shared-new", aadObjectId: "aad-shared", name: "New DM" }, - lastSeenAt: "2026-03-25T21:00:00.000Z", - }, - }, - ]); - - await store.upsert("conv-b", { - conversation: { id: "conv-b" }, - user: { id: "user-b", aadObjectId: "aad-b", name: "Bob" }, - }); - - await expect(store.get("conv-a")).resolves.toEqual({ - conversation: { id: "conv-a" }, - user: { id: "user-a", aadObjectId: "aad-a", name: "Alice" }, - }); - - await expect(store.list()).resolves.toEqual([ - { - conversationId: "conv-a", - reference: { - conversation: { id: "conv-a" }, - user: { id: "user-a", aadObjectId: "aad-a", name: "Alice" }, - }, - }, - { - conversationId: "dm-old", - reference: { - conversation: { id: "dm-old", conversationType: "personal" }, - user: { id: "user-shared-old", aadObjectId: "aad-shared", name: "Old DM" }, - lastSeenAt: "2026-03-25T20:00:00.000Z", - }, - }, - { - conversationId: "group-shared", - reference: { - conversation: { id: "group-shared", conversationType: "groupChat" }, - user: { id: "user-shared-group", aadObjectId: "aad-shared", name: "Group" }, - lastSeenAt: "2026-03-25T20:30:00.000Z", - }, - }, - { - conversationId: "dm-new", - reference: { - conversation: { id: "dm-new", conversationType: "personal" }, - user: { id: "user-shared-new", aadObjectId: "aad-shared", name: "New DM" }, - lastSeenAt: "2026-03-25T21:00:00.000Z", - }, - }, - { - conversationId: "conv-b", - reference: { - conversation: { id: "conv-b" }, - lastSeenAt: expect.any(String), - user: { id: "user-b", aadObjectId: "aad-b", name: "Bob" }, - }, - }, - ]); - - await expect(store.findByUserId(" aad-b ")).resolves.toEqual({ - conversationId: "conv-b", - reference: { - conversation: { id: "conv-b" }, - lastSeenAt: expect.any(String), - user: { id: "user-b", aadObjectId: "aad-b", name: "Bob" }, - }, - }); - await expect(store.findByUserId("user-a")).resolves.toEqual({ - conversationId: "conv-a", - reference: { - conversation: { id: "conv-a" }, - user: { id: "user-a", aadObjectId: "aad-a", name: "Alice" }, - }, - }); - await expect(store.findByUserId("aad-shared")).resolves.toEqual({ - conversationId: "dm-new", - reference: { - conversation: { id: "dm-new", conversationType: "personal" }, - user: { id: "user-shared-new", aadObjectId: "aad-shared", name: "New DM" }, - lastSeenAt: "2026-03-25T21:00:00.000Z", - }, - }); - await expect(store.findByUserId(" ")).resolves.toBeNull(); - - await expect(store.remove("conv-a")).resolves.toBe(true); - await expect(store.get("conv-a")).resolves.toBeNull(); - await expect(store.remove("missing")).resolves.toBe(false); - }); - - it("preserves existing timezone when upsert omits timezone, matching the fs store", async () => { - const store = createMSTeamsConversationStoreMemory(); - - await store.upsert("conv-tz", { - conversation: { id: "conv-tz" }, - channelId: "msteams", - serviceUrl: "https://service.example.com", - user: { id: "u1" }, - timezone: "Europe/London", - }); - - await store.upsert("conv-tz", { - conversation: { id: "conv-tz" }, - channelId: "msteams", - serviceUrl: "https://service.example.com", - user: { id: "u1" }, - }); - - await expect(store.get("conv-tz")).resolves.toMatchObject({ - timezone: "Europe/London", - }); - }); - - it("prefers the freshest personal conversation for repeated upserts of the same user", async () => { - vi.useFakeTimers(); - try { - const store = createMSTeamsConversationStoreMemory(); - - vi.setSystemTime(new Date("2026-03-25T20:00:00.000Z")); - await store.upsert("dm-old", { - conversation: { id: "dm-old", conversationType: "personal" }, - channelId: "msteams", - serviceUrl: "https://service.example.com", - user: { id: "user-shared-old", aadObjectId: "aad-shared", name: "Old DM" }, - }); - - vi.setSystemTime(new Date("2026-03-25T20:30:00.000Z")); - await store.upsert("group-shared", { - conversation: { id: "group-shared", conversationType: "groupChat" }, - channelId: "msteams", - serviceUrl: "https://service.example.com", - user: { id: "user-shared-group", aadObjectId: "aad-shared", name: "Group" }, - }); - - vi.setSystemTime(new Date("2026-03-25T21:00:00.000Z")); - await store.upsert("dm-new", { - conversation: { id: "dm-new", conversationType: "personal" }, - channelId: "msteams", - serviceUrl: "https://service.example.com", - user: { id: "user-shared-new", aadObjectId: "aad-shared", name: "New DM" }, - }); - - await expect(store.findByUserId("aad-shared")).resolves.toEqual({ - conversationId: "dm-new", - reference: { - conversation: { id: "dm-new", conversationType: "personal" }, - channelId: "msteams", - serviceUrl: "https://service.example.com", - user: { id: "user-shared-new", aadObjectId: "aad-shared", name: "New DM" }, - lastSeenAt: "2026-03-25T21:00:00.000Z", - }, - }); - } finally { - vi.useRealTimers(); - } - }); }); diff --git a/extensions/msteams/src/conversation-store-fs.ts b/extensions/msteams/src/conversation-store-fs.ts index 46d386762c3..8d67df49bf3 100644 --- a/extensions/msteams/src/conversation-store-fs.ts +++ b/extensions/msteams/src/conversation-store-fs.ts @@ -1,7 +1,10 @@ import { - findPreferredConversationByUserId, + findPreferredDmConversationByUserId, + mergeStoredConversationReference, + normalizeStoredConversationId, parseStoredConversationTimestamp, -} from "./conversation-store-selection.js"; + toConversationStoreEntries, +} from "./conversation-store-helpers.js"; import type { MSTeamsConversationStore, MSTeamsConversationStoreEntry, @@ -54,10 +57,6 @@ function pruneExpired( return { conversations: kept, removed }; } -function normalizeConversationId(raw: string): string { - return raw.split(";")[0] ?? raw; -} - export function createMSTeamsConversationStoreFs(params?: { env?: NodeJS.ProcessEnv; homedir?: () => string; @@ -93,36 +92,32 @@ export function createMSTeamsConversationStoreFs(params?: { const list = async (): Promise => { const store = await readStore(); - return Object.entries(store.conversations).map(([conversationId, reference]) => ({ - conversationId, - reference, - })); + return toConversationStoreEntries(Object.entries(store.conversations)); }; const get = async (conversationId: string): Promise => { const store = await readStore(); - return store.conversations[normalizeConversationId(conversationId)] ?? null; + return store.conversations[normalizeStoredConversationId(conversationId)] ?? null; }; - const findByUserId = async (id: string): Promise => { - return findPreferredConversationByUserId(await list(), id); + const findPreferredDmByUserId = async ( + id: string, + ): Promise => { + return findPreferredDmConversationByUserId(await list(), id); }; const upsert = async ( conversationId: string, reference: StoredConversationReference, ): Promise => { - const normalizedId = normalizeConversationId(conversationId); + const normalizedId = normalizeStoredConversationId(conversationId); await withFileLock(filePath, empty, async () => { const store = await readStore(); - const existing = store.conversations[normalizedId]; - store.conversations[normalizedId] = { - // Preserve fields from previous entry that may not be present on every activity - // (e.g. timezone is only sent when clientInfo entity is available). - ...(existing?.timezone && !reference.timezone ? { timezone: existing.timezone } : {}), - ...reference, - lastSeenAt: new Date().toISOString(), - }; + store.conversations[normalizedId] = mergeStoredConversationReference( + store.conversations[normalizedId], + reference, + new Date().toISOString(), + ); const nowMs = Date.now(); store.conversations = pruneExpired(store.conversations, nowMs, ttlMs).conversations; store.conversations = pruneToLimit(store.conversations); @@ -131,7 +126,7 @@ export function createMSTeamsConversationStoreFs(params?: { }; const remove = async (conversationId: string): Promise => { - const normalizedId = normalizeConversationId(conversationId); + const normalizedId = normalizeStoredConversationId(conversationId); return await withFileLock(filePath, empty, async () => { const store = await readStore(); if (!(normalizedId in store.conversations)) { @@ -143,5 +138,12 @@ export function createMSTeamsConversationStoreFs(params?: { }); }; - return { upsert, get, list, remove, findByUserId }; + return { + upsert, + get, + list, + remove, + findPreferredDmByUserId, + findByUserId: findPreferredDmByUserId, + }; } diff --git a/extensions/msteams/src/conversation-store-selection.ts b/extensions/msteams/src/conversation-store-helpers.ts similarity index 55% rename from extensions/msteams/src/conversation-store-selection.ts rename to extensions/msteams/src/conversation-store-helpers.ts index 5c3d4331dd9..6581d592f96 100644 --- a/extensions/msteams/src/conversation-store-selection.ts +++ b/extensions/msteams/src/conversation-store-helpers.ts @@ -1,4 +1,11 @@ -import type { MSTeamsConversationStoreEntry } from "./conversation-store.js"; +import type { + MSTeamsConversationStoreEntry, + StoredConversationReference, +} from "./conversation-store.js"; + +export function normalizeStoredConversationId(raw: string): string { + return raw.split(";")[0] ?? raw; +} export function parseStoredConversationTimestamp(value: string | undefined): number | null { if (!value) { @@ -11,7 +18,30 @@ export function parseStoredConversationTimestamp(value: string | undefined): num return parsed; } -export function findPreferredConversationByUserId( +export function toConversationStoreEntries( + entries: Iterable<[string, StoredConversationReference]>, +): MSTeamsConversationStoreEntry[] { + return Array.from(entries, ([conversationId, reference]) => ({ + conversationId, + reference, + })); +} + +export function mergeStoredConversationReference( + existing: StoredConversationReference | undefined, + incoming: StoredConversationReference, + nowIso: string, +): StoredConversationReference { + return { + // Preserve fields from previous entry that may not be present on every activity + // (e.g. timezone is only sent when clientInfo entity is available). + ...(existing?.timezone && !incoming.timezone ? { timezone: existing.timezone } : {}), + ...incoming, + lastSeenAt: nowIso, + }; +} + +export function findPreferredDmConversationByUserId( entries: Iterable, id: string, ): MSTeamsConversationStoreEntry | null { diff --git a/extensions/msteams/src/conversation-store-memory.ts b/extensions/msteams/src/conversation-store-memory.ts index 1a6a9ae2fd3..24164b60128 100644 --- a/extensions/msteams/src/conversation-store-memory.ts +++ b/extensions/msteams/src/conversation-store-memory.ts @@ -1,4 +1,9 @@ -import { findPreferredConversationByUserId } from "./conversation-store-selection.js"; +import { + findPreferredDmConversationByUserId, + mergeStoredConversationReference, + normalizeStoredConversationId, + toConversationStoreEntries, +} from "./conversation-store-helpers.js"; import type { MSTeamsConversationStore, MSTeamsConversationStoreEntry, @@ -9,41 +14,38 @@ export function createMSTeamsConversationStoreMemory( initial: MSTeamsConversationStoreEntry[] = [], ): MSTeamsConversationStore { const map = new Map(); - const normalizeConversationId = (raw: string): string => raw.split(";")[0] ?? raw; for (const { conversationId, reference } of initial) { - map.set(normalizeConversationId(conversationId), reference); + map.set(normalizeStoredConversationId(conversationId), reference); } + const findPreferredDmByUserId = async ( + id: string, + ): Promise => { + return findPreferredDmConversationByUserId(toConversationStoreEntries(map.entries()), id); + }; + return { upsert: async (conversationId, reference) => { - const normalizedId = normalizeConversationId(conversationId); - const existing = map.get(normalizedId); - map.set(normalizedId, { - ...(existing?.timezone && !reference.timezone ? { timezone: existing.timezone } : {}), - ...reference, - lastSeenAt: new Date().toISOString(), - }); - }, - get: async (conversationId) => { - return map.get(normalizeConversationId(conversationId)) ?? null; - }, - list: async () => { - return Array.from(map.entries()).map(([conversationId, reference]) => ({ - conversationId, - reference, - })); - }, - remove: async (conversationId) => { - return map.delete(normalizeConversationId(conversationId)); - }, - findByUserId: async (id) => { - return findPreferredConversationByUserId( - Array.from(map.entries()).map(([conversationId, reference]) => ({ - conversationId, + const normalizedId = normalizeStoredConversationId(conversationId); + map.set( + normalizedId, + mergeStoredConversationReference( + map.get(normalizedId), reference, - })), - id, + new Date().toISOString(), + ), ); }, + get: async (conversationId) => { + return map.get(normalizeStoredConversationId(conversationId)) ?? null; + }, + list: async () => { + return toConversationStoreEntries(map.entries()); + }, + remove: async (conversationId) => { + return map.delete(normalizeStoredConversationId(conversationId)); + }, + findPreferredDmByUserId, + findByUserId: findPreferredDmByUserId, }; } diff --git a/extensions/msteams/src/conversation-store.shared.test.ts b/extensions/msteams/src/conversation-store.shared.test.ts new file mode 100644 index 00000000000..44ad384a2d5 --- /dev/null +++ b/extensions/msteams/src/conversation-store.shared.test.ts @@ -0,0 +1,201 @@ +import fs from "node:fs"; +import os from "node:os"; +import path from "node:path"; +import { beforeEach, describe, expect, it, vi } from "vitest"; +import { createMSTeamsConversationStoreFs } from "./conversation-store-fs.js"; +import { createMSTeamsConversationStoreMemory } from "./conversation-store-memory.js"; +import type { MSTeamsConversationStore } from "./conversation-store.js"; +import { setMSTeamsRuntime } from "./runtime.js"; +import { msteamsRuntimeStub } from "./test-runtime.js"; + +type StoreFactory = { + name: string; + createStore: () => Promise; +}; + +const storeFactories: StoreFactory[] = [ + { + name: "fs", + createStore: async () => { + const stateDir = await fs.promises.mkdtemp(path.join(os.tmpdir(), "openclaw-msteams-store-")); + return createMSTeamsConversationStoreFs({ + env: { ...process.env, OPENCLAW_STATE_DIR: stateDir }, + ttlMs: 60_000, + }); + }, + }, + { + name: "memory", + createStore: async () => createMSTeamsConversationStoreMemory(), + }, +]; + +describe.each(storeFactories)("msteams conversation store ($name)", ({ createStore }) => { + beforeEach(() => { + setMSTeamsRuntime(msteamsRuntimeStub); + }); + + it("normalizes conversation ids consistently", async () => { + const store = await createStore(); + + await store.upsert("conv-norm;messageid=123", { + conversation: { id: "conv-norm" }, + channelId: "msteams", + serviceUrl: "https://service.example.com", + user: { id: "u1" }, + }); + + await expect(store.get("conv-norm")).resolves.toEqual( + expect.objectContaining({ + conversation: { id: "conv-norm" }, + }), + ); + await expect(store.remove("conv-norm")).resolves.toBe(true); + await expect(store.get("conv-norm;messageid=123")).resolves.toBeNull(); + }); + + it("upserts, lists, removes, and resolves users by both AAD and Bot Framework ids", async () => { + const store = await createStore(); + + await store.upsert("conv-a", { + conversation: { id: "conv-a" }, + channelId: "msteams", + serviceUrl: "https://service.example.com", + user: { id: "user-a", aadObjectId: "aad-a", name: "Alice" }, + }); + + await store.upsert("conv-b", { + conversation: { id: "conv-b" }, + channelId: "msteams", + serviceUrl: "https://service.example.com", + user: { id: "user-b", aadObjectId: "aad-b", name: "Bob" }, + }); + + await expect(store.get("conv-a")).resolves.toEqual({ + conversation: { id: "conv-a" }, + channelId: "msteams", + serviceUrl: "https://service.example.com", + user: { id: "user-a", aadObjectId: "aad-a", name: "Alice" }, + lastSeenAt: expect.any(String), + }); + + await expect(store.list()).resolves.toEqual([ + { + conversationId: "conv-a", + reference: { + conversation: { id: "conv-a" }, + channelId: "msteams", + serviceUrl: "https://service.example.com", + user: { id: "user-a", aadObjectId: "aad-a", name: "Alice" }, + lastSeenAt: expect.any(String), + }, + }, + { + conversationId: "conv-b", + reference: { + conversation: { id: "conv-b" }, + channelId: "msteams", + serviceUrl: "https://service.example.com", + user: { id: "user-b", aadObjectId: "aad-b", name: "Bob" }, + lastSeenAt: expect.any(String), + }, + }, + ]); + + await expect(store.findPreferredDmByUserId(" aad-b ")).resolves.toEqual({ + conversationId: "conv-b", + reference: { + conversation: { id: "conv-b" }, + channelId: "msteams", + serviceUrl: "https://service.example.com", + user: { id: "user-b", aadObjectId: "aad-b", name: "Bob" }, + lastSeenAt: expect.any(String), + }, + }); + await expect(store.findPreferredDmByUserId("user-a")).resolves.toEqual({ + conversationId: "conv-a", + reference: { + conversation: { id: "conv-a" }, + channelId: "msteams", + serviceUrl: "https://service.example.com", + user: { id: "user-a", aadObjectId: "aad-a", name: "Alice" }, + lastSeenAt: expect.any(String), + }, + }); + await expect(store.findByUserId("user-a")).resolves.toEqual( + await store.findPreferredDmByUserId("user-a"), + ); + await expect(store.findPreferredDmByUserId(" ")).resolves.toBeNull(); + + await expect(store.remove("conv-a")).resolves.toBe(true); + await expect(store.get("conv-a")).resolves.toBeNull(); + await expect(store.remove("missing")).resolves.toBe(false); + }); + + it("preserves existing timezone when upsert omits timezone", async () => { + const store = await createStore(); + + await store.upsert("conv-tz", { + conversation: { id: "conv-tz" }, + channelId: "msteams", + serviceUrl: "https://service.example.com", + user: { id: "u1" }, + timezone: "Europe/London", + }); + + await store.upsert("conv-tz", { + conversation: { id: "conv-tz" }, + channelId: "msteams", + serviceUrl: "https://service.example.com", + user: { id: "u1" }, + }); + + await expect(store.get("conv-tz")).resolves.toMatchObject({ + timezone: "Europe/London", + }); + }); + + it("prefers the freshest personal conversation for repeated upserts of the same user", async () => { + const store = await createStore(); + + vi.useFakeTimers(); + try { + vi.setSystemTime(new Date("2026-03-25T20:00:00.000Z")); + await store.upsert("dm-old", { + conversation: { id: "dm-old", conversationType: "personal" }, + channelId: "msteams", + serviceUrl: "https://service.example.com", + user: { id: "user-shared-old", aadObjectId: "aad-shared", name: "Old DM" }, + }); + + vi.setSystemTime(new Date("2026-03-25T20:30:00.000Z")); + await store.upsert("group-shared", { + conversation: { id: "group-shared", conversationType: "groupChat" }, + channelId: "msteams", + serviceUrl: "https://service.example.com", + user: { id: "user-shared-group", aadObjectId: "aad-shared", name: "Group" }, + }); + + vi.setSystemTime(new Date("2026-03-25T21:00:00.000Z")); + await store.upsert("dm-new", { + conversation: { id: "dm-new", conversationType: "personal" }, + channelId: "msteams", + serviceUrl: "https://service.example.com", + user: { id: "user-shared-new", aadObjectId: "aad-shared", name: "New DM" }, + }); + + await expect(store.findPreferredDmByUserId("aad-shared")).resolves.toEqual({ + conversationId: "dm-new", + reference: { + conversation: { id: "dm-new", conversationType: "personal" }, + channelId: "msteams", + serviceUrl: "https://service.example.com", + user: { id: "user-shared-new", aadObjectId: "aad-shared", name: "New DM" }, + lastSeenAt: "2026-03-25T21:00:00.000Z", + }, + }); + } finally { + vi.useRealTimers(); + } + }); +}); diff --git a/extensions/msteams/src/conversation-store.ts b/extensions/msteams/src/conversation-store.ts index a3804c63be6..04389b95ae3 100644 --- a/extensions/msteams/src/conversation-store.ts +++ b/extensions/msteams/src/conversation-store.ts @@ -48,5 +48,8 @@ export type MSTeamsConversationStore = { get: (conversationId: string) => Promise; list: () => Promise; remove: (conversationId: string) => Promise; + /** Person-targeted proactive lookup: prefer the freshest personal DM reference. */ + findPreferredDmByUserId: (id: string) => Promise; + /** @deprecated Use `findPreferredDmByUserId` for proactive user-targeted sends. */ findByUserId: (id: string) => Promise; }; diff --git a/extensions/msteams/src/graph-messages.test.ts b/extensions/msteams/src/graph-messages.test.ts index 95c4075ad10..7b68b509a49 100644 --- a/extensions/msteams/src/graph-messages.test.ts +++ b/extensions/msteams/src/graph-messages.test.ts @@ -17,7 +17,7 @@ const mockState = vi.hoisted(() => ({ postGraphJson: vi.fn(), postGraphBetaJson: vi.fn(), deleteGraphRequest: vi.fn(), - findByUserId: vi.fn(), + findPreferredDmByUserId: vi.fn(), })); vi.mock("./graph.js", async (importOriginal) => { @@ -34,7 +34,7 @@ vi.mock("./graph.js", async (importOriginal) => { vi.mock("./conversation-store-fs.js", () => ({ createMSTeamsConversationStoreFs: () => ({ - findByUserId: mockState.findByUserId, + findPreferredDmByUserId: mockState.findPreferredDmByUserId, }), })); @@ -49,7 +49,7 @@ describe("getMessageMSTeams", () => { }); it("resolves user: target using graphChatId from store", async () => { - mockState.findByUserId.mockResolvedValue({ + mockState.findPreferredDmByUserId.mockResolvedValue({ conversationId: "a:bot-framework-dm-id", reference: { graphChatId: "19:graph-native-chat@thread.tacv2" }, }); @@ -65,7 +65,7 @@ describe("getMessageMSTeams", () => { messageId: "msg-1", }); - expect(mockState.findByUserId).toHaveBeenCalledWith("aad-object-id-123"); + expect(mockState.findPreferredDmByUserId).toHaveBeenCalledWith("aad-object-id-123"); // Must use the graphChatId, not the Bot Framework conversation ID expect(mockState.fetchGraphJson).toHaveBeenCalledWith({ token: TOKEN, @@ -74,7 +74,7 @@ describe("getMessageMSTeams", () => { }); it("falls back to conversationId when it starts with 19:", async () => { - mockState.findByUserId.mockResolvedValue({ + mockState.findPreferredDmByUserId.mockResolvedValue({ conversationId: "19:resolved-chat@thread.tacv2", reference: {}, }); @@ -97,7 +97,7 @@ describe("getMessageMSTeams", () => { }); it("throws when user: target has no stored conversation", async () => { - mockState.findByUserId.mockResolvedValue(null); + mockState.findPreferredDmByUserId.mockResolvedValue(null); await expect( getMessageMSTeams({ @@ -109,7 +109,7 @@ describe("getMessageMSTeams", () => { }); it("throws when user: target has Bot Framework ID and no graphChatId", async () => { - mockState.findByUserId.mockResolvedValue({ + mockState.findPreferredDmByUserId.mockResolvedValue({ conversationId: "a:bot-framework-dm-id", reference: {}, }); @@ -394,7 +394,7 @@ describe("reactMessageMSTeams", () => { }); it("resolves user: target through conversation store", async () => { - mockState.findByUserId.mockResolvedValue({ + mockState.findPreferredDmByUserId.mockResolvedValue({ conversationId: "a:bot-id", reference: { graphChatId: "19:dm-chat@thread.tacv2" }, }); @@ -407,7 +407,7 @@ describe("reactMessageMSTeams", () => { reactionType: "like", }); - expect(mockState.findByUserId).toHaveBeenCalledWith("aad-user-1"); + expect(mockState.findPreferredDmByUserId).toHaveBeenCalledWith("aad-user-1"); expect(mockState.postGraphBetaJson).toHaveBeenCalledWith({ token: TOKEN, path: `/chats/${encodeURIComponent("19:dm-chat@thread.tacv2")}/messages/msg-1/setReaction`, @@ -729,7 +729,7 @@ describe("searchMessagesMSTeams", () => { }); it("resolves user: target through conversation store", async () => { - mockState.findByUserId.mockResolvedValue({ + mockState.findPreferredDmByUserId.mockResolvedValue({ conversationId: "a:bot-id", reference: { graphChatId: "19:dm-chat@thread.tacv2" }, }); @@ -741,7 +741,7 @@ describe("searchMessagesMSTeams", () => { query: "hello", }); - expect(mockState.findByUserId).toHaveBeenCalledWith("aad-user-1"); + expect(mockState.findPreferredDmByUserId).toHaveBeenCalledWith("aad-user-1"); const calledPath = mockState.fetchGraphJson.mock.calls[0][0].path as string; expect(calledPath).toContain( `/chats/${encodeURIComponent("19:dm-chat@thread.tacv2")}/messages?`, diff --git a/extensions/msteams/src/graph-messages.ts b/extensions/msteams/src/graph-messages.ts index ace83cc73b8..1d0d0933b21 100644 --- a/extensions/msteams/src/graph-messages.ts +++ b/extensions/msteams/src/graph-messages.ts @@ -74,7 +74,7 @@ async function resolveGraphConversationId(to: string): Promise { // user: — look up the conversation store for the real chat ID const store = createMSTeamsConversationStoreFs(); - const found = await store.findByUserId(cleaned); + const found = await store.findPreferredDmByUserId(cleaned); if (!found) { throw new Error( `No conversation found for user:${cleaned}. ` + diff --git a/extensions/msteams/src/monitor-handler.test-helpers.ts b/extensions/msteams/src/monitor-handler.test-helpers.ts index dbc0562a56b..099ddf291af 100644 --- a/extensions/msteams/src/monitor-handler.test-helpers.ts +++ b/extensions/msteams/src/monitor-handler.test-helpers.ts @@ -38,6 +38,7 @@ export function createMSTeamsMessageHandlerDeps(params?: { get: async () => null, list: async () => [], remove: async () => false, + findPreferredDmByUserId: async () => null, findByUserId: async () => null, }; const pollStore: MSTeamsPollStore = { diff --git a/extensions/msteams/src/send-context.ts b/extensions/msteams/src/send-context.ts index a6c92f7b0c8..2e991a099c6 100644 --- a/extensions/msteams/src/send-context.ts +++ b/extensions/msteams/src/send-context.ts @@ -92,7 +92,7 @@ async function findConversationReference(recipient: { return null; } - const found = await recipient.store.findByUserId(recipient.id); + const found = await recipient.store.findPreferredDmByUserId(recipient.id); if (!found) { return null; }