diff --git a/extensions/msteams/src/attachments.test.ts b/extensions/msteams/src/attachments.test.ts index d370c192110..caa50557f51 100644 --- a/extensions/msteams/src/attachments.test.ts +++ b/extensions/msteams/src/attachments.test.ts @@ -15,54 +15,67 @@ 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 GRAPH_HOST = "graph.microsoft.com"; +const SHAREPOINT_HOST = "contoso.sharepoint.com"; +const AZUREEDGE_HOST = "azureedge.net"; +const TEST_HOST = "x"; +const createUrlForHost = (host: string, pathSegment: string) => `https://${host}/${pathSegment}`; +const createTestUrl = (pathSegment: string) => createUrlForHost(TEST_HOST, pathSegment); 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_IMAGE = createTestUrl("img"); +const TEST_URL_IMAGE_PNG = createTestUrl("img.png"); +const TEST_URL_IMAGE_1_PNG = createTestUrl("1.png"); +const TEST_URL_IMAGE_2_JPG = createTestUrl("2.jpg"); +const TEST_URL_PDF = createTestUrl("x.pdf"); +const TEST_URL_PDF_1 = createTestUrl("1.pdf"); +const TEST_URL_PDF_2 = createTestUrl("2.pdf"); +const TEST_URL_HTML_A = createTestUrl("a.png"); +const TEST_URL_HTML_B = createTestUrl("b.png"); +const TEST_URL_INLINE_IMAGE = createTestUrl("inline.png"); +const TEST_URL_DOC_PDF = createTestUrl("doc.pdf"); +const TEST_URL_FILE_DOWNLOAD = createTestUrl("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 REDIRECT_STATUS_CODES = [301, 302, 303, 307, 308]; +const MAX_REDIRECT_HOPS = 5; +type RemoteMediaFetchParams = { + url: string; + maxBytes?: number; + filePathHint?: string; + fetchImpl?: (input: RequestInfo | URL, init?: RequestInit) => Promise; +}; const detectMimeMock = vi.fn(async () => CONTENT_TYPE_IMAGE_PNG); const saveMediaBufferMock = vi.fn(async () => ({ path: SAVED_PNG_PATH, contentType: CONTENT_TYPE_IMAGE_PNG, })); -const fetchRemoteMediaMock = vi.fn( - async (params: { - url: string; - maxBytes?: number; - filePathHint?: string; - fetchImpl?: (input: RequestInfo | URL, init?: RequestInit) => Promise; - }) => { - const fetchFn = params.fetchImpl ?? fetch; - const res = await fetchFn(params.url); - if (!res.ok) { - throw new Error(`HTTP ${res.status}`); - } - const buffer = Buffer.from(await res.arrayBuffer()); - if (typeof params.maxBytes === "number" && buffer.byteLength > params.maxBytes) { - throw new Error(`payload exceeds maxBytes ${params.maxBytes}`); - } - return { - buffer, - contentType: res.headers.get("content-type") ?? undefined, - fileName: params.filePathHint, - }; - }, -); +const readRemoteMediaResponse = async ( + res: Response, + params: Pick, +) => { + if (!res.ok) { + throw new Error(`HTTP ${res.status}`); + } + const buffer = Buffer.from(await res.arrayBuffer()); + if (typeof params.maxBytes === "number" && buffer.byteLength > params.maxBytes) { + throw new Error(`payload exceeds maxBytes ${params.maxBytes}`); + } + return { + buffer, + contentType: res.headers.get("content-type") ?? undefined, + fileName: params.filePathHint, + }; +}; +const fetchRemoteMediaMock = vi.fn(async (params: RemoteMediaFetchParams) => { + const fetchFn = params.fetchImpl ?? fetch; + const res = await fetchFn(params.url); + return readRemoteMediaResponse(res, params); +}); const runtimeStub = { media: { @@ -81,7 +94,6 @@ const runtimeStub = { type DownloadAttachmentsParams = Parameters[0]; type DownloadGraphMediaParams = Parameters[0]; type DownloadedMedia = Awaited>; -type DownloadedGraphMedia = Awaited>; type MSTeamsMediaPayload = ReturnType; type DownloadAttachmentsBuildOverrides = Partial< Omit @@ -97,6 +109,11 @@ type DownloadAttachmentsNoFetchOverrides = Partial< type DownloadGraphMediaOverrides = Partial< Omit >; +type FetchFn = typeof fetch; +type MSTeamsAttachments = DownloadAttachmentsParams["attachments"]; +type AttachmentPlaceholderInput = Parameters[0]; +type GraphMessageUrlParams = Parameters[0]; +type LabeledCase = { label: string }; type FetchCallExpectation = { expectFetchCalled?: boolean }; type DownloadedMediaExpectation = { path?: string; placeholder?: string }; type MSTeamsMediaPayloadExpectation = { @@ -105,19 +122,28 @@ type MSTeamsMediaPayloadExpectation = { types: string[]; }; -const DEFAULT_MESSAGE_URL = "https://graph.microsoft.com/v1.0/chats/19%3Achat/messages/123"; +const DEFAULT_MESSAGE_URL = `https://${GRAPH_HOST}/v1.0/chats/19%3Achat/messages/123`; +const GRAPH_SHARES_URL_PREFIX = `https://${GRAPH_HOST}/v1.0/shares/`; const DEFAULT_MAX_BYTES = 1024 * 1024; -const DEFAULT_ALLOW_HOSTS = ["x"]; -const DEFAULT_SHAREPOINT_ALLOW_HOSTS = ["graph.microsoft.com", "contoso.sharepoint.com"]; -const DEFAULT_SHARE_REFERENCE_URL = "https://contoso.sharepoint.com/site/file"; +const DEFAULT_ALLOW_HOSTS = [TEST_HOST]; +const DEFAULT_SHAREPOINT_ALLOW_HOSTS = [GRAPH_HOST, SHAREPOINT_HOST]; +const DEFAULT_SHARE_REFERENCE_URL = createUrlForHost(SHAREPOINT_HOST, "site/file"); const MEDIA_PLACEHOLDER_IMAGE = ""; const MEDIA_PLACEHOLDER_DOCUMENT = ""; +const formatImagePlaceholder = (count: number) => + count > 1 ? `${MEDIA_PLACEHOLDER_IMAGE} (${count} images)` : MEDIA_PLACEHOLDER_IMAGE; +const formatDocumentPlaceholder = (count: number) => + count > 1 ? `${MEDIA_PLACEHOLDER_DOCUMENT} (${count} files)` : 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 withLabel = (label: string, fields: T): T & LabeledCase => ({ + label, + ...fields, +}); const buildAttachment = >(contentType: string, props: T) => ({ contentType, ...props, @@ -127,10 +153,12 @@ const createHtmlAttachment = (content: string) => const buildHtmlImageTag = (src: string) => ``; const createHtmlImageAttachments = (sources: string[], prefix = "") => asSingleItemArray(createHtmlAttachment(`${prefix}${sources.map(buildHtmlImageTag).join("")}`)); +const createContentUrlAttachments = (contentType: string, ...contentUrls: string[]) => + contentUrls.map((contentUrl) => buildAttachment(contentType, { contentUrl })); const createImageAttachments = (...contentUrls: string[]) => - contentUrls.map((contentUrl) => buildAttachment(CONTENT_TYPE_IMAGE_PNG, { contentUrl })); + createContentUrlAttachments(CONTENT_TYPE_IMAGE_PNG, ...contentUrls); const createPdfAttachments = (...contentUrls: string[]) => - contentUrls.map((contentUrl) => buildAttachment(CONTENT_TYPE_APPLICATION_PDF, { contentUrl })); + createContentUrlAttachments(CONTENT_TYPE_APPLICATION_PDF, ...contentUrls); const createTeamsFileDownloadInfoAttachments = ( downloadUrl = TEST_URL_FILE_DOWNLOAD, fileType = "png", @@ -140,30 +168,38 @@ const createTeamsFileDownloadInfoAttachments = ( content: { downloadUrl, fileType }, }), ); +const createMediaEntriesWithType = (contentType: string, ...paths: string[]) => + paths.map((path) => ({ path, contentType })); +const createHostedContentsWithType = (contentType: string, ...ids: string[]) => + ids.map((id) => ({ id, contentType, contentBytes: PNG_BASE64 })); const createImageMediaEntries = (...paths: string[]) => - paths.map((path) => ({ path, contentType: CONTENT_TYPE_IMAGE_PNG })); + createMediaEntriesWithType(CONTENT_TYPE_IMAGE_PNG, ...paths); const createHostedImageContents = (...ids: string[]) => - ids.map((id) => ({ id, contentType: CONTENT_TYPE_IMAGE_PNG, contentBytes: PNG_BASE64 })); + createHostedContentsWithType(CONTENT_TYPE_IMAGE_PNG, ...ids); const createPdfResponse = (payload: Buffer | string = PDF_BUFFER) => { + return createBufferResponse(payload, CONTENT_TYPE_APPLICATION_PDF); +}; +const createBufferResponse = (payload: Buffer | string, contentType: string, status = 200) => { const raw = Buffer.isBuffer(payload) ? payload : Buffer.from(payload); return new Response(new Uint8Array(raw), { - status: 200, - headers: { "content-type": CONTENT_TYPE_APPLICATION_PDF }, + status, + headers: { "content-type": contentType }, }); }; const createJsonResponse = (payload: unknown, status = 200) => new Response(JSON.stringify(payload), { status }); +const createTextResponse = (body: string, status = 200) => new Response(body, { status }); +const createGraphCollectionResponse = (value: unknown[]) => createJsonResponse({ value }); +const createNotFoundResponse = () => new Response("not found", { status: 404 }); +const createRedirectResponse = (location: string, status = 302) => + new Response(null, { status, headers: { location } }); const createOkFetchMock = (contentType: string, payload = "png") => - vi.fn(async () => { - return new Response(Buffer.from(payload), { - status: 200, - headers: { "content-type": contentType }, - }); - }); + vi.fn(async () => createBufferResponse(payload, contentType)); +const asFetchFn = (fetchFn: unknown): FetchFn => fetchFn as FetchFn; const buildDownloadParams = ( - attachments: DownloadAttachmentsParams["attachments"], + attachments: MSTeamsAttachments, overrides: DownloadAttachmentsBuildOverrides = {}, ): DownloadAttachmentsParams => { return { @@ -175,53 +211,30 @@ const buildDownloadParams = ( }; }; -const buildDownloadParamsWithFetch = ( - attachments: DownloadAttachmentsParams["attachments"], - fetchFn: unknown, - overrides: DownloadAttachmentsNoFetchOverrides = {}, -): DownloadAttachmentsParams => { - return buildDownloadParams(attachments, { - ...overrides, - fetchFn: fetchFn as unknown as typeof fetch, - }); -}; - const downloadAttachmentsWithFetch = async ( - attachments: DownloadAttachmentsParams["attachments"], + attachments: MSTeamsAttachments, fetchFn: unknown, overrides: DownloadAttachmentsNoFetchOverrides = {}, options: FetchCallExpectation = {}, ) => { const media = await downloadMSTeamsAttachments( - buildDownloadParamsWithFetch(attachments, fetchFn, overrides), + buildDownloadParams(attachments, { + ...overrides, + fetchFn: asFetchFn(fetchFn), + }), ); expectMockCallState(fetchFn, options.expectFetchCalled ?? true); return media; }; -const downloadAttachmentsWithOkImageFetch = ( - attachments: DownloadAttachmentsParams["attachments"], - overrides: DownloadAttachmentsNoFetchOverrides = {}, - options: FetchCallExpectation = {}, -) => { - return downloadAttachmentsWithFetch( - attachments, - createOkFetchMock(CONTENT_TYPE_IMAGE_PNG), - overrides, - options, - ); -}; const createAuthAwareImageFetchMock = (params: { unauthStatus: number; unauthBody: string }) => vi.fn(async (_url: string, opts?: RequestInit) => { const headers = new Headers(opts?.headers); const hasAuth = Boolean(headers.get("Authorization")); if (!hasAuth) { - return new Response(params.unauthBody, { status: params.unauthStatus }); + return createTextResponse(params.unauthBody, params.unauthStatus); } - return new Response(PNG_BUFFER, { - status: 200, - headers: { "content-type": CONTENT_TYPE_IMAGE_PNG }, - }); + return createBufferResponse(PNG_BUFFER, CONTENT_TYPE_IMAGE_PNG); }); const expectMockCallState = (mockFn: unknown, shouldCall: boolean) => { if (shouldCall) { @@ -231,18 +244,6 @@ const expectMockCallState = (mockFn: unknown, shouldCall: boolean) => { } }; -const buildDownloadGraphParams = ( - fetchFn: unknown, - overrides: DownloadGraphMediaOverrides = {}, -): DownloadGraphMediaParams => { - return { - messageUrl: DEFAULT_MESSAGE_URL, - tokenProvider: createTokenProvider(), - maxBytes: DEFAULT_MAX_BYTES, - fetchFn: fetchFn as unknown as typeof fetch, - ...overrides, - }; -}; const DEFAULT_CHANNEL_TEAM_ID = "team-id"; const DEFAULT_CHANNEL_ID = "chan-id"; const createChannelGraphMessageUrlParams = (params: { @@ -262,45 +263,14 @@ const buildExpectedChannelMessagePath = (params: { messageId: string; 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: DownloadGraphMediaOverrides = {}, -) => { - return downloadMSTeamsGraphMedia(buildDownloadGraphParams(fetchFn, overrides)); -}; -const expectFirstGraphUrlContains = ( - params: Parameters[0], - expectedPath: string, -) => { - const urls = buildMSTeamsGraphMessageUrls(params); - expect(urls[0]).toContain(expectedPath); -}; -const expectAttachmentPlaceholder = ( - attachments: Parameters[0], - expected: string, -) => { - 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 expectAttachmentMediaLength = (media: DownloadedMedia, expectedLength: number) => { + expect(media).toHaveLength(expectedLength); }; const expectSingleMedia = (media: DownloadedMedia, expected: DownloadedMediaExpectation = {}) => { - expectMediaLength(media, 1); + expectAttachmentMediaLength(media, 1); expectFirstMedia(media, expected); }; -const expectNoGraphMedia = (media: DownloadedGraphMedia) => { - expectGraphMediaLength(media, 0); -}; -const expectMediaSaved = () => { +const expectMediaBufferSaved = () => { expect(saveMediaBufferMock).toHaveBeenCalled(); }; const expectFirstMedia = (media: DownloadedMedia, expected: DownloadedMediaExpectation) => { @@ -322,14 +292,19 @@ const expectMSTeamsMediaPayload = ( expect(payload.MediaUrls).toEqual(expected.paths); expect(payload.MediaTypes).toEqual(expected.types); }; -type AttachmentPlaceholderCase = { - label: string; - attachments: Parameters[0]; +type AttachmentPlaceholderCase = LabeledCase & { + attachments: AttachmentPlaceholderInput; expected: string; }; -type AttachmentDownloadSuccessCase = { - label: string; - attachments: DownloadAttachmentsParams["attachments"]; +type CountedAttachmentPlaceholderCaseDef = LabeledCase & { + attachments: AttachmentPlaceholderCase["attachments"]; + count: number; + formatPlaceholder: (count: number) => string; +}; +type AttachmentDownloadSuccessCase = LabeledCase & { + attachments: MSTeamsAttachments; + buildFetchFn?: () => unknown; + beforeDownload?: () => void; assert?: (media: DownloadedMedia) => void; }; type AttachmentAuthRetryScenario = { @@ -338,26 +313,186 @@ type AttachmentAuthRetryScenario = { unauthBody: string; overrides?: Omit; }; -type AttachmentAuthRetryCase = { - label: string; +type AttachmentAuthRetryCase = LabeledCase & { scenario: AttachmentAuthRetryScenario; expectedMediaLength: number; expectTokenFetch: boolean; }; -type GraphUrlExpectationCase = { - label: string; - params: Parameters[0]; +type GraphUrlExpectationCase = LabeledCase & { + params: GraphMessageUrlParams; expectedPath: string; }; -type GraphMediaSuccessCase = { - label: string; +type ChannelGraphUrlCaseParams = { + messageId: string; + replyToId?: string; + conversationId?: string; +}; +type GraphMediaDownloadResult = { + fetchMock: ReturnType; + media: Awaited>; +}; +type GraphMediaSuccessCase = LabeledCase & { buildOptions: () => GraphFetchMockOptions; expectedLength: number; - assert?: (params: { - fetchMock: ReturnType; - media: Awaited>; - }) => void; + assert?: (params: GraphMediaDownloadResult) => void; }; +const EMPTY_ATTACHMENT_PLACEHOLDER_CASES: AttachmentPlaceholderCase[] = [ + withLabel("returns empty string when no attachments", { attachments: undefined, expected: "" }), + withLabel("returns empty string when attachments are empty", { attachments: [], expected: "" }), +]; +const COUNTED_ATTACHMENT_PLACEHOLDER_CASE_DEFS: CountedAttachmentPlaceholderCaseDef[] = [ + withLabel("returns image placeholder for one image attachment", { + attachments: createImageAttachments(TEST_URL_IMAGE_PNG), + count: 1, + formatPlaceholder: formatImagePlaceholder, + }), + withLabel("returns image placeholder with count for many image attachments", { + attachments: [ + ...createImageAttachments(TEST_URL_IMAGE_1_PNG), + { contentType: "image/jpeg", contentUrl: TEST_URL_IMAGE_2_JPG }, + ], + count: 2, + formatPlaceholder: formatImagePlaceholder, + }), + withLabel("treats Teams file.download.info image attachments as images", { + attachments: createTeamsFileDownloadInfoAttachments(), + count: 1, + formatPlaceholder: formatImagePlaceholder, + }), + withLabel("returns document placeholder for non-image attachments", { + attachments: createPdfAttachments(TEST_URL_PDF), + count: 1, + formatPlaceholder: formatDocumentPlaceholder, + }), + withLabel("returns document placeholder with count for many non-image attachments", { + attachments: createPdfAttachments(TEST_URL_PDF_1, TEST_URL_PDF_2), + count: 2, + formatPlaceholder: formatDocumentPlaceholder, + }), + withLabel("counts one inline image in html attachments", { + attachments: createHtmlImageAttachments([TEST_URL_HTML_A], "

hi

"), + count: 1, + formatPlaceholder: formatImagePlaceholder, + }), + withLabel("counts many inline images in html attachments", { + attachments: createHtmlImageAttachments([TEST_URL_HTML_A, TEST_URL_HTML_B]), + count: 2, + formatPlaceholder: formatImagePlaceholder, + }), +]; +const ATTACHMENT_PLACEHOLDER_CASES: AttachmentPlaceholderCase[] = [ + ...EMPTY_ATTACHMENT_PLACEHOLDER_CASES, + ...COUNTED_ATTACHMENT_PLACEHOLDER_CASE_DEFS.map((testCase) => + withLabel(testCase.label, { + attachments: testCase.attachments, + expected: testCase.formatPlaceholder(testCase.count), + }), + ), +]; +const ATTACHMENT_DOWNLOAD_SUCCESS_CASES: AttachmentDownloadSuccessCase[] = [ + withLabel("downloads and stores image contentUrl attachments", { + attachments: asSingleItemArray(IMAGE_ATTACHMENT), + assert: (media) => { + expectFirstMedia(media, { path: SAVED_PNG_PATH }); + expectMediaBufferSaved(); + }, + }), + withLabel("supports Teams file.download.info downloadUrl attachments", { + attachments: createTeamsFileDownloadInfoAttachments(), + }), + withLabel("downloads inline image URLs from html attachments", { + attachments: createHtmlImageAttachments([TEST_URL_INLINE_IMAGE]), + }), + withLabel("downloads non-image file attachments (PDF)", { + attachments: createPdfAttachments(TEST_URL_DOC_PDF), + buildFetchFn: () => createOkFetchMock(CONTENT_TYPE_APPLICATION_PDF, "pdf"), + beforeDownload: () => { + detectMimeMock.mockResolvedValueOnce(CONTENT_TYPE_APPLICATION_PDF); + saveMediaBufferMock.mockResolvedValueOnce({ + path: SAVED_PDF_PATH, + contentType: CONTENT_TYPE_APPLICATION_PDF, + }); + }, + assert: (media) => { + expectSingleMedia(media, { + path: SAVED_PDF_PATH, + placeholder: formatDocumentPlaceholder(1), + }); + }, + }), +]; +const ATTACHMENT_AUTH_RETRY_CASES: AttachmentAuthRetryCase[] = [ + withLabel("retries with auth when the first request is unauthorized", { + scenario: { + attachmentUrl: IMAGE_ATTACHMENT.contentUrl, + unauthStatus: 401, + unauthBody: "unauthorized", + overrides: { authAllowHosts: [TEST_HOST] }, + }, + expectedMediaLength: 1, + expectTokenFetch: true, + }), + withLabel("skips auth retries when the host is not in auth allowlist", { + scenario: { + attachmentUrl: createUrlForHost(AZUREEDGE_HOST, "img"), + unauthStatus: 403, + unauthBody: "forbidden", + overrides: { + allowHosts: [AZUREEDGE_HOST], + authAllowHosts: [GRAPH_HOST], + }, + }, + expectedMediaLength: 0, + expectTokenFetch: false, + }), +]; +const GRAPH_MEDIA_SUCCESS_CASES: GraphMediaSuccessCase[] = [ + withLabel("downloads hostedContents images", { + buildOptions: () => ({ hostedContents: createHostedImageContents("1") }), + expectedLength: 1, + assert: ({ fetchMock }) => { + expect(fetchMock).toHaveBeenCalled(); + expectMediaBufferSaved(); + }, + }), + withLabel("merges SharePoint reference attachments with hosted content", { + buildOptions: () => { + return { + hostedContents: createHostedImageContents("hosted-1"), + ...buildDefaultShareReferenceGraphFetchOptions({ + onShareRequest: () => createPdfResponse(), + }), + }; + }, + expectedLength: 2, + }), +]; +const CHANNEL_GRAPH_URL_CASES: Array = [ + withLabel("builds channel message urls", { + conversationId: "19:thread@thread.tacv2", + messageId: "123", + }), + withLabel("builds channel reply urls when replyToId is present", { + messageId: "reply-id", + replyToId: "root-id", + }), +]; +const GRAPH_URL_EXPECTATION_CASES: GraphUrlExpectationCase[] = [ + ...CHANNEL_GRAPH_URL_CASES.map(({ label, ...params }) => + withLabel(label, { + params: createChannelGraphMessageUrlParams(params), + expectedPath: buildExpectedChannelMessagePath(params), + }), + ), + withLabel("builds chat message urls", { + params: { + conversationType: "groupChat" as const, + conversationId: "19:chat@thread.v2", + messageId: "456", + }, + expectedPath: "/chats/19%3Achat%40thread.v2/messages/456", + }), +]; type GraphFetchMockOptions = { hostedContents?: unknown[]; @@ -390,37 +525,88 @@ const buildDefaultShareReferenceGraphFetchOptions = ( referenceAttachment: createReferenceAttachment(), ...params, }); +type GraphEndpointResponseHandler = { + suffix: string; + buildResponse: () => Response; +}; +const createGraphEndpointResponseHandlers = (params: { + hostedContents: unknown[]; + attachments: unknown[]; + messageAttachments: unknown[]; +}): GraphEndpointResponseHandler[] => [ + { + suffix: "/hostedContents", + buildResponse: () => createGraphCollectionResponse(params.hostedContents), + }, + { + suffix: "/attachments", + buildResponse: () => createGraphCollectionResponse(params.attachments), + }, + { + suffix: "/messages/123", + buildResponse: () => createJsonResponse({ attachments: params.messageAttachments }), + }, +]; +const resolveGraphEndpointResponse = ( + url: string, + handlers: GraphEndpointResponseHandler[], +): Response | undefined => { + const handler = handlers.find((entry) => url.endsWith(entry.suffix)); + return handler ? handler.buildResponse() : undefined; +}; const createGraphFetchMock = (options: GraphFetchMockOptions = {}) => { const hostedContents = options.hostedContents ?? []; const attachments = options.attachments ?? []; const messageAttachments = options.messageAttachments ?? []; + const endpointHandlers = createGraphEndpointResponseHandlers({ + hostedContents, + attachments, + messageAttachments, + }); return vi.fn(async (url: string) => { - if (url.endsWith("/hostedContents")) { - return createJsonResponse({ value: hostedContents }); + const endpointResponse = resolveGraphEndpointResponse(url, endpointHandlers); + if (endpointResponse) { + return endpointResponse; } - if (url.endsWith("/attachments")) { - return createJsonResponse({ value: attachments }); - } - if (url.endsWith("/messages/123")) { - return createJsonResponse({ attachments: messageAttachments }); - } - if (url.startsWith("https://graph.microsoft.com/v1.0/shares/") && options.onShareRequest) { + if (url.startsWith(GRAPH_SHARES_URL_PREFIX) && options.onShareRequest) { return options.onShareRequest(url); } const unhandled = options.onUnhandled ? await options.onUnhandled(url) : undefined; - return unhandled ?? new Response("not found", { status: 404 }); + return unhandled ?? createNotFoundResponse(); }); }; const downloadGraphMediaWithMockOptions = async ( options: GraphFetchMockOptions = {}, overrides: DownloadGraphMediaOverrides = {}, -) => { +): Promise => { const fetchMock = createGraphFetchMock(options); - const media = await downloadGraphMediaWithFetch(fetchMock, overrides); + const media = await downloadMSTeamsGraphMedia({ + messageUrl: DEFAULT_MESSAGE_URL, + tokenProvider: createTokenProvider(), + maxBytes: DEFAULT_MAX_BYTES, + fetchFn: asFetchFn(fetchMock), + ...overrides, + }); return { fetchMock, media }; }; -const runAttachmentAuthRetryScenario = async (scenario: AttachmentAuthRetryScenario) => { +const runAttachmentDownloadSuccessCase = async ({ + attachments, + buildFetchFn, + beforeDownload, + assert, +}: AttachmentDownloadSuccessCase) => { + const fetchFn = (buildFetchFn ?? (() => createOkFetchMock(CONTENT_TYPE_IMAGE_PNG)))(); + beforeDownload?.(); + const media = await downloadAttachmentsWithFetch(attachments, fetchFn); + expectSingleMedia(media); + assert?.(media); +}; +const runAttachmentAuthRetryCase = async ({ + scenario, + expectedMediaLength, + expectTokenFetch, +}: AttachmentAuthRetryCase) => { const tokenProvider = createTokenProvider(); const fetchMock = createAuthAwareImageFetchMock({ unauthStatus: scenario.unauthStatus, @@ -431,7 +617,17 @@ const runAttachmentAuthRetryScenario = async (scenario: AttachmentAuthRetryScena fetchMock, { tokenProvider, ...scenario.overrides }, ); - return { tokenProvider, media }; + expectAttachmentMediaLength(media, expectedMediaLength); + expectMockCallState(tokenProvider.getAccessToken, expectTokenFetch); +}; +const runGraphMediaSuccessCase = async ({ + buildOptions, + expectedLength, + assert, +}: GraphMediaSuccessCase) => { + const { fetchMock, media } = await downloadGraphMediaWithMockOptions(buildOptions()); + expectAttachmentMediaLength(media.media, expectedLength); + assert?.({ fetchMock, media }); }; describe("msteams attachments", () => { @@ -443,94 +639,19 @@ describe("msteams attachments", () => { }); describe("buildMSTeamsAttachmentPlaceholder", () => { - it.each([ - { label: "returns empty string when no attachments", attachments: undefined, expected: "" }, - { label: "returns empty string when attachments are empty", attachments: [], expected: "" }, - { - label: "returns image placeholder for one image attachment", - attachments: createImageAttachments(TEST_URL_IMAGE_PNG), - expected: MEDIA_PLACEHOLDER_IMAGE, + it.each(ATTACHMENT_PLACEHOLDER_CASES)( + "$label", + ({ attachments, expected }) => { + expect(buildMSTeamsAttachmentPlaceholder(attachments)).toBe(expected); }, - { - label: "returns image placeholder with count for many image attachments", - attachments: [ - ...createImageAttachments(TEST_URL_IMAGE_1_PNG), - { contentType: "image/jpeg", contentUrl: TEST_URL_IMAGE_2_JPG }, - ], - expected: `${MEDIA_PLACEHOLDER_IMAGE} (2 images)`, - }, - { - label: "treats Teams file.download.info image attachments as images", - attachments: createTeamsFileDownloadInfoAttachments(), - expected: MEDIA_PLACEHOLDER_IMAGE, - }, - { - label: "returns document placeholder for non-image attachments", - attachments: createPdfAttachments(TEST_URL_PDF), - expected: MEDIA_PLACEHOLDER_DOCUMENT, - }, - { - label: "returns document placeholder with count for many non-image attachments", - 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: createHtmlImageAttachments([TEST_URL_HTML_A], "

hi

"), - expected: MEDIA_PLACEHOLDER_IMAGE, - }, - { - label: "counts many inline images in html attachments", - attachments: createHtmlImageAttachments([TEST_URL_HTML_A, TEST_URL_HTML_B]), - expected: `${MEDIA_PLACEHOLDER_IMAGE} (2 images)`, - }, - ])("$label", ({ attachments, expected }) => { - expectAttachmentPlaceholder(attachments, expected); - }); + ); }); describe("downloadMSTeamsAttachments", () => { - it.each([ - { - label: "downloads and stores image contentUrl attachments", - attachments: asSingleItemArray(IMAGE_ATTACHMENT), - assert: (media) => { - expectMediaSaved(); - expectFirstMedia(media, { path: SAVED_PNG_PATH }); - }, - }, - { - label: "supports Teams file.download.info downloadUrl attachments", - attachments: createTeamsFileDownloadInfoAttachments(), - }, - { - label: "downloads inline image URLs from html attachments", - attachments: createHtmlImageAttachments([TEST_URL_INLINE_IMAGE]), - }, - ])("$label", async ({ attachments, assert }) => { - const media = await downloadAttachmentsWithOkImageFetch(attachments); - expectSingleMedia(media); - assert?.(media); - }); - - it("downloads non-image file attachments (PDF)", async () => { - const fetchMock = createOkFetchMock(CONTENT_TYPE_APPLICATION_PDF, "pdf"); - detectMimeMock.mockResolvedValueOnce(CONTENT_TYPE_APPLICATION_PDF); - saveMediaBufferMock.mockResolvedValueOnce({ - path: SAVED_PDF_PATH, - contentType: CONTENT_TYPE_APPLICATION_PDF, - }); - - const media = await downloadAttachmentsWithFetch( - createPdfAttachments(TEST_URL_DOC_PDF), - fetchMock, - ); - - expectSingleMedia(media, { - path: SAVED_PDF_PATH, - placeholder: MEDIA_PLACEHOLDER_DOCUMENT, - }); - }); + it.each(ATTACHMENT_DOWNLOAD_SUCCESS_CASES)( + "$label", + runAttachmentDownloadSuccessCase, + ); it("stores inline data:image base64 payloads", async () => { const media = await downloadMSTeamsAttachments( @@ -540,40 +661,13 @@ describe("msteams attachments", () => { ); expectSingleMedia(media); - expectMediaSaved(); + expectMediaBufferSaved(); }); - it.each([ - { - label: "retries with auth when the first request is unauthorized", - scenario: { - attachmentUrl: IMAGE_ATTACHMENT.contentUrl, - unauthStatus: 401, - unauthBody: "unauthorized", - overrides: { authAllowHosts: ["x"] }, - }, - expectedMediaLength: 1, - expectTokenFetch: true, - }, - { - label: "skips auth retries when the host is not in auth allowlist", - scenario: { - attachmentUrl: "https://attacker.azureedge.net/img", - unauthStatus: 403, - unauthBody: "forbidden", - overrides: { - allowHosts: ["azureedge.net"], - authAllowHosts: ["graph.microsoft.com"], - }, - }, - expectedMediaLength: 0, - expectTokenFetch: false, - }, - ])("$label", async ({ scenario, expectedMediaLength, expectTokenFetch }) => { - const { tokenProvider, media } = await runAttachmentAuthRetryScenario(scenario); - expectMediaLength(media, expectedMediaLength); - expectMockCallState(tokenProvider.getAccessToken, expectTokenFetch); - }); + it.each(ATTACHMENT_AUTH_RETRY_CASES)( + "$label", + runAttachmentAuthRetryCase, + ); it("skips urls outside the allowlist", async () => { const fetchMock = vi.fn(); @@ -581,90 +675,34 @@ describe("msteams attachments", () => { createImageAttachments(TEST_URL_OUTSIDE_ALLOWLIST), fetchMock, { - allowHosts: ["graph.microsoft.com"], + allowHosts: [GRAPH_HOST], resolveFn: undefined, }, { expectFetchCalled: false }, ); - expectNoMedia(media); + expectAttachmentMediaLength(media, 0); }); }); describe("buildMSTeamsGraphMessageUrls", () => { - const cases: GraphUrlExpectationCase[] = [ - { - label: "builds channel message urls", - params: createChannelGraphMessageUrlParams({ - conversationId: "19:thread@thread.tacv2", - messageId: "123", - }), - expectedPath: buildExpectedChannelMessagePath({ messageId: "123" }), - }, - { - label: "builds channel reply urls when replyToId is present", - params: createChannelGraphMessageUrlParams({ - messageId: "reply-id", - replyToId: "root-id", - }), - expectedPath: buildExpectedChannelMessagePath({ - messageId: "reply-id", - replyToId: "root-id", - }), - }, - { - label: "builds chat message urls", - params: { - conversationType: "groupChat" as const, - conversationId: "19:chat@thread.v2", - messageId: "456", - }, - expectedPath: "/chats/19%3Achat%40thread.v2/messages/456", - }, - ]; - - it.each(cases)("$label", ({ params, expectedPath }) => { - expectFirstGraphUrlContains(params, expectedPath); + it.each(GRAPH_URL_EXPECTATION_CASES)("$label", ({ params, expectedPath }) => { + const urls = buildMSTeamsGraphMessageUrls(params); + expect(urls[0]).toContain(expectedPath); }); }); describe("downloadMSTeamsGraphMedia", () => { - 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.each(GRAPH_MEDIA_SUCCESS_CASES)("$label", runGraphMediaSuccessCase); it("blocks SharePoint redirects to hosts outside allowHosts", async () => { const escapedUrl = "https://evil.example/internal.pdf"; fetchRemoteMediaMock.mockImplementationOnce(async (params) => { const fetchFn = params.fetchImpl ?? fetch; let currentUrl = params.url; - for (let i = 0; i < 5; i += 1) { + for (let i = 0; i < MAX_REDIRECT_HOPS; i += 1) { const res = await fetchFn(currentUrl, { redirect: "manual" }); - if ([301, 302, 303, 307, 308].includes(res.status)) { + if (REDIRECT_STATUS_CODES.includes(res.status)) { const location = res.headers.get("location"); if (!location) { throw new Error("redirect missing location"); @@ -672,14 +710,7 @@ describe("msteams attachments", () => { currentUrl = new URL(location, currentUrl).toString(); continue; } - if (!res.ok) { - throw new Error(`HTTP ${res.status}`); - } - return { - buffer: Buffer.from(await res.arrayBuffer()), - contentType: res.headers.get("content-type") ?? undefined, - fileName: params.filePathHint, - }; + return readRemoteMediaResponse(res, params); } throw new Error("too many redirects"); }); @@ -687,11 +718,7 @@ describe("msteams attachments", () => { const { fetchMock, media } = await downloadGraphMediaWithMockOptions( { ...buildDefaultShareReferenceGraphFetchOptions({ - onShareRequest: () => - new Response(null, { - status: 302, - headers: { location: escapedUrl }, - }), + onShareRequest: () => createRedirectResponse(escapedUrl), onUnhandled: (url) => { if (url === escapedUrl) { return createPdfResponse("should-not-be-fetched"); @@ -705,11 +732,9 @@ describe("msteams attachments", () => { }, ); - expectNoGraphMedia(media); + expectAttachmentMediaLength(media.media, 0); const calledUrls = fetchMock.mock.calls.map((call) => String(call[0])); - expect( - calledUrls.some((url) => url.startsWith("https://graph.microsoft.com/v1.0/shares/")), - ).toBe(true); + expect(calledUrls.some((url) => url.startsWith(GRAPH_SHARES_URL_PREFIX))).toBe(true); expect(calledUrls).not.toContain(escapedUrl); }); }); diff --git a/src/agents/bash-tools.test.ts b/src/agents/bash-tools.test.ts index eb7e415fb87..db0a910f2c8 100644 --- a/src/agents/bash-tools.test.ts +++ b/src/agents/bash-tools.test.ts @@ -17,6 +17,16 @@ 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 BACKGROUND_POLL_OPTIONS = { + timeout: BACKGROUND_POLL_TIMEOUT_MS, + interval: POLL_INTERVAL_MS, +}; +const NOTIFY_POLL_OPTIONS = { + timeout: NOTIFY_EVENT_TIMEOUT_MS, + interval: POLL_INTERVAL_MS, +}; +const SHELL_ENV_KEYS = ["SHELL"] as const; +const PATH_SHELL_ENV_KEYS = ["PATH", "SHELL"] as const; const PROCESS_STATUS_RUNNING = "running"; const PROCESS_STATUS_COMPLETED = "completed"; const PROCESS_STATUS_FAILED = "failed"; @@ -24,12 +34,15 @@ 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 shellEcho = (message: string) => (isWin ? `Write-Output ${message}` : `echo ${message}`); +const COMMAND_ECHO_HELLO = shellEcho("hello"); +const COMMAND_PRINT_PATH = isWin ? "Write-Output $env:PATH" : "echo $PATH"; +const COMMAND_EXIT_WITH_ERROR = "exit 1"; 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"; +const ECHO_HI_COMMAND = shellEcho("hi"); let callIdCounter = 0; const nextCallId = () => `call${++callIdCounter}`; type ExecToolInstance = ReturnType; @@ -37,6 +50,8 @@ type ProcessToolInstance = ReturnType; type ExecToolArgs = Parameters[1]; type ProcessToolArgs = Parameters[1]; type ExecToolConfig = Exclude[0], undefined>; +type ExecToolRunOptions = Omit; +type LabeledCase = { label: string }; const createTestExecTool = ( defaults?: Parameters[0], ): ReturnType => createExecTool({ ...TEST_EXEC_DEFAULTS, ...defaults }); @@ -62,10 +77,14 @@ const createScopedToolSet = (scopeKey: string) => ({ }); const execTool = createTestExecTool(); const processTool = createProcessTool(); +const withLabel = (label: string, fields: T): T & LabeledCase => ({ + label, + ...fields, +}); // Both PowerShell and bash use ; for command separation const joinCommands = (commands: string[]) => commands.join("; "); -const echoAfterDelay = (message: string) => joinCommands([shortDelayCmd, `echo ${message}`]); -const echoLines = (lines: string[]) => joinCommands(lines.map((line) => `echo ${line}`)); +const echoAfterDelay = (message: string) => joinCommands([shortDelayCmd, shellEcho(message)]); +const echoLines = (lines: string[]) => joinCommands(lines.map((line) => shellEcho(line))); const normalizeText = (value?: string) => sanitizeBinaryOutput(value ?? "") .replace(/\r\n/g, "\n") @@ -85,9 +104,6 @@ const readTotalLines = (details: unknown) => (details as { totalLines?: number } 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, @@ -104,36 +120,22 @@ const expectTextContainsValues = ( } } }; -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 hasSession = (sessions: ProcessSessionSummary[], sessionId: string) => + sessions.some((session) => session.sessionId === sessionId); const executeExecTool = (tool: ExecToolInstance, params: ExecToolArgs) => tool.execute(nextCallId(), params); +const executeExecCommand = ( + tool: ExecToolInstance, + command: string, + options: ExecToolRunOptions = {}, +) => executeExecTool(tool, { command, ...options }); 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); + return (list.details as { sessions: ProcessSessionSummary[] }).sessions; } async function pollProcessSession(params: { tool: ProcessToolInstance; @@ -148,7 +150,6 @@ async function pollProcessSession(params: { output: readTextContent(poll.content), }; } - function applyDefaultShellEnv() { if (!isWin && defaultShell) { process.env.SHELL = defaultShell; @@ -168,20 +169,13 @@ function useCapturedEnv(keys: string[], afterCapture?: () => void) { }); } -function useCapturedShellEnv() { - useCapturedEnv(["SHELL"], applyDefaultShellEnv); -} - async function waitForCompletion(sessionId: string) { let status = PROCESS_STATUS_RUNNING; await expect - .poll( - async () => { - status = (await pollProcessSession({ tool: processTool, sessionId })).status; - return status; - }, - { timeout: BACKGROUND_POLL_TIMEOUT_MS, interval: POLL_INTERVAL_MS }, - ) + .poll(async () => { + status = (await pollProcessSession({ tool: processTool, sessionId })).status; + return status; + }, BACKGROUND_POLL_OPTIONS) .not.toBe(PROCESS_STATUS_RUNNING); return status; } @@ -192,6 +186,10 @@ function requireSessionId(details: { sessionId?: string }): string { } return details.sessionId; } +const requireRunningSessionId = (result: { details: unknown }) => { + expect(readProcessStatus(result.details)).toBe(PROCESS_STATUS_RUNNING); + return requireSessionId(result.details as { sessionId?: string }); +}; function hasNotifyEventForPrefix(prefix: string): boolean { return peekSystemEvents(DEFAULT_NOTIFY_SESSION_KEY).some((event) => event.includes(prefix)); @@ -202,14 +200,11 @@ async function waitForNotifyEvent(sessionId: string) { 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 }, - ) + .poll(() => { + finished = getFinishedSession(sessionId); + hasEvent = hasNotifyEventForPrefix(prefix); + return Boolean(finished && hasEvent); + }, NOTIFY_POLL_OPTIONS) .toBe(true); return { finished: finished ?? getFinishedSession(sessionId), @@ -217,22 +212,14 @@ async function waitForNotifyEvent(sessionId: string) { }; } -async function startBackgroundSession(params: { tool: ExecToolInstance; command: string }) { - const result = await executeExecTool(params.tool, { - command: params.command, - background: true, - }); - expectProcessStatus(result.details, PROCESS_STATUS_RUNNING); - return requireSessionId(result.details as { sessionId?: string }); +async function startBackgroundCommand(tool: ExecToolInstance, command: string) { + const result = await executeExecCommand(tool, command, { background: true }); + return requireRunningSessionId(result); } - -async function runBackgroundEchoLines(lines: string[]) { - const sessionId = await startBackgroundSession({ - tool: execTool, - command: echoLines(lines), - }); - await waitForCompletion(sessionId); - return sessionId; +async function runBackgroundCommandToCompletion(tool: ExecToolInstance, command: string) { + const sessionId = await startBackgroundCommand(tool, command); + const status = await waitForCompletion(sessionId); + return { sessionId, status }; } type ProcessLogWindow = { offset?: number; limit?: number }; @@ -244,65 +231,87 @@ async function readProcessLog(sessionId: string, options: ProcessLogWindow = {}) }); } -type ProcessLogResult = Awaited>; -const readLogSnapshot = (log: ProcessLogResult) => ({ - text: readTextContent(log.content) ?? "", - lines: readTrimmedLines(log.content), - totalLines: readTotalLines(log.details), -}); -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: ProcessLogWindow = {}) { - const sessionId = await runBackgroundEchoLines(lines); - return readProcessLog(sessionId, options); -} -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; +type LongLogExpectationCase = LabeledCase & { options?: ProcessLogWindow; firstLine: string; lastLine?: string; mustContain?: string[]; mustNotContain?: string[]; }; -type ShortLogExpectationCase = { - label: string; +type ShortLogExpectationCase = LabeledCase & { lines: string[]; options: ProcessLogWindow; expectedText: string; expectedTotalLines: number; }; -type DisallowedElevationCase = { - label: string; +type ProcessLogSnapshot = { + text: string; + normalizedText: string; + lines: string[]; + totalLines: number | undefined; +}; +const EXPECTED_TOTAL_LINES_THREE = 3; +type DisallowedElevationCase = LabeledCase & { defaultLevel: "off" | "on"; overrides?: Partial; requestElevated?: boolean; expectedError?: string; expectedOutputIncludes?: string; }; -type NotifyNoopCase = { - label: string; +type NotifyNoopCase = LabeledCase & { 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", + withLabel("default behavior skips no-op completion events", { notifyOnExitEmptySuccess: false }), + withLabel("explicitly enabling no-op completion emits completion events", { notifyOnExitEmptySuccess: true, - }, + }), +]; +const DISALLOWED_ELEVATION_CASES: DisallowedElevationCase[] = [ + withLabel("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", + }), + withLabel("does not default to elevated when not allowed", { + defaultLevel: "on", + overrides: { + backgroundMs: 1000, + timeoutSec: 5, + }, + expectedOutputIncludes: "hi", + }), +]; +const SHORT_LOG_EXPECTATION_CASES: ShortLogExpectationCase[] = [ + withLabel("logs line-based slices and defaults to last lines", { + lines: ["one", "two", "three"], + options: { limit: 2 }, + expectedText: "two\nthree", + expectedTotalLines: EXPECTED_TOTAL_LINES_THREE, + }), + withLabel("supports line offsets for log slices", { + lines: ["alpha", "beta", "gamma"], + options: { offset: 1, limit: 1 }, + expectedText: "beta", + expectedTotalLines: EXPECTED_TOTAL_LINES_THREE, + }), +]; +const LONG_LOG_EXPECTATION_CASES: LongLogExpectationCase[] = [ + withLabel("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"], + }), + withLabel("keeps offset-only log requests unbounded by default tail mode", { + options: { offset: 30 }, + firstLine: "line-31", + lastLine: "line-201", + mustNotContain: ["showing last 200"], + }), ]; const expectNotifyNoopEvents = ( events: string[], @@ -319,16 +328,79 @@ const expectNotifyNoopEvents = ( label, ).toBe(true); }; +const runDisallowedElevationCase = async ({ + defaultLevel, + overrides, + requestElevated, + expectedError, + expectedOutputIncludes, +}: DisallowedElevationCase) => { + const customBash = createDisallowedElevatedExecTool(defaultLevel, overrides); + if (expectedError) { + await expect( + executeExecCommand(customBash, ECHO_HI_COMMAND, { elevated: requestElevated }), + ).rejects.toThrow(expectedError); + return; + } -async function runBackgroundAndWaitForCompletion(params: { - tool: ExecToolInstance; - command: string; -}) { - const sessionId = await startBackgroundSession(params); - const status = await waitForCompletion(sessionId); + const result = await executeExecCommand(customBash, ECHO_HI_COMMAND); + if (expectedOutputIncludes === undefined) { + throw new Error("expected text assertion value"); + } + expect(readTextContent(result.content) ?? "").toContain(expectedOutputIncludes); +}; +const runShortLogExpectationCase = async ({ + lines, + options, + expectedText, + expectedTotalLines, +}: ShortLogExpectationCase) => { + const snapshot = await readBackgroundLogSnapshot(lines, options); + expect(snapshot.normalizedText).toBe(expectedText); + expect(snapshot.totalLines).toBe(expectedTotalLines); +}; +const readBackgroundLogSnapshot = async ( + lines: string[], + options: ProcessLogWindow = {}, +): Promise => { + const { sessionId } = await runBackgroundCommandToCompletion(execTool, echoLines(lines)); + const log = await readProcessLog(sessionId, options); + return { + text: readTextContent(log.content) ?? "", + normalizedText: readNormalizedTextContent(log.content), + lines: readTrimmedLines(log.content), + totalLines: readTotalLines(log.details), + }; +}; +const runLongLogExpectationCase = async ({ + options, + firstLine, + lastLine, + mustContain, + mustNotContain, +}: LongLogExpectationCase) => { + const snapshot = await readBackgroundLogSnapshot( + Array.from({ length: LONG_LOG_LINE_COUNT }, (_value, index) => `line-${index + 1}`), + 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); + expectTextContainsValues(snapshot.text, mustContain, true); + expectTextContainsValues(snapshot.text, mustNotContain, false); +}; +const runNotifyNoopCase = async ({ label, notifyOnExitEmptySuccess }: NotifyNoopCase) => { + const tool = createNotifyOnExitExecTool( + notifyOnExitEmptySuccess ? { notifyOnExitEmptySuccess: true } : {}, + ); + + const { status } = await runBackgroundCommandToCompletion(tool, shortDelayCmd); expect(status).toBe(PROCESS_STATUS_COMPLETED); - return { sessionId }; -} + const events = peekSystemEvents(DEFAULT_NOTIFY_SESSION_KEY); + expectNotifyNoopEvents(events, notifyOnExitEmptySuccess, label); +}; beforeEach(() => { callIdCounter = 0; @@ -337,54 +409,44 @@ beforeEach(() => { }); describe("exec tool backgrounding", () => { - useCapturedShellEnv(); + useCapturedEnv([...SHELL_ENV_KEYS], applyDefaultShellEnv); it( "backgrounds after yield and can be polled", async () => { - const result = await executeExecTool(execTool, { - command: joinCommands([yieldDelayCmd, "echo done"]), - yieldMs: 10, - }); + const result = await executeExecCommand( + execTool, + joinCommands([yieldDelayCmd, shellEcho(OUTPUT_DONE)]), + { yieldMs: 10 }, + ); // Timing can race here: command may already be complete before the first response. if (result.details.status === PROCESS_STATUS_COMPLETED) { - const text = readTextContent(result.content) ?? ""; - expectTextContains(text, OUTPUT_DONE); + expect(readTextContent(result.content) ?? "").toContain(OUTPUT_DONE); return; } - expectProcessStatus(result.details, PROCESS_STATUS_RUNNING); - const sessionId = requireSessionId(result.details as { sessionId?: string }); + const sessionId = requireRunningSessionId(result); let output = ""; await expect - .poll( - async () => { - const pollResult = await pollProcessSession({ - tool: processTool, - sessionId, - }); - output = pollResult.output ?? ""; - return pollResult.status; - }, - { timeout: BACKGROUND_POLL_TIMEOUT_MS, interval: POLL_INTERVAL_MS }, - ) + .poll(async () => { + const pollResult = await pollProcessSession({ tool: processTool, sessionId }); + output = pollResult.output ?? ""; + return pollResult.status; + }, BACKGROUND_POLL_OPTIONS) .toBe(PROCESS_STATUS_COMPLETED); - expectTextContains(output, OUTPUT_DONE); + expect(output).toContain(OUTPUT_DONE); }, isWin ? 15_000 : 5_000, ); it("supports explicit background and derives session name from the command", async () => { - const sessionId = await startBackgroundSession({ - tool: execTool, - command: COMMAND_ECHO_HELLO, - }); + const sessionId = await startBackgroundCommand(execTool, COMMAND_ECHO_HELLO); const sessions = await listProcessSessions(processTool); - expectSessionMembership(sessions, sessionId, true); + expect(hasSession(sessions, sessionId)).toBe(true); expect(sessions.find((s) => s.sessionId === sessionId)?.name).toBe(COMMAND_ECHO_HELLO); }); @@ -394,114 +456,30 @@ describe("exec tool backgrounding", () => { backgroundMs: 10, allowBackground: false, }); - await expect( - executeExecTool(customBash, { - command: longDelayCmd, - }), - ).rejects.toThrow(/timed out/i); + await expect(executeExecCommand(customBash, longDelayCmd)).rejects.toThrow(/timed out/i); }); - 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", - }, - ])( + it.each(DISALLOWED_ELEVATION_CASES)( "$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; - } - - const result = await executeExecTool(customBash, { - command: ECHO_HI_COMMAND, - }); - expectTextContains(readTextContent(result.content), expectedOutputIncludes); - }, + runDisallowedElevationCase, ); - 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.each(SHORT_LOG_EXPECTATION_CASES)( + "$label", + runShortLogExpectationCase, + ); - 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.each(LONG_LOG_EXPECTATION_CASES)("$label", runLongLogExpectationCase); it("scopes process sessions by scopeKey", async () => { const alphaTools = createScopedToolSet(SCOPE_KEY_ALPHA); const betaTools = createScopedToolSet(SCOPE_KEY_BETA); - const sessionA = await startBackgroundSession({ - tool: alphaTools.exec, - command: shortDelayCmd, - }); - const sessionB = await startBackgroundSession({ - tool: betaTools.exec, - command: shortDelayCmd, - }); + const sessionA = await startBackgroundCommand(alphaTools.exec, shortDelayCmd); + const sessionB = await startBackgroundCommand(betaTools.exec, shortDelayCmd); const sessionsA = await listProcessSessions(alphaTools.process); - expectSessionMembership(sessionsA, sessionA, true); - expectSessionMembership(sessionsA, sessionB, false); + expect(hasSession(sessionsA, sessionA)).toBe(true); + expect(hasSession(sessionsA, sessionB)).toBe(false); const pollB = await pollProcessSession({ tool: betaTools.process, @@ -512,19 +490,18 @@ describe("exec tool backgrounding", () => { }); describe("exec exit codes", () => { - useCapturedShellEnv(); + useCapturedEnv([...SHELL_ENV_KEYS], applyDefaultShellEnv); it("treats non-zero exits as completed and appends exit code", async () => { - const command = isWin - ? joinCommands(["Write-Output nope", "exit 1"]) - : joinCommands([`echo ${OUTPUT_NOPE}`, "exit 1"]); - const result = await executeExecTool(execTool, { command }); + const command = joinCommands([shellEcho(OUTPUT_NOPE), COMMAND_EXIT_WITH_ERROR]); + const result = await executeExecCommand(execTool, command); const resultDetails = result.details as { status?: string; exitCode?: number | null }; - expectProcessStatus(resultDetails, PROCESS_STATUS_COMPLETED); + expect(readProcessStatus(resultDetails)).toBe(PROCESS_STATUS_COMPLETED); expect(resultDetails.exitCode).toBe(1); const text = readNormalizedTextContent(result.content); - expectTextContainsAll(text, [OUTPUT_NOPE, OUTPUT_EXIT_CODE_1]); + expect(text).toContain(OUTPUT_NOPE); + expect(text).toContain(OUTPUT_EXIT_CODE_1); }); }); @@ -532,10 +509,7 @@ describe("exec notifyOnExit", () => { it("enqueues a system event when a backgrounded exec exits", async () => { const tool = createNotifyOnExitExecTool(); - const sessionId = await startBackgroundSession({ - tool, - command: echoAfterDelay("notify"), - }); + const sessionId = await startBackgroundCommand(tool, echoAfterDelay("notify")); const { finished, hasEvent } = await waitForNotifyEvent(sessionId); @@ -543,25 +517,11 @@ describe("exec notifyOnExit", () => { expect(hasEvent).toBe(true); }); - it.each(NOOP_NOTIFY_CASES)( - "$label", - async ({ label, notifyOnExitEmptySuccess }) => { - const tool = createNotifyOnExitExecTool( - notifyOnExitEmptySuccess ? { notifyOnExitEmptySuccess: true } : {}, - ); - - await runBackgroundAndWaitForCompletion({ - tool, - command: shortDelayCmd, - }); - const events = peekSystemEvents(DEFAULT_NOTIFY_SESSION_KEY); - expectNotifyNoopEvents(events, notifyOnExitEmptySuccess, label); - }, - ); + it.each(NOOP_NOTIFY_CASES)("$label", runNotifyNoopCase); }); describe("exec PATH handling", () => { - useCapturedEnv(["PATH", "SHELL"], applyDefaultShellEnv); + useCapturedEnv([...PATH_SHELL_ENV_KEYS], applyDefaultShellEnv); it("prepends configured path entries", async () => { const basePath = isWin ? "C:\\Windows\\System32" : "/usr/bin"; @@ -569,9 +529,7 @@ describe("exec PATH handling", () => { process.env.PATH = basePath; const tool = createTestExecTool({ pathPrepend: prepend }); - const result = await executeExecTool(tool, { - command: isWin ? "Write-Output $env:PATH" : "echo $PATH", - }); + const result = await executeExecCommand(tool, COMMAND_PRINT_PATH); const text = readNormalizedTextContent(result.content); const entries = text.split(path.delimiter);