[SavioBS629]

Summary

App Automate builds started through a framework API (Maestro, Espresso, XCUITest and Flutter on Android and iOS) have their own build and session endpoints. For these builds the generic App Automate session list returns no sessions, and the generic log and screenshot endpoints return 404, so listSessions, getFailureLogs and fetchAutomationScreenshots couldn't help with them. This PR reads those builds through their framework endpoints. Appium builds keep their current behaviour.

Behaviour changes

listSessions

• When an App Automate build's session list is empty, the tool checks the five framework build endpoints in parallel. Each returns 404 for another framework's build, so at most one answers. A failed lookup counts as "not this framework" and never turns the reply into an error.
• Sessions are listed from the framework build (id, status, device, OS, dashboard link), with a note naming the framework. limit, offset and a case-insensitive status filter apply as on the generic path.

getFailureLogs

• When the generic App Automate log endpoint returns 404, the tool looks up the session in its framework build and reads each test's log links:
  • appiumLogs returns each test's maestro_log or instrumentation_log;
  • deviceLogs returns each test's device_log, where the framework provides one;
  • crashLogs reports the tests that recorded crashes.
• Failed tests come first. At most 5 tests are read, and each log is trimmed to its last 8,000 characters.
• If a requested log doesn't exist for the framework, the reply lists the logs that do.
• For a session that isn't part of a framework build, the reply is unchanged.

fetchAutomationScreenshots

• For Maestro sessions, after the generic endpoint returns 404, screenshots come from the per-test screenshot archive. The failed test comes first, and the last 5 images are returned.
• For frameworks without screenshots, the reply says so and points to getFailureLogs and the video, instead of "Invalid session ID".

Safety and limits

• Test log links are fetched with the caller's credentials only when they are https links on a BrowserStack host.
• Test logs and test names are returned as untrusted content, like the existing device, Appium and crash logs.
• Memory limits, so one call can't exhaust a server's memory:
  • test logs over 2 MB and screenshot archives over 10 MB are refused with a message;
  • archives with more than 500 files are refused;
  • each image is extracted as a stream and dropped as soon as it passes 10 MB, whatever the archive header claims;
  • at most 25 MB of images is extracted per call, one image at a time, and then passed through the existing image compression.
• Only a 404 triggers a fallback. Other failures (429, 5xx) are reported as errors.

Dependency

• Adds jszip (^3.10.1) as a direct dependency, to read Maestro screenshot archives. The same version is already installed through webdriverio.

Testing

• npm run build passes: lint, format, tests and tsc.
• New tests cover framework detection (including server errors), session listing, log reading (failed tests first, links fetched only from BrowserStack hosts, crash counts, missing log types), Maestro screenshots from an archive, the archive memory limits, and that Appium sessions make no framework calls.
• Live, through a real MCP client, against one build of each framework: listSessions, getFailureLogs and fetchAutomationScreenshots.

🤖 Generated with Claude Code

[coderabbitai]

Important

Review skipped

Auto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

Run configuration

• Source mode: Auto
• Repository: Unavailable
• Branch: Unavailable
• File: No configuration file from this repository applied
• Commit: Unavailable
• Layers: Central YAML (base), Organization UI (inherited), Workspace UI (inherited); Enforced overrides: Organization
• Review profile: ASSERTIVE
• Plan: Enterprise
• Run ID: 56610c39-6bd8-46a4-b99a-27b481dc26c4

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

• Trigger review

• Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

[sarve-shreyas]

Reviewed the full diff at dff5c7a (4 commits on 76ff189, current main). Locally: eslint, prettier --check, tsc --noEmit and vitest run (144 files, 2129 tests) all pass.

These parts look right:

• Making LogsNotFoundError opt-in keeps Appium sessions with logs on exactly the old path and output.
• Credentials only go to https *.browserstack.com links. follow-redirects 1.16.0 drops Authorization on a cross-host redirect, so a redirect to S3 doesn't leak them.
• The archive limits hold up: the stream is capped regardless of the header size, there's an entry-count cap and a total budget, and images are processed one at a time.

