Merge fix/SDK-4177-cli-flow-screenshot-log - #208
Merged
Merged
Conversation
`InsightsHandler.browserCommand` is the only producer of TEST_SCREENSHOT logs, and its two call sites both sat behind `!BrowserstackCLI.isRunning()`. Since `CLISupportedFrameworks = ['mocha']`, every Mocha run took the binary flow and lost screenshots entirely — Observability received a test run with no TEST_LOG artifact, so no screenshots manifest. Jasmine and Cucumber, which stay on the Direct flow, were unaffected. Subscribe to the result event on the binary path as well. `command` (beforeCommand) is deliberately left unsubscribed: it only fills the map the HTTP-log half of `browserCommand` reads, and that half emits on the JS listener pipeline the binary owns here — so the screenshot upload, which rides its own JWT-authenticated endpoint, stays the single effect. Also honour an explicit denial: `allow_screenshots` is an optional *string* on the wire, so a denial arrives as `'false'`, which `Boolean()` read as permission granted. Same defect class `shouldProcessEventForTesthub` already guards against with `isTrue`. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
`.github/PULL_REQUEST_TEMPLATE.md` states the changeset is generated from the PR's Release section as `.changeset/pr-<number>.md`, so a hand-written one is redundant here. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Follow-up to the previous commit, which subscribed the result event on the
binary path but stopped one step short — verified end to end and the
screenshot still never reached Observability. Two further links were broken:
1. `browserCommand` uploaded via `listener.onScreenshot`, whose endpoint
answers 401 to the binary's JWT (`[screenshot_upload] Failed ... status:
401`). On this path the binary owns reporting, so the entry now rides the
same LOG rail `appendTestItemLog` uses: the CLI stamps the test uuid and
forwards it over gRPC.
2. `loadLogEntries` hardcoded `logRecord.kind = KIND_LOG`, destructuring only
`{level, message, timestamp}` and discarding the producer's kind — so the
screenshot arrived labelled as a console log and no screenshots manifest
was built. It now keeps an explicit kind and falls back to KIND_LOG, which
is what the previously-unreferenced KIND_SCREENSHOT constant was for.
Console logs are unchanged: `StdLog.kind` is already 'TEST_LOG'.
Verified on a real build: two `"kind":"TEST_SCREENSHOT"` entries on the gRPC
wire, `retries[].logs == ['TEST_LOG']` on both leaves, and BStackAutomation's
`validate_o11y_screenshot` returning True for both test ids (0/2 failures).
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…ng it Review finding 1. `getTestFramework()` can be undefined while `isRunning()` is true — the dev-env short-circuit returns true before `setupTestFramework()` has run, and that only assigns for webdriverio-mocha — so the non-null assertion could throw. `o11yClassErrorHandler` wraps every InsightsHandler method and catches async rejections, so it could not break the customer's test, but the screenshot was thrown away silently. Resolve the framework and fall back to the direct upload when it is absent, so an untracked framework still gets its one chance at reporting the screenshot. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
bsautomation
requested review from
AdityaHirapara and
rounak610
and removed request for
a team
September 18, 2026 13:13
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Central YAML (base), Organization UI (inherited), Workspace UI (inherited) Review profile: ASSERTIVE Plan: Advanced Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Comment |
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
3 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.
Merge into sdk_v9_pre_prod :by automationbs