Repository navigation
fix(automate): read visual-log screenshots in fetchAutomationScreenshots - #481
Open
SavioBS629 wants to merge 3 commits into
Open
SavioBS629 wants to merge 3 commits into
SavioBS629 wants to merge 3 commits into
Conversation
Session logs carry screenshots in two forms: `REQUEST GET /screenshot` followed by a RESPONSE value (test-initiated), and `DEBUG <url>` lines written for every click/goto (Playwright) or listed command (Selenium) when browserstack.debug is on. Only the first form was parsed, so debug-mode sessions without explicit screenshot calls returned nothing. - Parse both forms in log order, dedupe, keep the last five - Accept s3-debug, b2-debug and direct bucket URLs on DEBUG lines - Match the RESPONSE after the screenshot request even when other log lines sit in between - Report the detected image mime type instead of always image/png - Point to browserstack.debug in the empty-result message Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Request and response payloads are written verbatim into session logs, so unanchored patterns could pick up an attacker-supplied " DEBUG <url>" or "value" and make the server fetch it (SSRF from hosted pods). - Anchor REQUEST/RESPONSE/DEBUG patterns to the log line prefix - Require a top-level "value" in the screenshot RESPONSE - Only fetch https URLs on browserstack.com, S3 or B2 hosts Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configuration
You can disable this status message by setting the Use the checkbox below for a quick retry:
Comment |
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
sarve-shreyas
approved these changes
Oct 13, 2026
SavioBS629
added a commit
to SavioBS629/mcp-server
that referenced
this pull request
Oct 15, 2026
PR browserstack#481 and this branch both rewrote fetchAndCompress, so they could not be merged independently. This brings browserstack#481's changes onto the size-cap work: - parse DEBUG visual-log screenshots alongside test-initiated ones, anchored on the timestamped log prefix so a REQUEST cannot borrow a later RESPONSE - restrict screenshot URLs to https on BrowserStack, S3 and B2 hosts - detect the image mime type from the bytes, after compression: sharp re-encodes anything over 1 MB to PNG, so reading the original bytes would label a large JPEG wrongly - report the empty result as a debug-mode hint rather than an unexpected error Screenshot keeps both shapes: it carries mimeType and fetchAndCompress still returns null for an oversized download so the remaining screenshots load. The size-cap tests now use timestamped log lines and allowlisted https URLs, routed to the local server by the apiClient spy; the previous fixtures were written against the older permissive parser and would otherwise parse to no URLs at all. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
fetchAutomationScreenshotsreturned "No screenshots found" for sessions that do have screenshots in their logs.Session logs (written by SeleniumHub) carry screenshots in two forms:
REQUEST … GET /session/<id>/screenshotthenRESPONSE {"value":"<url>"}driver.save_screenshot()/page.screenshot(); no capability neededDEBUG <url>browserstack.debug: true; after eachclick/goto(Playwright) or each command inconfig/commandlist.txt(Selenium)Only the first form was parsed. Debug-mode sessions without explicit screenshot calls came back empty, and Selenium sessions with both returned only the test-initiated ones.
Changes
s3-debug,b2-debug, direct bucket)PERFORMANCE,SELFHEAL, …) sit in between, without crossing into the next REQUESTimage/pngbrowserstack.debug: true" DEBUG <url>"or a"value"in their own commands and make the server fetch it. All patterns are now anchored to the log line prefix, the screenshot RESPONSE must have a top-levelvalue, and only https URLs onbrowserstack.com, S3 or B2 hosts are fetched. This also closes the same gap in the pre-existingGET /screenshotpath.Verification
npm run buildpasses on Node 22 (82 files, 1278 tests). Newtests/tools/fetch-screenshots.test.tsuses log lines taken from real sessions.Against live sessions:
9eaca30c…c239114f…page.screenshot()94d6e8ef…🤖 Generated with Claude Code