From 45a3aa6fb6743bb576cfbb9902c880ad45244226 Mon Sep 17 00:00:00 2001 From: Harshit Date: Mon, 20 Jul 2026 14:58:56 +0530 Subject: [PATCH 1/5] fix: detect git branch on detached-HEAD CI checkouts (SDK-7009) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit CI systems check out a commit SHA directly (`git checkout `), leaving the repo in a detached-HEAD state. There, `.git/HEAD` holds a raw SHA rather than `ref: refs/heads/`, so git-repo-info returns no branch and `getGitMetaData` reported `version_control.branch = null`. Builds then dropped out of branch-based dashboards and filters even though they were on master. Add a branch fallback in `getGitMetaData`: when git-repo-info yields no branch, resolve it from the CI provider's branch env var (covering the CI systems getCiInfo already detects, plus an explicit BROWSERSTACK_GIT_BRANCH override), then from a best-effort `git for-each-ref --points-at HEAD` backstop. Values are sanitized via normalizeBranchName (strips refs/heads|origin/, rejects unsafe input). Fully graceful — never throws, degrades to the prior behaviour. Co-Authored-By: Claude Opus 4.8 --- .changeset/sdk-7009-ci-branch-detection.md | 5 + packages/browserstack-service/src/util.ts | 96 ++++++++++++++++++- .../browserstack-service/tests/util.test.ts | 31 ++++++ 3 files changed, 131 insertions(+), 1 deletion(-) create mode 100644 .changeset/sdk-7009-ci-branch-detection.md diff --git a/.changeset/sdk-7009-ci-branch-detection.md b/.changeset/sdk-7009-ci-branch-detection.md new file mode 100644 index 0000000..04f18b1 --- /dev/null +++ b/.changeset/sdk-7009-ci-branch-detection.md @@ -0,0 +1,5 @@ +--- +"@wdio/browserstack-service": patch +--- + +Detect the git branch in detached-HEAD CI checkouts. CI systems typically check out a commit SHA directly (`git checkout `), leaving the repo in a detached-HEAD state where `git-repo-info` cannot resolve a branch — so builds reported no branch and dropped out of branch-based dashboards and filters. `getGitMetaData` now falls back to the CI provider's branch env var (with an explicit `BROWSERSTACK_GIT_BRANCH` override) and a `git for-each-ref` backstop. (SDK-7009) diff --git a/packages/browserstack-service/src/util.ts b/packages/browserstack-service/src/util.ts index 136eebd..5c2a651 100644 --- a/packages/browserstack-service/src/util.ts +++ b/packages/browserstack-service/src/util.ts @@ -5,6 +5,7 @@ import zlib from 'node:zlib' import { format, promisify } from 'node:util' import path from 'node:path' import util from 'node:util' +import { spawnSync } from 'node:child_process' import type { Capabilities, Frameworks, Options } from '@wdio/types' import type { BeforeCommandArgs, AfterCommandArgs } from '@wdio/reporter' @@ -981,6 +982,90 @@ export function getCiInfo () { return null } +// Branch names may only contain these characters; anything else is treated as +// untrusted input and rejected (env vars/CI values are attacker-influenceable). +const SAFE_BRANCH_PATTERN = /^[\w./-]+$/ + +// CI branch env vars in priority order — covers the CI systems getCiInfo() +// detects. In a detached-HEAD CI checkout git-repo-info can't resolve the +// branch, but the CI system almost always exposes it via one of these. +const CI_BRANCH_ENV_VARS = [ + 'BROWSERSTACK_GIT_BRANCH', // explicit override (support/customer escape hatch) + 'GITHUB_HEAD_REF', // GitHub Actions (PR) + 'GITHUB_REF_NAME', // GitHub Actions (push/tag) + 'CI_COMMIT_REF_NAME', // GitLab + 'CI_COMMIT_BRANCH', // GitLab + 'CIRCLE_BRANCH', // CircleCI + 'TRAVIS_PULL_REQUEST_BRANCH', // Travis (PR) + 'TRAVIS_BRANCH', // Travis + 'BITBUCKET_BRANCH', // Bitbucket + 'DRONE_COMMIT_BRANCH', // Drone + 'DRONE_BRANCH', // Drone + 'SEMAPHORE_GIT_BRANCH', // Semaphore + 'BUILDKITE_BRANCH', // Buildkite + 'BUILD_SOURCEBRANCHNAME', // Azure DevOps / VSTS (clean name) + 'BUILD_SOURCEBRANCH', // Azure DevOps / VSTS (refs/heads/...) + 'APPVEYOR_REPO_BRANCH', // Appveyor + 'CODEBUILD_WEBHOOK_HEAD_REF', // AWS CodeBuild + 'bamboo_planRepository_branchName', // Bamboo + 'bamboo_repository_branch_name', // Bamboo + 'WERCKER_GIT_BRANCH', // Wercker + 'VERCEL_GIT_COMMIT_REF', // Vercel + 'BRANCH', // Netlify / generic + 'GIT_LOCAL_BRANCH', // Jenkins git plugin (clean) + 'BRANCH_NAME', // Jenkins multibranch + 'GIT_BRANCH', // Jenkins (often origin/) +] + +// Strip refs/heads|tags and a leading origin/, reject HEAD and unsafe values. +export function normalizeBranchName (raw?: string | null): string | undefined { + if (!raw) { + return undefined + } + const branch = raw.trim() + .replace(/^refs\/(heads|tags)\//, '') + .replace(/^origin\//, '') + if (!branch || branch === 'HEAD' || !SAFE_BRANCH_PATTERN.test(branch)) { + return undefined + } + return branch +} + +function getBranchFromCIEnv (): string | undefined { + for (const key of CI_BRANCH_ENV_VARS) { + const val = normalizeBranchName(process.env[key]) + if (val) { + return val + } + } + return undefined +} + +// Best-effort CI-agnostic fallback: the branch whose tip is the checked-out +// commit. Resolves detached-HEAD checkouts where the commit is a branch tip +// (the common merge-to-master → CI-builds-master case). Never throws. +function getBranchFromGit (cwd?: string): string | undefined { + try { + const result = spawnSync( + 'git', + ['for-each-ref', '--points-at', 'HEAD', '--format=%(refname:short)', 'refs/heads', 'refs/remotes'], + { cwd, encoding: 'utf-8', timeout: 5000 } + ) + if (result.status !== 0 || !result.stdout) { + return undefined + } + for (const line of result.stdout.split('\n')) { + const branch = normalizeBranchName(line) + if (branch) { + return branch + } + } + } catch { + // best-effort — degrade silently + } + return undefined +} + export async function getGitMetaData () { const info: GitRepoInfo = gitRepoInfo() if (!info.commonGitDir) { @@ -989,11 +1074,20 @@ export async function getGitMetaData () { const { remote } = await pGitconfig(info.commonGitDir) const remotes = remote ? Object.keys(remote).map(remoteName => ({ name: remoteName, url: remote[remoteName].url })) : [] + // git-repo-info reads branch from .git/HEAD; on a detached HEAD (the default + // for most CI checkouts, e.g. `git checkout `) it returns no branch. + // Fall back to CI env vars, then to the branch whose tip is HEAD, so builds + // don't drop out of branch-based dashboards/filters. Ref: SDK-7009. + let branch = info.branch + if (!branch) { + branch = getBranchFromCIEnv() ?? getBranchFromGit(info.root) ?? info.branch + } + let gitMetaData : GitMetaData = { name: 'git', sha: info.sha, short_sha: info.abbreviatedSha, - branch: info.branch, + branch: branch, tag: info.tag, committer: info.committer, committer_date: info.committerDate, diff --git a/packages/browserstack-service/tests/util.test.ts b/packages/browserstack-service/tests/util.test.ts index 1260156..368541b 100644 --- a/packages/browserstack-service/tests/util.test.ts +++ b/packages/browserstack-service/tests/util.test.ts @@ -68,6 +68,11 @@ const log = logger('test') vi.mock('fetch') vi.mock('git-repo-info') +// getGitMetaData promisifies gitconfiglocal; the repo also mocks `fs`, so the +// real gitconfiglocal cannot read a config file. Resolve it with no remotes. +vi.mock('gitconfiglocal', () => ({ + default: (_dir: string, cb: (err: Error | null, config: unknown) => void) => cb(null, { remote: {} }) +})) // Fake only Date (not `performance`): these tests need a deterministic system // clock but never advance timers. Faking `performance` too makes performance.now() // negative under the 2020 system time, which Node 18's perf_hooks rejects with @@ -958,6 +963,32 @@ describe('getGitMetaData', () => { // } }) + + // SDK-7009: git-repo-info returns no branch on a detached HEAD (the default + // for CI checkouts). getGitMetaData must fall back to CI env vars so the + // build's version_control still reports the branch. + it('uses git-repo-info branch when present (no fallback)', async () => { + delete process.env.BROWSERSTACK_GIT_BRANCH + vi.mocked(gitRepoInfo).mockReturnValue({ commonGitDir: '/tmp', worktreeGitDir: '/tmp', branch: 'develop', sha: 'sha1' } as any) + const result: any = await getGitMetaData() + expect(result.branch).toEqual('develop') + }) + + it('falls back to CI branch env var on detached HEAD', async () => { + process.env.BROWSERSTACK_GIT_BRANCH = 'master' + vi.mocked(gitRepoInfo).mockReturnValue({ commonGitDir: '/tmp', worktreeGitDir: '/tmp', branch: undefined, sha: 'sha1' } as any) + const result: any = await getGitMetaData() + expect(result.branch).toEqual('master') + delete process.env.BROWSERSTACK_GIT_BRANCH + }) + + it('normalizes CI env branch (strips refs/heads/ and origin/)', async () => { + process.env.BROWSERSTACK_GIT_BRANCH = 'refs/heads/release/1.2' + vi.mocked(gitRepoInfo).mockReturnValue({ commonGitDir: '/tmp', worktreeGitDir: '/tmp', branch: undefined, sha: 'sha1' } as any) + const result: any = await getGitMetaData() + expect(result.branch).toEqual('release/1.2') + delete process.env.BROWSERSTACK_GIT_BRANCH + }) }) describe('getHookType', () => { From 18ff9952f59951ec78588a12d3766782d4e9c2df Mon Sep 17 00:00:00 2001 From: Harshit Date: Mon, 20 Jul 2026 16:32:26 +0530 Subject: [PATCH 2/5] =?UTF-8?q?fix(SDK-7009):=20address=20SDK=20PR=20revie?= =?UTF-8?q?w=20=E2=80=94=20branch=20fallback=20correctness=20+=20tests?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - BROWSERSTACK_GIT_BRANCH now overrides even a git-repo-info-resolved branch (moved ahead of info.branch), matching its documented "override" role. - GitLab: prefer CI_COMMIT_BRANCH over CI_COMMIT_REF_NAME (the latter is the tag name on tag pipelines). - Split CI branch env vars into provider-specific (trusted directly) vs generic (BRANCH/GIT_BRANCH/…, only trusted when getCiInfo() detects a CI) to stop a stray local env var being misreported as the branch. - Add CodeFresh CF_BRANCH (parity with getCiInfo detection). - git backstop: query refs/heads (lstrip=2) then refs/remotes (lstrip=3) as two ordered calls, yielding bare branch names for ANY remote (not just origin) and making heads-before-remotes explicit rather than relying on refspec order. - normalizeBranchName: denylist unsafe/illegal ref chars instead of an allowlist, so legal names (e.g. containing '#') are no longer dropped; kept internal (un-exported) so this stays a patch. - Tests: cover the git for-each-ref backstop (mocked spawnSync) and the null-result path; assert override precedence; all env mutations via a save/restore helper (try/finally) to prevent cross-test leakage. Co-Authored-By: Claude Opus 4.8 --- .changeset/sdk-7009-ci-branch-detection.md | 2 +- packages/browserstack-service/src/util.ts | 109 ++++++++++++------ .../browserstack-service/tests/util.test.ts | 100 +++++++++++++--- 3 files changed, 156 insertions(+), 55 deletions(-) diff --git a/.changeset/sdk-7009-ci-branch-detection.md b/.changeset/sdk-7009-ci-branch-detection.md index 04f18b1..e75d912 100644 --- a/.changeset/sdk-7009-ci-branch-detection.md +++ b/.changeset/sdk-7009-ci-branch-detection.md @@ -2,4 +2,4 @@ "@wdio/browserstack-service": patch --- -Detect the git branch in detached-HEAD CI checkouts. CI systems typically check out a commit SHA directly (`git checkout `), leaving the repo in a detached-HEAD state where `git-repo-info` cannot resolve a branch — so builds reported no branch and dropped out of branch-based dashboards and filters. `getGitMetaData` now falls back to the CI provider's branch env var (with an explicit `BROWSERSTACK_GIT_BRANCH` override) and a `git for-each-ref` backstop. (SDK-7009) +Detect the git branch in detached-HEAD CI checkouts. CI systems typically check out a commit SHA directly (`git checkout `), leaving the repo in a detached-HEAD state where `git-repo-info` cannot resolve a branch — so builds reported no branch and dropped out of branch-based dashboards and filters. `getGitMetaData` now resolves the branch in this precedence: an explicit `BROWSERSTACK_GIT_BRANCH` override (which wins even over a git-repo-info-resolved branch, so it can correct a mis-detected one) → `git-repo-info` → the CI provider's branch env var → a `git for-each-ref --points-at HEAD` backstop. Provider-specific env vars are trusted directly; generic names (`BRANCH`, `GIT_BRANCH`, …) are only used when a CI environment is detected. (SDK-7009) diff --git a/packages/browserstack-service/src/util.ts b/packages/browserstack-service/src/util.ts index 5c2a651..c60a55a 100644 --- a/packages/browserstack-service/src/util.ts +++ b/packages/browserstack-service/src/util.ts @@ -982,19 +982,22 @@ export function getCiInfo () { return null } -// Branch names may only contain these characters; anything else is treated as -// untrusted input and rejected (env vars/CI values are attacker-influenceable). -const SAFE_BRANCH_PATTERN = /^[\w./-]+$/ - -// CI branch env vars in priority order — covers the CI systems getCiInfo() -// detects. In a detached-HEAD CI checkout git-repo-info can't resolve the -// branch, but the CI system almost always exposes it via one of these. -const CI_BRANCH_ENV_VARS = [ - 'BROWSERSTACK_GIT_BRANCH', // explicit override (support/customer escape hatch) - 'GITHUB_HEAD_REF', // GitHub Actions (PR) - 'GITHUB_REF_NAME', // GitHub Actions (push/tag) - 'CI_COMMIT_REF_NAME', // GitLab - 'CI_COMMIT_BRANCH', // GitLab +// Reject anything a git ref can't legally be OR that is unsafe to propagate: +// whitespace/control chars, the git-forbidden set (~ ^ : ? * [ \), and `..`. +// This is a denylist (git ref names are otherwise permissive — `#`, `+`, `/`, +// `-`, `_`, `.` are all valid), so a legal branch name is never dropped. +// eslint-disable-next-line no-control-regex -- control chars (\x00-\x1f) are exactly what we reject +const UNSAFE_BRANCH_PATTERN = /[\x00-\x20~^:?*[\\]/ + +// CI-specific, namespaced branch env vars in priority order. These are unique +// to their CI provider, so reading them unconditionally is safe. Values from a +// recognised CI's own variable are trusted even when getCiInfo() can't classify +// the run (some providers set the branch var but not the marker getCiInfo keys on). +const CI_SPECIFIC_BRANCH_ENV = [ + 'GITHUB_HEAD_REF', // GitHub Actions (PR — source branch) + 'GITHUB_REF_NAME', // GitHub Actions (push) + 'CI_COMMIT_BRANCH', // GitLab (branch pipelines only — empty on tag pipelines) + 'CI_COMMIT_REF_NAME', // GitLab (also set to the tag name on tag pipelines) 'CIRCLE_BRANCH', // CircleCI 'TRAVIS_PULL_REQUEST_BRANCH', // Travis (PR) 'TRAVIS_BRANCH', // Travis @@ -1011,57 +1014,89 @@ const CI_BRANCH_ENV_VARS = [ 'bamboo_repository_branch_name', // Bamboo 'WERCKER_GIT_BRANCH', // Wercker 'VERCEL_GIT_COMMIT_REF', // Vercel - 'BRANCH', // Netlify / generic + 'CF_BRANCH', // CodeFresh +] + +// Generic, non-namespaced branch env vars. These collide with unrelated +// variables a developer might have exported locally, so they are only trusted +// when getCiInfo() confirms we are actually inside a recognised CI run. +const CI_GENERIC_BRANCH_ENV = [ 'GIT_LOCAL_BRANCH', // Jenkins git plugin (clean) 'BRANCH_NAME', // Jenkins multibranch 'GIT_BRANCH', // Jenkins (often origin/) + 'BRANCH', // Netlify / generic ] -// Strip refs/heads|tags and a leading origin/, reject HEAD and unsafe values. -export function normalizeBranchName (raw?: string | null): string | undefined { +// Strip a leading refs/heads|tags/ or origin/ prefix (Jenkins GIT_BRANCH is +// `origin/`), reject HEAD and anything that isn't a legal/safe ref. +function normalizeBranchName (raw?: string | null): string | undefined { if (!raw) { return undefined } const branch = raw.trim() .replace(/^refs\/(heads|tags)\//, '') .replace(/^origin\//, '') - if (!branch || branch === 'HEAD' || !SAFE_BRANCH_PATTERN.test(branch)) { + if (!branch || branch.length > 256 || branch === 'HEAD' || branch.includes('..') || UNSAFE_BRANCH_PATTERN.test(branch)) { return undefined } return branch } function getBranchFromCIEnv (): string | undefined { - for (const key of CI_BRANCH_ENV_VARS) { + for (const key of CI_SPECIFIC_BRANCH_ENV) { const val = normalizeBranchName(process.env[key]) if (val) { return val } } + // Generic names are only trusted inside a recognised CI — otherwise a stray + // local `BRANCH`/`GIT_BRANCH` export would be misreported as the git branch. + if (getCiInfo() !== null) { + for (const key of CI_GENERIC_BRANCH_ENV) { + const val = normalizeBranchName(process.env[key]) + if (val) { + return val + } + } + } return undefined } +function gitForEachRef (args: string[], cwd?: string): string[] { + try { + const result = spawnSync('git', ['for-each-ref', '--points-at', 'HEAD', ...args], { + cwd, encoding: 'utf-8', timeout: 5000 + }) + if (result.status !== 0 || !result.stdout) { + return [] + } + return result.stdout.split('\n').map(s => s.trim()).filter(Boolean) + } catch { + // best-effort — degrade silently + return [] + } +} + // Best-effort CI-agnostic fallback: the branch whose tip is the checked-out // commit. Resolves detached-HEAD checkouts where the commit is a branch tip // (the common merge-to-master → CI-builds-master case). Never throws. +// Local heads are queried before remotes; each format strips the ref prefix so +// the value is a bare branch name regardless of the remote (lstrip handles any +// remote, not just `origin`). git orders its own output alphabetically, so the +// two separate calls — not the refspec argument order — control heads-first. function getBranchFromGit (cwd?: string): string | undefined { - try { - const result = spawnSync( - 'git', - ['for-each-ref', '--points-at', 'HEAD', '--format=%(refname:short)', 'refs/heads', 'refs/remotes'], - { cwd, encoding: 'utf-8', timeout: 5000 } - ) - if (result.status !== 0 || !result.stdout) { - return undefined + const candidates = [ + ...gitForEachRef(['--format=%(refname:lstrip=2)', 'refs/heads'], cwd), + ...gitForEachRef(['--format=%(refname:lstrip=3)', 'refs/remotes'], cwd), + ] + for (const candidate of candidates) { + if (candidate === 'HEAD') { + continue // refs/remotes//HEAD → skip the symbolic pointer } - for (const line of result.stdout.split('\n')) { - const branch = normalizeBranchName(line) - if (branch) { - return branch - } + const branch = normalizeBranchName(candidate) + if (branch) { + return branch } - } catch { - // best-effort — degrade silently } return undefined } @@ -1076,9 +1111,11 @@ export async function getGitMetaData () { // git-repo-info reads branch from .git/HEAD; on a detached HEAD (the default // for most CI checkouts, e.g. `git checkout `) it returns no branch. - // Fall back to CI env vars, then to the branch whose tip is HEAD, so builds - // don't drop out of branch-based dashboards/filters. Ref: SDK-7009. - let branch = info.branch + // Precedence: explicit BROWSERSTACK_GIT_BRANCH override (wins even over a + // git-repo-info-resolved branch, so it can correct a mis-resolved one) → + // git-repo-info → CI branch env vars → branch whose tip is HEAD. Keeps builds + // in branch-based dashboards/filters. Ref: SDK-7009. + let branch = normalizeBranchName(process.env.BROWSERSTACK_GIT_BRANCH) || info.branch if (!branch) { branch = getBranchFromCIEnv() ?? getBranchFromGit(info.root) ?? info.branch } diff --git a/packages/browserstack-service/tests/util.test.ts b/packages/browserstack-service/tests/util.test.ts index 368541b..6f88c8a 100644 --- a/packages/browserstack-service/tests/util.test.ts +++ b/packages/browserstack-service/tests/util.test.ts @@ -3,6 +3,7 @@ import type { LaunchResponse } from '../src/types.js' import { describe, expect, it, vi, beforeEach, afterEach, beforeAll, afterAll } from 'vitest' import gitRepoInfo from 'git-repo-info' +import { spawnSync } from 'node:child_process' import CrashReporter from '../src/crash-reporter.js' import logger from '@wdio/logger' import * as utils from '../src/util.js' @@ -73,6 +74,12 @@ vi.mock('git-repo-info') vi.mock('gitconfiglocal', () => ({ default: (_dir: string, cb: (err: Error | null, config: unknown) => void) => cb(null, { remote: {} }) })) +// Mock only spawnSync (getGitMetaData's git for-each-ref backstop); keep the +// rest of node:child_process real so unrelated imports are unaffected. +vi.mock('node:child_process', async (importActual) => { + const actual = await importActual>() + return { ...actual, spawnSync: vi.fn() } +}) // Fake only Date (not `performance`): these tests need a deterministic system // clock but never advance timers. Faking `performance` too makes performance.now() // negative under the 2020 system time, which Node 18's perf_hooks rejects with @@ -965,29 +972,86 @@ describe('getGitMetaData', () => { }) // SDK-7009: git-repo-info returns no branch on a detached HEAD (the default - // for CI checkouts). getGitMetaData must fall back to CI env vars so the - // build's version_control still reports the branch. + // for CI checkouts). getGitMetaData must recover the branch so the build's + // version_control still reports it. All env mutations are restored in + // finally so a failed assertion can't leak state into other tests. + + // Every branch-carrying env var the fallback reads. Cleared before the + // detached-HEAD tests so the ambient CI (e.g. GitHub Actions sets + // GITHUB_REF_NAME) can't pre-empt the code path under test. + const BRANCH_ENV_KEYS = [ + 'BROWSERSTACK_GIT_BRANCH', 'GITHUB_HEAD_REF', 'GITHUB_REF_NAME', 'CI_COMMIT_BRANCH', + 'CI_COMMIT_REF_NAME', 'CIRCLE_BRANCH', 'TRAVIS_PULL_REQUEST_BRANCH', 'TRAVIS_BRANCH', + 'BITBUCKET_BRANCH', 'DRONE_COMMIT_BRANCH', 'DRONE_BRANCH', 'SEMAPHORE_GIT_BRANCH', + 'BUILDKITE_BRANCH', 'BUILD_SOURCEBRANCHNAME', 'BUILD_SOURCEBRANCH', 'APPVEYOR_REPO_BRANCH', + 'CODEBUILD_WEBHOOK_HEAD_REF', 'bamboo_planRepository_branchName', 'bamboo_repository_branch_name', + 'WERCKER_GIT_BRANCH', 'VERCEL_GIT_COMMIT_REF', 'CF_BRANCH', + 'GIT_LOCAL_BRANCH', 'BRANCH_NAME', 'GIT_BRANCH', 'BRANCH', + ] + const withEnv = async (overrides: Record, fn: () => Promise) => { + const saved: Record = {} + for (const k of BRANCH_ENV_KEYS) { saved[k] = process.env[k]; delete process.env[k] } + for (const [k, v] of Object.entries(overrides)) { + if (v === undefined) { delete process.env[k] } else { process.env[k] = v } + } + try { await fn() } finally { + for (const k of BRANCH_ENV_KEYS) { + if (saved[k] === undefined) { delete process.env[k] } else { process.env[k] = saved[k] } + } + } + } + it('uses git-repo-info branch when present (no fallback)', async () => { - delete process.env.BROWSERSTACK_GIT_BRANCH - vi.mocked(gitRepoInfo).mockReturnValue({ commonGitDir: '/tmp', worktreeGitDir: '/tmp', branch: 'develop', sha: 'sha1' } as any) - const result: any = await getGitMetaData() - expect(result.branch).toEqual('develop') + await withEnv({}, async () => { + vi.mocked(gitRepoInfo).mockReturnValue({ commonGitDir: '/tmp', worktreeGitDir: '/tmp', branch: 'develop', sha: 'sha1' } as any) + const result: any = await getGitMetaData() + expect(result.branch).toEqual('develop') + }) }) - it('falls back to CI branch env var on detached HEAD', async () => { - process.env.BROWSERSTACK_GIT_BRANCH = 'master' - vi.mocked(gitRepoInfo).mockReturnValue({ commonGitDir: '/tmp', worktreeGitDir: '/tmp', branch: undefined, sha: 'sha1' } as any) - const result: any = await getGitMetaData() - expect(result.branch).toEqual('master') - delete process.env.BROWSERSTACK_GIT_BRANCH + it('BROWSERSTACK_GIT_BRANCH overrides even a git-repo-info-resolved branch', async () => { + await withEnv({ BROWSERSTACK_GIT_BRANCH: 'release/1.2' }, async () => { + vi.mocked(gitRepoInfo).mockReturnValue({ commonGitDir: '/tmp', worktreeGitDir: '/tmp', branch: 'wrong-branch', sha: 'sha1' } as any) + const result: any = await getGitMetaData() + expect(result.branch).toEqual('release/1.2') + }) }) - it('normalizes CI env branch (strips refs/heads/ and origin/)', async () => { - process.env.BROWSERSTACK_GIT_BRANCH = 'refs/heads/release/1.2' - vi.mocked(gitRepoInfo).mockReturnValue({ commonGitDir: '/tmp', worktreeGitDir: '/tmp', branch: undefined, sha: 'sha1' } as any) - const result: any = await getGitMetaData() - expect(result.branch).toEqual('release/1.2') - delete process.env.BROWSERSTACK_GIT_BRANCH + it('falls back to a CI branch env var on detached HEAD', async () => { + await withEnv({ GITHUB_REF_NAME: 'master' }, async () => { + vi.mocked(gitRepoInfo).mockReturnValue({ commonGitDir: '/tmp', worktreeGitDir: '/tmp', branch: undefined, sha: 'sha1' } as any) + const result: any = await getGitMetaData() + expect(result.branch).toEqual('master') + }) + }) + + it('normalizes CI env branch (strips refs/heads/ prefix)', async () => { + await withEnv({ BROWSERSTACK_GIT_BRANCH: 'refs/heads/release/1.2' }, async () => { + vi.mocked(gitRepoInfo).mockReturnValue({ commonGitDir: '/tmp', worktreeGitDir: '/tmp', branch: undefined, sha: 'sha1' } as any) + const result: any = await getGitMetaData() + expect(result.branch).toEqual('release/1.2') + }) + }) + + it('falls back to the git for-each-ref backstop when no branch env is set', async () => { + await withEnv({}, async () => { + // No CI branch env → getBranchFromGit runs. Mock the git backstop to + // report the branch whose tip is HEAD (local heads queried first). + vi.mocked(spawnSync).mockReturnValueOnce({ status: 0, stdout: 'master\n', stderr: '' } as any) + vi.mocked(gitRepoInfo).mockReturnValue({ commonGitDir: '/tmp', worktreeGitDir: '/tmp', branch: undefined, root: '/tmp', sha: 'sha1' } as any) + const result: any = await getGitMetaData() + expect(result.branch).toEqual('master') + expect(vi.mocked(spawnSync)).toHaveBeenCalledWith('git', expect.arrayContaining(['for-each-ref', '--points-at', 'HEAD']), expect.anything()) + }) + }) + + it('leaves branch undefined when detached HEAD has no env and no branch tip', async () => { + await withEnv({}, async () => { + vi.mocked(spawnSync).mockReturnValue({ status: 0, stdout: '', stderr: '' } as any) + vi.mocked(gitRepoInfo).mockReturnValue({ commonGitDir: '/tmp', worktreeGitDir: '/tmp', branch: undefined, root: '/tmp', sha: 'sha1' } as any) + const result: any = await getGitMetaData() + expect(result.branch).toBeUndefined() + }) }) }) From 8ad2f979f3bbdc451a83d265314245f13266b47b Mon Sep 17 00:00:00 2001 From: Harshit Date: Mon, 20 Jul 2026 17:02:28 +0530 Subject: [PATCH 3/5] =?UTF-8?q?fix(SDK-7009):=20address=20SDK=20PR=20revie?= =?UTF-8?q?w=20round=202=20=E2=80=94=20worktree,=20async=20backstop,=20tes?= =?UTF-8?q?ts?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - getBranchFromGit no longer passes git-repo-info's `.root` (which can point at the main repo dir under a git worktree); the backstop runs in process.cwd(), the actual checkout being instrumented. - Convert the git for-each-ref backstop from synchronous spawnSync to async promisified execFile, and make it lazy (query refs/heads, return early, only then refs/remotes) with a 3s timeout — no blocking subprocess on getGitMetaData's async hot path. - Tests: add coverage for the generic-var anti-collision gate (BRANCH ignored outside a recognised CI; GIT_BRANCH honoured inside Jenkins), CI-specific vs generic priority, and the remotes fall-through; env helper now also neutralises CI-detection markers so getCiInfo() is deterministic under ambient CI. Co-Authored-By: Claude Opus 4.8 --- packages/browserstack-service/src/util.ts | 57 +++++++------ .../browserstack-service/tests/util.test.ts | 85 ++++++++++++++++--- 2 files changed, 106 insertions(+), 36 deletions(-) diff --git a/packages/browserstack-service/src/util.ts b/packages/browserstack-service/src/util.ts index c60a55a..0f7dd92 100644 --- a/packages/browserstack-service/src/util.ts +++ b/packages/browserstack-service/src/util.ts @@ -5,7 +5,7 @@ import zlib from 'node:zlib' import { format, promisify } from 'node:util' import path from 'node:path' import util from 'node:util' -import { spawnSync } from 'node:child_process' +import { execFile } from 'node:child_process' import type { Capabilities, Frameworks, Options } from '@wdio/types' import type { BeforeCommandArgs, AfterCommandArgs } from '@wdio/reporter' @@ -59,6 +59,7 @@ import AccessibilityScripts from './scripts/accessibility-scripts.js' import { _fetch as fetch } from './fetchWrapper.js' const pGitconfig = promisify(gitconfig) +const pExecFile = promisify(execFile) export type GitMetaData = { name: string; @@ -1062,38 +1063,30 @@ function getBranchFromCIEnv (): string | undefined { return undefined } -function gitForEachRef (args: string[], cwd?: string): string[] { +// Async (non-blocking) — a synchronous subprocess on getGitMetaData's hot path +// would stall the event loop. execFile takes an argv array (no shell), so the +// ref args cannot be interpreted by a shell. +async function gitForEachRef (args: string[]): Promise { try { - const result = spawnSync('git', ['for-each-ref', '--points-at', 'HEAD', ...args], { - cwd, encoding: 'utf-8', timeout: 5000 + // No explicit cwd → runs in process.cwd(), the actual checkout the SDK + // is instrumenting. (git-repo-info's `.root` can point at the main repo + // dir under a git worktree, which would resolve the wrong branch.) + const { stdout } = await pExecFile('git', ['for-each-ref', '--points-at', 'HEAD', ...args], { + encoding: 'utf-8', timeout: 3000 }) - if (result.status !== 0 || !result.stdout) { - return [] - } - return result.stdout.split('\n').map(s => s.trim()).filter(Boolean) + return stdout.split('\n').map(s => s.trim()).filter(Boolean) } catch { - // best-effort — degrade silently + // best-effort — degrade silently (git missing, not a repo, timeout, …) return [] } } -// Best-effort CI-agnostic fallback: the branch whose tip is the checked-out -// commit. Resolves detached-HEAD checkouts where the commit is a branch tip -// (the common merge-to-master → CI-builds-master case). Never throws. -// Local heads are queried before remotes; each format strips the ref prefix so -// the value is a bare branch name regardless of the remote (lstrip handles any -// remote, not just `origin`). git orders its own output alphabetically, so the -// two separate calls — not the refspec argument order — control heads-first. -function getBranchFromGit (cwd?: string): string | undefined { - const candidates = [ - ...gitForEachRef(['--format=%(refname:lstrip=2)', 'refs/heads'], cwd), - ...gitForEachRef(['--format=%(refname:lstrip=3)', 'refs/remotes'], cwd), - ] - for (const candidate of candidates) { - if (candidate === 'HEAD') { +const pickBranch = (lines: string[]): string | undefined => { + for (const line of lines) { + if (line === 'HEAD') { continue // refs/remotes//HEAD → skip the symbolic pointer } - const branch = normalizeBranchName(candidate) + const branch = normalizeBranchName(line) if (branch) { return branch } @@ -1101,6 +1094,20 @@ function getBranchFromGit (cwd?: string): string | undefined { return undefined } +// Best-effort CI-agnostic fallback: the branch whose tip is the checked-out +// commit. Resolves detached-HEAD checkouts where the commit is a branch tip +// (the common merge-to-master → CI-builds-master case). Never throws. +// Local heads are queried (and returned) before remotes; each format strips the +// ref prefix via lstrip so the value is a bare branch name for ANY remote, not +// just `origin`. Lazy — the remotes query only runs if no local head matched. +async function getBranchFromGit (): Promise { + const fromHeads = pickBranch(await gitForEachRef(['--format=%(refname:lstrip=2)', 'refs/heads'])) + if (fromHeads) { + return fromHeads + } + return pickBranch(await gitForEachRef(['--format=%(refname:lstrip=3)', 'refs/remotes'])) +} + export async function getGitMetaData () { const info: GitRepoInfo = gitRepoInfo() if (!info.commonGitDir) { @@ -1117,7 +1124,7 @@ export async function getGitMetaData () { // in branch-based dashboards/filters. Ref: SDK-7009. let branch = normalizeBranchName(process.env.BROWSERSTACK_GIT_BRANCH) || info.branch if (!branch) { - branch = getBranchFromCIEnv() ?? getBranchFromGit(info.root) ?? info.branch + branch = getBranchFromCIEnv() ?? (await getBranchFromGit()) ?? info.branch } let gitMetaData : GitMetaData = { diff --git a/packages/browserstack-service/tests/util.test.ts b/packages/browserstack-service/tests/util.test.ts index 6f88c8a..9c37343 100644 --- a/packages/browserstack-service/tests/util.test.ts +++ b/packages/browserstack-service/tests/util.test.ts @@ -3,7 +3,7 @@ import type { LaunchResponse } from '../src/types.js' import { describe, expect, it, vi, beforeEach, afterEach, beforeAll, afterAll } from 'vitest' import gitRepoInfo from 'git-repo-info' -import { spawnSync } from 'node:child_process' +import { execFile } from 'node:child_process' import CrashReporter from '../src/crash-reporter.js' import logger from '@wdio/logger' import * as utils from '../src/util.js' @@ -74,11 +74,11 @@ vi.mock('git-repo-info') vi.mock('gitconfiglocal', () => ({ default: (_dir: string, cb: (err: Error | null, config: unknown) => void) => cb(null, { remote: {} }) })) -// Mock only spawnSync (getGitMetaData's git for-each-ref backstop); keep the -// rest of node:child_process real so unrelated imports are unaffected. +// Mock only execFile (getGitMetaData's async git for-each-ref backstop); keep +// the rest of node:child_process real so unrelated imports are unaffected. vi.mock('node:child_process', async (importActual) => { const actual = await importActual>() - return { ...actual, spawnSync: vi.fn() } + return { ...actual, execFile: vi.fn() } }) // Fake only Date (not `performance`): these tests need a deterministic system // clock but never advance timers. Faking `performance` too makes performance.now() @@ -988,18 +988,44 @@ describe('getGitMetaData', () => { 'WERCKER_GIT_BRANCH', 'VERCEL_GIT_COMMIT_REF', 'CF_BRANCH', 'GIT_LOCAL_BRANCH', 'BRANCH_NAME', 'GIT_BRANCH', 'BRANCH', ] + // CI-detection markers getCiInfo() keys on. Cleared too, so getCiInfo() is + // null unless a test opts a specific CI in — the generic-var gate + // (`getCiInfo() !== null`) is otherwise non-deterministic under ambient CI. + const CI_MARKER_KEYS = [ + 'CI', 'GITHUB_ACTIONS', 'JENKINS_URL', 'JENKINS_HOME', 'CIRCLECI', 'TRAVIS', 'GITLAB_CI', + 'BITBUCKET_BUILD_NUMBER', 'DRONE', 'SEMAPHORE', 'BUILDKITE', 'TF_BUILD', 'APPVEYOR', + 'AZURE_HTTP_USER_AGENT', 'CODEBUILD_BUILD_ID', 'bamboo_buildNumber', 'WERCKER', 'NETLIFY', + 'VERCEL', 'TEAMCITY_VERSION', 'CF_BUILD_ID', 'GO_JOB_NAME', + ] + const ALL_ENV_KEYS = [...BRANCH_ENV_KEYS, ...CI_MARKER_KEYS] const withEnv = async (overrides: Record, fn: () => Promise) => { const saved: Record = {} - for (const k of BRANCH_ENV_KEYS) { saved[k] = process.env[k]; delete process.env[k] } + for (const k of ALL_ENV_KEYS) { saved[k] = process.env[k]; delete process.env[k] } for (const [k, v] of Object.entries(overrides)) { if (v === undefined) { delete process.env[k] } else { process.env[k] = v } } try { await fn() } finally { - for (const k of BRANCH_ENV_KEYS) { + for (const k of ALL_ENV_KEYS) { if (saved[k] === undefined) { delete process.env[k] } else { process.env[k] = saved[k] } } } } + // Mock the async git backstop (promisify(execFile) → resolves {stdout,stderr}). + // A per-call queue lets a test feed the heads query then the remotes query. + const mockGitRefs = (...outputs: string[]) => { + let i = 0 + vi.mocked(execFile).mockImplementation(((...args: unknown[]) => { + const cb = args[args.length - 1] as (e: unknown, r: unknown) => void + const out = i < outputs.length ? outputs[i] : '' + i += 1 + cb(null, { stdout: out, stderr: '' }) + return {} as any + }) as any) + } + + // Default: resolve the git backstop with empty output so any test reaching it + // (without calling mockGitRefs) never hangs on the unresolved-callback mock. + beforeEach(() => mockGitRefs()) it('uses git-repo-info branch when present (no fallback)', async () => { await withEnv({}, async () => { @@ -1035,24 +1061,61 @@ describe('getGitMetaData', () => { it('falls back to the git for-each-ref backstop when no branch env is set', async () => { await withEnv({}, async () => { - // No CI branch env → getBranchFromGit runs. Mock the git backstop to - // report the branch whose tip is HEAD (local heads queried first). - vi.mocked(spawnSync).mockReturnValueOnce({ status: 0, stdout: 'master\n', stderr: '' } as any) + // No CI branch env → getBranchFromGit runs. Local heads query resolves + // the branch whose tip is HEAD. + mockGitRefs('master\n') vi.mocked(gitRepoInfo).mockReturnValue({ commonGitDir: '/tmp', worktreeGitDir: '/tmp', branch: undefined, root: '/tmp', sha: 'sha1' } as any) const result: any = await getGitMetaData() expect(result.branch).toEqual('master') - expect(vi.mocked(spawnSync)).toHaveBeenCalledWith('git', expect.arrayContaining(['for-each-ref', '--points-at', 'HEAD']), expect.anything()) + expect(vi.mocked(execFile)).toHaveBeenCalledWith('git', expect.arrayContaining(['for-each-ref', '--points-at', 'HEAD']), expect.anything(), expect.anything()) + }) + }) + + it('git backstop falls through to remotes (bare name) when no local head matches', async () => { + await withEnv({}, async () => { + // heads query empty → remotes query returns a lstrip-3 bare name for a + // non-origin remote (must not leak the remote prefix). + mockGitRefs('', 'main\n') + vi.mocked(gitRepoInfo).mockReturnValue({ commonGitDir: '/tmp', worktreeGitDir: '/tmp', branch: undefined, root: '/tmp', sha: 'sha1' } as any) + const result: any = await getGitMetaData() + expect(result.branch).toEqual('main') }) }) it('leaves branch undefined when detached HEAD has no env and no branch tip', async () => { await withEnv({}, async () => { - vi.mocked(spawnSync).mockReturnValue({ status: 0, stdout: '', stderr: '' } as any) + mockGitRefs('', '') vi.mocked(gitRepoInfo).mockReturnValue({ commonGitDir: '/tmp', worktreeGitDir: '/tmp', branch: undefined, root: '/tmp', sha: 'sha1' } as any) const result: any = await getGitMetaData() expect(result.branch).toBeUndefined() }) }) + + it('ignores a generic BRANCH env var when NOT in a recognised CI', async () => { + // A stray local `BRANCH` export must not be misreported as the branch. + await withEnv({ BRANCH: 'stray-local-value' }, async () => { + mockGitRefs('', '') // no CI marker set → getCiInfo() is null; backstop also empty + vi.mocked(gitRepoInfo).mockReturnValue({ commonGitDir: '/tmp', worktreeGitDir: '/tmp', branch: undefined, root: '/tmp', sha: 'sha1' } as any) + const result: any = await getGitMetaData() + expect(result.branch).toBeUndefined() + }) + }) + + it('uses a generic GIT_BRANCH var only inside a recognised CI (Jenkins), stripping origin/', async () => { + await withEnv({ JENKINS_URL: 'http://jenkins.local', GIT_BRANCH: 'origin/main' }, async () => { + vi.mocked(gitRepoInfo).mockReturnValue({ commonGitDir: '/tmp', worktreeGitDir: '/tmp', branch: undefined, root: '/tmp', sha: 'sha1' } as any) + const result: any = await getGitMetaData() + expect(result.branch).toEqual('main') + }) + }) + + it('prefers a CI-specific env var over a generic one', async () => { + await withEnv({ GITHUB_ACTIONS: 'true', GITHUB_REF_NAME: 'feature/x', BRANCH: 'generic-loser' }, async () => { + vi.mocked(gitRepoInfo).mockReturnValue({ commonGitDir: '/tmp', worktreeGitDir: '/tmp', branch: undefined, root: '/tmp', sha: 'sha1' } as any) + const result: any = await getGitMetaData() + expect(result.branch).toEqual('feature/x') + }) + }) }) describe('getHookType', () => { From c119ae03daa7e274b41cfdcfe06af60a74f73fd0 Mon Sep 17 00:00:00 2001 From: Harshit Date: Mon, 20 Jul 2026 17:19:19 +0530 Subject: [PATCH 4/5] test(SDK-7009): make CI-detection test isolation drift-proof MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Round-3 review: the test's CI_MARKER_KEYS list (used to force getCiInfo() null in the generic-var-gating tests) didn't cover every branch the real getCiInfo() checks (Concourse, Google Cloud, Shippable, some CodeBuild/Wercker vars), so a stray ambient CI marker could silently flip those tests to the wrong branch. - Complete CI_MARKER_KEYS against every gating var in getCiInfo(). - Add self-checking guards — assert utils.getCiInfo() is null (outside-CI test) / non-null (Jenkins test) before the branch assertion — so any future drift between the list and the real detector fails loudly instead of silently. Test-only change; production code unchanged. Co-Authored-By: Claude Opus 4.8 --- .../browserstack-service/tests/util.test.ts | 25 +++++++++++++------ 1 file changed, 18 insertions(+), 7 deletions(-) diff --git a/packages/browserstack-service/tests/util.test.ts b/packages/browserstack-service/tests/util.test.ts index 9c37343..0d2d2f0 100644 --- a/packages/browserstack-service/tests/util.test.ts +++ b/packages/browserstack-service/tests/util.test.ts @@ -988,14 +988,20 @@ describe('getGitMetaData', () => { 'WERCKER_GIT_BRANCH', 'VERCEL_GIT_COMMIT_REF', 'CF_BRANCH', 'GIT_LOCAL_BRANCH', 'BRANCH_NAME', 'GIT_BRANCH', 'BRANCH', ] - // CI-detection markers getCiInfo() keys on. Cleared too, so getCiInfo() is - // null unless a test opts a specific CI in — the generic-var gate + // Every env var getCiInfo() gates a non-null return on. Cleared so getCiInfo() + // is null unless a test opts a specific CI in — the generic-var gate // (`getCiInfo() !== null`) is otherwise non-deterministic under ambient CI. + // Kept exhaustive against src/util.ts getCiInfo(); the getCiInfo()-null/non-null + // guard asserts in the tests below fail loudly if this list ever drifts. const CI_MARKER_KEYS = [ - 'CI', 'GITHUB_ACTIONS', 'JENKINS_URL', 'JENKINS_HOME', 'CIRCLECI', 'TRAVIS', 'GITLAB_CI', - 'BITBUCKET_BUILD_NUMBER', 'DRONE', 'SEMAPHORE', 'BUILDKITE', 'TF_BUILD', 'APPVEYOR', - 'AZURE_HTTP_USER_AGENT', 'CODEBUILD_BUILD_ID', 'bamboo_buildNumber', 'WERCKER', 'NETLIFY', - 'VERCEL', 'TEAMCITY_VERSION', 'CF_BUILD_ID', 'GO_JOB_NAME', + 'CI', 'CIRCLECI', 'TRAVIS', 'CI_NAME', 'DRONE', 'SEMAPHORE', 'GITLAB_CI', 'BUILDKITE', 'VERCEL', + 'JENKINS_URL', 'JENKINS_HOME', 'BITBUCKET_COMMIT', 'BITBUCKET_BUILD_NUMBER', + 'TF_BUILD', 'TF_BUILD_BUILDNUMBER', 'APPVEYOR', 'AZURE_HTTP_USER_AGENT', + 'CODEBUILD_BUILD_ID', 'CODEBUILD_RESOLVED_SOURCE_VERSION', 'CODEBUILD_SOURCE_VERSION', + 'bamboo_buildNumber', 'WERCKER', 'WERCKER_MAIN_PIPELINE_STARTED', + 'GCP_PROJECT', 'GCLOUD_PROJECT', 'GOOGLE_CLOUD_PROJECT', 'SHIPPABLE', 'NETLIFY', + 'GITHUB_ACTIONS', 'TEAMCITY_VERSION', 'CONCOURSE', 'CONCOURSE_URL', 'CONCOURSE_USERNAME', + 'CONCOURSE_TEAM', 'GO_JOB_NAME', 'CF_BUILD_ID', ] const ALL_ENV_KEYS = [...BRANCH_ENV_KEYS, ...CI_MARKER_KEYS] const withEnv = async (overrides: Record, fn: () => Promise) => { @@ -1094,7 +1100,11 @@ describe('getGitMetaData', () => { it('ignores a generic BRANCH env var when NOT in a recognised CI', async () => { // A stray local `BRANCH` export must not be misreported as the branch. await withEnv({ BRANCH: 'stray-local-value' }, async () => { - mockGitRefs('', '') // no CI marker set → getCiInfo() is null; backstop also empty + // Guard: assert the real detector actually sees "no CI" here — if a + // CI marker leaks past CI_MARKER_KEYS this fails loudly instead of + // silently exercising the in-CI branch. + expect(utils.getCiInfo()).toBeNull() + mockGitRefs('', '') // backstop also empty vi.mocked(gitRepoInfo).mockReturnValue({ commonGitDir: '/tmp', worktreeGitDir: '/tmp', branch: undefined, root: '/tmp', sha: 'sha1' } as any) const result: any = await getGitMetaData() expect(result.branch).toBeUndefined() @@ -1103,6 +1113,7 @@ describe('getGitMetaData', () => { it('uses a generic GIT_BRANCH var only inside a recognised CI (Jenkins), stripping origin/', async () => { await withEnv({ JENKINS_URL: 'http://jenkins.local', GIT_BRANCH: 'origin/main' }, async () => { + expect(utils.getCiInfo()).not.toBeNull() // guard: detector sees Jenkins vi.mocked(gitRepoInfo).mockReturnValue({ commonGitDir: '/tmp', worktreeGitDir: '/tmp', branch: undefined, root: '/tmp', sha: 'sha1' } as any) const result: any = await getGitMetaData() expect(result.branch).toEqual('main') From c4891798b79d3b92a16e88787c856b9a772062a8 Mon Sep 17 00:00:00 2001 From: Harshit Date: Mon, 20 Jul 2026 21:01:30 +0530 Subject: [PATCH 5/5] fix(SDK-7009): lazy-import child_process in git backstop (fix CI Build&test) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The module-top `import { execFile }` + `promisify(execFile)` broke every test that partial-mocks `node:child_process` (e.g. tests/cli/frameworks/index.test.ts mocks it as `{ spawn }` only) — `execFile` was undefined at util.ts load, so those suites failed to load with "No execFile export is defined on the mock". Import child_process lazily inside gitForEachRef (within its existing try/catch) so module load never references execFile. It's only touched when the git backstop actually runs (covered by util.test.ts's full mock); suites that never hit the backstop are unaffected, and a missing/partial mock degrades to []. Co-Authored-By: Claude Opus 4.8 --- packages/browserstack-service/src/util.ts | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/packages/browserstack-service/src/util.ts b/packages/browserstack-service/src/util.ts index 0f7dd92..ee1dfe0 100644 --- a/packages/browserstack-service/src/util.ts +++ b/packages/browserstack-service/src/util.ts @@ -5,7 +5,6 @@ import zlib from 'node:zlib' import { format, promisify } from 'node:util' import path from 'node:path' import util from 'node:util' -import { execFile } from 'node:child_process' import type { Capabilities, Frameworks, Options } from '@wdio/types' import type { BeforeCommandArgs, AfterCommandArgs } from '@wdio/reporter' @@ -59,7 +58,6 @@ import AccessibilityScripts from './scripts/accessibility-scripts.js' import { _fetch as fetch } from './fetchWrapper.js' const pGitconfig = promisify(gitconfig) -const pExecFile = promisify(execFile) export type GitMetaData = { name: string; @@ -1065,13 +1063,16 @@ function getBranchFromCIEnv (): string | undefined { // Async (non-blocking) — a synchronous subprocess on getGitMetaData's hot path // would stall the event loop. execFile takes an argv array (no shell), so the -// ref args cannot be interpreted by a shell. +// ref args cannot be interpreted by a shell. child_process is imported lazily +// (inside the try) so module load never touches it — keeps every test that +// partial-mocks node:child_process working, and degrades to [] if it's absent. async function gitForEachRef (args: string[]): Promise { try { + const { execFile } = await import('node:child_process') // No explicit cwd → runs in process.cwd(), the actual checkout the SDK // is instrumenting. (git-repo-info's `.root` can point at the main repo // dir under a git worktree, which would resolve the wrong branch.) - const { stdout } = await pExecFile('git', ['for-each-ref', '--points-at', 'HEAD', ...args], { + const { stdout } = await promisify(execFile)('git', ['for-each-ref', '--points-at', 'HEAD', ...args], { encoding: 'utf-8', timeout: 3000 }) return stdout.split('\n').map(s => s.trim()).filter(Boolean)