From 7c7e10ab853ef74707a68ca5210d15f888fe9a88 Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Sun, 23 Aug 2026 11:50:09 -0700 Subject: [PATCH] perf(msteams): reuse prepared activity on retries (#128329) Amp-Thread-ID: https://ampcode.com/threads/T-01a021f5-984a-7628-a30c-491c166ff247 Co-authored-by: Amp --- extensions/msteams/src/messenger.test.ts | 19 ++++++++++++++----- extensions/msteams/src/messenger.ts | 12 ++++++------ 2 files changed, 20 insertions(+), 11 deletions(-) diff --git a/extensions/msteams/src/messenger.test.ts b/extensions/msteams/src/messenger.test.ts index e52828fbdb48..2dc95629ee6e 100644 --- a/extensions/msteams/src/messenger.test.ts +++ b/extensions/msteams/src/messenger.test.ts @@ -508,13 +508,14 @@ describe("msteams messenger", () => { expect(retryEvents).toEqual([{ nextAttempt: 2, delayMs: 0 }]); }); - it("retries full activity preparation when media upload fails transiently", async () => { + it("retries media preparation but reuses it after provider dispatch starts", async () => { const tmpDir = await mkdtemp(path.join(resolvePreferredOpenClawTmpDir(), "msteams-retry-")); const localFile = path.join(tmpDir, "retry.txt"); await writeFile(localFile, "hello"); try { const attempts: string[] = []; + const providerPayloads: string[] = []; const retryEvents: Array<{ nextAttempt: number; delayMs: number }> = []; let uploadAttempts = 0; graphUploadMockState.uploadAndShareSharePoint.mockImplementation(async () => { @@ -535,8 +536,12 @@ describe("msteams messenger", () => { name: "retry.txt", }); + const sendActivity = createRecordedSendActivity(attempts, 429); const ctx = { - sendActivity: createRecordedSendActivity(attempts), + sendActivity: async (activity: unknown) => { + providerPayloads.push(JSON.stringify(activity)); + return await sendActivity(activity); + }, }; const ids = await sendMSTeamsMessages({ replyStyle: "thread", @@ -555,14 +560,18 @@ describe("msteams messenger", () => { getAccessToken: async () => "token", }, sharePointSiteId: "site-123", - retry: { maxAttempts: 2, baseDelayMs: 0, maxDelayMs: 0 }, + retry: { maxAttempts: 3, baseDelayMs: 0, maxDelayMs: 0 }, onRetry: (e) => retryEvents.push({ nextAttempt: e.nextAttempt, delayMs: e.delayMs }), }); expect(uploadAttempts).toBe(2); - expect(attempts).toEqual(["one"]); + expect(attempts).toEqual(["one", "one"]); + expect(providerPayloads[1]).toBe(providerPayloads[0]); expect(ids).toEqual(["id:one"]); - expect(retryEvents).toEqual([{ nextAttempt: 2, delayMs: 0 }]); + expect(retryEvents).toEqual([ + { nextAttempt: 2, delayMs: 0 }, + { nextAttempt: 3, delayMs: 0 }, + ]); } finally { await rm(tmpDir, { recursive: true, force: true }); } diff --git a/extensions/msteams/src/messenger.ts b/extensions/msteams/src/messenger.ts index bd3dc1f8662f..24294f3c73d6 100644 --- a/extensions/msteams/src/messenger.ts +++ b/extensions/msteams/src/messenger.ts @@ -452,12 +452,15 @@ export async function sendMSTeamsMessages(params: { message: MSTeamsRenderedMessage, messageIndex: number, ): Promise => { + let activity: Record | undefined; let pendingUploadId: string | undefined; let response: unknown; try { response = await sendWithRetry( async () => { - const activity = await buildActivity( + // Retry failed preparation, but keep its successful I/O and SharePoint work + // out of subsequent provider retries. + activity ??= await buildActivity( message, params.conversationRef, params.tokenProvider, @@ -466,14 +469,11 @@ export async function sendMSTeamsMessages(params: { { feedbackLoopEnabled: params.feedbackLoopEnabled }, ); - // Extract and strip the internal-only pending upload tag before sending. - pendingUploadId = + pendingUploadId ??= typeof activity["_pendingUploadId"] === "string" ? activity["_pendingUploadId"] : undefined; - if (pendingUploadId) { - delete activity["_pendingUploadId"]; - } + delete activity["_pendingUploadId"]; providerDispatchStarted = true; return await sendFn(activity);