⚠️ Warning — [MAINTAINABILITY] The update_cli → BROWSERSTACK_BINARY_URL fallback has side effects that aren't documented

Problem

This PR's description covers only the bsstag host matching in util.ts. It also changes how the CLI binary is obtained here: when requestToUpdateCLI throws and BROWSERSTACK_BINARY_URL is set, the version check is skipped and the binary is downloaded from that URL. The changeset sdk-6948-v8-binary-url-fallback.md documents this, but the PR description doesn't.

The fallback is opt-in. With the env var unset, the error propagates as before, and the new test covers that. Two side effects are worth fixing or stating:

• Re-download every time. The fallback calls downloadLatestBinary even when a usable existingCliPath is already on disk, so each run where update_cli fails downloads the binary again.
• The original error is lost. If downloadLatestBinary also fails, its error surfaces and the original update_cli error (the 401 from the comment) is never logged. Only its status is in the warn line.

Suggested Fix

• If existingCliPath exists, return it instead of re-downloading, or at least prefer it.
• Wrap the fallback download in a try/catch that logs or rethrows with both errors.
• Add the fallback to the PR description, so reviewers and release notes don't treat this as a pure host-matching port.

Verified against the diff at 25529d20: the fallback path calls downloadLatestBinary without checking existingCliPath, and err is not kept if the download throws.

Partly fixed in c4d9545.

Fixed:

• The original error is no longer lost. The update_cli error is logged at debug. If the BROWSERSTACK_BINARY_URL download then fails with no binary on disk, both errors are logged together before the download error is rethrown.
• A failed download now falls back to the binary on disk. cliUtils.ts fallbackToBinaryUrl returns the existing binary when there is one, and the run fails only when there isn't. This matches main/v9 fix(cli): fall back to BROWSERSTACK_BINARY_URL when update_cli fails (SDK-6948) #224.
• The PR description and both changesets now describe the fallback.

Not changed: re-downloading instead of reusing the binary on disk. When update_cli fails, BROWSERSTACK_BINARY_URL intentionally wins over the binary on disk:

