GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — Merge A11Y-13909-forward-observability-api-url by bsautomation · Pull Request #1215 · browserstack/browserstack-cypress-cli

Conversation

@bsautomation

@bsautomation bsautomation commented Nov 5, 2026 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Collaborator

Merge into pre_prod :by automationbs

  • Mainline PR: A11 y 13909 forward observability api url #1214 (into master)
  • Last commit: 8513d91 by manish-bothra
  • Merged at: 12:31 PM IST, 5 Nov

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
manish-bothra and others added 12 commits October 30, 2026 12:04
@manish-bothra @claude
Forward the Test Observability collector URL to the remote reporter
024cd0a
The reporter that sends TestRunStarted/TestRunFinished runs on the BrowserStack machine from its own install of this package, which has the prod collector hard-coded. Against any other collector (staging environments) those events never arrive, so TestHub keeps nameless test cases and Accessibility collapses every test in a spec into one. Make API_URL overridable via BROWSERSTACK_TEST_OBSERVABILITY_API_URL and forward the launching cli's effective collector in system_env_vars, the same channel BS_TESTOPS_JWT already uses. Prod behaviour is unchanged.

Jira: A11Y-13909

Co-Authored-By: Claude Fable 5.1 <[email protected]>
@manish-bothra @claude
Allow a separate collector for the remote reporter
bf87243
On staging the collector the cli talks to is a private ingress the BrowserStack machine cannot reach, while the machine-facing public ingress is a different host. BROWSERSTACK_TEST_OBSERVABILITY_REMOTE_API_URL, when set, is what gets forwarded to the machine; otherwise the cli's own collector is forwarded as before. Read at call time so the tests need no module reloads.

Jira: A11Y-13909

Co-Authored-By: Claude Fable 5.1 <[email protected]>
@manish-bothra @claude
Require https for the collector overrides and isolate the env vars in…
0166a03
… tests

Review feedback on #1210. Collector requests carry BS_TESTOPS_JWT, so an http:// override would send it in cleartext: reject such a value instead of falling back to the default collector. The assignment moves out of the try block in setSystemEnvs because that block swallows errors, which would have turned the rejection into a silent fallback. Tests now clear both URL variables before each case and restore the originals after.

Jira: A11Y-13909

Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
@manish-bothra @claude
Parse the collector override as a URL and reject a query or fragment
e7d01ac
Request urls are built as `${API_URL}/${path}`, so an override carrying a query or fragment would swallow the endpoint path and the upload would hit / instead. Parse with URL, require an https protocol and a hostname, reject search and hash, and trim a trailing slash so paths are not doubled.

Jira: A11Y-13909

Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
@manish-bothra @claude
Normalise the collector override and keep its value out of errors
fc37d26
A bare trailing ? or # leaves URL.search and URL.hash empty, so the value passed validation and the appended endpoint became a query or fragment; return origin + pathname with no trailing separator instead. The override can also carry credentials and callers log the error, so the messages no longer echo the value.

Jira: A11Y-13909

Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
@manish-bothra @claude
Cover the request destination, not just the forwarded env var
c40a12b
setSystemEnvs tests only prove the url is handed to the remote machine. These load the helper with the override set and a stubbed axios, and assert nodeRequest builds its request url from it, with the default covered when nothing is set.

Jira: A11Y-13909

Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
@manish-bothra @claude
Read the collector override lazily and validate it on the runs path
3bf5017
constants.js is loaded through bin/helpers/utils.js, which every command loads, so validating at module load meant a bad BROWSERSTACK_TEST_OBSERVABILITY_API_URL crashed init, info, stop and the reporter on the machine at startup, outside any handler. API_URL becomes apiUrl(), read when a request is built; nodeRequest rejects rather than throws, since its executor is async and a throw there would never reach the returned promise. validateCollectorUrls() runs at runs.js:136, before TestHubHandler.launchBuild, so a bad value fails before a build exists. Comments in constants.js trimmed to the one that earns its place.

