Disable accessibility when browserstackAccessibility plugin not loaded by pranay-v29 · Pull Request #1132 · browserstack/browserstack-cypress-cli

pranay-v29 · 2026-06-22T05:58:50Z

When a build requests accessibility (browserstack.json/browser caps) but the browserstackAccessibility plugin is not wired into the cypress config, the CLI now detects this before the build start event, explicitly disables accessibility so the build is not counted as an a11y build, warns the user with the setup doc link, and instruments the end-of-session EDS event (accessibility_plugin_not_loaded) so such builds can be excluded from stability queries.

Detection is code-based: the plugin sets BROWSERSTACK_ACCESSIBILITY_PLUGIN_LOADED when its module is loaded/invoked by the cypress config; requireModule writes a flag file the parent process reads back. Falls back to a raw-source scan only when the config could not be required (e.g. TS before bstack packages are installed).

Detection is also done at import and invocation of plugin.

When a build requests accessibility (browserstack.json/browser caps) but the browserstackAccessibility plugin is not wired into the cypress config, the CLI now detects this before the build start event, explicitly disables accessibility so the build is not counted as an a11y build, warns the user with the setup doc link, and instruments the end-of-session EDS event (accessibility_plugin_not_loaded) so such builds can be excluded from stability queries. Detection is code-based: the plugin sets BROWSERSTACK_ACCESSIBILITY_PLUGIN_LOADED when its module is loaded/invoked by the cypress config; requireModule writes a flag file the parent process reads back. Falls back to a raw-source scan only when the config could not be required (e.g. TS before bstack packages are installed). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

…-cli into disable-accessibility-when-plugin-not-loaded

