From cc7a498ace94087f4ffe1b2b6a4df85373a09af2 Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Mon, 23 Feb 2026 17:59:46 +0000 Subject: [PATCH] refactor(tests): deduplicate repeated fixtures in msteams and bash tests --- extensions/msteams/src/attachments.test.ts | 416 +++++++++++------- src/agents/bash-tools.test.ts | 469 +++++++++++++-------- 2 files changed, 567 insertions(+), 318 deletions(-) diff --git a/extensions/msteams/src/attachments.test.ts b/extensions/msteams/src/attachments.test.ts index 71cddb08d62..d370c192110 100644 --- a/extensions/msteams/src/attachments.test.ts +++ b/extensions/msteams/src/attachments.test.ts @@ -15,11 +15,30 @@ vi.mock("openclaw/plugin-sdk", () => ({ /** Mock DNS resolver that always returns a public IP (for anti-SSRF validation in tests). */ const publicResolveFn = async () => ({ address: "13.107.136.10" }); +const SAVED_PNG_PATH = "/tmp/saved.png"; +const SAVED_PDF_PATH = "/tmp/saved.pdf"; +const TEST_URL_IMAGE = "https://x/img"; +const TEST_URL_IMAGE_PNG = "https://x/img.png"; +const TEST_URL_IMAGE_1_PNG = "https://x/1.png"; +const TEST_URL_IMAGE_2_JPG = "https://x/2.jpg"; +const TEST_URL_PDF = "https://x/x.pdf"; +const TEST_URL_PDF_1 = "https://x/1.pdf"; +const TEST_URL_PDF_2 = "https://x/2.pdf"; +const TEST_URL_HTML_A = "https://x/a.png"; +const TEST_URL_HTML_B = "https://x/b.png"; +const TEST_URL_INLINE_IMAGE = "https://x/inline.png"; +const TEST_URL_DOC_PDF = "https://x/doc.pdf"; +const TEST_URL_FILE_DOWNLOAD = "https://x/dl"; +const TEST_URL_OUTSIDE_ALLOWLIST = "https://evil.test/img"; +const CONTENT_TYPE_IMAGE_PNG = "image/png"; +const CONTENT_TYPE_APPLICATION_PDF = "application/pdf"; +const CONTENT_TYPE_TEXT_HTML = "text/html"; +const CONTENT_TYPE_TEAMS_FILE_DOWNLOAD_INFO = "application/vnd.microsoft.teams.file.download.info"; -const detectMimeMock = vi.fn(async () => "image/png"); +const detectMimeMock = vi.fn(async () => CONTENT_TYPE_IMAGE_PNG); const saveMediaBufferMock = vi.fn(async () => ({ - path: "/tmp/saved.png", - contentType: "image/png", + path: SAVED_PNG_PATH, + contentType: CONTENT_TYPE_IMAGE_PNG, })); const fetchRemoteMediaMock = vi.fn( async (params: { @@ -62,6 +81,8 @@ const runtimeStub = { type DownloadAttachmentsParams = Parameters[0]; type DownloadGraphMediaParams = Parameters[0]; type DownloadedMedia = Awaited>; +type DownloadedGraphMedia = Awaited>; +type MSTeamsMediaPayload = ReturnType; type DownloadAttachmentsBuildOverrides = Partial< Omit > & @@ -73,33 +94,65 @@ type DownloadAttachmentsNoFetchOverrides = Partial< > > & Pick; +type DownloadGraphMediaOverrides = Partial< + Omit +>; +type FetchCallExpectation = { expectFetchCalled?: boolean }; +type DownloadedMediaExpectation = { path?: string; placeholder?: string }; +type MSTeamsMediaPayloadExpectation = { + firstPath: string; + paths: string[]; + types: string[]; +}; const DEFAULT_MESSAGE_URL = "https://graph.microsoft.com/v1.0/chats/19%3Achat/messages/123"; const DEFAULT_MAX_BYTES = 1024 * 1024; const DEFAULT_ALLOW_HOSTS = ["x"]; -const IMAGE_ATTACHMENT = { contentType: "image/png", contentUrl: "https://x/img" }; +const DEFAULT_SHAREPOINT_ALLOW_HOSTS = ["graph.microsoft.com", "contoso.sharepoint.com"]; +const DEFAULT_SHARE_REFERENCE_URL = "https://contoso.sharepoint.com/site/file"; +const MEDIA_PLACEHOLDER_IMAGE = ""; +const MEDIA_PLACEHOLDER_DOCUMENT = ""; +const IMAGE_ATTACHMENT = { contentType: CONTENT_TYPE_IMAGE_PNG, contentUrl: TEST_URL_IMAGE }; const PNG_BUFFER = Buffer.from("png"); const PNG_BASE64 = PNG_BUFFER.toString("base64"); const PDF_BUFFER = Buffer.from("pdf"); const createTokenProvider = () => ({ getAccessToken: vi.fn(async () => "token") }); +const asSingleItemArray = (value: T) => [value]; const buildAttachment = >(contentType: string, props: T) => ({ contentType, ...props, }); -const createHtmlAttachment = (content: string) => buildAttachment("text/html", { content }); -const createImageAttachment = (contentUrl: string) => buildAttachment("image/png", { contentUrl }); -const createPdfAttachment = (contentUrl: string) => - buildAttachment("application/pdf", { contentUrl }); -const createTeamsFileDownloadInfoAttachment = (downloadUrl = "https://x/dl", fileType = "png") => - buildAttachment("application/vnd.microsoft.teams.file.download.info", { - content: { downloadUrl, fileType }, +const createHtmlAttachment = (content: string) => + buildAttachment(CONTENT_TYPE_TEXT_HTML, { content }); +const buildHtmlImageTag = (src: string) => ``; +const createHtmlImageAttachments = (sources: string[], prefix = "") => + asSingleItemArray(createHtmlAttachment(`${prefix}${sources.map(buildHtmlImageTag).join("")}`)); +const createImageAttachments = (...contentUrls: string[]) => + contentUrls.map((contentUrl) => buildAttachment(CONTENT_TYPE_IMAGE_PNG, { contentUrl })); +const createPdfAttachments = (...contentUrls: string[]) => + contentUrls.map((contentUrl) => buildAttachment(CONTENT_TYPE_APPLICATION_PDF, { contentUrl })); +const createTeamsFileDownloadInfoAttachments = ( + downloadUrl = TEST_URL_FILE_DOWNLOAD, + fileType = "png", +) => + asSingleItemArray( + buildAttachment(CONTENT_TYPE_TEAMS_FILE_DOWNLOAD_INFO, { + content: { downloadUrl, fileType }, + }), + ); +const createImageMediaEntries = (...paths: string[]) => + paths.map((path) => ({ path, contentType: CONTENT_TYPE_IMAGE_PNG })); +const createHostedImageContents = (...ids: string[]) => + ids.map((id) => ({ id, contentType: CONTENT_TYPE_IMAGE_PNG, contentBytes: PNG_BASE64 })); +const createPdfResponse = (payload: Buffer | string = PDF_BUFFER) => { + const raw = Buffer.isBuffer(payload) ? payload : Buffer.from(payload); + return new Response(new Uint8Array(raw), { + status: 200, + headers: { "content-type": CONTENT_TYPE_APPLICATION_PDF }, }); -const createImageMediaEntry = (path: string) => ({ path, contentType: "image/png" }); -const createHostedImageContent = (id: string) => ({ - id, - contentType: "image/png", - contentBytes: PNG_BASE64, -}); +}; +const createJsonResponse = (payload: unknown, status = 200) => + new Response(JSON.stringify(payload), { status }); const createOkFetchMock = (contentType: string, payload = "png") => vi.fn(async () => { @@ -137,26 +190,22 @@ const downloadAttachmentsWithFetch = async ( attachments: DownloadAttachmentsParams["attachments"], fetchFn: unknown, overrides: DownloadAttachmentsNoFetchOverrides = {}, - options: { expectFetchCalled?: boolean } = {}, + options: FetchCallExpectation = {}, ) => { const media = await downloadMSTeamsAttachments( buildDownloadParamsWithFetch(attachments, fetchFn, overrides), ); - if (options.expectFetchCalled ?? true) { - expect(fetchFn).toHaveBeenCalled(); - } else { - expect(fetchFn).not.toHaveBeenCalled(); - } + expectMockCallState(fetchFn, options.expectFetchCalled ?? true); return media; }; const downloadAttachmentsWithOkImageFetch = ( attachments: DownloadAttachmentsParams["attachments"], overrides: DownloadAttachmentsNoFetchOverrides = {}, - options: { expectFetchCalled?: boolean } = {}, + options: FetchCallExpectation = {}, ) => { return downloadAttachmentsWithFetch( attachments, - createOkFetchMock("image/png"), + createOkFetchMock(CONTENT_TYPE_IMAGE_PNG), overrides, options, ); @@ -171,15 +220,20 @@ const createAuthAwareImageFetchMock = (params: { unauthStatus: number; unauthBod } return new Response(PNG_BUFFER, { status: 200, - headers: { "content-type": "image/png" }, + headers: { "content-type": CONTENT_TYPE_IMAGE_PNG }, }); }); +const expectMockCallState = (mockFn: unknown, shouldCall: boolean) => { + if (shouldCall) { + expect(mockFn).toHaveBeenCalled(); + } else { + expect(mockFn).not.toHaveBeenCalled(); + } +}; const buildDownloadGraphParams = ( fetchFn: unknown, - overrides: Partial< - Omit - > = {}, + overrides: DownloadGraphMediaOverrides = {}, ): DownloadGraphMediaParams => { return { messageUrl: DEFAULT_MESSAGE_URL, @@ -189,12 +243,28 @@ const buildDownloadGraphParams = ( ...overrides, }; }; +const DEFAULT_CHANNEL_TEAM_ID = "team-id"; +const DEFAULT_CHANNEL_ID = "chan-id"; +const createChannelGraphMessageUrlParams = (params: { + messageId: string; + replyToId?: string; + conversationId?: string; +}) => ({ + conversationType: "channel" as const, + ...params, + channelData: { + team: { id: DEFAULT_CHANNEL_TEAM_ID }, + channel: { id: DEFAULT_CHANNEL_ID }, + }, +}); +const buildExpectedChannelMessagePath = (params: { messageId: string; replyToId?: string }) => + params.replyToId + ? `/teams/${DEFAULT_CHANNEL_TEAM_ID}/channels/${DEFAULT_CHANNEL_ID}/messages/${params.replyToId}/replies/${params.messageId}` + : `/teams/${DEFAULT_CHANNEL_TEAM_ID}/channels/${DEFAULT_CHANNEL_ID}/messages/${params.messageId}`; const downloadGraphMediaWithFetch = ( fetchFn: unknown, - overrides: Partial< - Omit - > = {}, + overrides: DownloadGraphMediaOverrides = {}, ) => { return downloadMSTeamsGraphMedia(buildDownloadGraphParams(fetchFn, overrides)); }; @@ -211,6 +281,47 @@ const expectAttachmentPlaceholder = ( ) => { expect(buildMSTeamsAttachmentPlaceholder(attachments)).toBe(expected); }; +const expectLength = (value: { length: number }, expectedLength: number) => { + expect(value).toHaveLength(expectedLength); +}; +const expectMediaLength = (media: DownloadedMedia, expectedLength: number) => { + expectLength(media, expectedLength); +}; +const expectGraphMediaLength = (media: DownloadedGraphMedia, expectedLength: number) => { + expectLength(media.media, expectedLength); +}; +const expectNoMedia = (media: DownloadedMedia) => { + expectMediaLength(media, 0); +}; +const expectSingleMedia = (media: DownloadedMedia, expected: DownloadedMediaExpectation = {}) => { + expectMediaLength(media, 1); + expectFirstMedia(media, expected); +}; +const expectNoGraphMedia = (media: DownloadedGraphMedia) => { + expectGraphMediaLength(media, 0); +}; +const expectMediaSaved = () => { + expect(saveMediaBufferMock).toHaveBeenCalled(); +}; +const expectFirstMedia = (media: DownloadedMedia, expected: DownloadedMediaExpectation) => { + const first = media[0]; + if (expected.path !== undefined) { + expect(first?.path).toBe(expected.path); + } + if (expected.placeholder !== undefined) { + expect(first?.placeholder).toBe(expected.placeholder); + } +}; +const expectMSTeamsMediaPayload = ( + payload: MSTeamsMediaPayload, + expected: MSTeamsMediaPayloadExpectation, +) => { + expect(payload.MediaPath).toBe(expected.firstPath); + expect(payload.MediaUrl).toBe(expected.firstPath); + expect(payload.MediaPaths).toEqual(expected.paths); + expect(payload.MediaUrls).toEqual(expected.paths); + expect(payload.MediaTypes).toEqual(expected.types); +}; type AttachmentPlaceholderCase = { label: string; attachments: Parameters[0]; @@ -238,6 +349,15 @@ type GraphUrlExpectationCase = { params: Parameters[0]; expectedPath: string; }; +type GraphMediaSuccessCase = { + label: string; + buildOptions: () => GraphFetchMockOptions; + expectedLength: number; + assert?: (params: { + fetchMock: ReturnType; + media: Awaited>; + }) => void; +}; type GraphFetchMockOptions = { hostedContents?: unknown[]; @@ -247,16 +367,29 @@ type GraphFetchMockOptions = { onUnhandled?: (url: string) => Response | Promise | undefined; }; -const createReferenceAttachment = (shareUrl: string) => ({ +const createReferenceAttachment = (shareUrl = DEFAULT_SHARE_REFERENCE_URL) => ({ id: "ref-1", contentType: "reference", contentUrl: shareUrl, name: "report.pdf", }); -const createShareReferenceFixture = (shareUrl = "https://contoso.sharepoint.com/site/file") => ({ - shareUrl, - referenceAttachment: createReferenceAttachment(shareUrl), +const buildShareReferenceGraphFetchOptions = (params: { + referenceAttachment: ReturnType; + onShareRequest?: GraphFetchMockOptions["onShareRequest"]; + onUnhandled?: GraphFetchMockOptions["onUnhandled"]; +}) => ({ + attachments: [params.referenceAttachment], + messageAttachments: [params.referenceAttachment], + ...(params.onShareRequest ? { onShareRequest: params.onShareRequest } : {}), + ...(params.onUnhandled ? { onUnhandled: params.onUnhandled } : {}), }); +const buildDefaultShareReferenceGraphFetchOptions = ( + params: Omit[0], "referenceAttachment">, +) => + buildShareReferenceGraphFetchOptions({ + referenceAttachment: createReferenceAttachment(), + ...params, + }); const createGraphFetchMock = (options: GraphFetchMockOptions = {}) => { const hostedContents = options.hostedContents ?? []; @@ -264,13 +397,13 @@ const createGraphFetchMock = (options: GraphFetchMockOptions = {}) => { const messageAttachments = options.messageAttachments ?? []; return vi.fn(async (url: string) => { if (url.endsWith("/hostedContents")) { - return new Response(JSON.stringify({ value: hostedContents }), { status: 200 }); + return createJsonResponse({ value: hostedContents }); } if (url.endsWith("/attachments")) { - return new Response(JSON.stringify({ value: attachments }), { status: 200 }); + return createJsonResponse({ value: attachments }); } if (url.endsWith("/messages/123")) { - return new Response(JSON.stringify({ attachments: messageAttachments }), { status: 200 }); + return createJsonResponse({ attachments: messageAttachments }); } if (url.startsWith("https://graph.microsoft.com/v1.0/shares/") && options.onShareRequest) { return options.onShareRequest(url); @@ -281,9 +414,7 @@ const createGraphFetchMock = (options: GraphFetchMockOptions = {}) => { }; const downloadGraphMediaWithMockOptions = async ( options: GraphFetchMockOptions = {}, - overrides: Partial< - Omit - > = {}, + overrides: DownloadGraphMediaOverrides = {}, ) => { const fetchMock = createGraphFetchMock(options); const media = await downloadGraphMediaWithFetch(fetchMock, overrides); @@ -296,7 +427,7 @@ const runAttachmentAuthRetryScenario = async (scenario: AttachmentAuthRetryScena unauthBody: scenario.unauthBody, }); const media = await downloadAttachmentsWithFetch( - [createImageAttachment(scenario.attachmentUrl)], + createImageAttachments(scenario.attachmentUrl), fetchMock, { tokenProvider, ...scenario.overrides }, ); @@ -317,46 +448,41 @@ describe("msteams attachments", () => { { label: "returns empty string when attachments are empty", attachments: [], expected: "" }, { label: "returns image placeholder for one image attachment", - attachments: [createImageAttachment("https://x/img.png")], - expected: "", + attachments: createImageAttachments(TEST_URL_IMAGE_PNG), + expected: MEDIA_PLACEHOLDER_IMAGE, }, { label: "returns image placeholder with count for many image attachments", attachments: [ - createImageAttachment("https://x/1.png"), - { contentType: "image/jpeg", contentUrl: "https://x/2.jpg" }, + ...createImageAttachments(TEST_URL_IMAGE_1_PNG), + { contentType: "image/jpeg", contentUrl: TEST_URL_IMAGE_2_JPG }, ], - expected: " (2 images)", + expected: `${MEDIA_PLACEHOLDER_IMAGE} (2 images)`, }, { label: "treats Teams file.download.info image attachments as images", - attachments: [createTeamsFileDownloadInfoAttachment()], - expected: "", + attachments: createTeamsFileDownloadInfoAttachments(), + expected: MEDIA_PLACEHOLDER_IMAGE, }, { label: "returns document placeholder for non-image attachments", - attachments: [createPdfAttachment("https://x/x.pdf")], - expected: "", + attachments: createPdfAttachments(TEST_URL_PDF), + expected: MEDIA_PLACEHOLDER_DOCUMENT, }, { label: "returns document placeholder with count for many non-image attachments", - attachments: [ - createPdfAttachment("https://x/1.pdf"), - createPdfAttachment("https://x/2.pdf"), - ], - expected: " (2 files)", + attachments: createPdfAttachments(TEST_URL_PDF_1, TEST_URL_PDF_2), + expected: `${MEDIA_PLACEHOLDER_DOCUMENT} (2 files)`, }, { label: "counts one inline image in html attachments", - attachments: [createHtmlAttachment('

hi

')], - expected: "", + attachments: createHtmlImageAttachments([TEST_URL_HTML_A], "

hi

"), + expected: MEDIA_PLACEHOLDER_IMAGE, }, { label: "counts many inline images in html attachments", - attachments: [ - createHtmlAttachment(''), - ], - expected: " (2 images)", + attachments: createHtmlImageAttachments([TEST_URL_HTML_A, TEST_URL_HTML_B]), + expected: `${MEDIA_PLACEHOLDER_IMAGE} (2 images)`, }, ])("$label", ({ attachments, expected }) => { expectAttachmentPlaceholder(attachments, expected); @@ -367,53 +493,54 @@ describe("msteams attachments", () => { it.each([ { label: "downloads and stores image contentUrl attachments", - attachments: [IMAGE_ATTACHMENT], + attachments: asSingleItemArray(IMAGE_ATTACHMENT), assert: (media) => { - expect(saveMediaBufferMock).toHaveBeenCalled(); - expect(media[0]?.path).toBe("/tmp/saved.png"); + expectMediaSaved(); + expectFirstMedia(media, { path: SAVED_PNG_PATH }); }, }, { label: "supports Teams file.download.info downloadUrl attachments", - attachments: [createTeamsFileDownloadInfoAttachment()], + attachments: createTeamsFileDownloadInfoAttachments(), }, { label: "downloads inline image URLs from html attachments", - attachments: [createHtmlAttachment('')], + attachments: createHtmlImageAttachments([TEST_URL_INLINE_IMAGE]), }, ])("$label", async ({ attachments, assert }) => { const media = await downloadAttachmentsWithOkImageFetch(attachments); - expect(media).toHaveLength(1); + expectSingleMedia(media); assert?.(media); }); it("downloads non-image file attachments (PDF)", async () => { - const fetchMock = createOkFetchMock("application/pdf", "pdf"); - detectMimeMock.mockResolvedValueOnce("application/pdf"); + const fetchMock = createOkFetchMock(CONTENT_TYPE_APPLICATION_PDF, "pdf"); + detectMimeMock.mockResolvedValueOnce(CONTENT_TYPE_APPLICATION_PDF); saveMediaBufferMock.mockResolvedValueOnce({ - path: "/tmp/saved.pdf", - contentType: "application/pdf", + path: SAVED_PDF_PATH, + contentType: CONTENT_TYPE_APPLICATION_PDF, }); const media = await downloadAttachmentsWithFetch( - [createPdfAttachment("https://x/doc.pdf")], + createPdfAttachments(TEST_URL_DOC_PDF), fetchMock, ); - expect(media).toHaveLength(1); - expect(media[0]?.path).toBe("/tmp/saved.pdf"); - expect(media[0]?.placeholder).toBe(""); + expectSingleMedia(media, { + path: SAVED_PDF_PATH, + placeholder: MEDIA_PLACEHOLDER_DOCUMENT, + }); }); it("stores inline data:image base64 payloads", async () => { const media = await downloadMSTeamsAttachments( buildDownloadParams([ - createHtmlAttachment(``), + ...createHtmlImageAttachments([`data:image/png;base64,${PNG_BASE64}`]), ]), ); - expect(media).toHaveLength(1); - expect(saveMediaBufferMock).toHaveBeenCalled(); + expectSingleMedia(media); + expectMediaSaved(); }); it.each([ @@ -444,18 +571,14 @@ describe("msteams attachments", () => { }, ])("$label", async ({ scenario, expectedMediaLength, expectTokenFetch }) => { const { tokenProvider, media } = await runAttachmentAuthRetryScenario(scenario); - expect(media).toHaveLength(expectedMediaLength); - if (expectTokenFetch) { - expect(tokenProvider.getAccessToken).toHaveBeenCalled(); - } else { - expect(tokenProvider.getAccessToken).not.toHaveBeenCalled(); - } + expectMediaLength(media, expectedMediaLength); + expectMockCallState(tokenProvider.getAccessToken, expectTokenFetch); }); it("skips urls outside the allowlist", async () => { const fetchMock = vi.fn(); const media = await downloadAttachmentsWithFetch( - [createImageAttachment("https://evil.test/img")], + createImageAttachments(TEST_URL_OUTSIDE_ALLOWLIST), fetchMock, { allowHosts: ["graph.microsoft.com"], @@ -464,7 +587,7 @@ describe("msteams attachments", () => { { expectFetchCalled: false }, ); - expect(media).toHaveLength(0); + expectNoMedia(media); }); }); @@ -472,23 +595,22 @@ describe("msteams attachments", () => { const cases: GraphUrlExpectationCase[] = [ { label: "builds channel message urls", - params: { - conversationType: "channel" as const, + params: createChannelGraphMessageUrlParams({ conversationId: "19:thread@thread.tacv2", messageId: "123", - channelData: { team: { id: "team-id" }, channel: { id: "chan-id" } }, - }, - expectedPath: "/teams/team-id/channels/chan-id/messages/123", + }), + expectedPath: buildExpectedChannelMessagePath({ messageId: "123" }), }, { label: "builds channel reply urls when replyToId is present", - params: { - conversationType: "channel" as const, + params: createChannelGraphMessageUrlParams({ messageId: "reply-id", replyToId: "root-id", - channelData: { team: { id: "team-id" }, channel: { id: "chan-id" } }, - }, - expectedPath: "/teams/team-id/channels/chan-id/messages/root-id/replies/reply-id", + }), + expectedPath: buildExpectedChannelMessagePath({ + messageId: "reply-id", + replyToId: "root-id", + }), }, { label: "builds chat message urls", @@ -507,34 +629,35 @@ describe("msteams attachments", () => { }); describe("downloadMSTeamsGraphMedia", () => { - it("downloads hostedContents images", async () => { - const { fetchMock, media } = await downloadGraphMediaWithMockOptions({ - hostedContents: [createHostedImageContent("1")], - }); - - expect(media.media).toHaveLength(1); - expect(fetchMock).toHaveBeenCalled(); - expect(saveMediaBufferMock).toHaveBeenCalled(); - }); - - it("merges SharePoint reference attachments with hosted content", async () => { - const { referenceAttachment } = createShareReferenceFixture(); - const { media } = await downloadGraphMediaWithMockOptions({ - hostedContents: [createHostedImageContent("hosted-1")], - attachments: [referenceAttachment], - messageAttachments: [referenceAttachment], - onShareRequest: () => - new Response(PDF_BUFFER, { - status: 200, - headers: { "content-type": "application/pdf" }, - }), - }); - - expect(media.media).toHaveLength(2); + it.each([ + { + label: "downloads hostedContents images", + buildOptions: () => ({ hostedContents: createHostedImageContents("1") }), + expectedLength: 1, + assert: ({ fetchMock }) => { + expect(fetchMock).toHaveBeenCalled(); + expectMediaSaved(); + }, + }, + { + label: "merges SharePoint reference attachments with hosted content", + buildOptions: () => { + return { + hostedContents: createHostedImageContents("hosted-1"), + ...buildDefaultShareReferenceGraphFetchOptions({ + onShareRequest: () => createPdfResponse(), + }), + }; + }, + expectedLength: 2, + }, + ])("$label", async ({ buildOptions, expectedLength, assert }) => { + const { fetchMock, media } = await downloadGraphMediaWithMockOptions(buildOptions()); + expectGraphMediaLength(media, expectedLength); + assert?.({ fetchMock, media }); }); it("blocks SharePoint redirects to hosts outside allowHosts", async () => { - const { referenceAttachment } = createShareReferenceFixture(); const escapedUrl = "https://evil.example/internal.pdf"; fetchRemoteMediaMock.mockImplementationOnce(async (params) => { const fetchFn = params.fetchImpl ?? fetch; @@ -563,28 +686,26 @@ describe("msteams attachments", () => { const { fetchMock, media } = await downloadGraphMediaWithMockOptions( { - messageAttachments: [referenceAttachment], - onShareRequest: () => - new Response(null, { - status: 302, - headers: { location: escapedUrl }, - }), - onUnhandled: (url) => { - if (url === escapedUrl) { - return new Response(Buffer.from("should-not-be-fetched"), { - status: 200, - headers: { "content-type": "application/pdf" }, - }); - } - return undefined; - }, + ...buildDefaultShareReferenceGraphFetchOptions({ + onShareRequest: () => + new Response(null, { + status: 302, + headers: { location: escapedUrl }, + }), + onUnhandled: (url) => { + if (url === escapedUrl) { + return createPdfResponse("should-not-be-fetched"); + } + return undefined; + }, + }), }, { - allowHosts: ["graph.microsoft.com", "contoso.sharepoint.com"], + allowHosts: DEFAULT_SHAREPOINT_ALLOW_HOSTS, }, ); - expect(media.media).toHaveLength(0); + expectNoGraphMedia(media); const calledUrls = fetchMock.mock.calls.map((call) => String(call[0])); expect( calledUrls.some((url) => url.startsWith("https://graph.microsoft.com/v1.0/shares/")), @@ -595,15 +716,12 @@ describe("msteams attachments", () => { describe("buildMSTeamsMediaPayload", () => { it("returns single and multi-file fields", async () => { - const payload = buildMSTeamsMediaPayload([ - createImageMediaEntry("/tmp/a.png"), - createImageMediaEntry("/tmp/b.png"), - ]); - expect(payload.MediaPath).toBe("/tmp/a.png"); - expect(payload.MediaUrl).toBe("/tmp/a.png"); - expect(payload.MediaPaths).toEqual(["/tmp/a.png", "/tmp/b.png"]); - expect(payload.MediaUrls).toEqual(["/tmp/a.png", "/tmp/b.png"]); - expect(payload.MediaTypes).toEqual(["image/png", "image/png"]); + const payload = buildMSTeamsMediaPayload(createImageMediaEntries("/tmp/a.png", "/tmp/b.png")); + expectMSTeamsMediaPayload(payload, { + firstPath: "/tmp/a.png", + paths: ["/tmp/a.png", "/tmp/b.png"], + types: [CONTENT_TYPE_IMAGE_PNG, CONTENT_TYPE_IMAGE_PNG], + }); }); }); }); diff --git a/src/agents/bash-tools.test.ts b/src/agents/bash-tools.test.ts index 5006d8e8611..eb7e415fb87 100644 --- a/src/agents/bash-tools.test.ts +++ b/src/agents/bash-tools.test.ts @@ -17,12 +17,37 @@ const longDelayCmd = isWin ? "Start-Sleep -Milliseconds 72" : "sleep 0.072"; const POLL_INTERVAL_MS = 15; const BACKGROUND_POLL_TIMEOUT_MS = isWin ? 8000 : 1200; const NOTIFY_EVENT_TIMEOUT_MS = isWin ? 12_000 : 5_000; +const PROCESS_STATUS_RUNNING = "running"; +const PROCESS_STATUS_COMPLETED = "completed"; +const PROCESS_STATUS_FAILED = "failed"; +const OUTPUT_DONE = "done"; +const OUTPUT_NOPE = "nope"; +const OUTPUT_EXEC_COMPLETED = "Exec completed"; +const OUTPUT_EXIT_CODE_1 = "Command exited with code 1"; +const COMMAND_ECHO_HELLO = "echo hello"; +const SCOPE_KEY_ALPHA = "agent:alpha"; +const SCOPE_KEY_BETA = "agent:beta"; const TEST_EXEC_DEFAULTS = { security: "full" as const, ask: "off" as const }; const DEFAULT_NOTIFY_SESSION_KEY = "agent:main:main"; +const ECHO_HI_COMMAND = "echo hi"; +let callIdCounter = 0; +const nextCallId = () => `call${++callIdCounter}`; +type ExecToolInstance = ReturnType; +type ProcessToolInstance = ReturnType; +type ExecToolArgs = Parameters[1]; +type ProcessToolArgs = Parameters[1]; type ExecToolConfig = Exclude[0], undefined>; const createTestExecTool = ( defaults?: Parameters[0], ): ReturnType => createExecTool({ ...TEST_EXEC_DEFAULTS, ...defaults }); +const createDisallowedElevatedExecTool = ( + defaultLevel: "off" | "on", + overrides: Partial = {}, +) => + createTestExecTool({ + elevated: { enabled: true, allowed: false, defaultLevel }, + ...overrides, + }); const createNotifyOnExitExecTool = (overrides: Partial = {}) => createTestExecTool({ allowBackground: true, @@ -57,6 +82,72 @@ const readNormalizedTextContent = (content: ToolTextContent) => const readTrimmedLines = (content: ToolTextContent) => (readTextContent(content) ?? "").split("\n").map((line) => line.trim()); const readTotalLines = (details: unknown) => (details as { totalLines?: number }).totalLines; +const readProcessStatus = (details: unknown) => (details as { status?: string }).status; +const readProcessStatusOrRunning = (details: unknown) => + readProcessStatus(details) ?? PROCESS_STATUS_RUNNING; +const expectProcessStatus = (details: unknown, expected: string) => { + expect(readProcessStatus(details)).toBe(expected); +}; +const expectTextContainsValues = ( + text: string, + values: string[] | undefined, + shouldContain: boolean, +) => { + if (!values) { + return; + } + for (const value of values) { + if (shouldContain) { + expect(text).toContain(value); + } else { + expect(text).not.toContain(value); + } + } +}; +const expectTextContainsAll = (text: string, expected?: string[]) => { + expectTextContainsValues(text, expected, true); +}; +const expectTextContainsNone = (text: string, forbidden?: string[]) => { + expectTextContainsValues(text, forbidden, false); +}; +const expectTextContains = (text: string | undefined, expected?: string) => { + if (expected === undefined) { + throw new Error("expected text assertion value"); + } + expectTextContainsAll(text ?? "", [expected]); +}; +type ProcessSessionSummary = { sessionId: string; name?: string }; +const readProcessSessions = (details: unknown) => + (details as { sessions: ProcessSessionSummary[] }).sessions; +const expectSessionMembership = ( + sessions: ProcessSessionSummary[], + sessionId: string, + shouldExist: boolean, +) => { + expect(sessions.some((session) => session.sessionId === sessionId)).toBe(shouldExist); +}; +const executeExecTool = (tool: ExecToolInstance, params: ExecToolArgs) => + tool.execute(nextCallId(), params); +const executeProcessTool = (tool: ProcessToolInstance, params: ProcessToolArgs) => + tool.execute(nextCallId(), params); +type ProcessPollResult = { status: string; output?: string }; +async function listProcessSessions(tool: ProcessToolInstance) { + const list = await executeProcessTool(tool, { action: "list" }); + return readProcessSessions(list.details); +} +async function pollProcessSession(params: { + tool: ProcessToolInstance; + sessionId: string; +}): Promise { + const poll = await executeProcessTool(params.tool, { + action: "poll", + sessionId: params.sessionId, + }); + return { + status: readProcessStatusOrRunning(poll.details), + output: readTextContent(poll.content), + }; +} function applyDefaultShellEnv() { if (!isWin && defaultShell) { @@ -82,20 +173,16 @@ function useCapturedShellEnv() { } async function waitForCompletion(sessionId: string) { - let status = "running"; + let status = PROCESS_STATUS_RUNNING; await expect .poll( async () => { - const poll = await processTool.execute("call-wait", { - action: "poll", - sessionId, - }); - status = (poll.details as { status: string }).status; + status = (await pollProcessSession({ tool: processTool, sessionId })).status; return status; }, { timeout: BACKGROUND_POLL_TIMEOUT_MS, interval: POLL_INTERVAL_MS }, ) - .not.toBe("running"); + .not.toBe(PROCESS_STATUS_RUNNING); return status; } @@ -110,34 +197,47 @@ function hasNotifyEventForPrefix(prefix: string): boolean { return peekSystemEvents(DEFAULT_NOTIFY_SESSION_KEY).some((event) => event.includes(prefix)); } -async function startBackgroundSession(params: { - tool: ReturnType; - callId: string; - command: string; -}) { - const result = await params.tool.execute(params.callId, { +async function waitForNotifyEvent(sessionId: string) { + const prefix = sessionId.slice(0, 8); + let finished = getFinishedSession(sessionId); + let hasEvent = hasNotifyEventForPrefix(prefix); + await expect + .poll( + () => { + finished = getFinishedSession(sessionId); + hasEvent = hasNotifyEventForPrefix(prefix); + return Boolean(finished && hasEvent); + }, + { timeout: NOTIFY_EVENT_TIMEOUT_MS, interval: POLL_INTERVAL_MS }, + ) + .toBe(true); + return { + finished: finished ?? getFinishedSession(sessionId), + hasEvent: hasEvent || hasNotifyEventForPrefix(prefix), + }; +} + +async function startBackgroundSession(params: { tool: ExecToolInstance; command: string }) { + const result = await executeExecTool(params.tool, { command: params.command, background: true, }); - expect(result.details.status).toBe("running"); + expectProcessStatus(result.details, PROCESS_STATUS_RUNNING); return requireSessionId(result.details as { sessionId?: string }); } async function runBackgroundEchoLines(lines: string[]) { const sessionId = await startBackgroundSession({ tool: execTool, - callId: "call1", command: echoLines(lines), }); await waitForCompletion(sessionId); return sessionId; } -async function readProcessLog( - sessionId: string, - options: { offset?: number; limit?: number } = {}, -) { - return processTool.execute("call-log", { +type ProcessLogWindow = { offset?: number; limit?: number }; +async function readProcessLog(sessionId: string, options: ProcessLogWindow = {}) { + return executeProcessTool(processTool, { action: "log", sessionId, ...options, @@ -154,28 +254,84 @@ const createNumberedLines = (count: number) => Array.from({ length: count }, (_value, index) => `line-${index + 1}`); const LONG_LOG_LINE_COUNT = 201; -async function runBackgroundAndReadProcessLog( - lines: string[], - options: { offset?: number; limit?: number } = {}, -) { +async function runBackgroundAndReadProcessLog(lines: string[], options: ProcessLogWindow = {}) { const sessionId = await runBackgroundEchoLines(lines); return readProcessLog(sessionId, options); } -const readLongProcessLog = (options: { offset?: number; limit?: number } = {}) => +const readLogSlice = async (lines: string[], options: ProcessLogWindow = {}) => { + const log = await runBackgroundAndReadProcessLog(lines, options); + return { + text: readNormalizedTextContent(log.content), + totalLines: readTotalLines(log.details), + }; +}; +const readLongProcessLog = (options: ProcessLogWindow = {}) => runBackgroundAndReadProcessLog(createNumberedLines(LONG_LOG_LINE_COUNT), options); +type LongLogExpectationCase = { + label: string; + options?: ProcessLogWindow; + firstLine: string; + lastLine?: string; + mustContain?: string[]; + mustNotContain?: string[]; +}; +type ShortLogExpectationCase = { + label: string; + lines: string[]; + options: ProcessLogWindow; + expectedText: string; + expectedTotalLines: number; +}; +type DisallowedElevationCase = { + label: string; + defaultLevel: "off" | "on"; + overrides?: Partial; + requestElevated?: boolean; + expectedError?: string; + expectedOutputIncludes?: string; +}; +type NotifyNoopCase = { + label: string; + notifyOnExitEmptySuccess: boolean; +}; +const NOOP_NOTIFY_CASES: NotifyNoopCase[] = [ + { + label: "default behavior skips no-op completion events", + notifyOnExitEmptySuccess: false, + }, + { + label: "explicitly enabling no-op completion emits completion events", + notifyOnExitEmptySuccess: true, + }, +]; +const expectNotifyNoopEvents = ( + events: string[], + notifyOnExitEmptySuccess: boolean, + label: string, +) => { + if (!notifyOnExitEmptySuccess) { + expect(events, label).toEqual([]); + return; + } + expect(events.length, label).toBeGreaterThan(0); + expect( + events.some((event) => event.includes(OUTPUT_EXEC_COMPLETED)), + label, + ).toBe(true); +}; async function runBackgroundAndWaitForCompletion(params: { - tool: ReturnType; - callId: string; + tool: ExecToolInstance; command: string; }) { const sessionId = await startBackgroundSession(params); const status = await waitForCompletion(sessionId); - expect(status).toBe("completed"); + expect(status).toBe(PROCESS_STATUS_COMPLETED); return { sessionId }; } beforeEach(() => { + callIdCounter = 0; resetProcessRegistryForTests(); resetSystemEventsForTest(); }); @@ -186,38 +342,37 @@ describe("exec tool backgrounding", () => { it( "backgrounds after yield and can be polled", async () => { - const result = await execTool.execute("call1", { + const result = await executeExecTool(execTool, { command: joinCommands([yieldDelayCmd, "echo done"]), yieldMs: 10, }); // Timing can race here: command may already be complete before the first response. - if (result.details.status === "completed") { + if (result.details.status === PROCESS_STATUS_COMPLETED) { const text = readTextContent(result.content) ?? ""; - expect(text).toContain("done"); + expectTextContains(text, OUTPUT_DONE); return; } - expect(result.details.status).toBe("running"); + expectProcessStatus(result.details, PROCESS_STATUS_RUNNING); const sessionId = requireSessionId(result.details as { sessionId?: string }); let output = ""; await expect .poll( async () => { - const poll = await processTool.execute("call2", { - action: "poll", + const pollResult = await pollProcessSession({ + tool: processTool, sessionId, }); - const status = (poll.details as { status: string }).status; - output = readTextContent(poll.content) ?? ""; - return status; + output = pollResult.output ?? ""; + return pollResult.status; }, { timeout: BACKGROUND_POLL_TIMEOUT_MS, interval: POLL_INTERVAL_MS }, ) - .toBe("completed"); + .toBe(PROCESS_STATUS_COMPLETED); - expect(output).toContain("done"); + expectTextContains(output, OUTPUT_DONE); }, isWin ? 15_000 : 5_000, ); @@ -225,15 +380,12 @@ describe("exec tool backgrounding", () => { it("supports explicit background and derives session name from the command", async () => { const sessionId = await startBackgroundSession({ tool: execTool, - callId: "call1", - command: "echo hello", + command: COMMAND_ECHO_HELLO, }); - const list = await processTool.execute("call2", { action: "list" }); - const sessions = (list.details as { sessions: Array<{ sessionId: string; name?: string }> }) - .sessions; - expect(sessions.some((s) => s.sessionId === sessionId)).toBe(true); - expect(sessions.find((s) => s.sessionId === sessionId)?.name).toBe("echo hello"); + const sessions = await listProcessSessions(processTool); + expectSessionMembership(sessions, sessionId, true); + expect(sessions.find((s) => s.sessionId === sessionId)?.name).toBe(COMMAND_ECHO_HELLO); }); it("uses default timeout when timeout is omitted", async () => { @@ -243,102 +395,119 @@ describe("exec tool backgrounding", () => { allowBackground: false, }); await expect( - customBash.execute("call1", { + executeExecTool(customBash, { command: longDelayCmd, }), ).rejects.toThrow(/timed out/i); }); - it("rejects elevated requests when not allowed", async () => { - const customBash = createTestExecTool({ - elevated: { enabled: true, allowed: false, defaultLevel: "off" }, - messageProvider: "telegram", - sessionKey: DEFAULT_NOTIFY_SESSION_KEY, - }); + it.each([ + { + label: "rejects elevated requests when not allowed", + defaultLevel: "off", + overrides: { + messageProvider: "telegram", + sessionKey: DEFAULT_NOTIFY_SESSION_KEY, + }, + requestElevated: true, + expectedError: "Context: provider=telegram session=agent:main:main", + }, + { + label: "does not default to elevated when not allowed", + defaultLevel: "on", + overrides: { + backgroundMs: 1000, + timeoutSec: 5, + }, + expectedOutputIncludes: "hi", + }, + ])( + "$label", + async ({ defaultLevel, overrides, requestElevated, expectedError, expectedOutputIncludes }) => { + const customBash = createDisallowedElevatedExecTool(defaultLevel, overrides); + if (expectedError) { + await expect( + executeExecTool(customBash, { + command: ECHO_HI_COMMAND, + elevated: requestElevated, + }), + ).rejects.toThrow(expectedError); + return; + } - await expect( - customBash.execute("call1", { - command: "echo hi", - elevated: true, - }), - ).rejects.toThrow("Context: provider=telegram session=agent:main:main"); + const result = await executeExecTool(customBash, { + command: ECHO_HI_COMMAND, + }); + expectTextContains(readTextContent(result.content), expectedOutputIncludes); + }, + ); + + it.each([ + { + label: "logs line-based slices and defaults to last lines", + lines: ["one", "two", "three"], + options: { limit: 2 }, + expectedText: "two\nthree", + expectedTotalLines: 3, + }, + { + label: "supports line offsets for log slices", + lines: ["alpha", "beta", "gamma"], + options: { offset: 1, limit: 1 }, + expectedText: "beta", + expectedTotalLines: 3, + }, + ])("$label", async ({ lines, options, expectedText, expectedTotalLines }) => { + const slice = await readLogSlice(lines, options); + expect(slice.text).toBe(expectedText); + expect(slice.totalLines).toBe(expectedTotalLines); }); - it("does not default to elevated when not allowed", async () => { - const customBash = createTestExecTool({ - elevated: { enabled: true, allowed: false, defaultLevel: "on" }, - backgroundMs: 1000, - timeoutSec: 5, - }); - - const result = await customBash.execute("call1", { - command: "echo hi", - }); - const text = readTextContent(result.content) ?? ""; - expect(text).toContain("hi"); - }); - - it("logs line-based slices and defaults to last lines", async () => { - const { sessionId } = await runBackgroundAndWaitForCompletion({ - tool: execTool, - callId: "call1", - command: echoLines(["one", "two", "three"]), - }); - - const log = await readProcessLog(sessionId, { limit: 2 }); - expect(readNormalizedTextContent(log.content)).toBe("two\nthree"); - expect(readTotalLines(log.details)).toBe(3); - }); - - it("applies default tail only when no explicit log window is provided", async () => { - const snapshot = readLogSnapshot(await readLongProcessLog()); - expect(snapshot.text).toContain("showing last 200 of 201 lines"); - expect(snapshot.lines[0]).toBe("line-2"); - expect(snapshot.text).toContain("line-2"); - expect(snapshot.text).toContain("line-201"); - expect(snapshot.totalLines).toBe(LONG_LOG_LINE_COUNT); - }); - - it("supports line offsets for log slices", async () => { - const sessionId = await runBackgroundEchoLines(["alpha", "beta", "gamma"]); - - const log = await readProcessLog(sessionId, { offset: 1, limit: 1 }); - expect(readNormalizedTextContent(log.content)).toBe("beta"); - }); - - it("keeps offset-only log requests unbounded by default tail mode", async () => { - const snapshot = readLogSnapshot(await readLongProcessLog({ offset: 30 })); - expect(snapshot.lines[0]).toBe("line-31"); - expect(snapshot.lines[snapshot.lines.length - 1]).toBe("line-201"); - expect(snapshot.text).not.toContain("showing last 200"); + it.each([ + { + label: "applies default tail only when no explicit log window is provided", + firstLine: "line-2", + mustContain: ["showing last 200 of 201 lines", "line-2", "line-201"], + }, + { + label: "keeps offset-only log requests unbounded by default tail mode", + options: { offset: 30 }, + firstLine: "line-31", + lastLine: "line-201", + mustNotContain: ["showing last 200"], + }, + ])("$label", async ({ options, firstLine, lastLine, mustContain, mustNotContain }) => { + const snapshot = readLogSnapshot(await readLongProcessLog(options)); + expect(snapshot.lines[0]).toBe(firstLine); + if (lastLine) { + expect(snapshot.lines[snapshot.lines.length - 1]).toBe(lastLine); + } expect(snapshot.totalLines).toBe(LONG_LOG_LINE_COUNT); + expectTextContainsAll(snapshot.text, mustContain); + expectTextContainsNone(snapshot.text, mustNotContain); }); it("scopes process sessions by scopeKey", async () => { - const alphaTools = createScopedToolSet("agent:alpha"); - const betaTools = createScopedToolSet("agent:beta"); + const alphaTools = createScopedToolSet(SCOPE_KEY_ALPHA); + const betaTools = createScopedToolSet(SCOPE_KEY_BETA); const sessionA = await startBackgroundSession({ tool: alphaTools.exec, - callId: "call1", command: shortDelayCmd, }); const sessionB = await startBackgroundSession({ tool: betaTools.exec, - callId: "call2", command: shortDelayCmd, }); - const listA = await alphaTools.process.execute("call3", { action: "list" }); - const sessionsA = (listA.details as { sessions: Array<{ sessionId: string }> }).sessions; - expect(sessionsA.some((s) => s.sessionId === sessionA)).toBe(true); - expect(sessionsA.some((s) => s.sessionId === sessionB)).toBe(false); + const sessionsA = await listProcessSessions(alphaTools.process); + expectSessionMembership(sessionsA, sessionA, true); + expectSessionMembership(sessionsA, sessionB, false); - const pollB = await betaTools.process.execute("call4", { - action: "poll", + const pollB = await pollProcessSession({ + tool: betaTools.process, sessionId: sessionA, }); - const pollBDetails = pollB.details as { status?: string }; - expect(pollBDetails.status).toBe("failed"); + expect(pollB.status).toBe(PROCESS_STATUS_FAILED); }); }); @@ -348,15 +517,14 @@ describe("exec exit codes", () => { it("treats non-zero exits as completed and appends exit code", async () => { const command = isWin ? joinCommands(["Write-Output nope", "exit 1"]) - : joinCommands(["echo nope", "exit 1"]); - const result = await execTool.execute("call1", { command }); + : joinCommands([`echo ${OUTPUT_NOPE}`, "exit 1"]); + const result = await executeExecTool(execTool, { command }); const resultDetails = result.details as { status?: string; exitCode?: number | null }; - expect(resultDetails.status).toBe("completed"); + expectProcessStatus(resultDetails, PROCESS_STATUS_COMPLETED); expect(resultDetails.exitCode).toBe(1); const text = readNormalizedTextContent(result.content); - expect(text).toContain("nope"); - expect(text).toContain("Command exited with code 1"); + expectTextContainsAll(text, [OUTPUT_NOPE, OUTPUT_EXIT_CODE_1]); }); }); @@ -366,67 +534,30 @@ describe("exec notifyOnExit", () => { const sessionId = await startBackgroundSession({ tool, - callId: "call1", command: echoAfterDelay("notify"), }); - const prefix = sessionId.slice(0, 8); - let finished = getFinishedSession(sessionId); - let hasEvent = hasNotifyEventForPrefix(prefix); - await expect - .poll( - () => { - finished = getFinishedSession(sessionId); - hasEvent = hasNotifyEventForPrefix(prefix); - return Boolean(finished && hasEvent); - }, - { timeout: NOTIFY_EVENT_TIMEOUT_MS, interval: POLL_INTERVAL_MS }, - ) - .toBe(true); - if (!finished) { - finished = getFinishedSession(sessionId); - } - if (!hasEvent) { - hasEvent = hasNotifyEventForPrefix(prefix); - } + const { finished, hasEvent } = await waitForNotifyEvent(sessionId); expect(finished).toBeTruthy(); expect(hasEvent).toBe(true); }); - it("handles no-op completion events based on notifyOnExitEmptySuccess", async () => { - for (const testCase of [ - { - label: "default behavior skips no-op completion events", - notifyOnExitEmptySuccess: false, - }, - { - label: "explicitly enabling no-op completion emits completion events", - notifyOnExitEmptySuccess: true, - }, - ]) { - resetSystemEventsForTest(); + it.each(NOOP_NOTIFY_CASES)( + "$label", + async ({ label, notifyOnExitEmptySuccess }) => { const tool = createNotifyOnExitExecTool( - testCase.notifyOnExitEmptySuccess ? { notifyOnExitEmptySuccess: true } : {}, + notifyOnExitEmptySuccess ? { notifyOnExitEmptySuccess: true } : {}, ); await runBackgroundAndWaitForCompletion({ tool, - callId: "call-noop", command: shortDelayCmd, }); const events = peekSystemEvents(DEFAULT_NOTIFY_SESSION_KEY); - if (!testCase.notifyOnExitEmptySuccess) { - expect(events, testCase.label).toEqual([]); - } else { - expect(events.length, testCase.label).toBeGreaterThan(0); - expect( - events.some((event) => event.includes("Exec completed")), - testCase.label, - ).toBe(true); - } - } - }); + expectNotifyNoopEvents(events, notifyOnExitEmptySuccess, label); + }, + ); }); describe("exec PATH handling", () => { @@ -438,7 +569,7 @@ describe("exec PATH handling", () => { process.env.PATH = basePath; const tool = createTestExecTool({ pathPrepend: prepend }); - const result = await tool.execute("call1", { + const result = await executeExecTool(tool, { command: isWin ? "Write-Output $env:PATH" : "echo $PATH", });