From 0e6b0da9bca9a678ea0ad81e4024e0c26fcecfe4 Mon Sep 17 00:00:00 2001 From: Kamalpreet Kaur Date: Tue, 8 Sep 2026 22:40:21 +0530 Subject: [PATCH 1/2] fix(a11y): keep the binary flow alive on a degenerate config and stop double scanning MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two defects that only surface together, and only away from production hostnames. 1. loadModules() dereferenced config.apis unconditionally. A config with no apis block — an auth failure, or a config server that never answered (measured: a 60s hang, after which the binary echoes the input config back) — threw here, the caller tore the binary down, and the entire run silently fell through to the Direct flow. Keeping the default endpoints is strictly better than losing the binary flow. 2. getCloudProvider() classifies a session by hostname.includes('browserstack'), so a hub served from a host without that substring reads as a third-party grid. isBrowserstackSession() then goes false, _isCliAccessibilityFlow() with it, and the service runs the classic accessibility handler WHILE the binary runs its CLI accessibility module — both wrapping the same commands. Measured on one such run: 22 classic + 19 CLI scans where 19 were expected. Duplicate scans inflate scan_count against effective_expected and corrupt the App-A11y stability metric. The gate asks the CLI module registry rather than the hostname, and asks at SCAN time rather than session start: service.ts must choose a flow before the binary has necessarily finished booting, so a decision made then can be stale by the time a command fires. Verified: 48/48 accessibility-handler tests pass. service + cli suites show 4 failures, identical to untouched main (cliUtils network tests). Co-Authored-By: Claude Opus 5 --- .../src/accessibility-handler.ts | 19 ++++++++++++++++-- .../browserstack-service/src/cli/index.ts | 11 +++++++++- packages/browserstack-service/src/service.ts | 13 ++++++++++++ .../tests/accessibility-handler.test.ts | 20 +++++++++++++++++++ 4 files changed, 60 insertions(+), 3 deletions(-) diff --git a/packages/browserstack-service/src/accessibility-handler.ts b/packages/browserstack-service/src/accessibility-handler.ts index 69c3350..4e93284 100644 --- a/packages/browserstack-service/src/accessibility-handler.ts +++ b/packages/browserstack-service/src/accessibility-handler.ts @@ -78,6 +78,17 @@ import * as PERFORMANCE_SDK_EVENTS from './instrumentation/performance/constants import { BStackLogger } from './bstackLogger.js' class _AccessibilityHandler { + // Evaluated AT SCAN TIME, not at session start. service.ts decides which flow owns + // accessibility once, before the binary has necessarily finished booting; on a slow + // environment that decision lands on this handler and the binary then comes up and wraps the + // same commands through the CLI module, so every command gets scanned twice. Asking again + // when a scan is about to fire is the only check that can be right. + private _cliOwnsAccessibility: () => boolean = () => false + + setCliOwnershipCheck(check: () => boolean) { + this._cliOwnsAccessibility = check + } + /** * Frameworks whose per-test lifecycle flows through beforeTest/afterTest. * WDIO's jasmine adapter emits the same service hooks as mocha (SDK-7190); @@ -512,8 +523,12 @@ class _AccessibilityHandler { !AccessibilityHandler.shouldPatchExecuteScript(args.length ? args[0] as string : null) ) ) { - BStackLogger.debug(`Performing scan for ${command.class} ${command.name}`) - await performA11yScan(this.isAppAutomate, this._browser, true, true, command.name, undefined, this._currentHookRunUuid) + if (this._cliOwnsAccessibility()) { + BStackLogger.debug('Skipping accessibility scan: the binary flow owns accessibility for this session') + } else { + BStackLogger.debug(`Performing scan for ${command.class} ${command.name}`) + await performA11yScan(this.isAppAutomate, this._browser, true, true, command.name, undefined, this._currentHookRunUuid) + } } else if (skipScanForBidiWindowCommand) { BStackLogger.debug(`SDK-5047: skipping accessibility scan for BiDi window/context command '${command.name}' to avoid racing the WebdriverIO ContextManager during session-start window churn`) } diff --git a/packages/browserstack-service/src/cli/index.ts b/packages/browserstack-service/src/cli/index.ts index b1b3cc3..c1dd171 100644 --- a/packages/browserstack-service/src/cli/index.ts +++ b/packages/browserstack-service/src/cli/index.ts @@ -151,7 +151,16 @@ export class BrowserstackCLI { // credentials) before any downstream error. this.logBuildErrors(startBinResponse) - APIUtils.updateURLSForGRR(this.config.apis as GRRUrls) + // A degenerate config carries no apis block — an auth failure, or a config server that + // never answered (measured: a 60s hang against an internal environment, after which the + // binary echoes the input config straight back). Dereferencing it throws, and the caller + // then tears the binary down, silently dropping the whole run to the Direct flow. Keeping + // the default endpoints is strictly better than that. + if (this.config.apis) { + APIUtils.updateURLSForGRR(this.config.apis as GRRUrls) + } else { + this.logger.warn('loadModules: config carries no apis block; keeping default endpoints') + } this.setupTestFramework() this.setupAutomationFramework() diff --git a/packages/browserstack-service/src/service.ts b/packages/browserstack-service/src/service.ts index b996be4..c043c22 100644 --- a/packages/browserstack-service/src/service.ts +++ b/packages/browserstack-service/src/service.ts @@ -289,6 +289,19 @@ export default class BrowserstackService implements Services.ServiceInstance { this._options.accessibilityOptions ) + // Re-asked at scan time, and deliberately NOT via _isCliAccessibilityFlow(): + // that predicate requires isBrowserstackSession(), which decides on + // `hostname.includes('browserstack')` and so is FALSE on every internal + // environment (hub-.bsstag.com). There the service takes the classic + // branch while the binary still runs the CLI module, and both wrap the same + // commands — measured on one internal-env run as 22 classic + 19 CLI scans. + // Asking the module registry is hostname-independent: the module exists only + // when the binary owns accessibility for this session. + this._accessibilityHandler.setCliOwnershipCheck(() => { + const cliA11y = BrowserstackCLI.getInstance().modules?.[AccessibilityModule.MODULE_NAME] as AccessibilityModule | undefined + return Boolean(cliA11y && (cliA11y.accessibility || cliA11y.isAppAccessibility)) + }) + if (this._isCliAccessibilityFlow()){ BStackLogger.info(`CLI is running, tracking accessibility event for before: ${sessionId}`) // BrowserstackCLI.getInstance().getTestFramework()!.trackEvent(AutomationFrameworkState.CREATE, HookState.POST, { sessionId }) diff --git a/packages/browserstack-service/tests/accessibility-handler.test.ts b/packages/browserstack-service/tests/accessibility-handler.test.ts index 1d75f54..99c5546 100644 --- a/packages/browserstack-service/tests/accessibility-handler.test.ts +++ b/packages/browserstack-service/tests/accessibility-handler.test.ts @@ -503,6 +503,26 @@ describe('beforeHook / afterHook (hook scans)', () => { expect(lastCall[lastCall.length - 1]).toBeNull() }) + it('performs NO scan when the binary flow owns accessibility for the session', async () => { + // service.ts can pick the classic branch before the binary has booted; the CLI module then + // wraps the same commands, and without this check every command is scanned twice. + vi.spyOn(utils, 'shouldScanTestForAccessibility').mockReturnValue(true) + const scanSpy = vi.spyOn(utils, 'performA11yScan').mockResolvedValue(undefined) + await accessibilityHandler.beforeHook( + { title: '"before each" hook', parent: 'suite' } as any, + { currentTest: { parent: 'suite', title: 'test' } }, + 'hook-uuid-cli' + ) + accessibilityHandler.setCliOwnershipCheck(() => true) + + const orig = vi.fn().mockResolvedValue('ok') + await accessibilityHandler['commandWrapper']({ name: 'click', class: 'Element' } as any, undefined as any, orig, 'arg') + + // the command still runs — only the duplicate scan is suppressed + expect(orig).toHaveBeenCalled() + expect(scanSpy).not.toHaveBeenCalled() + }) + it('_getParamsForAppAccessibility puts the hook uuid on the scan payload as thHookRunUuid', () => { expect(utils._getParamsForAppAccessibility('click', 'testName', 'hook-uuid-9').thHookRunUuid).toBe('hook-uuid-9') expect(utils._getParamsForAppAccessibility('click', 'testName').thHookRunUuid).toBeUndefined() From a1330d1a96a97d996b6c0bf21c1d4132b14a4333 Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" <41898282+github-actions[bot]@users.noreply.github.com> Date: Tue, 8 Sep 2026 17:11:40 +0000 Subject: [PATCH 2/2] chore(changeset): auto-generate from PR template (patch) --- .changeset/pr-186.md | 6 ++++++ 1 file changed, 6 insertions(+) create mode 100644 .changeset/pr-186.md diff --git a/.changeset/pr-186.md b/.changeset/pr-186.md new file mode 100644 index 0000000..73392a7 --- /dev/null +++ b/.changeset/pr-186.md @@ -0,0 +1,6 @@ +--- +"@wdio/browserstack-service": patch +--- + +- Fixed accessibility scans being sent twice for a single command in some environments, which made scan counts inaccurate. +- Fixed the SDK silently falling back to a non-binary flow when the configuration response was incomplete.