+};
+
+const isBindingCalled = (content, binding) => {
+  const callRegex = new RegExp('\\b' + binding.replace(/[.*+?^${}()|[\]\\]/g, '\\$&') + '\\s*\\(');


kamal-kaur04

Automated SDK-style review — accessibility-plugin-not-loaded detection.

Design is sound and degrades safely; nothing build-breaking. Inline notes below: 2 warnings worth resolving before merge (false-negative on unusual import syntax; redundant TS config compile on every a11y run) and 3 cleanup suggestions. Posted as comments — final approve/merge call is the human reviewer's.

kamal-kaur04 · 2026-06-23T10:04:43Z

+    // If accessibility is requested but the BrowserStack accessibility plugin is
+    // not loaded in the cypress config, explicitly disable accessibility before
+    // the build start event so the build is not treated as an accessibility build.
+    if (isAccessibilitySession && isBrowserstackInfra && !isAccessibilityPluginLoaded(bsConfig)) {


⚠️ Warning — redundant config read/compile on every a11y build.

isAccessibilityPluginLoaded(bsConfig) calls readCypressConfigFile, which is already invoked at capabilityHelper.js:251 and helper.js:378 during a normal run. This adds a third call. For a TS config each call runs convertTsConfig → cp.execSync(tsc …) (a fresh sync TypeScript compile) plus a child-process require that re-executes the user config's top-level side-effects.

Consider reading the config once and threading the detection signal through, or memoizing on BROWSERSTACK_ACCESSIBILITY_PLUGIN_LOADED once an earlier read in the same run has populated it, only forcing a fresh read when it's still undefined.

added a fix for this

kamal-kaur04 · 2026-06-23T10:04:43Z

+
+// Finds the symbol the accessibility plugin is imported as, via require() or
+// import, regardless of path style. Returns the binding name or null.
+const getAccessibilityPluginBinding = (content) => {


⚠️ Warning — binding regex misses valid import forms → silent a11y disable.

These regexes only match const/let/var X = require('…/plugin') and import X from '…/plugin'. They do not match import * as X from …, import { X } from …, or dynamic await import(…).

On the require-succeeded path this is benign (the invocation scan is lenient). But on the inconclusive path (TS config before bstack packages are installed), isAccessibilityPluginImportedAndCalledInSource is strict and returns false when the binding isn't found — so a correctly-wired config using namespace/dynamic import gets accessibility silently disabled (a billed feature).

Suggest widening the regex to cover import * as X / import { X }, or biasing the strict fallback toward keeping a11y on when the plugin path is present in source at all.

kamal-kaur04 · 2026-06-23T10:04:43Z

+// Strip JS/TS comments so that commented-out plugin imports/calls are ignored
+// by the static scans below. Best-effort: handles block and line comments while
+// avoiding `://` in URLs.
+const stripComments = (src) => {


💡 Suggestion — stripComments can corrupt string literals.

The :// guard protects URLs, but a // inside a non-URL string (e.g. const re = "a//b") is still stripped, and the block-comment pass strips /* */ inside strings too. ^ isn't multiline-anchored (a preceding \n satisfies [^:], so line-start comments are still handled). Impact is low since the require-load marker is the primary signal — worth a comment noting the scan is intentionally lossy.

Ignore, a suggestion comment

kamal-kaur04 · 2026-06-23T10:04:43Z

+// sending the build start event, and uses this marker to determine whether the
+// accessibility plugin is actually wired in. Unlike a static text scan of the
+// config file, this does NOT false-positive on commented-out requires.
+process.env.BROWSERSTACK_ACCESSIBILITY_PLUGIN_LOADED = 'true';


💡 Suggestion — load-bearing process.env mutation as an import side-effect.

Setting BROWSERSTACK_ACCESSIBILITY_PLUGIN_LOADED='true' at module top level is the intended detection mechanism, but a global mutation on require is easy to miss. Since the whole feature depends on it, consider making the invariant unmissable so anyone refactoring this module preserves it.

kamal-kaur04 · 2026-06-23T10:04:43Z

+try {
+    const accessibilityPluginLoaded = process.env.BROWSERSTACK_ACCESSIBILITY_PLUGIN_LOADED === 'true';
+    fs.writeFileSync(
+        config.accessibilityPluginFlagFileName,


💡 Suggestion — temp flag file can be left behind.

tmpA11yPluginLoaded.json is unlinked only inside loadJsFile after a successful read. If loadJsFile returns early (e.g. the configJsonFileName read throws before the propagation block), the file lingers, and a stale file from a crashed prior run could be read on a path that bypasses a fresh write. The child overwrites it on the normal path, so risk is low — but a guaranteed cleanup (or writing under an os-temp dir) would be safer.

added a fix for this

- Memoize parsed cypress config in readCypressConfigFile (module-level cache keyed by resolved path) so the detection, capabilityHelper and video-config call sites share a single compile + child require per run. - Widen static binding detection to cover `import * as X` and bias the strict source-scan fallback toward keeping accessibility enabled when the plugin path is present but the binding can't be statically parsed (named/dynamic imports), avoiding silently disabling a billed feature. Still disables on positive evidence (path absent, or binding parsed and never called). - Guarantee cleanup of the detection flag file: delete any stale flag before the child runs, and unlink it in readCypressConfigFile's finally block. - Document the stripComments scrubber as intentionally best-effort/lossy. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

kamal-kaur04

Approving — both warnings from the earlier review are properly resolved in 6bea7529:

Redundant config read/compile → fixed via a parsedCypressConfigCache memoization in readCypressConfigUtil.js; the TS compile + child-process require now runs once per run across all three call sites.
Binding-regex false-negative → fixed; namespace imports now matched, and the strict fallback biases toward keeping accessibility on when the import binding can't be confidently parsed, so a billed feature is no longer silently disabled on a lossy scan.
Temp flag file → guaranteed cleanup added (pre-run unlink + finally block).
stripComments → documented as intentionally lossy with a clear rationale.

Non-blocking nits: consider re-adding a one-line comment on the process.env.BROWSERSTACK_ACCESSIBILITY_PLUGIN_LOADED marker in plugin/index.js so the detection invariant isn't silently fragile, and mark the Semgrep new RegExp finding resolved (binding is an escaped identifier — ReDoS isn't reachable). Neither blocks merge.

LGTM. 👍

pranay-v29 requested a review from a team as a code owner June 22, 2026 05:58

pranay-v29 requested review from kamal-kaur04 and rounak610 June 22, 2026 05:58

pranay-v29 added 2 commits June 22, 2026 11:29

Merge branch 'master' of github.com:browserstack/browserstack-cypress…

34aaa6f

…-cli into disable-accessibility-when-plugin-not-loaded

fixed the case of plugin loaded but not invoked

a7bea68

github-advanced-security AI found potential problems Jun 22, 2026

View reviewed changes

Comment thread bin/accessibility-automation/helper.js

};

const isBindingCalled = (content, binding) => {

const callRegex = new RegExp('\\b' + binding.replace(/[.*+?^${}()|[\]\\]/g, '\\$&') + '\\s*\\(');

kamal-kaur04 reviewed Jun 23, 2026

View reviewed changes

pranay-v29 requested review from shivam5643 and removed request for rounak610 June 24, 2026 11:07

kamal-kaur04 approved these changes Jun 25, 2026

View reviewed changes

shivam5643 approved these changes Jun 25, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Disable accessibility when browserstackAccessibility plugin not loaded#1132

Disable accessibility when browserstackAccessibility plugin not loaded#1132
pranay-v29 wants to merge 4 commits into
masterfrom
disable-accessibility-when-plugin-not-loaded

pranay-v29 commented Jun 22, 2026 •

edited

Loading

Uh oh!

kamal-kaur04 left a comment

Uh oh!

kamal-kaur04 Jun 23, 2026

Uh oh!

pranay-v29 Jun 25, 2026

Uh oh!

kamal-kaur04 Jun 23, 2026

Uh oh!

kamal-kaur04 Jun 23, 2026

Uh oh!

pranay-v29 Jun 25, 2026

Uh oh!

kamal-kaur04 Jun 23, 2026

Uh oh!

pranay-v29 Jun 25, 2026

Uh oh!

kamal-kaur04 Jun 23, 2026

Uh oh!

pranay-v29 Jun 25, 2026

Uh oh!

kamal-kaur04 left a comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Uh oh!

Conversation

pranay-v29 commented Jun 22, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

kamal-kaur04 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

kamal-kaur04 left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

pranay-v29 commented Jun 22, 2026 •

edited

Loading