From 13b8161aac387632b3d08b1185a9bbc47bf54810 Mon Sep 17 00:00:00 2001 From: Savio Dias Date: Tue, 10 Nov 2026 17:07:54 +0530 Subject: [PATCH 1/5] fix(appautomate): support framework builds in listSessions, getFailureLogs and screenshots Co-Authored-By: Claude Opus 5.5 --- package-lock.json | 1 + package.json | 1 + .../appautomate-utils/framework-builds.ts | 177 ++++++++++++ src/tools/automate-utils/fetch-screenshots.ts | 83 +++++- src/tools/automate.ts | 18 ++ src/tools/failurelogs-utils/framework.ts | 99 +++++++ src/tools/get-failure-logs.ts | 43 ++- tests/tools/frameworkBuilds.test.ts | 253 ++++++++++++++++++ 8 files changed, 671 insertions(+), 4 deletions(-) create mode 100644 src/tools/appautomate-utils/framework-builds.ts create mode 100644 src/tools/failurelogs-utils/framework.ts create mode 100644 tests/tools/frameworkBuilds.test.ts diff --git a/package-lock.json b/package-lock.json index f717cbb5..ccf00599 100644 --- a/package-lock.json +++ b/package-lock.json @@ -16,6 +16,7 @@ "csv-parse": "^6.2.1", "dotenv": "^17.4.0", "form-data": "^4.0.5", + "jszip": "^3.10.1", "pino": "^10.3.1", "pino-pretty": "^13.1.3", "sharp": "^0.35.5", diff --git a/package.json b/package.json index deb59cc3..286bced6 100644 --- a/package.json +++ b/package.json @@ -49,6 +49,7 @@ "csv-parse": "^6.2.1", "dotenv": "^17.4.0", "form-data": "^4.0.5", + "jszip": "^3.10.1", "pino": "^10.3.1", "pino-pretty": "^13.1.3", "sharp": "^0.35.5", diff --git a/src/tools/appautomate-utils/framework-builds.ts b/src/tools/appautomate-utils/framework-builds.ts new file mode 100644 index 00000000..a448ac1c --- /dev/null +++ b/src/tools/appautomate-utils/framework-builds.ts @@ -0,0 +1,177 @@ +import { apiClient } from "../../lib/apiClient.js"; +import { HttpError } from "../../lib/error.js"; +import { getBrowserStackAuth } from "../../lib/get-auth.js"; +import { getRegionalHost } from "../../lib/regional-host.js"; +import { BrowserStackConfig } from "../../lib/types.js"; +import { SessionIdRecord } from "../automate-utils/list-session-ids.js"; + +/** + * App Automate builds run through a framework API (Maestro, Espresso, XCUITest, + * Flutter) have their own build and session endpoints. The generic App Automate + * session list returns nothing for them and the generic log endpoints return 404; + * their logs are links on each test case of the framework session. + */ +export const FRAMEWORKS = [ + { name: "maestro", path: "maestro/v2" }, + { name: "espresso", path: "espresso/v2" }, + { name: "xcuitest", path: "xcuitest/v2" }, + { name: "flutter-android", path: "flutter-integration-tests/v2/android" }, + { name: "flutter-ios", path: "flutter-integration-tests/v2/ios" }, +] as const; + +export type Framework = (typeof FRAMEWORKS)[number]; + +export interface FrameworkBuild { + framework: Framework; + buildId: string; + data: any; +} + +export interface FrameworkTestCase { + name: string; + className?: string; + status?: string; + /** Link fields as returned: video, maestro_log, instrumentation_log, ... */ + fields: Record; +} + +function headers(config: BrowserStackConfig): Record { + return { + "Content-Type": "application/json", + Authorization: `Basic ${Buffer.from(getBrowserStackAuth(config)).toString("base64")}`, + }; +} + +function frameworkUrl(framework: Framework, path: string): string { + return `${getRegionalHost("appAutomateCloud")}/app-automate/${framework.path}/${path}`; +} + +async function getJson(url: string, config: BrowserStackConfig) { + const response = await apiClient.get({ + url, + headers: headers(config), + raise_error: false, + }); + if (response.status === 404) return undefined; + if (!response.ok) { + throw new HttpError( + `Failed to read ${url.split("/app-automate/")[1] ?? url}: ${response.status} ${response.statusText}`.trim(), + response.status, + ); + } + return response.data; +} + +/** + * The framework build with this id, or undefined if none has it. Each framework's + * build endpoint returns 404 for another framework's build, so this is at most + * one call per framework. + */ +export async function findFrameworkBuild( + buildId: string, + config: BrowserStackConfig, +): Promise { + const id = encodeURIComponent(buildId.trim()); + for (const framework of FRAMEWORKS) { + const data = await getJson(frameworkUrl(framework, `builds/${id}`), config); + if (data) return { framework, buildId: buildId.trim(), data }; + } + return undefined; +} + +export function frameworkSessions(build: FrameworkBuild): SessionIdRecord[] { + const records: SessionIdRecord[] = []; + for (const device of build.data?.devices ?? []) { + for (const session of device?.sessions ?? []) { + if (typeof session?.id !== "string") continue; + records.push({ + sessionId: session.id, + status: session.status, + os: device.os, + osVersion: device.os_version, + device: device.device ?? null, + browserUrl: `https://app-automate.browserstack.com/dashboard/v2/builds/${build.buildId}/sessions/${session.id}`, + }); + } + } + return records; +} + +export async function getFrameworkTestCases( + build: FrameworkBuild, + sessionId: string, + config: BrowserStackConfig, +): Promise { + const data = await getJson( + frameworkUrl( + build.framework, + `builds/${encodeURIComponent(build.buildId)}/sessions/${encodeURIComponent(sessionId)}`, + ), + config, + ); + if (!data) return undefined; + const cases: FrameworkTestCase[] = []; + for (const group of data?.testcases?.data ?? []) { + for (const test of group?.testcases ?? []) { + const { name, status, ...fields } = test ?? {}; + cases.push({ + name: typeof name === "string" ? name : "unnamed test", + className: typeof group?.class === "string" ? group.class : undefined, + status: typeof status === "string" ? status : undefined, + fields, + }); + } + } + return cases; +} + +const FAILED = new Set(["failed", "error", "timedout"]); + +/** Failed tests first, then the rest, in their original order. */ +export function failedFirst(cases: FrameworkTestCase[]): FrameworkTestCase[] { + return [ + ...cases.filter((c) => FAILED.has(c.status ?? "")), + ...cases.filter((c) => !FAILED.has(c.status ?? "")), + ]; +} + +/** Only BrowserStack hosts get the caller's credentials. */ +export function isBrowserStackUrl(value: unknown): value is string { + if (typeof value !== "string") return false; + try { + const url = new URL(value); + return ( + url.protocol === "https:" && + (url.hostname === "browserstack.com" || + url.hostname.endsWith(".browserstack.com")) + ); + } catch { + return false; + } +} + +/** Download a test-case link with the caller's credentials. */ +export async function downloadTestCaseFile( + url: string, + config: BrowserStackConfig, + options: { binary?: boolean; maxBytes: number }, +) { + if (!isBrowserStackUrl(url)) { + throw new Error("refusing to fetch a log link outside browserstack.com"); + } + const response = await apiClient.get({ + url, + headers: { Authorization: headers(config).Authorization }, + ...(options.binary ? { responseType: "arraybuffer" as const } : {}), + maxContentLength: options.maxBytes, + raise_error: false, + }); + if (response.status === 413) return { tooLarge: true as const }; + if (!response.ok) { + throw new HttpError( + `Failed to download a test log: ${response.status} ${response.statusText}`.trim(), + response.status, + ); + } + return { tooLarge: false as const, data: response.data }; +} diff --git a/src/tools/automate-utils/fetch-screenshots.ts b/src/tools/automate-utils/fetch-screenshots.ts index 0145a996..72a054a8 100644 --- a/src/tools/automate-utils/fetch-screenshots.ts +++ b/src/tools/automate-utils/fetch-screenshots.ts @@ -6,6 +6,15 @@ import { apiClient } from "../../lib/apiClient.js"; import globalConfig from "../../config.js"; import { sessionApiHost } from "../../lib/regional-host.js"; import { MAX_LOG_BYTES } from "../failurelogs-utils/utils.js"; +import JSZip from "jszip"; +import { + downloadTestCaseFile, + failedFirst, + findFrameworkBuild, + getFrameworkTestCases, + isBrowserStackUrl, +} from "../appautomate-utils/framework-builds.js"; +import { resolveAppAutomateBuildId } from "../failurelogs-utils/resolve-app-build-id.js"; export const MAX_SCREENSHOT_BYTES = globalConfig.REMOTE_MCP ? 10 * 1024 * 1024 @@ -172,7 +181,15 @@ export async function fetchAutomationScreenshots( sessionType: SessionType = SessionType.Automate, config: BrowserStackConfig, ) { - const urls = await extractScreenshotUrls(sessionId, sessionType, config); + let urls: string[]; + try { + urls = await extractScreenshotUrls(sessionId, sessionType, config); + } catch (error) { + if (sessionType !== SessionType.AppAutomate) throw error; + const framework = await frameworkScreenshots(sessionId, config); + if (!framework) throw error; + return framework; + } if (urls.length === 0) { return []; } @@ -181,3 +198,67 @@ export async function fetchAutomationScreenshots( const lastFiveUrls = urls.slice(-5); return await convertUrlsToBase64(lastFiveUrls); } + +const MAX_ZIP_BYTES = 10 * 1024 * 1024; +const MAX_IMAGE_BYTES = 10 * 1024 * 1024; +const IMAGE_FILE = /\.(png|jpe?g)$/i; + +/** + * Screenshots of a framework session (Maestro packs them per test as a zip), or + * undefined when the session is not part of a framework build. Frameworks without + * screenshots throw a message saying where the evidence is instead. + */ +async function frameworkScreenshots( + sessionId: string, + config: BrowserStackConfig, +): Promise { + const buildId = await resolveAppAutomateBuildId(sessionId, config); + if (!buildId) return undefined; + const build = await findFrameworkBuild(buildId, config); + if (!build) return undefined; + + const cases = (await getFrameworkTestCases(build, sessionId, config)) ?? []; + const withShots = failedFirst(cases).find((c) => + isBrowserStackUrl(c.fields.screenshots), + ); + if (!withShots) { + throw new Error( + `This ${build.framework.name} session has no screenshots. Use getFailureLogs ` + + "for its test logs, or the video log type for its recording.", + ); + } + + const file = await downloadTestCaseFile( + withShots.fields.screenshots as string, + config, + { binary: true, maxBytes: MAX_ZIP_BYTES }, + ); + if (file.tooLarge) { + throw new Error( + "The screenshots for this session are over 10 MB. Open the session in the BrowserStack dashboard to view them.", + ); + } + const zip = await JSZip.loadAsync(Buffer.from(file.data)); + const images = Object.values(zip.files) + .filter((entry) => !entry.dir && IMAGE_FILE.test(entry.name)) + .sort((a, b) => a.name.localeCompare(b.name, undefined, { numeric: true })) + .slice(-5); + + const screenshots: Screenshot[] = []; + for (const entry of images) { + // Checked before inflating, so a crafted archive cannot exhaust memory. + const declared = ( + entry as unknown as { _data?: { uncompressedSize?: number } } + )._data?.uncompressedSize; + if (typeof declared !== "number" || declared > MAX_IMAGE_BYTES) continue; + const buffer = await entry.async("nodebuffer"); + if (buffer.length > MAX_IMAGE_BYTES) continue; + const base64 = await maybeCompressBase64(buffer.toString("base64")); + screenshots.push({ + url: `${withShots.fields.screenshots as string}#${entry.name}`, + base64, + mimeType: detectImageMimeType(Buffer.from(base64, "base64")), + }); + } + return screenshots; +} diff --git a/src/tools/automate.ts b/src/tools/automate.ts index 54fda09f..ada8ab9c 100644 --- a/src/tools/automate.ts +++ b/src/tools/automate.ts @@ -15,6 +15,10 @@ import { resolveHashedBuildId, } from "./automate-utils/resolve-hashed-build-id.js"; import { SessionType } from "../lib/constants.js"; +import { + findFrameworkBuild, + frameworkSessions, +} from "./appautomate-utils/framework-builds.js"; import { trackMCP } from "../lib/instrumentation.js"; import logger from "../logger.js"; import { BrowserStackConfig } from "../lib/types.js"; @@ -160,6 +164,20 @@ export async function listSessionIdsTool( } } + // Framework builds (Maestro, Espresso, XCUITest, Flutter) are empty on the + // generic App Automate list; their own build endpoint has the sessions. + if (sessions.length === 0 && sessionType === SessionType.AppAutomate) { + const framework = await findFrameworkBuild(buildId, config); + if (framework) { + sessions = frameworkSessions(framework).filter( + (s) => !args.status || s.status === args.status, + ); + notes.push( + `This build ran through the ${framework.framework.name} framework API; sessions are listed from it.`, + ); + } + } + const content: CallToolResult["content"] = [ { type: "text", diff --git a/src/tools/failurelogs-utils/framework.ts b/src/tools/failurelogs-utils/framework.ts new file mode 100644 index 00000000..adbd232a --- /dev/null +++ b/src/tools/failurelogs-utils/framework.ts @@ -0,0 +1,99 @@ +import { AppAutomateLogType } from "../../lib/constants.js"; +import { BrowserStackConfig } from "../../lib/types.js"; +import { + downloadTestCaseFile, + failedFirst, + FrameworkBuild, + FrameworkTestCase, + getFrameworkTestCases, + isBrowserStackUrl, +} from "../appautomate-utils/framework-builds.js"; + +// Per test-case link field, by the log type the tool was asked for. +const LOG_FIELDS: Partial> = { + [AppAutomateLogType.AppiumLogs]: ["maestro_log", "instrumentation_log"], + [AppAutomateLogType.DeviceLogs]: ["device_log"], +}; + +const MAX_TESTS = 5; +const MAX_LOG_DOWNLOAD = 2 * 1024 * 1024; +// The end of a test log is where a failure shows up. +const TAIL_CHARS = 8_000; + +function label(test: FrameworkTestCase): string { + const name = test.className ? `${test.className} › ${test.name}` : test.name; + return `${name} (${test.status ?? "unknown"})`; +} + +function tail(text: string): string { + return text.length > TAIL_CHARS + ? `… (last ${TAIL_CHARS} characters)\n${text.slice(-TAIL_CHARS)}` + : text; +} + +/** + * Logs of one framework session for a requested log type, read from each test + * case's links. Failed tests come first; at most MAX_TESTS tests are read. + */ +export async function retrieveFrameworkLogs( + build: FrameworkBuild, + sessionId: string, + logType: AppAutomateLogType, + config: BrowserStackConfig, +): Promise { + const cases = await getFrameworkTestCases(build, sessionId, config); + const framework = build.framework.name; + if (!cases) { + return `Session ${sessionId} was not found in ${framework} build ${build.buildId}.`; + } + + if (logType === AppAutomateLogType.CrashLogs) { + const crashed = cases.filter( + (c) => + typeof c.fields.crash_logs_count === "number" && + c.fields.crash_logs_count > 0, + ); + return crashed.length + ? `Crashes recorded in this ${framework} session:\n` + + crashed + .map((c) => `- ${label(c)}: ${c.fields.crash_logs_count}`) + .join("\n") + + "\nOpen the session on the dashboard to read the crash reports." + : `No crashes recorded in this ${framework} session.`; + } + + const fields = LOG_FIELDS[logType] ?? []; + const withLog = failedFirst(cases).filter((c) => + fields.some((f) => isBrowserStackUrl(c.fields[f])), + ); + if (!withLog.length) { + return `This ${framework} session has no ${logType} for its tests. Available per test: ${ + [...new Set(cases.flatMap((c) => Object.keys(c.fields)))] + .filter((f) => /log|video|screenshots|commands/.test(f)) + .join(", ") || "none" + }.`; + } + + const parts: string[] = []; + for (const test of withLog.slice(0, MAX_TESTS)) { + const field = fields.find((f) => isBrowserStackUrl(test.fields[f]))!; + const file = await downloadTestCaseFile( + test.fields[field] as string, + config, + { + maxBytes: MAX_LOG_DOWNLOAD, + }, + ); + const text = file.tooLarge + ? "(log over 2 MB; open the session on the dashboard)" + : tail( + typeof file.data === "string" ? file.data : JSON.stringify(file.data), + ); + parts.push(`### ${label(test)} · ${field}\n${text}`); + } + const more = + withLog.length > MAX_TESTS + ? `\n\n${withLog.length - MAX_TESTS} more test(s) have this log; failed tests are shown first.` + : ""; + return `${framework} session ${sessionId}, ${logType}:\n\n${parts.join("\n\n")}${more}`; +} diff --git a/src/tools/get-failure-logs.ts b/src/tools/get-failure-logs.ts index 99cf47d6..f5ee3968 100644 --- a/src/tools/get-failure-logs.ts +++ b/src/tools/get-failure-logs.ts @@ -23,6 +23,11 @@ import { isTraBuildId, } from "./automate-utils/resolve-hashed-build-id.js"; import { retrieveSessionVideo } from "./failurelogs-utils/video.js"; +import { + findFrameworkBuild, + FrameworkBuild, +} from "./appautomate-utils/framework-builds.js"; +import { retrieveFrameworkLogs } from "./failurelogs-utils/framework.js"; import { AppAutomateLogType, @@ -152,6 +157,27 @@ export async function getFailureLogs( }; } + // Framework builds (Maestro, Espresso, XCUITest, Flutter) 404 on the generic App + // Automate log endpoints. Checked once, and only after a generic request fails, + // so Appium sessions make no extra calls. + let framework: FrameworkBuild | null | undefined; + const appLogs = async ( + logType: AppAutomateLogType, + generic: () => Promise, + ): Promise => { + try { + return await generic(); + } catch (error) { + if (framework === undefined) { + framework = buildId + ? ((await findFrameworkBuild(buildId, config)) ?? null) + : null; + } + if (!framework) throw error; + return retrieveFrameworkLogs(framework, args.sessionId, logType, config); + } + }; + let response; // eslint-disable-next-line no-useless-catch try { @@ -186,19 +212,25 @@ export async function getFailureLogs( } case AppAutomateLogType.DeviceLogs: { - response = await retrieveDeviceLogs(args.sessionId, buildId!, config); + response = await appLogs(logType, () => + retrieveDeviceLogs(args.sessionId, buildId!, config), + ); results.push({ type: "text", text: response }); break; } case AppAutomateLogType.AppiumLogs: { - response = await retrieveAppiumLogs(args.sessionId, buildId!, config); + response = await appLogs(logType, () => + retrieveAppiumLogs(args.sessionId, buildId!, config), + ); results.push({ type: "text", text: response }); break; } case AppAutomateLogType.CrashLogs: { - response = await retrieveCrashLogs(args.sessionId, buildId!, config); + response = await appLogs(logType, () => + retrieveCrashLogs(args.sessionId, buildId!, config), + ); results.push({ type: "text", text: response }); break; } @@ -208,6 +240,11 @@ export async function getFailureLogs( throw error; } + if (framework) { + notes.push( + `This session ran through the ${framework.framework.name} framework API, so its logs come from each test's results.`, + ); + } if (notes.length > 0) { results.unshift({ type: "text", text: notes.join("\n") }); } diff --git a/tests/tools/frameworkBuilds.test.ts b/tests/tools/frameworkBuilds.test.ts new file mode 100644 index 00000000..29e68566 --- /dev/null +++ b/tests/tools/frameworkBuilds.test.ts @@ -0,0 +1,253 @@ +import { describe, it, expect, vi, beforeEach, Mock } from "vitest"; +import JSZip from "jszip"; +import { apiClient } from "../../src/lib/apiClient"; +import { AppAutomateLogType, SessionType } from "../../src/lib/constants"; +import { + findFrameworkBuild, + frameworkSessions, + isBrowserStackUrl, +} from "../../src/tools/appautomate-utils/framework-builds"; +import { retrieveFrameworkLogs } from "../../src/tools/failurelogs-utils/framework"; +import { fetchAutomationScreenshots } from "../../src/tools/automate-utils/fetch-screenshots"; + +vi.mock("../../src/lib/apiClient", () => ({ apiClient: { get: vi.fn() } })); +vi.mock("../../src/lib/get-auth", () => ({ + getBrowserStackAuth: () => "user:key", +})); +vi.mock("../../src/logger", () => ({ + default: { warn: vi.fn(), error: vi.fn(), info: vi.fn(), debug: vi.fn() }, +})); +vi.mock("../../src/lib/image", () => ({ + maybeCompressBase64: async (b: string) => b, +})); +vi.mock("../../src/tools/failurelogs-utils/resolve-app-build-id", () => ({ + resolveAppAutomateBuildId: vi.fn(async () => BUILD), +})); + +const config = { + "browserstack-username": "user", + "browserstack-access-key": "key", +}; +const BUILD = "1b463b0f5e7bb27c29c4ffe8d7666d91451cc600"; +const SESSION = "d0dcef26a61f9f8fc8588f8b12c45f2a2c0dcf29"; +const ok = (data: unknown) => ({ ok: true, status: 200, data }); +const notFound = { ok: false, status: 404, statusText: "Not Found", data: {} }; + +// Shapes recorded from live Maestro / XCUITest builds. +const maestroBuild = { + id: BUILD, + framework: "maestro", + devices: [ + { + device: "Google Pixel 8", + os: "android", + os_version: "14.0", + sessions: [{ id: SESSION, status: "failed" }], + }, + ], +}; +const maestroSession = { + id: SESSION, + testcases: { + data: [ + { + class: "flows", + testcases: [ + { + name: "dice-roller", + status: "passed", + maestro_log: "https://api.browserstack.com/logs/pass.txt", + screenshots: "https://api.browserstack.com/shots/pass.zip", + }, + { + name: "random-character", + status: "failed", + crash_logs_count: 1, + maestro_log: "https://api.browserstack.com/logs/fail.txt", + screenshots: "https://api.browserstack.com/shots/fail.zip", + }, + ], + }, + ], + }, +}; + +/** Route by URL: only the Maestro framework knows the build. */ +function routeMaestro(extra: (url: string) => unknown = () => undefined) { + (apiClient.get as Mock).mockImplementation(async ({ url }) => { + const answer = extra(url); + if (answer) return answer; + if (url.endsWith(`/maestro/v2/builds/${BUILD}`)) return ok(maestroBuild); + if (url.endsWith(`/maestro/v2/builds/${BUILD}/sessions/${SESSION}`)) + return ok(maestroSession); + if (url.includes("/v2/")) return notFound; + throw new Error(`unexpected url ${url}`); + }); +} + +describe("findFrameworkBuild", () => { + beforeEach(() => vi.resetAllMocks()); + + it("tries each framework once and stops at the one that knows the build", async () => { + (apiClient.get as Mock).mockImplementation(async ({ url }) => + url.includes("/xcuitest/v2/") ? ok({ id: BUILD, devices: [] }) : notFound, + ); + + const build = await findFrameworkBuild(BUILD, config); + + expect(build?.framework.name).toBe("xcuitest"); + expect((apiClient.get as Mock).mock.calls.map(([r]) => r.url)).toEqual([ + `https://api-cloud.browserstack.com/app-automate/maestro/v2/builds/${BUILD}`, + `https://api-cloud.browserstack.com/app-automate/espresso/v2/builds/${BUILD}`, + `https://api-cloud.browserstack.com/app-automate/xcuitest/v2/builds/${BUILD}`, + ]); + }); + + it("returns undefined after five 404s, for an ordinary Appium build", async () => { + (apiClient.get as Mock).mockResolvedValue(notFound); + + await expect(findFrameworkBuild(BUILD, config)).resolves.toBeUndefined(); + expect(apiClient.get).toHaveBeenCalledTimes(5); + }); + + it("surfaces a server error instead of reading it as 'not this framework'", async () => { + (apiClient.get as Mock).mockResolvedValue({ + ok: false, + status: 503, + statusText: "Service Unavailable", + }); + + await expect(findFrameworkBuild(BUILD, config)).rejects.toThrow(/503/); + }); +}); + +describe("frameworkSessions", () => { + it("flattens devices and sessions into session records", () => { + expect( + frameworkSessions({ + framework: { name: "maestro", path: "maestro/v2" }, + buildId: BUILD, + data: maestroBuild, + }), + ).toEqual([ + { + sessionId: SESSION, + status: "failed", + os: "android", + osVersion: "14.0", + device: "Google Pixel 8", + browserUrl: `https://app-automate.browserstack.com/dashboard/v2/builds/${BUILD}/sessions/${SESSION}`, + }, + ]); + }); +}); + +describe("isBrowserStackUrl", () => { + it("allows only https BrowserStack hosts", () => { + expect(isBrowserStackUrl("https://api.browserstack.com/x")).toBe(true); + expect(isBrowserStackUrl("https://browserstack.com.evil.io/x")).toBe(false); + expect(isBrowserStackUrl("http://api.browserstack.com/x")).toBe(false); + expect(isBrowserStackUrl(42)).toBe(false); + }); +}); + +describe("retrieveFrameworkLogs", () => { + beforeEach(() => vi.resetAllMocks()); + const build = { + framework: { name: "maestro", path: "maestro/v2" } as const, + buildId: BUILD, + data: maestroBuild, + }; + + it("returns the failed test's log first, from its link", async () => { + routeMaestro((url) => + url.endsWith("fail.txt") + ? ok("step 1\nAssertion failed: dice not visible") + : url.endsWith("pass.txt") + ? ok("all good") + : undefined, + ); + + const text = await retrieveFrameworkLogs( + build, + SESSION, + AppAutomateLogType.AppiumLogs, + config, + ); + + expect(text.indexOf("random-character (failed)")).toBeLessThan( + text.indexOf("dice-roller (passed)"), + ); + expect(text).toContain("Assertion failed: dice not visible"); + const fetched = (apiClient.get as Mock).mock.calls.find(([r]) => + r.url.endsWith("fail.txt"), + )![0]; + expect(fetched.headers.Authorization).toMatch(/^Basic /); + }); + + it("reports crash counts without downloading anything", async () => { + routeMaestro(); + + const text = await retrieveFrameworkLogs( + build, + SESSION, + AppAutomateLogType.CrashLogs, + config, + ); + + expect(text).toContain("random-character (failed): 1"); + }); + + it("says which logs exist when the requested one does not", async () => { + routeMaestro(); + + const text = await retrieveFrameworkLogs( + build, + SESSION, + AppAutomateLogType.DeviceLogs, + config, + ); + + expect(text).toMatch(/no deviceLogs[\s\S]*maestro_log, screenshots/); + }); +}); + +describe("fetchAutomationScreenshots on a Maestro session", () => { + beforeEach(() => vi.resetAllMocks()); + + it("reads the failed test's screenshot zip when the generic logs 404", async () => { + const zip = new JSZip(); + const png = Buffer.from([0x89, 0x50, 0x4e, 0x47, 1, 2, 3]); + zip.file("screenshot-1.png", png); + zip.file("screenshot-2.png", png); + zip.file("notes.txt", "not an image"); + const zipped = await zip.generateAsync({ type: "nodebuffer" }); + + routeMaestro((url) => { + if (url.endsWith(`/app-automate/sessions/${SESSION}/logs`)) + return notFound; + if (url.endsWith("fail.zip")) return ok(zipped); + return undefined; + }); + + const shots = await fetchAutomationScreenshots( + SESSION, + SessionType.AppAutomate, + config, + ); + + expect(shots).toHaveLength(2); + expect(shots[0].url).toBe( + "https://api.browserstack.com/shots/fail.zip#screenshot-1.png", + ); + expect(shots[0].mimeType).toBe("image/png"); + }); + + it("keeps the original error for an Appium session", async () => { + (apiClient.get as Mock).mockResolvedValue(notFound); + + await expect( + fetchAutomationScreenshots(SESSION, SessionType.AppAutomate, config), + ).rejects.toThrow(/Invalid session ID/); + }); +}); From 730f42794ecc1ce0bfd86d0d80fc6f07d7ce37d3 Mon Sep 17 00:00:00 2001 From: Savio Dias Date: Tue, 10 Nov 2026 17:12:22 +0530 Subject: [PATCH 2/5] fix(appautomate): fall back to framework logs only on a 404 The App Automate log helpers return a "not available" message on 404. They now throw LogsNotFoundError when asked (getFailureLogs does), so the framework fallback runs only on a real 404 and Appium sessions keep their existing output. Adds tool-level tests for listSessions and getFailureLogs. Co-Authored-By: Claude Opus 5.5 --- src/tools/failurelogs-utils/app-automate.ts | 37 ++++++++++- src/tools/get-failure-logs.ts | 21 +++++-- tests/tools/frameworkBuilds.test.ts | 70 +++++++++++++++++++++ tests/tools/getFailureLogs.test.ts | 16 ++--- 4 files changed, 128 insertions(+), 16 deletions(-) diff --git a/src/tools/failurelogs-utils/app-automate.ts b/src/tools/failurelogs-utils/app-automate.ts index c9aa2750..3f2266ac 100644 --- a/src/tools/failurelogs-utils/app-automate.ts +++ b/src/tools/failurelogs-utils/app-automate.ts @@ -9,11 +9,25 @@ import { apiClient } from "../../lib/apiClient.js"; import { wrapUntrusted } from "../../lib/untrusted-content.js"; import { getRegionalHost } from "../../lib/regional-host.js"; +/** The session has no such log here (404). Opt-in, so the caller can try elsewhere. */ +export class LogsNotFoundError extends Error { + constructor(message: string) { + super(message); + this.name = "LogsNotFoundError"; + } +} + +export interface AppLogOptions { + /** Throw LogsNotFoundError on 404 instead of returning the "not available" text. */ + throwOnNotFound?: boolean; +} + // DEVICE LOGS export async function retrieveDeviceLogs( sessionId: string, buildId: string, config: BrowserStackConfig, + options: AppLogOptions = {}, ): Promise { const url = `${getRegionalHost("appAutomateApi")}/app-automate/builds/${buildId}/sessions/${sessionId}/devicelogs`; const authString = getBrowserStackAuth(config); @@ -30,7 +44,12 @@ export async function retrieveDeviceLogs( }); const validationError = validateLogResponse(response, "device logs"); - if (validationError) return validationError.message!; + if (validationError) { + if (response.status === 404 && options.throwOnNotFound) { + throw new LogsNotFoundError(validationError.message!); + } + return validationError.message!; + } const logText = typeof response.data === "string" @@ -47,6 +66,7 @@ export async function retrieveAppiumLogs( sessionId: string, buildId: string, config: BrowserStackConfig, + options: AppLogOptions = {}, ): Promise { const url = `${getRegionalHost("appAutomateApi")}/app-automate/builds/${buildId}/sessions/${sessionId}/appiumlogs`; const authString = getBrowserStackAuth(config); @@ -63,7 +83,12 @@ export async function retrieveAppiumLogs( }); const validationError = validateLogResponse(response, "Appium logs"); - if (validationError) return validationError.message!; + if (validationError) { + if (response.status === 404 && options.throwOnNotFound) { + throw new LogsNotFoundError(validationError.message!); + } + return validationError.message!; + } const logText = typeof response.data === "string" @@ -80,6 +105,7 @@ export async function retrieveCrashLogs( sessionId: string, buildId: string, config: BrowserStackConfig, + options: AppLogOptions = {}, ): Promise { const url = `${getRegionalHost("appAutomateApi")}/app-automate/builds/${buildId}/sessions/${sessionId}/crashlogs`; const authString = getBrowserStackAuth(config); @@ -96,7 +122,12 @@ export async function retrieveCrashLogs( }); const validationError = validateLogResponse(response, "crash logs"); - if (validationError) return validationError.message!; + if (validationError) { + if (response.status === 404 && options.throwOnNotFound) { + throw new LogsNotFoundError(validationError.message!); + } + return validationError.message!; + } const logText = typeof response.data === "string" diff --git a/src/tools/get-failure-logs.ts b/src/tools/get-failure-logs.ts index f5ee3968..8745a4e8 100644 --- a/src/tools/get-failure-logs.ts +++ b/src/tools/get-failure-logs.ts @@ -12,6 +12,7 @@ import { } from "./failurelogs-utils/automate.js"; import { + LogsNotFoundError, retrieveDeviceLogs, retrieveAppiumLogs, retrieveCrashLogs, @@ -158,8 +159,8 @@ export async function getFailureLogs( } // Framework builds (Maestro, Espresso, XCUITest, Flutter) 404 on the generic App - // Automate log endpoints. Checked once, and only after a generic request fails, - // so Appium sessions make no extra calls. + // Automate log endpoints. Checked once, and only after a generic 404, so Appium + // sessions with logs make no extra calls. let framework: FrameworkBuild | null | undefined; const appLogs = async ( logType: AppAutomateLogType, @@ -168,12 +169,14 @@ export async function getFailureLogs( try { return await generic(); } catch (error) { + if (!(error instanceof LogsNotFoundError)) throw error; if (framework === undefined) { framework = buildId ? ((await findFrameworkBuild(buildId, config)) ?? null) : null; } - if (!framework) throw error; + // Not a framework build either: the same "not available" text as before. + if (!framework) return error.message; return retrieveFrameworkLogs(framework, args.sessionId, logType, config); } }; @@ -213,7 +216,9 @@ export async function getFailureLogs( case AppAutomateLogType.DeviceLogs: { response = await appLogs(logType, () => - retrieveDeviceLogs(args.sessionId, buildId!, config), + retrieveDeviceLogs(args.sessionId, buildId!, config, { + throwOnNotFound: true, + }), ); results.push({ type: "text", text: response }); break; @@ -221,7 +226,9 @@ export async function getFailureLogs( case AppAutomateLogType.AppiumLogs: { response = await appLogs(logType, () => - retrieveAppiumLogs(args.sessionId, buildId!, config), + retrieveAppiumLogs(args.sessionId, buildId!, config, { + throwOnNotFound: true, + }), ); results.push({ type: "text", text: response }); break; @@ -229,7 +236,9 @@ export async function getFailureLogs( case AppAutomateLogType.CrashLogs: { response = await appLogs(logType, () => - retrieveCrashLogs(args.sessionId, buildId!, config), + retrieveCrashLogs(args.sessionId, buildId!, config, { + throwOnNotFound: true, + }), ); results.push({ type: "text", text: response }); break; diff --git a/tests/tools/frameworkBuilds.test.ts b/tests/tools/frameworkBuilds.test.ts index 29e68566..e151b3a0 100644 --- a/tests/tools/frameworkBuilds.test.ts +++ b/tests/tools/frameworkBuilds.test.ts @@ -9,11 +9,14 @@ import { } from "../../src/tools/appautomate-utils/framework-builds"; import { retrieveFrameworkLogs } from "../../src/tools/failurelogs-utils/framework"; import { fetchAutomationScreenshots } from "../../src/tools/automate-utils/fetch-screenshots"; +import { listSessionIdsTool } from "../../src/tools/automate"; +import { getFailureLogs } from "../../src/tools/get-failure-logs"; vi.mock("../../src/lib/apiClient", () => ({ apiClient: { get: vi.fn() } })); vi.mock("../../src/lib/get-auth", () => ({ getBrowserStackAuth: () => "user:key", })); +vi.mock("../../src/lib/instrumentation", () => ({ trackMCP: vi.fn() })); vi.mock("../../src/logger", () => ({ default: { warn: vi.fn(), error: vi.fn(), info: vi.fn(), debug: vi.fn() }, })); @@ -251,3 +254,70 @@ describe("fetchAutomationScreenshots on a Maestro session", () => { ).rejects.toThrow(/Invalid session ID/); }); }); + +describe("tools on a framework build", () => { + beforeEach(() => vi.resetAllMocks()); + + it("listSessions lists a Maestro build's sessions when the generic list is empty", async () => { + routeMaestro((url) => + url.endsWith(`/app-automate/builds/${BUILD}/sessions.json`) + ? ok([]) + : undefined, + ); + + const result = await listSessionIdsTool( + { sessionType: SessionType.AppAutomate, buildId: BUILD }, + config, + ); + + const sessions = JSON.parse(result.content[0].text as string); + expect(sessions[0]).toMatchObject({ sessionId: SESSION, status: "failed" }); + expect(result.content.map((c: any) => c.text).join("\n")).toContain( + "ran through the maestro framework API", + ); + }); + + it("getFailureLogs falls back to the framework session when generic logs 404", async () => { + routeMaestro((url) => { + if (url.endsWith("/appiumlogs")) return notFound; + if (url.endsWith("fail.txt")) + return ok("Assertion failed: dice not visible"); + if (url.endsWith("pass.txt")) return ok("ok"); + return undefined; + }); + + const result = await getFailureLogs( + { + sessionId: SESSION, + sessionType: SessionType.AppAutomate, + logTypes: [AppAutomateLogType.AppiumLogs], + }, + config, + ); + + const text = result.content.map((c: any) => c.text).join("\n"); + expect(result.isError).toBeFalsy(); + expect(result.content[0].text).toContain("maestro framework API"); + expect(text).toContain("Assertion failed: dice not visible"); + }); + + it("getFailureLogs makes no framework calls for an Appium session", async () => { + (apiClient.get as Mock).mockImplementation(async ({ url }) => { + if (url.endsWith("/appiumlogs")) return ok("appium log line"); + throw new Error(`unexpected url ${url}`); + }); + + await getFailureLogs( + { + sessionId: SESSION, + sessionType: SessionType.AppAutomate, + logTypes: [AppAutomateLogType.AppiumLogs], + }, + config, + ); + + expect( + (apiClient.get as Mock).mock.calls.some(([r]) => r.url.includes("/v2/")), + ).toBe(false); + }); +}); diff --git a/tests/tools/getFailureLogs.test.ts b/tests/tools/getFailureLogs.test.ts index 57d32158..adb9e19a 100644 --- a/tests/tools/getFailureLogs.test.ts +++ b/tests/tools/getFailureLogs.test.ts @@ -132,7 +132,7 @@ describe('BrowserStack Failure Logs', () => { sessionType: 'app-automate' }, mockServer); - expect(appAutomate.retrieveDeviceLogs).toHaveBeenCalledWith('test-session', 'hashed-app-build', mockServer); + expect(appAutomate.retrieveDeviceLogs).toHaveBeenCalledWith('test-session', 'hashed-app-build', mockServer, { throwOnNotFound: true }); }); it('should resolve buildId from the session when not provided', async () => { @@ -151,6 +151,7 @@ describe('BrowserStack Failure Logs', () => { 'test-session', 'resolved-build-id', mockServer, + { throwOnNotFound: true }, ); expect(result.isError).toBeFalsy(); }); @@ -171,6 +172,7 @@ describe('BrowserStack Failure Logs', () => { 'test-session', 'explicit-build-id', mockServer, + { throwOnNotFound: true }, ); }); @@ -201,7 +203,7 @@ describe('BrowserStack Failure Logs', () => { }, mockServer); expect(result.isError).toBeFalsy(); - expect(appAutomate.retrieveDeviceLogs).toHaveBeenCalledWith('test-session', 'app-build', mockServer); + expect(appAutomate.retrieveDeviceLogs).toHaveBeenCalledWith('test-session', 'app-build', mockServer, { throwOnNotFound: true }); }); it('fetches appiumLogs for sessionLogs and reports networkLogs as unavailable on an app session', async () => { @@ -217,7 +219,7 @@ describe('BrowserStack Failure Logs', () => { }, mockServer); expect(result.isError).toBeFalsy(); - expect(appAutomate.retrieveAppiumLogs).toHaveBeenCalledWith('test-session', 'app-build', mockServer); + expect(appAutomate.retrieveAppiumLogs).toHaveBeenCalledWith('test-session', 'app-build', mockServer, { throwOnNotFound: true }); expect(appAutomate.retrieveDeviceLogs).not.toHaveBeenCalled(); expect(automate.retrieveSessionFailures).not.toHaveBeenCalled(); const texts = result.content.map((c: any) => c.text).join('\n'); @@ -329,7 +331,7 @@ describe('BrowserStack Failure Logs', () => { sessionType: 'app-automate' }, mockServer); - expect(appAutomate.retrieveDeviceLogs).toHaveBeenCalledWith(mockSessionId, mockBuildId, expect.anything()); + expect(appAutomate.retrieveDeviceLogs).toHaveBeenCalledWith(mockSessionId, mockBuildId, expect.anything(), { throwOnNotFound: true }); if (result.content?.[0]?.type === 'text') { expect(result.content[0].text).toContain('Device Failures (1 found)'); expect(result.content[0].text).toContain('Fatal Exception'); @@ -345,7 +347,7 @@ describe('BrowserStack Failure Logs', () => { sessionType: 'app-automate' }, mockServer); - expect(appAutomate.retrieveAppiumLogs).toHaveBeenCalledWith(mockSessionId, mockBuildId, expect.anything()); + expect(appAutomate.retrieveAppiumLogs).toHaveBeenCalledWith(mockSessionId, mockBuildId, expect.anything(), { throwOnNotFound: true }); if (result.content?.[0]?.type === 'text') { expect(result.content[0].text).toContain('Appium Failures (1 found)'); expect(result.content[0].text).toContain('Element not found'); @@ -361,7 +363,7 @@ describe('BrowserStack Failure Logs', () => { sessionType: 'app-automate' }, mockServer); - expect(appAutomate.retrieveCrashLogs).toHaveBeenCalledWith(mockSessionId, mockBuildId, expect.anything()); + expect(appAutomate.retrieveCrashLogs).toHaveBeenCalledWith(mockSessionId, mockBuildId, expect.anything(), { throwOnNotFound: true }); if (result.content?.[0]?.type === 'text') { expect(result.content[0].text).toContain('Crash Failures (1 found)'); expect(result.content[0].text).toContain('signal 11'); @@ -503,7 +505,7 @@ console.error('Uncaught TypeError') }, mockServer); expect(resolveAppAutomateBuildId).toHaveBeenCalledTimes(1); - expect(appAutomate.retrieveAppiumLogs).toHaveBeenCalledWith('sess-3', 'resolved-build', mockServer); + expect(appAutomate.retrieveAppiumLogs).toHaveBeenCalledWith('sess-3', 'resolved-build', mockServer, { throwOnNotFound: true }); expect(retrieveSessionVideo).toHaveBeenCalledWith('sess-3', 'app-automate', mockServer); expect(result.content.map((c: any) => c.text)).toEqual([ 'Appium Failures (1 found)', From e86fcece30f7b80d1bc43a93682ea05f89d35481 Mon Sep 17 00:00:00 2001 From: Savio Dias Date: Tue, 10 Nov 2026 17:13:04 +0530 Subject: [PATCH 3/5] test: use made-up ids and names in framework fixtures Co-Authored-By: Claude Opus 5.5 --- tests/tools/frameworkBuilds.test.ts | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/tests/tools/frameworkBuilds.test.ts b/tests/tools/frameworkBuilds.test.ts index e151b3a0..a8af7650 100644 --- a/tests/tools/frameworkBuilds.test.ts +++ b/tests/tools/frameworkBuilds.test.ts @@ -31,8 +31,8 @@ const config = { "browserstack-username": "user", "browserstack-access-key": "key", }; -const BUILD = "1b463b0f5e7bb27c29c4ffe8d7666d91451cc600"; -const SESSION = "d0dcef26a61f9f8fc8588f8b12c45f2a2c0dcf29"; +const BUILD = "f00d000000000000000000000000000000000001"; +const SESSION = "5e55000000000000000000000000000000000001"; const ok = (data: unknown) => ({ ok: true, status: 200, data }); const notFound = { ok: false, status: 404, statusText: "Not Found", data: {} }; @@ -57,13 +57,13 @@ const maestroSession = { class: "flows", testcases: [ { - name: "dice-roller", + name: "login-flow", status: "passed", maestro_log: "https://api.browserstack.com/logs/pass.txt", screenshots: "https://api.browserstack.com/shots/pass.zip", }, { - name: "random-character", + name: "checkout-flow", status: "failed", crash_logs_count: 1, maestro_log: "https://api.browserstack.com/logs/fail.txt", @@ -165,7 +165,7 @@ describe("retrieveFrameworkLogs", () => { it("returns the failed test's log first, from its link", async () => { routeMaestro((url) => url.endsWith("fail.txt") - ? ok("step 1\nAssertion failed: dice not visible") + ? ok("step 1\nAssertion failed: button not visible") : url.endsWith("pass.txt") ? ok("all good") : undefined, @@ -178,10 +178,10 @@ describe("retrieveFrameworkLogs", () => { config, ); - expect(text.indexOf("random-character (failed)")).toBeLessThan( - text.indexOf("dice-roller (passed)"), + expect(text.indexOf("checkout-flow (failed)")).toBeLessThan( + text.indexOf("login-flow (passed)"), ); - expect(text).toContain("Assertion failed: dice not visible"); + expect(text).toContain("Assertion failed: button not visible"); const fetched = (apiClient.get as Mock).mock.calls.find(([r]) => r.url.endsWith("fail.txt"), )![0]; @@ -198,7 +198,7 @@ describe("retrieveFrameworkLogs", () => { config, ); - expect(text).toContain("random-character (failed): 1"); + expect(text).toContain("checkout-flow (failed): 1"); }); it("says which logs exist when the requested one does not", async () => { @@ -281,7 +281,7 @@ describe("tools on a framework build", () => { routeMaestro((url) => { if (url.endsWith("/appiumlogs")) return notFound; if (url.endsWith("fail.txt")) - return ok("Assertion failed: dice not visible"); + return ok("Assertion failed: button not visible"); if (url.endsWith("pass.txt")) return ok("ok"); return undefined; }); @@ -298,7 +298,7 @@ describe("tools on a framework build", () => { const text = result.content.map((c: any) => c.text).join("\n"); expect(result.isError).toBeFalsy(); expect(result.content[0].text).toContain("maestro framework API"); - expect(text).toContain("Assertion failed: dice not visible"); + expect(text).toContain("Assertion failed: button not visible"); }); it("getFailureLogs makes no framework calls for an Appium session", async () => { From dff5c7a1149a12c341295d413ed4e733dbda6fb4 Mon Sep 17 00:00:00 2001 From: Savio Dias Date: Tue, 10 Nov 2026 18:00:25 +0530 Subject: [PATCH 4/5] fix(appautomate): hard memory limits for Maestro screenshot archives Inflate each image as a stream and stop at 10 MB whatever the archive header claims, cap the images read per call at 25 MB in total, process them one at a time, and refuse archives with more than 500 files. Co-Authored-By: Claude Opus 5.5 --- src/tools/automate-utils/fetch-screenshots.ts | 79 ++++++++++++++++--- tests/tools/frameworkBuilds.test.ts | 69 +++++++++++++++- 2 files changed, 137 insertions(+), 11 deletions(-) diff --git a/src/tools/automate-utils/fetch-screenshots.ts b/src/tools/automate-utils/fetch-screenshots.ts index 72a054a8..c9a26d44 100644 --- a/src/tools/automate-utils/fetch-screenshots.ts +++ b/src/tools/automate-utils/fetch-screenshots.ts @@ -199,10 +199,56 @@ export async function fetchAutomationScreenshots( return await convertUrlsToBase64(lastFiveUrls); } -const MAX_ZIP_BYTES = 10 * 1024 * 1024; -const MAX_IMAGE_BYTES = 10 * 1024 * 1024; +// Limits for Maestro screenshot archives, so one call cannot exhaust a pod's memory. +export const MAX_ZIP_BYTES = 10 * 1024 * 1024; +export const MAX_ZIP_ENTRIES = 500; +export const MAX_IMAGE_BYTES = 10 * 1024 * 1024; +export const MAX_TOTAL_IMAGE_BYTES = 25 * 1024 * 1024; const IMAGE_FILE = /\.(png|jpe?g)$/i; +/** + * Inflate one archive entry, stopping as soon as it passes `maxBytes`. The size + * in the archive header is not trusted: a crafted entry can claim to be small. + */ +export function readEntryCapped( + entry: JSZip.JSZipObject, + maxBytes: number, +): Promise { + return new Promise((resolve, reject) => { + const chunks: Buffer[] = []; + let total = 0; + let settled = false; + const stream = entry.nodeStream("nodebuffer") as NodeJS.ReadableStream & { + destroy?: () => void; + }; + stream.on("data", (chunk: Buffer) => { + if (settled) return; + total += chunk.length; + if (total > maxBytes) { + settled = true; + chunks.length = 0; + stream.pause(); + stream.destroy?.(); + resolve(null); + return; + } + chunks.push(chunk); + }); + stream.on("error", (error: Error) => { + if (!settled) { + settled = true; + reject(error); + } + }); + stream.on("end", () => { + if (!settled) { + settled = true; + resolve(Buffer.concat(chunks)); + } + }); + }); +} + /** * Screenshots of a framework session (Maestro packs them per test as a zip), or * undefined when the session is not part of a framework build. Frameworks without @@ -239,20 +285,28 @@ async function frameworkScreenshots( ); } const zip = await JSZip.loadAsync(Buffer.from(file.data)); - const images = Object.values(zip.files) + const entries = Object.values(zip.files); + if (entries.length > MAX_ZIP_ENTRIES) { + throw new Error( + "This session's screenshot archive has too many files to read here. Open the session in the BrowserStack dashboard to view them.", + ); + } + const images = entries .filter((entry) => !entry.dir && IMAGE_FILE.test(entry.name)) .sort((a, b) => a.name.localeCompare(b.name, undefined, { numeric: true })) .slice(-5); const screenshots: Screenshot[] = []; + let budget = MAX_TOTAL_IMAGE_BYTES; + // One image at a time, newest last, so at most one inflated image is held. for (const entry of images) { - // Checked before inflating, so a crafted archive cannot exhaust memory. - const declared = ( - entry as unknown as { _data?: { uncompressedSize?: number } } - )._data?.uncompressedSize; - if (typeof declared !== "number" || declared > MAX_IMAGE_BYTES) continue; - const buffer = await entry.async("nodebuffer"); - if (buffer.length > MAX_IMAGE_BYTES) continue; + if (budget <= 0) break; + const buffer = await readEntryCapped( + entry, + Math.min(MAX_IMAGE_BYTES, budget), + ); + if (!buffer) continue; + budget -= buffer.length; const base64 = await maybeCompressBase64(buffer.toString("base64")); screenshots.push({ url: `${withShots.fields.screenshots as string}#${entry.name}`, @@ -260,5 +314,10 @@ async function frameworkScreenshots( mimeType: detectImageMimeType(Buffer.from(base64, "base64")), }); } + if (images.length > 0 && screenshots.length === 0) { + throw new Error( + "The screenshots in this session are too large (over 10 MB each) to fetch here. Open the session in the BrowserStack dashboard to view them.", + ); + } return screenshots; } diff --git a/tests/tools/frameworkBuilds.test.ts b/tests/tools/frameworkBuilds.test.ts index a8af7650..0fb7f24b 100644 --- a/tests/tools/frameworkBuilds.test.ts +++ b/tests/tools/frameworkBuilds.test.ts @@ -8,7 +8,13 @@ import { isBrowserStackUrl, } from "../../src/tools/appautomate-utils/framework-builds"; import { retrieveFrameworkLogs } from "../../src/tools/failurelogs-utils/framework"; -import { fetchAutomationScreenshots } from "../../src/tools/automate-utils/fetch-screenshots"; +import { + fetchAutomationScreenshots, + MAX_IMAGE_BYTES, + MAX_TOTAL_IMAGE_BYTES, + MAX_ZIP_ENTRIES, + readEntryCapped, +} from "../../src/tools/automate-utils/fetch-screenshots"; import { listSessionIdsTool } from "../../src/tools/automate"; import { getFailureLogs } from "../../src/tools/get-failure-logs"; @@ -321,3 +327,64 @@ describe("tools on a framework build", () => { ).toBe(false); }); }); + +describe("Maestro screenshot memory limits", () => { + beforeEach(() => vi.resetAllMocks()); + + async function shotsFrom(zip: JSZip) { + const zipped = await zip.generateAsync({ + type: "nodebuffer", + compression: "DEFLATE", + }); + routeMaestro((url) => { + if (url.endsWith(`/app-automate/sessions/${SESSION}/logs`)) + return notFound; + if (url.endsWith("fail.zip")) return ok(zipped); + return undefined; + }); + return fetchAutomationScreenshots(SESSION, SessionType.AppAutomate, config); + } + + it("stops inflating an entry at the cap, whatever its compressed size", async () => { + const zip = new JSZip(); + // 3 MB of zeros compresses to a few KB: the real size must be what counts. + zip.file("big.png", Buffer.alloc(3 * 1024 * 1024)); + const loaded = await JSZip.loadAsync( + await zip.generateAsync({ type: "nodebuffer", compression: "DEFLATE" }), + ); + const entry = loaded.file("big.png")!; + + await expect(readEntryCapped(entry, 1024 * 1024)).resolves.toBeNull(); + await expect(readEntryCapped(entry, 4 * 1024 * 1024)).resolves.toHaveLength( + 3 * 1024 * 1024, + ); + }); + + it("skips images over the per-image limit", async () => { + const zip = new JSZip(); + zip.file("1.png", Buffer.from([0x89, 0x50, 0x4e, 0x47])); + zip.file("2.png", Buffer.alloc(MAX_IMAGE_BYTES + 1)); + + const shots = await shotsFrom(zip); + + expect(shots.map((s) => s.url.split("#")[1])).toEqual(["1.png"]); + }); + + it("stops at the total budget across images", async () => { + const zip = new JSZip(); + const size = Math.floor(MAX_TOTAL_IMAGE_BYTES / 3) + 1; + for (let i = 1; i <= 4; i++) zip.file(`${i}.png`, Buffer.alloc(size)); + + const shots = await shotsFrom(zip); + + expect(shots.length).toBeLessThan(4); + expect(shots.length).toBeGreaterThan(0); + }); + + it("refuses an archive with too many files", async () => { + const zip = new JSZip(); + for (let i = 0; i <= MAX_ZIP_ENTRIES; i++) zip.file(`${i}.png`, "x"); + + await expect(shotsFrom(zip)).rejects.toThrow(/too many files/); + }); +}); From fb1d0c8fa4d55f001aafabdf4135e8798ac93576 Mon Sep 17 00:00:00 2001 From: Savio Dias Date: Tue, 10 Nov 2026 18:36:42 +0530 Subject: [PATCH 5/5] fix(appautomate): address review on framework builds - Wrap framework test logs and test names as untrusted content, and cover the path in the injection test. - Look up the five framework endpoints in parallel. A failed lookup counts as "not this framework", so it never turns an empty session list or a "no logs" reply into a tool error. - Fall back to Maestro screenshots only after a 404, and keep the original error if the framework check fails. - Apply limit, offset and a case-insensitive status filter to sessions listed from a framework build. Co-Authored-By: Claude Opus 5.5 --- .../appautomate-utils/framework-builds.ts | 28 ++++- src/tools/automate-utils/fetch-screenshots.ts | 38 +++++-- src/tools/automate.ts | 8 +- src/tools/failurelogs-utils/framework.ts | 16 ++- tests/manual/injection-e2e.test.ts | 40 +++++++ tests/tools/frameworkBuilds.test.ts | 106 ++++++++++++++++-- 6 files changed, 208 insertions(+), 28 deletions(-) diff --git a/src/tools/appautomate-utils/framework-builds.ts b/src/tools/appautomate-utils/framework-builds.ts index a448ac1c..b84b99c9 100644 --- a/src/tools/appautomate-utils/framework-builds.ts +++ b/src/tools/appautomate-utils/framework-builds.ts @@ -4,6 +4,7 @@ import { getBrowserStackAuth } from "../../lib/get-auth.js"; import { getRegionalHost } from "../../lib/regional-host.js"; import { BrowserStackConfig } from "../../lib/types.js"; import { SessionIdRecord } from "../automate-utils/list-session-ids.js"; +import logger from "../../logger.js"; /** * App Automate builds run through a framework API (Maestro, Espresso, XCUITest, @@ -64,17 +65,34 @@ async function getJson(url: string, config: BrowserStackConfig) { /** * The framework build with this id, or undefined if none has it. Each framework's - * build endpoint returns 404 for another framework's build, so this is at most - * one call per framework. + * build endpoint returns 404 for another framework's build, so the five lookups + * run in parallel and at most one answers. Used only as a fallback after an empty + * or 404 answer, so a failed lookup (401, 403, 5xx) is treated as "not this + * framework" rather than replacing the caller's original reply with an error. */ export async function findFrameworkBuild( buildId: string, config: BrowserStackConfig, ): Promise { const id = encodeURIComponent(buildId.trim()); - for (const framework of FRAMEWORKS) { - const data = await getJson(frameworkUrl(framework, `builds/${id}`), config); - if (data) return { framework, buildId: buildId.trim(), data }; + const results = await Promise.allSettled( + FRAMEWORKS.map((framework) => + getJson(frameworkUrl(framework, `builds/${id}`), config), + ), + ); + for (const [index, result] of results.entries()) { + if (result.status === "fulfilled" && result.value) { + return { + framework: FRAMEWORKS[index], + buildId: buildId.trim(), + data: result.value, + }; + } + if (result.status === "rejected") { + logger.debug( + `framework lookup failed for ${FRAMEWORKS[index].name}: ${result.reason}`, + ); + } } return undefined; } diff --git a/src/tools/automate-utils/fetch-screenshots.ts b/src/tools/automate-utils/fetch-screenshots.ts index c9a26d44..f65d33f4 100644 --- a/src/tools/automate-utils/fetch-screenshots.ts +++ b/src/tools/automate-utils/fetch-screenshots.ts @@ -16,6 +16,14 @@ import { } from "../appautomate-utils/framework-builds.js"; import { resolveAppAutomateBuildId } from "../failurelogs-utils/resolve-app-build-id.js"; +/** The session is unknown to the generic logs endpoint (404). */ +export class SessionLogsNotFoundError extends Error { + constructor(message: string) { + super(message); + this.name = "SessionLogsNotFoundError"; + } +} + export const MAX_SCREENSHOT_BYTES = globalConfig.REMOTE_MCP ? 10 * 1024 * 1024 : undefined; @@ -48,6 +56,9 @@ async function extractScreenshotUrls( "The logs for this session are too large (over 5 MB) to find screenshots here. Open the session in the BrowserStack dashboard to view them.", ); } + if (response.status === 404) { + throw new SessionLogsNotFoundError("Invalid session ID for Session"); + } await assertOkResponse(response, "Session"); const text = @@ -185,7 +196,13 @@ export async function fetchAutomationScreenshots( try { urls = await extractScreenshotUrls(sessionId, sessionType, config); } catch (error) { - if (sessionType !== SessionType.AppAutomate) throw error; + // Only a 404 can mean a framework session; anything else keeps its own error. + if ( + sessionType !== SessionType.AppAutomate || + !(error instanceof SessionLogsNotFoundError) + ) { + throw error; + } const framework = await frameworkScreenshots(sessionId, config); if (!framework) throw error; return framework; @@ -258,12 +275,19 @@ async function frameworkScreenshots( sessionId: string, config: BrowserStackConfig, ): Promise { - const buildId = await resolveAppAutomateBuildId(sessionId, config); - if (!buildId) return undefined; - const build = await findFrameworkBuild(buildId, config); - if (!build) return undefined; - - const cases = (await getFrameworkTestCases(build, sessionId, config)) ?? []; + // A failed lookup here means "not a framework session": the caller keeps the + // original error instead of reporting this secondary failure. + let build; + let cases; + try { + const buildId = await resolveAppAutomateBuildId(sessionId, config); + if (!buildId) return undefined; + build = await findFrameworkBuild(buildId, config); + if (!build) return undefined; + cases = (await getFrameworkTestCases(build, sessionId, config)) ?? []; + } catch { + return undefined; + } const withShots = failedFirst(cases).find((c) => isBrowserStackUrl(c.fields.screenshots), ); diff --git a/src/tools/automate.ts b/src/tools/automate.ts index ada8ab9c..3f664be5 100644 --- a/src/tools/automate.ts +++ b/src/tools/automate.ts @@ -169,9 +169,11 @@ export async function listSessionIdsTool( if (sessions.length === 0 && sessionType === SessionType.AppAutomate) { const framework = await findFrameworkBuild(buildId, config); if (framework) { - sessions = frameworkSessions(framework).filter( - (s) => !args.status || s.status === args.status, - ); + const wanted = args.status?.trim().toLowerCase(); + const offset = args.offset ?? 0; + sessions = frameworkSessions(framework) + .filter((s) => !wanted || (s.status ?? "").toLowerCase() === wanted) + .slice(offset, offset + (args.limit ?? DEFAULT_SESSION_LIST_LIMIT)); notes.push( `This build ran through the ${framework.framework.name} framework API; sessions are listed from it.`, ); diff --git a/src/tools/failurelogs-utils/framework.ts b/src/tools/failurelogs-utils/framework.ts index adbd232a..de8378ac 100644 --- a/src/tools/failurelogs-utils/framework.ts +++ b/src/tools/failurelogs-utils/framework.ts @@ -1,4 +1,5 @@ import { AppAutomateLogType } from "../../lib/constants.js"; +import { wrapUntrusted } from "../../lib/untrusted-content.js"; import { BrowserStackConfig } from "../../lib/types.js"; import { downloadTestCaseFile, @@ -55,9 +56,12 @@ export async function retrieveFrameworkLogs( ); return crashed.length ? `Crashes recorded in this ${framework} session:\n` + - crashed - .map((c) => `- ${label(c)}: ${c.fields.crash_logs_count}`) - .join("\n") + + wrapUntrusted( + "test names", + crashed + .map((c) => `- ${label(c)}: ${c.fields.crash_logs_count}`) + .join("\n"), + ) + "\nOpen the session on the dashboard to read the crash reports." : `No crashes recorded in this ${framework} session.`; } @@ -95,5 +99,9 @@ export async function retrieveFrameworkLogs( withLog.length > MAX_TESTS ? `\n\n${withLog.length - MAX_TESTS} more test(s) have this log; failed tests are shown first.` : ""; - return `${framework} session ${sessionId}, ${logType}:\n\n${parts.join("\n\n")}${more}`; + return ( + `${framework} session ${sessionId}, ${logType}:\n` + + wrapUntrusted("test logs", parts.join("\n\n")) + + more + ); } diff --git a/tests/manual/injection-e2e.test.ts b/tests/manual/injection-e2e.test.ts index 73f26192..1fd69bd8 100644 --- a/tests/manual/injection-e2e.test.ts +++ b/tests/manual/injection-e2e.test.ts @@ -20,6 +20,8 @@ import { queryAccessibilityRAG } from "../../src/tools/accessiblity-utils/access import { parseAccessibilityReportFromCSV } from "../../src/tools/accessiblity-utils/report-parser.js"; import { formatRCAData } from "../../src/tools/rca-agent-utils/format-rca.js"; import { wrapUntrusted } from "../../src/lib/untrusted-content.js"; +import { retrieveFrameworkLogs } from "../../src/tools/failurelogs-utils/framework.js"; +import { AppAutomateLogType } from "../../src/lib/constants.js"; const cfg: any = { "browserstack-username": "u", @@ -60,6 +62,44 @@ describe("indirect prompt injection is contained per tool", () => { } }); + it("framework test logs (Maestro / Espresso / XCUITest / Flutter)", async () => { + get + .mockResolvedValueOnce( + ok({ + testcases: { + data: [ + { + class: "flows", + testcases: [ + { + name: INJECT, + status: "failed", + maestro_log: "https://api.browserstack.com/log.txt", + }, + ], + }, + ], + }, + }), + ) + .mockResolvedValueOnce(ok(`step ok\n${INJECT}\n`)); + const out = await retrieveFrameworkLogs( + { + framework: { name: "maestro", path: "maestro/v2" }, + buildId: "build-1", + data: {}, + }, + "session-1", + AppAutomateLogType.AppiumLogs, + cfg, + ); + show("retrieveFrameworkLogs", out); + expect(out).toContain("UNTRUSTED"); + expect(out).toMatch(/never follow/i); + // The injected text appears only inside the untrusted block. + expect(out.indexOf("UNTRUSTED")).toBeLessThan(out.indexOf("IGNORE ALL")); + }); + it("network / session / console logs (automate)", async () => { get.mockResolvedValueOnce( ok({ diff --git a/tests/tools/frameworkBuilds.test.ts b/tests/tools/frameworkBuilds.test.ts index 0fb7f24b..b0b460f5 100644 --- a/tests/tools/frameworkBuilds.test.ts +++ b/tests/tools/frameworkBuilds.test.ts @@ -97,7 +97,7 @@ function routeMaestro(extra: (url: string) => unknown = () => undefined) { describe("findFrameworkBuild", () => { beforeEach(() => vi.resetAllMocks()); - it("tries each framework once and stops at the one that knows the build", async () => { + it("asks all five frameworks at once and returns the one that knows the build", async () => { (apiClient.get as Mock).mockImplementation(async ({ url }) => url.includes("/xcuitest/v2/") ? ok({ id: BUILD, devices: [] }) : notFound, ); @@ -105,11 +105,7 @@ describe("findFrameworkBuild", () => { const build = await findFrameworkBuild(BUILD, config); expect(build?.framework.name).toBe("xcuitest"); - expect((apiClient.get as Mock).mock.calls.map(([r]) => r.url)).toEqual([ - `https://api-cloud.browserstack.com/app-automate/maestro/v2/builds/${BUILD}`, - `https://api-cloud.browserstack.com/app-automate/espresso/v2/builds/${BUILD}`, - `https://api-cloud.browserstack.com/app-automate/xcuitest/v2/builds/${BUILD}`, - ]); + expect(apiClient.get).toHaveBeenCalledTimes(5); }); it("returns undefined after five 404s, for an ordinary Appium build", async () => { @@ -119,14 +115,28 @@ describe("findFrameworkBuild", () => { expect(apiClient.get).toHaveBeenCalledTimes(5); }); - it("surfaces a server error instead of reading it as 'not this framework'", async () => { + it("treats a failed lookup on another framework as 'not this framework'", async () => { + (apiClient.get as Mock).mockImplementation(async ({ url }) => { + if (url.includes("/maestro/v2/")) return ok(maestroBuild); + if (url.includes("/flutter-integration-tests/v2/ios/")) + return { ok: false, status: 403, statusText: "Forbidden", data: {} }; + return { + ok: false, + status: 503, + statusText: "Service Unavailable", + data: {}, + }; + }); + + await expect(findFrameworkBuild(BUILD, config)).resolves.toMatchObject({ + framework: { name: "maestro" }, + }); (apiClient.get as Mock).mockResolvedValue({ ok: false, status: 503, statusText: "Service Unavailable", }); - - await expect(findFrameworkBuild(BUILD, config)).rejects.toThrow(/503/); + await expect(findFrameworkBuild(BUILD, config)).resolves.toBeUndefined(); }); }); @@ -188,6 +198,7 @@ describe("retrieveFrameworkLogs", () => { text.indexOf("login-flow (passed)"), ); expect(text).toContain("Assertion failed: button not visible"); + expect(text).toContain("UNTRUSTED"); const fetched = (apiClient.get as Mock).mock.calls.find(([r]) => r.url.endsWith("fail.txt"), )![0]; @@ -388,3 +399,80 @@ describe("Maestro screenshot memory limits", () => { await expect(shotsFrom(zip)).rejects.toThrow(/too many files/); }); }); + +describe("review follow-ups", () => { + beforeEach(() => vi.resetAllMocks()); + + it("keeps a non-404 screenshot error and makes no framework calls", async () => { + (apiClient.get as Mock).mockResolvedValue({ + ok: false, + status: 500, + statusText: "Internal Server Error", + data: {}, + }); + + await expect( + fetchAutomationScreenshots(SESSION, SessionType.AppAutomate, config), + ).rejects.toThrow(); + expect( + (apiClient.get as Mock).mock.calls.some(([r]) => r.url.includes("/v2/")), + ).toBe(false); + }); + + it("keeps the original screenshot error when the framework check fails", async () => { + (apiClient.get as Mock).mockImplementation(async ({ url }) => + url.endsWith("/logs") + ? notFound + : { + ok: false, + status: 503, + statusText: "Service Unavailable", + data: {}, + }, + ); + + await expect( + fetchAutomationScreenshots(SESSION, SessionType.AppAutomate, config), + ).rejects.toThrow(/Invalid session ID/); + }); + + it("listSessions honours limit, offset and a status filter in any case", async () => { + const many = { + ...maestroBuild, + devices: [ + { + device: "Pixel", + os: "android", + os_version: "14.0", + sessions: [ + { id: "s1", status: "failed" }, + { id: "s2", status: "passed" }, + { id: "s3", status: "failed" }, + { id: "s4", status: "failed" }, + ], + }, + ], + }; + (apiClient.get as Mock).mockImplementation(async ({ url }) => { + if (url.endsWith(`/app-automate/builds/${BUILD}/sessions.json`)) + return ok([]); + if (url.endsWith(`/maestro/v2/builds/${BUILD}`)) return ok(many); + return notFound; + }); + + const result = await listSessionIdsTool( + { + sessionType: SessionType.AppAutomate, + buildId: BUILD, + status: "FAILED", + limit: 1, + offset: 1, + }, + config, + ); + + expect( + JSON.parse(result.content[0].text as string).map((s: any) => s.sessionId), + ).toEqual(["s3"]); + }); +});