• With update_cli failing, the SDK can't tell whether the binary on disk is current. On a staging env it is often an older build that sends session-marking calls to api-<env>, which returns 404 on non-preprod envs. Reusing it silently brings back the marking bug this work fixes (SDK-7872).
• The download only happens when update_cli fails and the user has set BROWSERSTACK_BINARY_URL, so production runs are unaffected.
• This is the rule all the SDK-6948 PRs follow (node-agent #2655, python #1282, csharp #1375, javaagent #2296, wdio main fix(cli): fall back to BROWSERSTACK_BINARY_URL when update_cli fails (SDK-6948) #224).

Tests in tests/cli/cliUtils.test.ts: download fails and the binary on disk is used; download fails with nothing on disk and the error is rethrown; a successful reply with no download link and nothing on disk falls back; a successful reply with no download link and a binary on disk keeps it.

⚠️ Warning — [TEST COVERAGE] No test for the new bsstag matching in getCloudProvider / isBrowserstackInfra

Problem

Both classifiers now treat bsstag hosts as BrowserStack: the single and multiremote branches of getCloudProvider, and isBrowserstack here. Nothing tests these branches. The only test added in this PR covers the CLI fallback. tests/util.test.ts:453-468 covers getCloudProvider only for hub.browserstack.com.

v8 uses .includes() where main/v9 uses === / endsWith, so the v9 tests don't protect this adaptation either. A later edit to either predicate could drop staging recognition without any test failing.

Suggested Fix

Add cases to tests/util.test.ts:

expect(getCloudProvider({ options: { hostname: 'hub-xyz.bsstag.com' } } as any)).toEqual('browserstack')
// multiremote: one instance with a bsstag hostname -> 'browserstack'
expect(isBrowserstackInfra({ hostname: 'hub.bsstag.com' } as any)).toBe(true)

Verified: tests/util.test.ts at 25529d20 has no bsstag case. The reviewer graded this as a Suggestion, and the engine's severity table (Test Coverage + KB-medium grounding) raised it to Warning.

Fixed in c4d9545.

The bsstag matching is also now limited to staging runs, to match main/v9 #275 and the other SDKs. A *.bsstag.com host counts as BrowserStack only when BROWSERSTACK_STAGING_ENV is a bare env name (trimmed, blank means unset) or BROWSERSTACK_ENV is staging, stag, preprod or pre-prod. The host must match exactly and case doesn't matter. This replaces the earlier match on any text containing bsstag, and the logic lives in a new isStagingEnvBsstagHost helper.

New tests in tests/util.test.ts, under 'SDK-6948 BROWSERSTACK_STAGING_ENV bsstag host recognition':

• getCloudProvider for a single instance and for multiremote, and isBrowserstackInfra with the host in the config and in capabilities
• trimming and case-insensitive matching, and the BROWSERSTACK_ENV values
• production behaviour unchanged when no staging env is set; hub.browserstack.com is still recognised
• blank or dotted BROWSERSTACK_STAGING_ENV values, and look-alike hosts such as hub.notbsstag.com

The full suite passes locally: 45/45 test files (npm run build && npm test).

🔴 Critical (reviewer note: should be ⚠️ Warning) — [CORRECTNESS] _update now runs for bsstag sessions with no error handling, while the session GET next to it was made non-fatal

Problem

With this PR, isStagingEnvBsstagHost makes *.bsstag.com sessions count as BrowserStack. isBrowserstackSession(this._browser) is now true there, so _update() (service.ts:791-810) starts sending got.put / got.patch to ${_sessionBaseUrl}/<id>.json. That is the same session REST host this hunk now guards.

This hunk wraps the GET in try/catch because "on a named staging env the session REST host may not serve this session". The writes to that host have no such guard:

• _setSessionName → await this._updateJob({ name }) (:887), called from beforeSuite / beforeTest / beforeFeature;
• after() → await this._updateJob({ status, … }) (:554), for the classic non-CLI flow;
• the multiremote reload path await this._update(oldSessionId, …) (:729).

If the host returns 401/404 for these sessions, which is the case this hunk was written for, each of these calls rejects and surfaces as a hook error instead of a debug log.

Why Warning rather than Critical: this only triggers when BROWSERSTACK_STAGING_ENV (a bare env name) or BROWSERSTACK_ENV (staging/stag/preprod/pre-prod) is set. That means internal staging only; production hosts are unchanged. GRR also rewrites APIUtils.BROWSERSTACK_AUTOMATE_API_URL to the staging API, so the call may well succeed there. Runtime was not verified.

Suggested Fix

Make _update non-fatal the same way as _printSessionURL:

return got.put(sessionUrl, { … }).catch((err) => {
    const status = err?.response?.statusCode ?? err?.message
    BStackLogger.debug(`Could not update Browserstack session (${status}) at ${sessionUrl}`)
})

(Apply the same to the got.patch turbo-scale branch.) Add a service.test.ts case where got.put rejects with 401 for a bsstag session.

Verified against ccb370dd: _update has no catch, and its 3 call sites await it without one.

Correction to the after() bullet above. after() is wrapped in try/catch (service.ts:609), so a rejected _updateJob there is logged as Error in after hook and doesn't surface as a hook failure. It does skip everything after it in that try: Listener.onWorkerEnd(), Percy teardown, and the rest of the worker-end cleanup. The beforeSuite / beforeTest / beforeFeature paths through _setSessionName have no catch. The suggested fix (a non-fatal _update) covers all of these.

Fixed in d91e729. The finding is valid, and Warning is the right severity: it only triggers on a named staging env.

_update (service.ts) now attaches .catch(onHttpError) to both got.put and the TurboScale got.patch. That covers all three call sites: _setSessionName from beforeSuite / beforeTest / beforeFeature, after(), and the multiremote reload path.

• HTTP error replies (the error has a response, such as 401 or 404) are logged at debug as Could not update Browserstack session (<status>) at <url> and resolve. The hook doesn't fail, and the rest of after() still runs.
• Network errors (no response) still reject.

This matches main/v9 exactly. v9 sends these updates with fetch, which resolves on 4xx/5xx and rejects only on network failures. The v8 difference came only from got rejecting on non-2xx replies, so prod behaviour now lines up with v9 as well.

New tests in tests/service.test.ts under _update: a PUT returning 401 resolves and logs at debug; a TurboScale PATCH returning 404 resolves; a network error still rejects.

npm run build passes, and npm test passes 45/45 test files.

🔴 Critical (reviewer note: should be ⚠️ Warning) — [INTENT MISMATCH] In the classic (non-CLI) flow, session marking on bsstag goes to the prod API and is now swallowed, but the changeset says it works

Problem

_sessionBaseUrl is built from APIUtils.BROWSERSTACK_AUTOMATE_API_URL (service.ts:51, and :210/:214 for App Automate and TurboScale). That value is the prod default https://api.browserstack.com (cli/apiUtils.ts:3). The only code that changes it is APIUtils.updateURLSForGRR, called from cli/index.ts:131, which is the CLI flow.

So in the classic non-CLI flow on a bsstag hub, _update (session name, and after() status) now sends the staging session id and credentials to prod. The likely result is 401/404, which onHttpError swallows at debug. The run won't fail, but the session won't be marked. Meanwhile .changeset/pr-183.md says "so session marking and TRA linking work there".

Why Warning rather than Critical: this is internal-staging only (gated on BROWSERSTACK_STAGING_ENV / BROWSERSTACK_ENV), production is unchanged, and the CLI flow is unaffected. It is a claim/behaviour mismatch, not customer breakage. The prod 401 itself was not runtime-verified.

Suggested Fix

Either:

• point the session REST base at the staging API in the non-CLI flow too (derive it from the GRR / env config in the worker), or
• narrow the changeset: on bsstag, session marking works only with the CLI flow; in the classic flow it is skipped at debug.

Valid. The wording overclaimed. Fixed in d0a8fba by narrowing the changeset, which is your second option.

I confirmed the cause: APIUtils.BROWSERSTACK_AUTOMATE_API_URL is localized only by APIUtils.updateURLSForGRR, which is called only from cli/index.ts:131. In the classic (non-CLI) flow, _sessionBaseUrl stays on prod api.browserstack.com.

What changed:

• .changeset/sdk-6948-v8-bsstag-host-recognition.md and the PR's release notes (which regenerate .changeset/pr-183.md) now say marking and linking work with the CLI flow (the default), where the binary supplies the staging session REST host. The classic non-CLI flow on a named staging env is stated as not supported, and its session update is skipped at debug.
• The PR description has a matching Scope note.

Why I didn't add staging-host derivation to the classic flow:

• It is the same scope decision as main/v9 fix: recognize bsstag hosts and localize session REST host on named staging envs (SDK-6948) #275, whose changeset reads: "The classic (non-CLI) flow on a named staging env is not supported". The v8 port keeps parity with that.
• Internal staging runs go through the CLI flow, since the SDK-6948 work is built around the binary supplying the hosts.
• Building the staging API host in the worker would duplicate the binary's per-env host rules (api<env> vs api-preprod, binary #1889) in a second place, which could drift.
• Production is unaffected.

⚠️ Warning — [TEST COVERAGE] The TurboScale test says "logs at debug" but doesn't assert the debug log

Problem

it('logs at debug when a TurboScale session update gets an HTTP error (SDK-6948)') only checks that _update resolves and that got.patch was called once. There is no BStackLogger.debug spy or assertion, unlike the got.put test above it, so the patch branch of onHttpError logging is untested despite the name.

Suggested Fix

const debugSpy = vi.spyOn(bstackLogger.BStackLogger, 'debug')
...
expect(debugSpy).toHaveBeenCalledWith(expect.stringContaining('Could not update Browserstack session (404)'))

Alternatively, rename the test to match what it checks.

Verified at 61570d31.

Valid. Fixed in d0a8fba. The TurboScale _update test now spies on BStackLogger.debug and asserts 'Could not update Browserstack session (404)', so the patch branch's logging is covered just like the put test. npm test passes 45/45 test files.

💡 Suggestion (engine: 🔴 Critical, overridden: changelog nit) — [MAINTAINABILITY] The changelog will list each SDK-6948 fix twice

Problem

pr-183.md is auto-generated by .github/workflows/changeset-from-pr.yml from the PR body's ## Release section. It restates the two hand-written changesets, sdk-6948-v8-binary-url-fallback.md and sdk-6948-v8-bsstag-host-recognition.md. All three are patch for @wdio/browserstack-service, so the "Version Packages" changelog will carry each fix twice.

Suggested Fix (non-blocking)

Pick one source:

• Delete the two hand-written changesets and keep the bot's pr-183.md, which already covers all three points. Don't delete pr-183.md itself: the bot regenerates it on every PR body edit.
• Or add the skip-changeset label and keep the hand-written ones.

Verified at 0f1d1732: all three files are in .changeset/, and pr-183.md's commits are by github-actions[bot] ("auto-generate from PR template").