Jira: A11Y-13909

Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
@manish-bothra @claude
Validate the general collector override too, not only the remote one
7169d3d
remoteReporterApiUrl() short-circuits past apiUrl() when the remote override is set, so a malformed general override went unchecked until the first request. validateCollectorUrls() now calls both, with a test for the short-circuit case.

Jira: A11Y-13909

Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
@manish-bothra @claude
A11Y-13909: warn on a remote-only collector, and make the load-safety…
068bbe8
… test real

Two findings from the SDK PR review:

Setting only BROWSERSTACK_TEST_OBSERVABILITY_REMOTE_API_URL leaves this process
on its built-in collector while the machine reports to another one, so the build
and its test events can land in two places with nothing detecting it. That split
is correct in the Jenkins flow, where the built-in default has been pointed at
the same environment, and wrong anywhere else - which the code cannot tell apart,
so this warns rather than rejects.

The third case in collectorUrl.js promised that loading the module never throws
on a bad value, but proxyquire's noPreserveCache clears only the proxied module
and its stubs, and the url is resolved in constants.js. That stayed a cache hit,
so hoisting the resolution to module scope would not have turned the test red -
verified by doing exactly that, which now fails. The loader drops constants.js
and the test asserts the load explicitly.

Env var names move into constants.js so the validator and the resolvers share
one spelling.

Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
@manish-bothra @claude
A11Y-13909: keep the collector value out of the log, and share the en…
1c70e71
…v key

Second round of SDK PR review findings.

The warning added in the last commit interpolated the raw env value, which
contradicts the rule the validator states a few lines up: a collector url can
carry credentials and callers log it. It now logs the normalized accessor,
whose origin has already dropped any userinfo. The validator also rejects a
url carrying a username or password outright, so the stripping is explicit
rather than a silent side effect of normalization.

setSystemEnvs still spelled the forwarded key as a literal while every other
reference used the imported constant, so a rename could have split the
producer from the consumer - the exact failure this PR exists to prevent.

Tests: the no-override path (what every build takes) now has a case, and so
does the userinfo rejection.

Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
@manish-bothra @claude
A11Y-13909: name only the collector host in the split-collector warning
632b2c5
httpsOverride normalizes to origin + pathname, so the path survives and can
carry a token. The warning now names the host, which is the part that answers
the question it asks - whether the two collectors are the same environment -
and a test pins that a path in the value never reaches the log.

Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
@manish-bothra
Merge branch 'master' into A11Y-13909-forward-observability-api-url
8513d91
@bsautomation
bsautomation requested a review from a team as a code owner November 5, 2026 07:01
@bsautomation
bsautomation requested review from shivam5643 and vivianludrick and removed request for a team November 5, 2026 07:01
@coderabbitai

coderabbitai Bot commented Nov 5, 2026

Copy link
Copy Markdown

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

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: browserstack/browserstack-cypress-cli
  • Branch: A11Y-13909-forward-observability-api-url
  • File: .coderabbit.yaml
  • Commit: 8513d91
  • Layers: Repository YAML (base), Central YAML (inherited), Organization UI (inherited), Workspace UI (inherited); Enforced overrides: Organization
  • Review profile: ASSERTIVE
  • Plan: Advanced
  • Run ID: bb483e24-307c-46d5-ab92-7680e9a8377c

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
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@bsautomation
bsautomation merged commit 8fe6821 into pre_prod Nov 5, 2026
7 checks passed

Uh oh!

There was an error while loading. Please reload this page.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@shivam5643 shivam5643 Awaiting requested review from shivam5643 shivam5643 is a code owner automatically assigned from browserstack/sdk-dev
@vivianludrick vivianludrick Awaiting requested review from vivianludrick vivianludrick is a code owner automatically assigned from browserstack/sdk-dev

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

2 participants

@bsautomation @manish-bothra
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.
You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.