fix(observability): disable observability throughout the run when build-start is blocked

.changeset/cli-static-itskip-reporting.md

@@ -0,0 +1,5 @@
+---
+"@wdio/browserstack-service": patch
+---
+
+- Fixed a statically-skipped test (`it.skip`) being left orphaned as "in progress" on the Test Observability dashboard in the CLI flow. Such tests are reported from the un-awaited `onTestSkip` reporter hook, so their `TestRunFinished` event could still be pending when the worker tore down; the `after()` hook now drains these skip reports before the session closes so the test is correctly reported as skipped.

.changeset/pr-77.md

@@ -0,0 +1,5 @@
+---
+"@wdio/browserstack-service": patch
+---
+
+- Fixed Test Observability not being fully disabled for a run when the build could not be started (e.g. an unsupported framework) — such sessions no longer emit observability events or get linked to a non-existent build.

packages/browserstack-service/src/cli/skipReporter.ts

@@ -36,6 +36,15 @@ export function markTestStarted(identifier: string) {
     startedTests.add(identifier)
 }
 
+// wdio does not await the reporter's onTestSkip hook, so a static `it.skip` reported through
+// reportSkippedTest can have its TEST/POST (TestRunFinished) still pending when the worker tears
+// down — the test then stays "in progress" on the dashboard. The awaited after() hook drains this
+// so the chain completes while the session is still open. (Hook-skip cascades go via
+// reportSuiteSkipped inside afterHook, which is already awaited, so they were unaffected.)
+export function drainSkipReports(): Promise<void> {
+    return reportChain
+}
+
 export function reportSkippedTest(framework: TestFramework, identifier: string, test: Frameworks.Test, suiteTitle?: string): Promise<void> {
     if (startedTests.has(identifier) || reportedSkips.has(identifier)) {
         return reportChain

packages/browserstack-service/src/launcher.ts

@@ -421,7 +421,8 @@ export default class BrowserstackLauncherService implements Services.ServiceInst
         const shouldSetupPercy = this._options.percy || (isUndefined(this._options.percy) && this._options.app)
 
         let buildStartResponse = null
-        if (!BrowserstackCLI.getInstance().isRunning() && (this._options.testObservability || this._accessibilityAutomation || shouldSetupPercy)) {
+        const classicBuildStartAttempted = !BrowserstackCLI.getInstance().isRunning() && Boolean(this._options.testObservability || this._accessibilityAutomation || shouldSetupPercy)
+        if (classicBuildStartAttempted) {
             BStackLogger.debug('Sending launch start event')
 
             buildStartResponse = await launchTestSession(this._options, this._config, {
@@ -452,6 +453,18 @@ export default class BrowserstackLauncherService implements Services.ServiceInst
 
         this.browserStackConfig.accessibility = this._accessibilityAutomation
 
+        // Mirror the accessibility fold-back above for observability: if the classic build-start
+        // was attempted with observability requested but observability did not succeed
+        // (BROWSERSTACK_OBSERVABILITY !== 'true' — an explicit block sets 'false', a transport/parse
+        // failure leaves it unset because launchTestSession's error handler returns null), fold that
+        // outcome into config so the session's buildProductMap reports observability:false. Keying on
+        // the attempt (not on buildStartResponse being truthy) also covers the null-on-error case. The
+        // CLI/gRPC flow owns its own build-start, so classicBuildStartAttempted stays false there.
+        const observabilityBuildStartBlocked = classicBuildStartAttempted && Boolean(this._options.testObservability) && !isTrue(process.env[BROWSERSTACK_OBSERVABILITY])
+        if (observabilityBuildStartBlocked) {
+            this.browserStackConfig.testObservability.enabled = false
+        }
+
         if (this._accessibilityAutomation && this._options.accessibilityOptions) {
             // SDK-3737: coerce stringified booleans (e.g. autoScanning: 'false') to real
             // booleans so boolean-typed accessibility options are honoured instead of
@@ -486,7 +499,14 @@ export default class BrowserstackLauncherService implements Services.ServiceInst
             }
         }
 
-        this._updateCaps(capabilities as Capabilities.TestrunnerCapabilities, 'testhubBuildUuid')
+        // Skip stamping testhubBuildUuid only when the observability build-start was blocked
+        // and no other TestHub product (accessibility) succeeded — otherwise the Automate
+        // session gets orphan-linked to a TestHub build that was never created (a blocked
+        // build-start still returns a build_hashed_id), keeping it counted as an SDK
+        // observability session. Every other case keeps the prior behavior.
+        if (!(observabilityBuildStartBlocked && !this._accessibilityAutomation)) {
+            this._updateCaps(capabilities as Capabilities.TestrunnerCapabilities, 'testhubBuildUuid')
+        }
         this._updateCaps(capabilities as Capabilities.TestrunnerCapabilities, 'buildProductMap')
 
         if (isValidEnabledValue(this._options.testOrchestrationOptions?.runSmartSelection?.enabled)){

packages/browserstack-service/src/service.ts

@@ -32,7 +32,7 @@ import PerformanceTester from './instrumentation/performance/performance-tester.
 import * as PERFORMANCE_SDK_EVENTS from './instrumentation/performance/constants.js'
 import { EVENTS } from './instrumentation/performance/constants.js'
 import { BrowserstackCLI } from './cli/index.js'
-import { markTestStarted, reportSuiteSkipped } from './cli/skipReporter.js'
+import { drainSkipReports, markTestStarted, reportSuiteSkipped } from './cli/skipReporter.js'
 import { CLIUtils } from './cli/cliUtils.js'
 
 import { _fetch as fetch } from './fetchWrapper.js'
@@ -573,6 +573,14 @@ export default class BrowserstackService implements Services.ServiceInstance {
                 } catch (flushErr) {
                     BStackLogger.debug(`Exception flushing deferred test finish in after(): ${util.format(flushErr)}`)
                 }
+                // Drain skip reports queued from the un-awaited onTestSkip reporter hook (static
+                // `it.skip`) so their TestRunFinished lands before the session closes — otherwise
+                // the test is orphaned "in progress" on the dashboard.
+                try {
+                    await drainSkipReports()
+                } catch (skipDrainErr) {
+                    BStackLogger.debug(`Exception draining skip reports in after(): ${util.format(skipDrainErr)}`)
+                }
                 await BrowserstackCLI.getInstance().getAutomationFramework()!.trackEvent(AutomationFrameworkState.EXECUTE, HookState.POST, {})
             }
 

packages/browserstack-service/src/testHub/utils.ts

@@ -56,7 +56,11 @@ export const shouldProcessEventForTesthub = (eventType: string): boolean => {
     if (isTrue(process.env[BROWSERSTACK_PERCY]) && eventType) {
         return false
     }
-    return Boolean(process.env[BROWSERSTACK_ACCESSIBILITY] || process.env[BROWSERSTACK_OBSERVABILITY] || process.env[BROWSERSTACK_PERCY])!
+    // Evaluate with isTrue so a product flag explicitly set to the string 'false'
+    // (e.g. observability turned off after a failed/blocked build-start) reads as
+    // off. Raw Boolean('false') is truthy, which kept events flowing for an
+    // observability-only session even after the flag was disabled.
+    return isTrue(process.env[BROWSERSTACK_ACCESSIBILITY]) || isTrue(process.env[BROWSERSTACK_OBSERVABILITY]) || isTrue(process.env[BROWSERSTACK_PERCY])
 }
 
 export const handleErrorForObservability = (error: Errors | null): void => {

packages/browserstack-service/tests/launcher.test.ts

@@ -2,7 +2,7 @@ import fs from 'node:fs'
 import os from 'node:os'
 import path from 'node:path'
 
-import { describe, expect, it, vi, beforeEach } from 'vitest'
+import { describe, expect, it, vi, beforeEach, afterEach } from 'vitest'
 // @ts-expect-error mock feature
 import { Local, mockStart } from 'browserstack-local'
 import logger from '@wdio/logger'
@@ -76,9 +76,20 @@ describe('onPrepare', () => {
         key: '12345678901234567890',
         capabilities: []
     }
-    vi.spyOn(utils, 'launchTestSession').mockImplementation(() => {})
+    // Default: simulate a SUCCESSFUL observability build-start (sets the env flag 'true'),
+    // matching real behavior so the buildProductMap/testhubBuildUuid caps are stamped.
+    const launchTestSessionSuccess = () => { process.env.BROWSERSTACK_OBSERVABILITY = 'true' }
+    vi.spyOn(utils, 'launchTestSession').mockImplementation(launchTestSessionSuccess as any)
     vi.spyOn(utils, 'isBStackSession').mockImplementation(() => {return true})
 
+    // Reset shared state after every test so per-test overrides (launchTestSession / getProductMap /
+    // the observability env flag) never leak into unrelated tests, even if an assertion throws first.
+    afterEach(() => {
+        delete process.env.BROWSERSTACK_OBSERVABILITY
+        vi.spyOn(utils, 'launchTestSession').mockImplementation(launchTestSessionSuccess as any)
+        vi.spyOn(thUtils, 'getProductMap').mockImplementation(() => productMap)
+    })
+
     it('should not try to upload app is app is undefined', async () => {
         const service = new BrowserstackLauncher({ testObservability: false } as any, caps, config)
         await service.onPrepare(config, caps)
@@ -121,6 +132,56 @@ describe('onPrepare', () => {
         expect(capabilities.samsungGalaxy.capabilities).toEqual({ 'appium:app': 'bs://<app-id>', 'bstack:options': { buildProductMap: productMap, testhubBuildUuid: buildHashedId } })
     })
 
+    it('folds a blocked observability build-start into caps: buildProductMap.observability=false and no testhubBuildUuid stamped', async () => {
+        // Classic-flow build-start blocked (unsupported framework): launchTestSession turns the
+        // observability env flag off and returns a truthy response.
+        vi.spyOn(utils, 'launchTestSession').mockImplementation((() => {
+            process.env.BROWSERSTACK_OBSERVABILITY = 'false'
+            return {} as any
+        }) as any)
+        // Derive buildProductMap from config so the fold-back into config is observable in caps.
+        vi.spyOn(thUtils, 'getProductMap').mockImplementation((cfg: any) => ({
+            observability: cfg.testObservability.enabled,
+            accessibility: !!cfg.accessibility,
+            percy: cfg.percy,
+            automate: cfg.automate,
+            app_automate: cfg.appAutomate
+        }))
+
+        const service = new BrowserstackLauncher({ testObservability: true } as any, caps, config)
+        const capabilities: any = [{ 'bstack:options': {} }]
+        await service.onPrepare(config, capabilities)
+
+        // observability outcome folded into config -> buildProductMap reports observability:false
+        expect((service as any).browserStackConfig.testObservability.enabled).toBe(false)
+        expect(capabilities[0]['bstack:options'].buildProductMap.observability).toBe(false)
+        // session is not orphan-linked to a TestHub build that was never created
+        expect(capabilities[0]['bstack:options'].testhubBuildUuid).toBeUndefined()
+        // (mock/env restoration handled by afterEach)
+    })
+
+    it('folds a failed (transport-error) observability build-start into caps: observability off, no testhubBuildUuid', async () => {
+        // launchTestSession's error handler returns null WITHOUT setting the env flag (neither
+        // success 'true' nor explicit block 'false') — the fold must still fire since the classic
+        // build-start was attempted and observability did not succeed.
+        vi.spyOn(utils, 'launchTestSession').mockImplementation((() => null) as any)
+        vi.spyOn(thUtils, 'getProductMap').mockImplementation((cfg: any) => ({
+            observability: cfg.testObservability.enabled,
+            accessibility: !!cfg.accessibility,
+            percy: cfg.percy,
+            automate: cfg.automate,
+            app_automate: cfg.appAutomate
+        }))
+
+        const service = new BrowserstackLauncher({ testObservability: true } as any, caps, config)
+        const capabilities: any = [{ 'bstack:options': {} }]
+        await service.onPrepare(config, capabilities)
+
+        expect((service as any).browserStackConfig.testObservability.enabled).toBe(false)
+        expect(capabilities[0]['bstack:options'].buildProductMap.observability).toBe(false)
+        expect(capabilities[0]['bstack:options'].testhubBuildUuid).toBeUndefined()
+    })
+
     it('should add the "appium:app" property to a multiremote capability if "bstack:options" present', async () => {
         const options: BrowserstackConfig = { app: 'bs://<app-id>' }
         const service = new BrowserstackLauncher(options as any, caps, config)

packages/browserstack-service/tests/observability-build-start-success-false.test.ts

@@ -0,0 +1,130 @@
+import path from 'node:path'
+import { describe, it, expect, beforeEach, vi } from 'vitest'
+
+vi.mock('@wdio/logger', () => import(path.join(process.cwd(), '__mocks__', '@wdio/logger')))
+vi.mock('fetch')
+vi.mock('git-repo-info')
+vi.mock('./fileStream')
+vi.mock('./scripts/accessibility-scripts', () => ({
+    default: {
+        checkAndGetInstance: vi.fn(() => ({ update: vi.fn(), store: vi.fn() })),
+        update: vi.fn(),
+        store: vi.fn(),
+    }
+}))
+
+import { processTestObservabilityResponse } from '../src/util.js'
+import { shouldProcessEventForTesthub } from '../src/testHub/utils.js'
+import { BROWSERSTACK_OBSERVABILITY, BROWSERSTACK_ACCESSIBILITY, BROWSERSTACK_PERCY } from '../src/constants.js'
+import { BrowserstackCLI } from '../src/cli/index.js'
+import { CLIUtils } from '../src/cli/cliUtils.js'
+import ObservabilityModule from '../src/cli/modules/observabilityModule.js'
+import TestHubModule from '../src/cli/modules/testHubModule.js'
+
+const clearProductEnv = () => {
+    delete process.env[BROWSERSTACK_OBSERVABILITY]
+    delete process.env[BROWSERSTACK_ACCESSIBILITY]
+    delete process.env[BROWSERSTACK_PERCY]
+}
+
+// A config JSON with every apis.* key that loadModules -> updateURLSForGRR dereferences.
+const CLI_CONFIG = JSON.stringify({
+    apis: {
+        automate: { api: 'https://api.browserstack.com', upload: 'https://api.browserstack.com' },
+        appAutomate: { api: 'https://api-cloud.browserstack.com', upload: 'https://api-cloud.browserstack.com' },
+        percy: { api: 'https://percy.browserstack.com' },
+        appAccessibility: { api: 'https://app-a11y.browserstack.com' },
+        observability: { api: 'https://collector.observability.browserstack.com', upload: 'https://upload.observability.browserstack.com' },
+        edsInstrumentation: { api: 'https://eds.browserstack.com' },
+    }
+})
+
+describe('CLASSIC flow: build-start observability success:false', () => {
+    beforeEach(() => {
+        clearProductEnv()
+    })
+
+    it('success:true -> BROWSERSTACK_OBSERVABILITY="true" and event gate is true (positive control)', () => {
+        const response: any = { observability: { success: true, options: {} } }
+        processTestObservabilityResponse(response)
+        expect(process.env[BROWSERSTACK_OBSERVABILITY]).toEqual('true')
+        expect(shouldProcessEventForTesthub('TestRunStarted')).toEqual(true)
+    })
+
+    it('success:false -> flag turned OFF ("false") AND subsequent TestHub events are suppressed', () => {
+        const response: any = { observability: { success: false, errors: [{ key: 'ERROR_OBSERVABILITY_NOT_ALLOWED', message: 'unsupported framework' }] } }
+        processTestObservabilityResponse(response)
+
+        // (a) the flag IS turned off
+        expect(process.env[BROWSERSTACK_OBSERVABILITY]).toEqual('false')
+
+        // (b) a subsequent TestHub event is now suppressed (observability-only session:
+        // accessibility & percy unset). Post-fix the gate reads 'false' as off.
+        expect(process.env[BROWSERSTACK_ACCESSIBILITY]).toBeUndefined()
+        expect(process.env[BROWSERSTACK_PERCY]).toBeUndefined()
+        expect(shouldProcessEventForTesthub('TestRunStarted')).toEqual(false)
+    })
+
+    it('missing observability object -> flag "false", event gate suppressed', () => {
+        const response: any = {}
+        processTestObservabilityResponse(response)
+        expect(process.env[BROWSERSTACK_OBSERVABILITY]).toEqual('false')
+        expect(shouldProcessEventForTesthub('TestRunStarted')).toEqual(false)
+    })
+})
+
+describe('CLI/gRPC flow: startBinSession observability success:false', () => {
+    let instance: any
+
+    beforeEach(() => {
+        clearProductEnv()
+        // Mirror launcher.ts: register the mocha/WebdriverIO framework detail so
+        // loadModules -> setupTestFramework/setupAutomationFramework resolve names.
+        CLIUtils.setFrameworkDetail('webdriverio-mocha', 'WebdriverIO')
+        instance = BrowserstackCLI.getInstance()
+        instance.modulesLoaded = false
+        instance.modules = {}
+        instance.binSessionId = null
+        instance.config = {}
+        instance.browserstackConfig = {}
+        instance.options = {}
+    })
+
+    it('success:false -> ObservabilityModule NOT loaded, but TestHubModule loaded (event emission delegated to binary)', () => {
+        const response: any = {
+            binSessionId: 'bin-1',
+            config: CLI_CONFIG,
+            testhub: { jwt: 'jwt', buildHashedId: 'build-1' },
+            observability: { success: false },
+        }
+        instance.loadModules(response)
+
+        // observability module gated OFF by success:false
+        expect(instance.modules[ObservabilityModule.MODULE_NAME]).toBeUndefined()
+        expect(process.env[BROWSERSTACK_OBSERVABILITY]).not.toEqual('true')
+
+        // BUT the event-emitting TestHubModule is loaded unconditionally whenever
+        // testhub is present -- and it fires events with no observability gate.
+        expect(instance.modules[TestHubModule.MODULE_NAME]).toBeDefined()
+
+        // TestHubModule has no shouldProcessEventForTesthub-style gate: its send* paths
+        // call GrpcClient unconditionally.
+        const src = TestHubModule.toString() + Object.getOwnPropertyNames(TestHubModule.prototype)
+            .map((m) => (TestHubModule.prototype as any)[m]?.toString?.() ?? '').join('\n')
+        expect(src.includes('shouldProcessEventForTesthub')).toBe(false)
+    })
+
+    it('success:true -> ObservabilityModule loaded and BROWSERSTACK_OBSERVABILITY="true" (positive control)', () => {
+        const response: any = {
+            binSessionId: 'bin-2',
+            config: CLI_CONFIG,
+            testhub: { jwt: 'jwt', buildHashedId: 'build-2' },
+            observability: { success: true, options: {} },
+        }
+        instance.loadModules(response)
+
+        expect(process.env[BROWSERSTACK_OBSERVABILITY]).toEqual('true')
+        expect(instance.modules[ObservabilityModule.MODULE_NAME]).toBeDefined()
+        expect(instance.modules[TestHubModule.MODULE_NAME]).toBeDefined()
+    })
+})