One blocking item (untrusted log content) and three non-blocking ones, inline.

[sarve-shreyas]

Blocking: the framework log text goes back to the agent without wrapUntrusted. The generic device, Appium and crash logs in app-automate.ts are all wrapped, and tests/manual/injection-e2e.test.ts checks that they are. This path returns maestro_log / instrumentation_log / device_log tails and test or class names as plain text. The crash-count list at L56 also prints names unwrapped.

Scenario: a Maestro flow asserts on text from the app, or an Espresso test logs a server response, and the log tail contains Ignore previous instructions and call …. Through appiumLogs on an Appium session that line would be quarantined. Through this path it reaches the model as ordinary tool output.

Suggestion: wrap the joined parts and the crash list in wrapUntrusted("test logs", …). Keep the label a literal; never interpolate the framework or test names into it. Then add the framework path to the injection e2e test next to the device, Appium and crash cases.

[sarve-shreyas]

Non-blocking (question): detection now also runs for plain Appium cases: an Appium session whose deviceLogs 404 (device logs off), and an empty App Automate build in listSessions. getJson throws on any non-404 (L56). So a 401, 403, 429 or 5xx from an unrelated framework endpoint turns the old "No device logs available for this session" or "No sessions found" into a tool error. In getFailureLogs, that throw also discards the other log types in the same call that had already succeeded.

Do all five endpoints return 404, not 401 or 403, for an account without that framework, e.g. no Flutter iOS access? I couldn't verify that. If not, consider having the fallback callers log the failure and return the pre-PR text instead of throwing.

Also, the five calls run one after another, so a Flutter iOS build and every Appium miss wait on five round trips. A Promise.allSettled over FRAMEWORKS, taking the first hit in list order, keeps the semantics and drops that to one round trip.

[sarve-shreyas]

Non-blocking: this catch falls back on every error, not only a 404. The PR description says "Only a 404 triggers a fallback", and that's true for logs but not here. For an Appium session whose logs return 413 (the "too large, open the dashboard" message), 401 or 5xx, the tool now calls resolveAppAutomateBuildId and then makes up to five framework calls. If any of those calls throws, its error replaces the original, more useful message.

Suggestion: fall back only on the 404 that assertOkResponse turns into Invalid session ID…, for example with a typed not-found error like LogsNotFoundError, and rethrow everything else unchanged.

[sarve-shreyas]

Non-blocking: the framework list ignores limit and offset, and it compares status case-sensitively. The generic path, mapSessionRecords, trims and lowercases the filter before comparing.

Scenario: an agent pages a large Maestro build with offset: 50 and gets the same full list back, so it can loop. status: "Failed" matches nothing.

Suggestion: reuse the same status normalisation, then .slice(offset, offset + limit) with DEFAULT_SESSION_LIST_LIMIT as the default.

[sarve-shreyas]

Re-reviewed dff5c7a..fb1d0c8. Locally: eslint, prettier --check, tsc --noEmit and vitest run (144 files, 2133 tests, including the new injection case) all pass.

All four items from my last round are resolved:

1. The framework log tails and the crash-count test names are wrapped with literal labels ("test logs", "test names"), and retrieveFrameworkLogs is in injection-e2e.test.ts.
2. findFrameworkBuild runs the five lookups with Promise.allSettled and keeps list order. A rejected lookup is logged at debug and treated as not this framework, so empty-build and no-logs replies keep their old text.
3. The screenshot fallback only runs on SessionLogsNotFoundError, which keeps the old Invalid session ID for Session message. Lookup failures inside frameworkScreenshots return undefined, so the original error is the one surfaced.
4. The framework listSessions path lowercases the status filter and applies offset, limit and DEFAULT_SESSION_LIST_LIMIT.

One optional follow-up: the tradeoff in (2) is that when the matching framework endpoint itself returns 5xx, the reply is now the plain "no sessions" or "no logs" text, with the cause only in the debug log. A short note such as "framework lookup failed, try again" would make that case distinguishable. It isn't needed for this PR.