From f3ec0d9bf49f5f1b53c7012810370ce0a6312c65 Mon Sep 17 00:00:00 2001 From: amankansal-lt Date: Fri, 2 Oct 2026 19:45:11 +0530 Subject: [PATCH 1/9] refactor(provider-webdriver): share hub upload and app-reference helpers BrowserStack's app upload, install adapter, --provider-app resolution, session-details URL artifacts, and orientation check lived inline in the BrowserStack modules. Move the vendor-neutral mechanics into shared helpers that BrowserStack calls with its own form field, reference scheme, and response reader. This is groundwork for a WebDriver host port that accepts external provider definitions. - webdriver-utils.ts: postHubAppUpload, createHubUploadApp, resolveHubAppReference, appFileUploadForm, and requireProviderDeviceOrientation. - artifact-results.ts: urlArtifactFromDetails. - browserstack.ts: resolveBrowserStackAppReference, moved out of provider-definitions.ts. BrowserStack gains two fixes from the shared code: - An upload response that is not JSON (a gateway error page, an empty body) now fails with a typed COMMAND_FAILED that carries the HTTP status, instead of a raw JSON SyntaxError. - The http(s) scheme of a --provider-app URL is matched case-insensitively, so HTTPS://... is passed through to the hub rather than being treated as a local path. Co-Authored-By: Claude Opus 5.5 --- .../src/artifact-results.ts | 14 ++ .../src/browserstack-device-features.ts | 16 +-- .../src/browserstack.test.ts | 57 +++++++- .../provider-webdriver/src/browserstack.ts | 92 ++++++------- .../src/provider-definitions.ts | 64 +++------ .../src/webdriver-utils.test.ts | 109 ++++++++++++++- .../provider-webdriver/src/webdriver-utils.ts | 126 +++++++++++++++++- 7 files changed, 361 insertions(+), 117 deletions(-) diff --git a/packages/provider-webdriver/src/artifact-results.ts b/packages/provider-webdriver/src/artifact-results.ts index bf02c7c77e..de29b4ab3a 100644 --- a/packages/provider-webdriver/src/artifact-results.ts +++ b/packages/provider-webdriver/src/artifact-results.ts @@ -28,3 +28,17 @@ export function unavailableCloudArtifactsResult(options: { message: options.error instanceof Error ? options.error.message : String(options.error), }; } + +/** A ready URL artifact read off a provider's session-details record, or nothing when the field is absent. */ +export function urlArtifactFromDetails( + provider: string, + providerSessionId: string, + details: Record, + field: string, + kind: CloudArtifact['kind'], + name: string, +): CloudArtifact | undefined { + const url = details[field]; + if (typeof url !== 'string' || url.length === 0) return undefined; + return { provider, providerSessionId, kind, name, url, availability: 'ready' }; +} diff --git a/packages/provider-webdriver/src/browserstack-device-features.ts b/packages/provider-webdriver/src/browserstack-device-features.ts index 38669957ac..3ea041add7 100644 --- a/packages/provider-webdriver/src/browserstack-device-features.ts +++ b/packages/provider-webdriver/src/browserstack-device-features.ts @@ -3,6 +3,7 @@ import { type CloudProviderProfileFields, } from '@agent-device/contracts/remote'; import { AppError } from '@agent-device/kernel/errors'; +import { requireProviderDeviceOrientation } from './webdriver-utils.ts'; import type { CloudWebDriverPlatform } from './runtime.ts'; /** @@ -199,26 +200,13 @@ function assignStringField( value: string, ): void { if (spec.field === 'providerDeviceOrientation') { - fields.providerDeviceOrientation = requireDeviceOrientation(spec, value); + fields.providerDeviceOrientation = requireProviderDeviceOrientation(spec, value); return; } if (spec.field === 'providerNoResignApp') return; fields[spec.field] = value; } -function requireDeviceOrientation( - spec: BrowserStackDeviceFeatureSpec, - value: string, -): (typeof PROVIDER_DEVICE_ORIENTATIONS)[number] { - const match = PROVIDER_DEVICE_ORIENTATIONS.find((orientation) => orientation === value); - if (match) return match; - throw new AppError('INVALID_ARGS', `Invalid ${spec.flag} value: ${value}.`, { - hint: `Use ${PROVIDER_DEVICE_ORIENTATIONS.join('|')}.`, - flag: spec.flag, - capability: spec.capability, - }); -} - function requireSupportedPlatform( spec: BrowserStackDeviceFeatureSpec, platform: CloudWebDriverPlatform, diff --git a/packages/provider-webdriver/src/browserstack.test.ts b/packages/provider-webdriver/src/browserstack.test.ts index 125bc8ea8c..61cf7bdbb6 100644 --- a/packages/provider-webdriver/src/browserstack.test.ts +++ b/packages/provider-webdriver/src/browserstack.test.ts @@ -3,7 +3,8 @@ import { promises as fs } from 'node:fs'; import path from 'node:path'; import { afterEach, test } from 'vitest'; -import { uploadBrowserStackApp } from './browserstack.ts'; +import { AppError } from '@agent-device/kernel/errors'; +import { resolveBrowserStackAppReference, uploadBrowserStackApp } from './browserstack.ts'; import { mkdtempForTest } from './tmp-dir.fixtures.ts'; const realFetch = globalThis.fetch; @@ -46,3 +47,57 @@ test('BrowserStack upload aborts while the provider request is in flight', async await fs.rm(tempDir, { recursive: true, force: true }); } }); + +const upload = { clientVersion: '0.0.0-test', username: 'user', accessKey: 'key' }; + +test('BrowserStack upload sends the file field and fails typed on a gateway error page', async () => { + const tempDir = await mkdtempForTest('agent-device-browserstack-upload-error-'); + const appPath = path.join(tempDir, 'App.apk'); + try { + await fs.writeFile(appPath, 'placeholder'); + globalThis.fetch = async (_input, init) => { + assert.ok(init?.body instanceof FormData); + assert.ok(init.body.get('file') instanceof Blob); + return new Response('502 Bad Gateway', { status: 502 }); + }; + await assert.rejects(uploadBrowserStackApp(appPath, upload), (error: unknown) => { + assert.ok(error instanceof AppError); + assert.equal(error.code, 'COMMAND_FAILED'); + assert.equal(error.message, 'BrowserStack app upload failed.'); + assert.equal(error.details?.status, 502); + return true; + }); + } finally { + await fs.rm(tempDir, { recursive: true, force: true }); + } +}); + +test('BrowserStack passes bs:// ids and URLs to the hub and uploads only local paths', async () => { + const tempDir = await mkdtempForTest('agent-device-browserstack-resolve-'); + try { + await fs.writeFile(path.join(tempDir, 'App.apk'), 'placeholder'); + const fetched: string[] = []; + globalThis.fetch = async (input) => { + fetched.push(String(input)); + return new Response(JSON.stringify({ app_url: 'bs://uploaded' }), { status: 200 }); + }; + const resolve = async (app: string) => + await resolveBrowserStackAppReference(app, { ...upload, cwd: tempDir }); + + assert.equal(await resolve('bs://preuploaded'), 'bs://preuploaded'); + assert.equal(await resolve('https://builds.example/App.apk'), 'https://builds.example/App.apk'); + assert.equal(fetched.length, 0); + assert.equal(await resolve('App.apk'), 'bs://uploaded'); + assert.deepEqual(fetched, ['https://api-cloud.browserstack.com/app-automate/upload']); + await assert.rejects(resolve('missing.apk'), (error: unknown) => { + assert.ok(error instanceof AppError); + assert.equal( + error.message, + 'BrowserStack --provider-app must be a bs:// app id, URL, or existing local app path.', + ); + return true; + }); + } finally { + await fs.rm(tempDir, { recursive: true, force: true }); + } +}); diff --git a/packages/provider-webdriver/src/browserstack.ts b/packages/provider-webdriver/src/browserstack.ts index 6c8d10fd4b..800bd0c241 100644 --- a/packages/provider-webdriver/src/browserstack.ts +++ b/packages/provider-webdriver/src/browserstack.ts @@ -1,12 +1,17 @@ -import fs from 'node:fs/promises'; -import path from 'node:path'; import type { CloudArtifact, CloudArtifactsResult } from '@agent-device/contracts/observability'; import type { CloudWebDriverCapabilityOverrides } from './capabilities.ts'; import type { CloudWebDriverUploadApp } from './runtime.ts'; import { AppError } from '@agent-device/kernel/errors'; import { agentDeviceRequestHeaders } from './request-headers.ts'; -import { cloudArtifactsReadyOrPending } from './artifact-results.ts'; -import { basicAuthHeader, trimTrailingSlash } from './webdriver-utils.ts'; +import { cloudArtifactsReadyOrPending, urlArtifactFromDetails } from './artifact-results.ts'; +import { + appFileUploadForm, + basicAuthHeader, + createHubUploadApp, + postHubAppUpload, + resolveHubAppReference, + trimTrailingSlash, +} from './webdriver-utils.ts'; export const BROWSERSTACK_APP_AUTOMATE_ENDPOINT = 'https://hub-cloud.browserstack.com/wd/hub/'; export const BROWSERSTACK_APP_UPLOAD_ENDPOINT = @@ -76,41 +81,41 @@ export async function uploadBrowserStackApp( signal?: AbortSignal, ): Promise { signal?.throwIfAborted(); - const file = await fs.readFile(appPath); - const form = new FormData(); - form.set('file', new Blob([file]), path.basename(appPath)); - const response = await fetch(options.endpoint ?? BROWSERSTACK_APP_UPLOAD_ENDPOINT, { - method: 'POST', - headers: { - ...agentDeviceRequestHeaders(options.clientVersion), - Authorization: basicAuthHeader(options), + return await postHubAppUpload( + await appFileUploadForm(appPath, 'file'), + { + service: 'BrowserStack', + endpoint: options.endpoint ?? BROWSERSTACK_APP_UPLOAD_ENDPOINT, + clientVersion: options.clientVersion, + auth: options, + readAppReference: readBrowserStackAppUrl, }, - body: form, signal, - }); - const json = (await response.json()) as unknown; - const appUrl = readBrowserStackAppUrl(json); - if (!response.ok || !appUrl) { - throw new AppError('COMMAND_FAILED', 'BrowserStack app upload failed.', { - status: response.status, - response: json, - }); - } - return appUrl; + ); } export function createBrowserStackUploadApp( options: Required, ): CloudWebDriverUploadApp { - return async ({ appPath, options: installOptions, signal }) => { - const appReference = await uploadBrowserStackApp(appPath, options, signal); - return { - appReference, - bundleId: installOptions?.appIdentifierHint, - packageName: installOptions?.packageNameHint, - launchTarget: installOptions?.appIdentifierHint ?? installOptions?.packageNameHint, - }; - }; + return createHubUploadApp( + async (appPath, signal) => await uploadBrowserStackApp(appPath, options, signal), + ); +} + +/** The hub fetches a public URL itself, so only a local path is uploaded. */ +export async function resolveBrowserStackAppReference( + app: string, + options: BrowserStackUploadOptions & { cwd?: string; signal?: AbortSignal }, +): Promise { + return await resolveHubAppReference({ + service: 'BrowserStack', + app, + cwd: options.cwd, + referenceScheme: 'bs://', + referenceLabel: 'a bs:// app id', + uploadFile: async (appPath, signal) => await uploadBrowserStackApp(appPath, options, signal), + signal: options.signal, + }); } /** @@ -179,7 +184,7 @@ function mapBrowserStackArtifacts( details: Record, ): CloudArtifact[] { return [ - browserStackUrlArtifact( + urlArtifactFromDetails( provider, providerSessionId, details, @@ -187,7 +192,7 @@ function mapBrowserStackArtifacts( 'video', 'Session video', ), - browserStackUrlArtifact( + urlArtifactFromDetails( provider, providerSessionId, details, @@ -195,7 +200,7 @@ function mapBrowserStackArtifacts( 'appium-log', 'Appium logs', ), - browserStackUrlArtifact( + urlArtifactFromDetails( provider, providerSessionId, details, @@ -203,7 +208,7 @@ function mapBrowserStackArtifacts( 'device-log', 'Device logs', ), - browserStackUrlArtifact( + urlArtifactFromDetails( provider, providerSessionId, details, @@ -211,7 +216,7 @@ function mapBrowserStackArtifacts( 'provider-session', 'BrowserStack dashboard', ), - browserStackUrlArtifact( + urlArtifactFromDetails( provider, providerSessionId, details, @@ -222,19 +227,6 @@ function mapBrowserStackArtifacts( ].filter((artifact): artifact is CloudArtifact => artifact !== undefined); } -function browserStackUrlArtifact( - provider: string, - providerSessionId: string, - details: Record, - field: string, - kind: CloudArtifact['kind'], - name: string, -): CloudArtifact | undefined { - const url = details[field]; - if (typeof url !== 'string' || url.length === 0) return undefined; - return { provider, providerSessionId, kind, name, url, availability: 'ready' }; -} - function readBrowserStackAppUrl(value: unknown): string | undefined { if (!value || typeof value !== 'object') return undefined; const appUrl = (value as { app_url?: unknown }).app_url; diff --git a/packages/provider-webdriver/src/provider-definitions.ts b/packages/provider-webdriver/src/provider-definitions.ts index b2de85b689..713fd7d8ce 100644 --- a/packages/provider-webdriver/src/provider-definitions.ts +++ b/packages/provider-webdriver/src/provider-definitions.ts @@ -1,5 +1,3 @@ -import fs from 'node:fs'; -import path from 'node:path'; import type { CloudArtifactsResult } from '@agent-device/contracts/observability'; import type { LeaseLifecycleContext } from '@agent-device/contracts/device'; import { AppError } from '@agent-device/kernel/errors'; @@ -17,7 +15,7 @@ import { buildBrowserStackCapabilities, createBrowserStackUploadApp, listBrowserStackCloudArtifacts, - uploadBrowserStackApp, + resolveBrowserStackAppReference, } from './browserstack.ts'; import { buildBrowserStackDeviceFeatureCapabilities, @@ -108,22 +106,24 @@ export function createCloudWebDriverProviderDefinitions( 'providerOsVersion', 'BrowserStack requires --provider-os-version .', ); - const app = await resolveBrowserStackAppReference({ - clientVersion: dependencies.clientVersion, - app: requireFlag( + const app = await resolveBrowserStackAppReference( + requireFlag( request, 'providerApp', 'BrowserStack requires --provider-app .', ), - cwd: request.cwd, - username, - accessKey, - uploadEndpoint: env.BROWSERSTACK_APP_UPLOAD_ENDPOINT, - // A local IPA/APK upload can run long (130 MB is routine); an - // upload is not a billed resource, so the request's cancellation - // may simply abort it — unlike the session creation that follows. - signal: request.signal, - }); + { + clientVersion: dependencies.clientVersion, + username, + accessKey, + endpoint: env.BROWSERSTACK_APP_UPLOAD_ENDPOINT, + cwd: request.cwd, + // A local IPA/APK upload can run long (130 MB is routine); an + // upload is not a billed resource, so the request's cancellation + // may simply abort it — unlike the session creation that follows. + signal: request.signal, + }, + ); return { ...base, platform, @@ -249,40 +249,6 @@ export function createCloudWebDriverProviderDefinitions( ]; } -async function resolveBrowserStackAppReference(options: { - clientVersion: string; - app: string; - cwd?: string; - username: string; - accessKey: string; - uploadEndpoint?: string; - signal?: AbortSignal; -}): Promise { - if (isProviderAppReference(options.app)) return options.app; - const appPath = path.resolve(options.cwd ?? process.cwd(), options.app); - if (!fs.existsSync(appPath)) { - throw new AppError( - 'INVALID_ARGS', - 'BrowserStack --provider-app must be a bs:// app id, URL, or existing local app path.', - { providerApp: options.app }, - ); - } - return await uploadBrowserStackApp( - appPath, - { - clientVersion: options.clientVersion, - username: options.username, - accessKey: options.accessKey, - endpoint: options.uploadEndpoint, - }, - options.signal, - ); -} - -function isProviderAppReference(value: string): boolean { - return value.startsWith('bs://') || /^https?:\/\//.test(value); -} - function requireRequest( req: LeaseLifecycleContext | undefined, providerLabel: string, diff --git a/packages/provider-webdriver/src/webdriver-utils.test.ts b/packages/provider-webdriver/src/webdriver-utils.test.ts index 58d326c241..05c9e140f9 100644 --- a/packages/provider-webdriver/src/webdriver-utils.test.ts +++ b/packages/provider-webdriver/src/webdriver-utils.test.ts @@ -1,6 +1,23 @@ import assert from 'node:assert/strict'; -import { test } from 'vitest'; -import { trimLeadingSlash, trimTrailingSlash } from './webdriver-utils.ts'; +import { promises as fs } from 'node:fs'; +import path from 'node:path'; +import { afterEach, test, vi } from 'vitest'; +import { AppError } from '@agent-device/kernel/errors'; +import { asOptionalRecord } from '@agent-device/kernel/record'; +import { + createHubUploadApp, + postHubAppUpload, + resolveHubAppReference, + trimLeadingSlash, + trimTrailingSlash, +} from './webdriver-utils.ts'; +import { mkdtempForTest } from './tmp-dir.fixtures.ts'; + +const realFetch = globalThis.fetch; + +afterEach(() => { + globalThis.fetch = realFetch; +}); test('slash trimming utilities handle slash-heavy strings without regular expressions', () => { const slashRun = '/'.repeat(10_000); @@ -15,3 +32,91 @@ test('slash trimming utilities handle slash-heavy strings without regular expres assert.equal(trimLeadingSlash(slashRun), ''); assert.equal(trimTrailingSlash(slashRun), ''); }); + +const hub = { + service: 'Hub', + endpoint: 'https://upload.example.test/app', + clientVersion: '0.0.0-test', + auth: { username: 'user', accessKey: 'key' }, + readAppReference: (body: unknown) => asOptionalRecord(body)?.ref as string | undefined, +}; + +test('the hub upload helper posts with credentials and returns the vendor reference', async () => { + const form = new FormData(); + globalThis.fetch = async (input, init) => { + assert.equal(String(input), hub.endpoint); + assert.equal(init?.method, 'POST'); + assert.equal(init?.body, form); + const headers = init?.headers as Record; + assert.equal(headers.Authorization, `Basic ${Buffer.from('user:key').toString('base64')}`); + assert.equal(headers['x-agent-device-version'], '0.0.0-test'); + return new Response(JSON.stringify({ ref: 'hub://APP1' }), { status: 200 }); + }; + assert.equal(await postHubAppUpload(form, hub), 'hub://APP1'); +}); + +test('the hub upload helper fails typed with the status on an error page or a missing reference', async () => { + for (const response of [ + new Response('502 Bad Gateway', { status: 502 }), + new Response(JSON.stringify({ message: 'ok' }), { status: 200 }), + ]) { + globalThis.fetch = async () => response; + await assert.rejects(postHubAppUpload(new FormData(), hub), (error: unknown) => { + assert.ok(error instanceof AppError); + assert.equal(error.code, 'COMMAND_FAILED'); + assert.equal(error.message, 'Hub app upload failed.'); + assert.equal(error.details?.status, response.status); + return true; + }); + } +}); + +test('the hub install adapter uploads the build and launches the hinted app', async () => { + const upload = vi.fn(async () => 'hub://APP2'); + const signal = new AbortController().signal; + const result = await createHubUploadApp(upload)({ + appPath: '/builds/App.ipa', + options: { appIdentifierHint: 'com.example.app' }, + signal, + }); + assert.deepEqual(upload.mock.calls, [['/builds/App.ipa', signal]]); + assert.deepEqual(result, { + appReference: 'hub://APP2', + bundleId: 'com.example.app', + packageName: undefined, + launchTarget: 'com.example.app', + }); +}); + +test('the hub app resolver passes references through, uploads local files, and passes URLs through', async () => { + const tempDir = await mkdtempForTest('agent-device-hub-resolve-'); + try { + await fs.writeFile(path.join(tempDir, 'App.apk'), 'placeholder'); + const uploadFile = vi.fn(async (appPath: string) => `hub://${path.basename(appPath)}`); + const resolve = (app: string) => + resolveHubAppReference({ + service: 'Hub', + app, + cwd: tempDir, + referenceScheme: 'hub://', + referenceLabel: 'a hub:// app id', + uploadFile, + }); + + assert.equal(await resolve('hub://APP3'), 'hub://APP3'); + assert.equal(await resolve('https://builds.example/App.apk'), 'https://builds.example/App.apk'); + assert.equal(await resolve('App.apk'), 'hub://App.apk'); + assert.deepEqual(uploadFile.mock.calls, [[path.join(tempDir, 'App.apk'), undefined]]); + await assert.rejects(resolve('missing.apk'), (error: unknown) => { + assert.ok(error instanceof AppError); + assert.equal(error.code, 'INVALID_ARGS'); + assert.equal( + error.message, + 'Hub --provider-app must be a hub:// app id, URL, or existing local app path.', + ); + return true; + }); + } finally { + await fs.rm(tempDir, { recursive: true, force: true }); + } +}); diff --git a/packages/provider-webdriver/src/webdriver-utils.ts b/packages/provider-webdriver/src/webdriver-utils.ts index 5af69a9f4c..c9fb873e4a 100644 --- a/packages/provider-webdriver/src/webdriver-utils.ts +++ b/packages/provider-webdriver/src/webdriver-utils.ts @@ -1,5 +1,17 @@ -import type { DeviceLease } from '@agent-device/contracts/device'; +import fs from 'node:fs'; +import { readFile } from 'node:fs/promises'; +import path from 'node:path'; +import type { + DeviceLease, + ProviderDeviceInstallOptions, + ProviderDeviceInstallResult, +} from '@agent-device/contracts/device'; +import { + PROVIDER_DEVICE_ORIENTATIONS, + type ProviderDeviceOrientation, +} from '@agent-device/contracts/remote'; import { AppError, errorMessage } from '@agent-device/kernel/errors'; +import { agentDeviceRequestHeaders } from './request-headers.ts'; export type LeaseValue = T | ((lease: DeviceLease) => T); @@ -50,3 +62,115 @@ export function withTrailingSlash(url: URL): URL { copy.pathname = `${copy.pathname}/`; return copy; } + +type HubCredentials = { username: string; accessKey: string }; + +/** A multipart form carrying the local app file under the hub's field name. */ +export async function appFileUploadForm(appPath: string, fileField: string): Promise { + const form = new FormData(); + form.set(fileField, new Blob([await readFile(appPath)]), path.basename(appPath)); + return form; +} + +/** + * POSTs an app upload to a hosted hub and returns the hub's app reference. A non-2xx answer, a + * body that is not JSON, or one without a reference is `COMMAND_FAILED` with the HTTP status. + */ +export async function postHubAppUpload( + form: FormData, + options: { + service: string; + endpoint: string | URL; + clientVersion: string; + auth: HubCredentials; + readAppReference: (body: unknown) => string | undefined; + }, + signal?: AbortSignal, +): Promise { + const response = await fetch(options.endpoint, { + method: 'POST', + headers: { + ...agentDeviceRequestHeaders(options.clientVersion), + Authorization: basicAuthHeader(options.auth), + }, + body: form, + signal, + }); + const json = await readProviderJsonBody(response); + const appReference = options.readAppReference(json); + if (!response.ok || !appReference) { + throw new AppError('COMMAND_FAILED', `${options.service} app upload failed.`, { + status: response.status, + response: json, + }); + } + return appReference; +} + +/** The `install` adapter of a hosted hub: upload the local build, then launch the hinted app. */ +export function createHubUploadApp( + upload: (appPath: string, signal?: AbortSignal) => Promise, +): (params: { + appPath: string; + options?: ProviderDeviceInstallOptions; + signal?: AbortSignal; +}) => Promise { + return async ({ appPath, options, signal }) => ({ + appReference: await upload(appPath, signal), + bundleId: options?.appIdentifierHint, + packageName: options?.packageNameHint, + launchTarget: options?.appIdentifierHint ?? options?.packageNameHint, + }); +} + +/** + * Turns `--provider-app` into a reference the hub accepts: its own reference scheme and public + * URLs pass through, and anything else must be a local file to upload. + */ +export async function resolveHubAppReference(options: { + service: string; + app: string; + cwd?: string; + referenceScheme: string; + /** How the scheme reads in the error message, e.g. `a bs:// app id`. */ + referenceLabel: string; + uploadFile: (appPath: string, signal?: AbortSignal) => Promise; + signal?: AbortSignal; +}): Promise { + const { app } = options; + if (app.startsWith(options.referenceScheme) || /^https?:\/\//i.test(app)) return app; + const appPath = path.resolve(options.cwd ?? process.cwd(), app); + if (!fs.existsSync(appPath)) { + throw new AppError( + 'INVALID_ARGS', + `${options.service} --provider-app must be ${options.referenceLabel}, URL, or existing local app path.`, + { providerApp: app }, + ); + } + return await options.uploadFile(appPath, options.signal); +} + +/** A provider response body parsed as JSON, or `undefined` when it is empty or not JSON (a gateway error page). */ +async function readProviderJsonBody(response: Response): Promise { + const text = await response.text(); + if (text.length === 0) return undefined; + try { + return JSON.parse(text) as unknown; + } catch { + return undefined; + } +} + +/** Validates a device-orientation flag against the shared enum before it reaches a hub that would ignore it. */ +export function requireProviderDeviceOrientation( + spec: { flag: string; capability: string }, + value: string, +): ProviderDeviceOrientation { + const match = PROVIDER_DEVICE_ORIENTATIONS.find((orientation) => orientation === value); + if (match) return match; + throw new AppError('INVALID_ARGS', `Invalid ${spec.flag} value: ${value}.`, { + hint: `Use ${PROVIDER_DEVICE_ORIENTATIONS.join('|')}.`, + flag: spec.flag, + capability: spec.capability, + }); +} From ed98d1abb28ccedf3104c549fc9f241ae35e7c0e Mon Sep 17 00:00:00 2001 From: amankansal-lt Date: Fri, 2 Oct 2026 19:45:11 +0530 Subject: [PATCH 2/9] fix(provider-webdriver): bound and type session-details lookups The BrowserStack session-details lookup behind `artifacts` had no deadline, so a stalled API call could hang the command indefinitely. A transport failure or a body that was not JSON surfaced as an untyped fetch or SyntaxError, and a JSON array passed the object check and was read as session details. Add fetchProviderSessionDetails to webdriver-utils.ts and use it for BrowserStack. It sends basic auth with a 15 second deadline and reports every failure as COMMAND_FAILED: a timeout or network error with a retry hint and the original error as its cause, and a non-2xx answer or a body that is not a JSON object with the HTTP status and the parsed response. The test holds the fetch open until the armed signal aborts and checks that the signal is the 15 second deadline. Connection verification gets the same treatment through fetchProviderVerificationJson, which BrowserStack now uses in place of its private fetch, so one helper owns the 15 second provider API deadline. Behaviour is unchanged: 401/403 is UNAUTHORIZED with a credential hint, any other HTTP failure points at the provider's service status, and a transport failure points at network access. A new test pins the two non-credential hints. sameOsVersion moves alongside it. Co-Authored-By: Claude Opus 5.5 --- .../browserstack-connection-verification.ts | 48 ++------ .../src/browserstack.test.ts | 61 +++++++++- .../provider-webdriver/src/browserstack.ts | 23 +--- .../src/connection-verification.test.ts | 29 +++++ .../provider-webdriver/src/webdriver-utils.ts | 106 ++++++++++++++++++ 5 files changed, 210 insertions(+), 57 deletions(-) diff --git a/packages/provider-webdriver/src/browserstack-connection-verification.ts b/packages/provider-webdriver/src/browserstack-connection-verification.ts index efdfb4e3c8..ce44ddb459 100644 --- a/packages/provider-webdriver/src/browserstack-connection-verification.ts +++ b/packages/provider-webdriver/src/browserstack-connection-verification.ts @@ -1,7 +1,6 @@ import path from 'node:path'; import { AppError } from '@agent-device/kernel/errors'; -import { agentDeviceRequestHeaders } from './request-headers.ts'; -import { basicAuthHeader } from './webdriver-utils.ts'; +import { fetchProviderVerificationJson, sameOsVersion } from './webdriver-utils.ts'; import type { CloudWebDriverConnectionVerification, CloudWebDriverConnectionVerificationOptions, @@ -105,42 +104,15 @@ async function fetchBrowserStackJson( auth: { username: string; accessKey: string }, clientVersion: string, ): Promise { - try { - const response = await fetch(endpoint, { - headers: { - ...agentDeviceRequestHeaders(clientVersion), - Authorization: basicAuthHeader(auth), - }, - signal: AbortSignal.timeout(15_000), - }); - if (!response.ok) { - const unauthorized = response.status === 401 || response.status === 403; - throw new AppError( - unauthorized ? 'UNAUTHORIZED' : 'COMMAND_FAILED', - 'BrowserStack rejected connection verification.', - { - status: response.status, - hint: unauthorized - ? 'Check BROWSERSTACK_USERNAME and BROWSERSTACK_ACCESS_KEY.' - : 'Retry connect or check the BrowserStack service status.', - }, - ); - } - return (await response.json()) as unknown; - } catch (error) { - if (error instanceof AppError) throw error; - throw new AppError( - 'COMMAND_FAILED', - 'BrowserStack connection verification failed.', - { hint: 'Check network access to api-cloud.browserstack.com and retry connect.' }, - error, - ); - } -} - -function sameOsVersion(left: string, right: string): boolean { - const normalize = (value: string) => value.replace(/(?:\.0)+$/, ''); - return normalize(left) === normalize(right); + return await fetchProviderVerificationJson(endpoint, { + clientVersion, + auth, + hints: { + service: 'BrowserStack', + unauthorizedHint: 'Check BROWSERSTACK_USERNAME and BROWSERSTACK_ACCESS_KEY.', + networkHint: 'Check network access to api-cloud.browserstack.com and retry connect.', + }, + }); } function readBrowserStackDevices( diff --git a/packages/provider-webdriver/src/browserstack.test.ts b/packages/provider-webdriver/src/browserstack.test.ts index 61cf7bdbb6..70a6a38a95 100644 --- a/packages/provider-webdriver/src/browserstack.test.ts +++ b/packages/provider-webdriver/src/browserstack.test.ts @@ -2,9 +2,13 @@ import assert from 'node:assert/strict'; import { promises as fs } from 'node:fs'; import path from 'node:path'; -import { afterEach, test } from 'vitest'; +import { afterEach, test, vi } from 'vitest'; import { AppError } from '@agent-device/kernel/errors'; -import { resolveBrowserStackAppReference, uploadBrowserStackApp } from './browserstack.ts'; +import { + listBrowserStackCloudArtifacts, + resolveBrowserStackAppReference, + uploadBrowserStackApp, +} from './browserstack.ts'; import { mkdtempForTest } from './tmp-dir.fixtures.ts'; const realFetch = globalThis.fetch; @@ -101,3 +105,56 @@ test('BrowserStack passes bs:// ids and URLs to the hub and uploads only local p await fs.rm(tempDir, { recursive: true, force: true }); } }); + +test('BrowserStack session details lookup has a deadline and fails typed', async () => { + const lookup = async () => + await listBrowserStackCloudArtifacts('browserstack', 'SESSION1', upload); + // The fetch stays pending until the signal the lookup armed aborts, so only the 15 s deadline + // can end it; without that deadline the timeout spy is never called and the next assert fails. + const deadline = new AbortController(); + const timeoutSpy = vi.spyOn(AbortSignal, 'timeout').mockImplementation(() => deadline.signal); + let started: () => void = () => {}; + const fetchStarted = new Promise((resolve) => { + started = resolve; + }); + globalThis.fetch = async (_input, init) => + await new Promise((_resolve, reject) => { + init?.signal?.addEventListener('abort', () => reject(init.signal?.reason), { once: true }); + started(); + }); + try { + const pending = lookup(); + await fetchStarted; + assert.deepEqual(timeoutSpy.mock.calls, [[15_000]]); + const timeout = new DOMException('The operation was aborted due to timeout', 'TimeoutError'); + deadline.abort(timeout); + await assert.rejects(pending, (error: unknown) => { + assert.ok(error instanceof AppError); + assert.equal(error.code, 'COMMAND_FAILED'); + assert.equal(error.message, 'BrowserStack session details lookup failed.'); + assert.equal(error.cause, timeout); + return true; + }); + } finally { + timeoutSpy.mockRestore(); + } + + const networkFailure = new TypeError('fetch failed'); + globalThis.fetch = async () => { + throw networkFailure; + }; + await assert.rejects( + lookup(), + (error: unknown) => error instanceof AppError && error.cause === networkFailure, + ); + + for (const body of ['gateway', '[]']) { + globalThis.fetch = async () => new Response(body, { status: 200 }); + await assert.rejects(lookup(), (error: unknown) => { + assert.ok(error instanceof AppError); + assert.equal(error.code, 'COMMAND_FAILED'); + assert.equal(error.details?.status, 200); + return true; + }); + } +}); diff --git a/packages/provider-webdriver/src/browserstack.ts b/packages/provider-webdriver/src/browserstack.ts index 800bd0c241..0f942d132b 100644 --- a/packages/provider-webdriver/src/browserstack.ts +++ b/packages/provider-webdriver/src/browserstack.ts @@ -1,13 +1,11 @@ import type { CloudArtifact, CloudArtifactsResult } from '@agent-device/contracts/observability'; import type { CloudWebDriverCapabilityOverrides } from './capabilities.ts'; import type { CloudWebDriverUploadApp } from './runtime.ts'; -import { AppError } from '@agent-device/kernel/errors'; -import { agentDeviceRequestHeaders } from './request-headers.ts'; import { cloudArtifactsReadyOrPending, urlArtifactFromDetails } from './artifact-results.ts'; import { appFileUploadForm, - basicAuthHeader, createHubUploadApp, + fetchProviderSessionDetails, postHubAppUpload, resolveHubAppReference, trimTrailingSlash, @@ -161,21 +159,12 @@ async function fetchBrowserStackSessionDetails( const endpoint = new URL( `${trimTrailingSlash(String(options.endpoint ?? BROWSERSTACK_SESSION_DETAILS_ENDPOINT))}/${sessionId}.json`, ); - const response = await fetch(endpoint, { - headers: { - ...agentDeviceRequestHeaders(options.clientVersion), - Authorization: basicAuthHeader(options), - }, + const json = await fetchProviderSessionDetails(endpoint, { + clientVersion: options.clientVersion, + auth: options, + service: 'BrowserStack', }); - const json = (await response.json()) as unknown; - if (!response.ok || !json || typeof json !== 'object') { - throw new AppError('COMMAND_FAILED', 'BrowserStack session details lookup failed.', { - status: response.status, - response: json, - }); - } - const details = (json as { automation_session?: unknown }).automation_session ?? json; - return details && typeof details === 'object' ? (details as Record) : {}; + return asRecord(json.automation_session ?? json); } function mapBrowserStackArtifacts( diff --git a/packages/provider-webdriver/src/connection-verification.test.ts b/packages/provider-webdriver/src/connection-verification.test.ts index e9e7f9c726..c93e40109e 100644 --- a/packages/provider-webdriver/src/connection-verification.test.ts +++ b/packages/provider-webdriver/src/connection-verification.test.ts @@ -80,6 +80,35 @@ test('BrowserStack classifies rejected credentials without exposing them', async }); }); +test('BrowserStack points HTTP failures at its service status and transport failures at the network', async () => { + vi.stubGlobal( + 'fetch', + vi.fn(async () => jsonResponse({}, 503)), + ); + await assert.rejects(createProvider().verifyConnection(browserStackOptions), (error: unknown) => { + assert.equal((error as { code?: string }).code, 'COMMAND_FAILED'); + assert.equal( + (error as { details?: { hint?: string } }).details?.hint, + 'Retry connect or check the BrowserStack service status.', + ); + return true; + }); + + vi.stubGlobal( + 'fetch', + vi.fn(async () => { + throw new TypeError('fetch failed'); + }), + ); + await assert.rejects(createProvider().verifyConnection(browserStackOptions), (error: unknown) => { + assert.equal( + (error as { details?: { hint?: string } }).details?.hint, + 'Check network access to api-cloud.browserstack.com and retry connect.', + ); + return true; + }); +}); + test('BrowserStack defers a bs app reference outside the recent upload window', async () => { vi.stubGlobal( 'fetch', diff --git a/packages/provider-webdriver/src/webdriver-utils.ts b/packages/provider-webdriver/src/webdriver-utils.ts index c9fb873e4a..747a557779 100644 --- a/packages/provider-webdriver/src/webdriver-utils.ts +++ b/packages/provider-webdriver/src/webdriver-utils.ts @@ -150,6 +150,106 @@ export async function resolveHubAppReference(options: { return await options.uploadFile(appPath, options.signal); } +const PROVIDER_API_TIMEOUT_MS = 15_000; + +/** Service name and remediation hints a verification failure carries. */ +type ProviderJsonFailureHints = { + service: string; + unauthorizedHint: string; + networkHint: string; +}; + +/** + * Fetches JSON from a hosted provider's API during connection verification. A 401/403 is + * `UNAUTHORIZED` with a credential hint, any other non-2xx is `COMMAND_FAILED`, and a transport + * failure is wrapped so its cause survives without leaking the credentials. + */ +export async function fetchProviderVerificationJson( + endpoint: string | URL, + options: { + clientVersion: string; + auth: { username: string; accessKey: string }; + hints: ProviderJsonFailureHints; + }, +): Promise { + const { service, unauthorizedHint, networkHint } = options.hints; + try { + const response = await fetch(endpoint, { + headers: { + ...agentDeviceRequestHeaders(options.clientVersion), + Authorization: basicAuthHeader(options.auth), + }, + signal: AbortSignal.timeout(PROVIDER_API_TIMEOUT_MS), + }); + if (!response.ok) { + const unauthorized = response.status === 401 || response.status === 403; + throw new AppError( + unauthorized ? 'UNAUTHORIZED' : 'COMMAND_FAILED', + `${service} rejected connection verification.`, + { + status: response.status, + hint: unauthorized + ? unauthorizedHint + : `Retry connect or check the ${service} service status.`, + }, + ); + } + return (await response.json()) as unknown; + } catch (error) { + if (error instanceof AppError) throw error; + throw new AppError( + 'COMMAND_FAILED', + `${service} connection verification failed.`, + { hint: networkHint }, + error, + ); + } +} + +/** + * Fetches a provider's session-details JSON with basic auth under a deadline. A transport failure, + * a non-2xx answer, or a body that is not a JSON object is `COMMAND_FAILED`. + */ +export async function fetchProviderSessionDetails( + endpoint: string | URL, + options: { + clientVersion: string; + auth: { username: string; accessKey: string }; + service: string; + }, +): Promise> { + let response: Response; + let json: unknown; + try { + response = await fetch(endpoint, { + headers: { + ...agentDeviceRequestHeaders(options.clientVersion), + Authorization: basicAuthHeader(options.auth), + }, + signal: AbortSignal.timeout(PROVIDER_API_TIMEOUT_MS), + }); + json = await readProviderJsonBody(response); + } catch (error) { + throw new AppError( + 'COMMAND_FAILED', + `${options.service} session details lookup failed.`, + { hint: `Check network access to the ${options.service} API, then retry.` }, + error, + ); + } + const details = + json && typeof json === 'object' && !Array.isArray(json) + ? (json as Record) + : undefined; + if (!response.ok || !details) { + throw new AppError('COMMAND_FAILED', `${options.service} session details lookup failed.`, { + status: response.status, + response: json, + }); + } + return details; +} + /** A provider response body parsed as JSON, or `undefined` when it is empty or not JSON (a gateway error page). */ async function readProviderJsonBody(response: Response): Promise { const text = await response.text(); @@ -161,6 +261,12 @@ async function readProviderJsonBody(response: Response): Promise { } } +/** `1.0` and `1` name the same OS release on BrowserStack's catalog. */ +export function sameOsVersion(left: string, right: string): boolean { + const normalize = (value: string) => value.replace(/(?:\.0)+$/, ''); + return normalize(left) === normalize(right); +} + /** Validates a device-orientation flag against the shared enum before it reaches a hub that would ignore it. */ export function requireProviderDeviceOrientation( spec: { flag: string; capability: string }, From 8ec2de643903084e26e8d6ada4b2c5ffb7a2e773 Mon Sep 17 00:00:00 2001 From: amankansal-lt Date: Fri, 2 Oct 2026 22:28:39 +0530 Subject: [PATCH 3/9] fix(provider-webdriver): upload the file the materializer names for a hosted install Install from a remote source materializes an iOS build by extracting the `.app` bundle from a zipped simulator build or an .ipa. The WebDriver deployment runtime handed that extracted `.app` directory to the provider's uploader. Hosted upload APIs take a file, not a directory, so the upload could not succeed. Materialization now records the archive an installable was extracted from directly, and the Apple materializer declares the file a hosted provider uploads: the .ipa itself, or the zip a simulator .app was extracted from. An outer archive that merely wrapped either, such as a URL .zip around an .ipa, is never it, and nothing is named when no such file exists (a .tar, for instance). The deployment runtime uploads the declared file and otherwise the installable path, with no inference of its own. A provider without an uploader still installs the extracted bundle path, and the bundle id and launch target hints are unchanged. Co-Authored-By: Claude Opus 5.5 --- .../contracts/src/app-deployment-runtime.ts | 5 + .../src/core/install-artifact.ts | 27 ++++- .../src/runtime-deployment.test.ts | 104 ++++++++++++++++++ .../src/runtime-deployment.ts | 22 +++- .../src/install-source-archive.ts | 29 ++++- packages/provision-kit/src/install-source.ts | 5 + src/__tests__/install-source.test.ts | 38 +++++++ 7 files changed, 220 insertions(+), 10 deletions(-) diff --git a/packages/contracts/src/app-deployment-runtime.ts b/packages/contracts/src/app-deployment-runtime.ts index 52e581c3c9..711a6858c6 100644 --- a/packages/contracts/src/app-deployment-runtime.ts +++ b/packages/contracts/src/app-deployment-runtime.ts @@ -13,6 +13,11 @@ export type AppDeploymentSource = export type MaterializedAppSource = Readonly<{ archivePath?: string; installablePath: string; + /** + * The single file a hosted provider uploads for this build, named by the materializer that knows + * its format. Absent when the installable is already that file or no such file exists. + */ + uploadPath?: string; bundleId?: string; packageName?: string; appName?: string; diff --git a/packages/platform-apple/src/core/install-artifact.ts b/packages/platform-apple/src/core/install-artifact.ts index 46c4e913e6..5fe729906d 100644 --- a/packages/platform-apple/src/core/install-artifact.ts +++ b/packages/platform-apple/src/core/install-artifact.ts @@ -2,7 +2,11 @@ import path from 'node:path'; import type { LocalInstallSource } from '@agent-device/kernel/contracts'; import { readIosBundleInfo } from './bundle-info.ts'; import { AppError } from '@agent-device/kernel/errors'; -import { extractArchiveSafely, ArchiveBudget } from '@agent-device/host-kit/archive'; +import { + archiveTypeFromPath, + extractArchiveSafely, + ArchiveBudget, +} from '@agent-device/host-kit/archive'; import { installArtifactArchiveBudget, @@ -32,6 +36,7 @@ type IosPayloadAppBundle = { export type PreparedIosInstallArtifact = { archivePath?: string; installablePath: string; + uploadPath?: string; bundleId?: string; appName?: string; cleanup: () => Promise; @@ -70,9 +75,11 @@ async function prepareIosInstallArtifactInScope( ? materialized.installablePath : undefined); + const uploadPath = iosUploadPath(materialized); return { archivePath, installablePath: resolvedInstallable.installPath, + ...(uploadPath ? { uploadPath } : {}), bundleId: bundleInfo.bundleId, appName: bundleInfo.appName, cleanup: async () => { @@ -92,6 +99,24 @@ async function prepareIosInstallArtifactInScope( export { readIosBundleInfo } from './bundle-info.ts'; +/** + * The installable is an extracted `.app` directory, which no hosted upload API accepts. The file + * that carries it is the `.ipa` it was unpacked from, or the zip the `.app` was extracted from + * directly; an outer archive that merely wrapped either is never it. + */ +function iosUploadPath(materialized: { + containingArchivePath?: string; + installablePath: string; +}): string | undefined { + if (materialized.installablePath.toLowerCase().endsWith('.ipa')) { + return materialized.installablePath; + } + const { containingArchivePath } = materialized; + return containingArchivePath && archiveTypeFromPath(containingArchivePath) === 'zip' + ? containingArchivePath + : undefined; +} + async function resolveIosInstallablePath( appPath: string, options?: InstallIosArtifactOptions, diff --git a/packages/provider-webdriver/src/runtime-deployment.test.ts b/packages/provider-webdriver/src/runtime-deployment.test.ts index 67b06e14a9..d1f4430876 100644 --- a/packages/provider-webdriver/src/runtime-deployment.test.ts +++ b/packages/provider-webdriver/src/runtime-deployment.test.ts @@ -1,6 +1,9 @@ +import { promises as fs } from 'node:fs'; +import path from 'node:path'; import { expect, test, vi } from 'vitest'; import { createCloudWebDriverCapabilities } from './capabilities.ts'; import type { DeviceInfo } from '@agent-device/kernel/device'; +import { mkdtempForTest } from './tmp-dir.fixtures.ts'; import { createWebDriverDeploymentRuntime } from './runtime-deployment.ts'; import type { WebDriverProviderSession } from './runtime-session.ts'; import type { CloudWebDriverUploadApp } from './runtime.ts'; @@ -14,6 +17,8 @@ const device: DeviceInfo = { booted: true, }; +const iosDevice: DeviceInfo = { ...device, platform: 'apple', id: 'webdriver:ios' }; + test('keeps a stale WebDriver owner unavailable before any deployment attempt', () => { const deployment = createWebDriverDeploymentRuntime({ provider: 'webdriver-test', @@ -75,6 +80,105 @@ test('aborts a WebDriver provider deployment while its install is in flight', as expect(installApp).toHaveBeenCalledWith('bs://uploaded-app', controller.signal); }); +// The materializer knows which file carries the build; the deployment runtime must not second-guess +// it from extensions, or a URL zip wrapping an .ipa would upload the wrapper. +test('a hosted upload sends the file the materializer names, else the installable', async () => { + const uploaded: string[] = []; + const installApp = vi.fn(async () => undefined); + const deployment = createWebDriverDeploymentRuntime({ + provider: 'webdriver-test', + uploadApp: async ({ appPath }) => { + uploaded.push(appPath); + return { appReference: `hub://${uploaded.length}` }; + }, + findSessionForDevice: () => activeSession(installApp), + }); + const deploy = async (selected: DeviceInfo, artifact: Record) => + await deployment.deployMaterializedApp( + selected, + { artifact: { installablePath: '', ...artifact, cleanup: async () => {} } }, + new AbortController().signal, + ); + + const result = await deploy(iosDevice, { + archivePath: '/m/App.app.zip', + installablePath: '/m/extracted/App.app', + uploadPath: '/m/App.app.zip', + bundleId: 'com.example.app', + }); + await deploy(iosDevice, { + archivePath: '/m/wrapper.zip', + installablePath: '/m/x/Payload/App.app', + uploadPath: '/m/x/App.ipa', + }); + await deploy(iosDevice, { archivePath: '/m/App.tar.gz', installablePath: '/m/x/App.app' }); + await deploy(device, { archivePath: '/m/build.zip', installablePath: '/m/x/app.apk' }); + + expect(uploaded).toEqual(['/m/App.app.zip', '/m/x/App.ipa', '/m/x/App.app', '/m/x/app.apk']); + expect(result).toEqual({ bundleId: 'com.example.app', launchTarget: 'com.example.app' }); + expect(installApp).toHaveBeenNthCalledWith(1, 'hub://1', expect.any(AbortSignal)); +}); + +test('a provider without an uploader still installs the materialized bundle path', async () => { + const installApp = vi.fn(async () => undefined); + const deployment = createWebDriverDeploymentRuntime({ + provider: 'webdriver-test', + findSessionForDevice: () => activeSession(installApp), + }); + await deployment.deployMaterializedApp( + iosDevice, + { + artifact: { + archivePath: '/m/App.app.zip', + installablePath: '/m/extracted/App.app', + uploadPath: '/m/App.app.zip', + bundleId: 'com.example.app', + cleanup: async () => {}, + }, + }, + new AbortController().signal, + ); + expect(installApp).toHaveBeenCalledWith('/m/extracted/App.app', expect.any(AbortSignal)); +}); + +test('a hosted upload reads the zipped simulator build that install-from-source extracted', async () => { + const tempDir = await mkdtempForTest('agent-device-materialized-upload-'); + try { + const archivePath = path.join(tempDir, 'App.app.zip'); + const installablePath = path.join(tempDir, 'extracted', 'App.app'); + await fs.writeFile(archivePath, 'zip bytes'); + await fs.mkdir(installablePath, { recursive: true }); + const uploadedBytes: string[] = []; + const installApp = vi.fn(async () => undefined); + const deployment = createWebDriverDeploymentRuntime({ + provider: 'webdriver-test', + uploadApp: async ({ appPath }) => { + uploadedBytes.push(await fs.readFile(appPath, 'utf8')); + return { appReference: 'hub://APP42' }; + }, + findSessionForDevice: () => activeSession(installApp), + }); + + await deployment.deployMaterializedApp( + iosDevice, + { + artifact: { + archivePath, + installablePath, + uploadPath: archivePath, + cleanup: async () => {}, + }, + }, + new AbortController().signal, + ); + + expect(uploadedBytes).toEqual(['zip bytes']); + expect(installApp).toHaveBeenCalledWith('hub://APP42', expect.any(AbortSignal)); + } finally { + await fs.rm(tempDir, { recursive: true, force: true }); + } +}); + function activeSession( installApp: (appPath: string, signal?: AbortSignal) => Promise, ): WebDriverProviderSession { diff --git a/packages/provider-webdriver/src/runtime-deployment.ts b/packages/provider-webdriver/src/runtime-deployment.ts index 07e3ecf16d..fb2a058b86 100644 --- a/packages/provider-webdriver/src/runtime-deployment.ts +++ b/packages/provider-webdriver/src/runtime-deployment.ts @@ -49,10 +49,10 @@ export function createWebDriverDeploymentRuntime( findSessionForDevice(device: DeviceInfo): WebDriverProviderSession | undefined; }>, ): WebDriverDeploymentRuntime { - const installApp = async ( + const install = async ( device: DeviceInfo, app: string, - appPath: string, + paths: Readonly<{ appPath: string; uploadPath: string }>, installOptions?: ProviderDeviceInstallOptions, signal?: AbortSignal, ): Promise => { @@ -63,13 +63,20 @@ export function createWebDriverDeploymentRuntime( session, device, app, - appPath, + paths.uploadPath, installOptions, signal, ); - await session.client.installApp(upload?.appReference ?? appPath, signal); + await session.client.installApp(upload?.appReference ?? paths.appPath, signal); return providerInstallResult(upload, installOptions); }; + const installApp = async ( + device: DeviceInfo, + app: string, + appPath: string, + installOptions?: ProviderDeviceInstallOptions, + signal?: AbortSignal, + ) => await install(device, app, { appPath, uploadPath: appPath }, installOptions, signal); return Object.freeze({ fact: (device) => deploymentFact(options.findSessionForDevice(device)), installApp, @@ -92,10 +99,13 @@ export function createWebDriverDeploymentRuntime( ), deployMaterializedApp: async (device, input, signal) => deploymentResult( - await installApp( + await install( device, '', - input.artifact.installablePath, + { + appPath: input.artifact.installablePath, + uploadPath: input.artifact.uploadPath ?? input.artifact.installablePath, + }, { appIdentifierHint: input.artifact.bundleId, packageNameHint: input.artifact.packageName, diff --git a/packages/provision-kit/src/install-source-archive.ts b/packages/provision-kit/src/install-source-archive.ts index 366f63b68f..deef08ad38 100644 --- a/packages/provision-kit/src/install-source-archive.ts +++ b/packages/provision-kit/src/install-source-archive.ts @@ -15,10 +15,32 @@ type InstallableMatcher = ( stat: { isFile(): boolean; isDirectory(): boolean }, ) => boolean; +type ResolvedInstallableCandidate = { + /** The outermost archive the source arrived as. */ + archivePath?: string; + /** The innermost archive, the one the installable was extracted from directly. */ + containingArchivePath?: string; + installablePath: string; +}; + +function resolvedCandidate( + installablePath: string, + params: { archivePath: string | undefined; containingArchivePath?: string }, +): ResolvedInstallableCandidate { + return { + archivePath: params.archivePath, + ...(params.containingArchivePath + ? { containingArchivePath: params.containingArchivePath } + : {}), + installablePath, + }; +} + export async function resolveInstallableCandidate( candidatePath: string, params: { archivePath: string | undefined; + containingArchivePath?: string; isInstallablePath: InstallableMatcher; installableLabel: string; registerCleanup: (cleanup: () => Promise) => void; @@ -26,11 +48,11 @@ export async function resolveInstallableCandidate( archiveDepth: number; onArchiveAccepted?: (depth: number) => void; }, -): Promise<{ archivePath?: string; installablePath: string }> { +): Promise { const stat = await fs.stat(candidatePath).catch(() => null); if (!stat) throw new AppError('INVALID_ARGS', `App source not found: ${candidatePath}`); if (params.isInstallablePath(candidatePath, stat)) { - return { archivePath: params.archivePath, installablePath: candidatePath }; + return resolvedCandidate(candidatePath, params); } if (stat.isFile() && isArchivePath(candidatePath)) { return await resolveExtractedArchive(candidatePath, params); @@ -38,7 +60,7 @@ export async function resolveInstallableCandidate( if (stat.isDirectory()) { const installables = await collectMatchingPaths(candidatePath, params.isInstallablePath); if (installables.length === 1) { - return { archivePath: params.archivePath, installablePath: installables[0]! }; + return resolvedCandidate(installables[0]!, params); } if (installables.length > 1) { throw new AppError( @@ -77,6 +99,7 @@ async function resolveExtractedArchive( return await resolveInstallableCandidate(extracted.outputPath, { ...params, archivePath: params.archivePath ?? archivePath, + containingArchivePath: archivePath, archiveDepth: params.archiveDepth + 1, }); } diff --git a/packages/provision-kit/src/install-source.ts b/packages/provision-kit/src/install-source.ts index 1cbe44931c..3f005f8825 100644 --- a/packages/provision-kit/src/install-source.ts +++ b/packages/provision-kit/src/install-source.ts @@ -32,6 +32,8 @@ export type MaterializeInstallableOptions = { export type MaterializedInstallable = { archivePath?: string; + /** The archive the installable was extracted from directly, when it came out of one. */ + containingArchivePath?: string; installablePath: string; cleanup: () => Promise; }; @@ -67,6 +69,9 @@ export async function materializeInstallablePath( }); return { archivePath: resolved.archivePath, + ...(resolved.containingArchivePath + ? { containingArchivePath: resolved.containingArchivePath } + : {}), installablePath: resolved.installablePath, cleanup: async () => { await runCleanupTasks(cleanupTasks); diff --git a/src/__tests__/install-source.test.ts b/src/__tests__/install-source.test.ts index 082ed328d1..8ab235186c 100644 --- a/src/__tests__/install-source.test.ts +++ b/src/__tests__/install-source.test.ts @@ -380,6 +380,8 @@ test('prepareIosInstallArtifact extracts GitHub artifact ZIP containing nested a try { assert.equal(path.basename(result.installablePath), 'Demo.app'); + // A tar has no hosted upload API, so nothing is named uploadable. + assert.equal(result.uploadPath, undefined); assert.equal(result.bundleId, 'com.example.githubtar'); assert.equal(result.appName, 'GitHub Tar'); } finally { @@ -419,6 +421,9 @@ test('prepareIosInstallArtifact extracts GitHub artifact ZIP containing one IPA' try { assert.equal(path.basename(result.installablePath), 'Demo.app'); + // The .ipa, not the artifact zip that wrapped it, is what a hosted provider uploads. + assert.equal(path.basename(result.uploadPath ?? ''), 'Demo.ipa'); + assert.equal((await fs.stat(result.uploadPath ?? '')).isFile(), true); assert.equal(result.bundleId, 'com.example.githubipa'); assert.equal(result.appName, 'GitHub IPA'); } finally { @@ -430,6 +435,39 @@ test('prepareIosInstallArtifact extracts GitHub artifact ZIP containing one IPA' ); }); +test('prepareIosInstallArtifact names the zip a simulator app arrived in as its upload', async () => { + await withArchiveFixture( + { + extractions: [ + { + command: 'unzip', + populate: async (outputPath) => { + await fs.mkdir(path.join(outputPath, 'Demo.app')); + }, + }, + ], + }, + async () => { + await withIosBundleInfo('com.example.githubapp', 'GitHub App', async () => { + await withMockedInstallSourceFetch(Buffer.from('artifact fixture'), async () => { + const result = await prepareIosInstallArtifact({ + kind: 'url', + url: 'https://api.github.com/repos/acme/app/actions/artifacts/989/zip', + }); + + try { + assert.equal(path.basename(result.installablePath), 'Demo.app'); + assert.equal(result.uploadPath, result.archivePath); + assert.equal((await fs.stat(result.uploadPath ?? '')).isFile(), true); + } finally { + await result.cleanup(); + } + }); + }); + }, + ); +}); + test('prepareIosInstallArtifact cleans URL materialization when IPA payload resolution fails', async () => { await withArchiveFixture( { From 744a5ff0b67c73e89bd60b77ff971b386e76ccb5 Mon Sep 17 00:00:00 2001 From: amankansal-lt Date: Fri, 2 Oct 2026 22:55:35 +0530 Subject: [PATCH 4/9] fix(provider-webdriver): refuse anything but a regular file before any hosted upload A materialized build that names no uploadable file falls back to its installable path, which for iOS is the extracted .app directory. The deployment runtime handed that to the hub's uploader, and BrowserStack's read it as a file and failed with a raw EISDIR. A --provider-app that points at a directory failed the same way. appFileUploadForm, which every hub upload builds its form with, now refuses any path that is not an existing regular file with INVALID_ARGS before any request is sent. The message names the hub's display label, the details carry the provider id and path, and the hint lists the files a hosted upload takes. Both the materialized install and --provider-app routes go through it. Co-Authored-By: Claude Opus 5.5 --- .../src/browserstack.test.ts | 26 +++++++++++ .../provider-webdriver/src/browserstack.ts | 6 ++- .../src/runtime-deployment.test.ts | 45 ++++++++++++++++++- .../src/webdriver-utils.test.ts | 30 +++++++++++++ .../provider-webdriver/src/webdriver-utils.ts | 26 +++++++++-- 5 files changed, 128 insertions(+), 5 deletions(-) diff --git a/packages/provider-webdriver/src/browserstack.test.ts b/packages/provider-webdriver/src/browserstack.test.ts index 70a6a38a95..d487c0c74f 100644 --- a/packages/provider-webdriver/src/browserstack.test.ts +++ b/packages/provider-webdriver/src/browserstack.test.ts @@ -106,6 +106,32 @@ test('BrowserStack passes bs:// ids and URLs to the hub and uploads only local p } }); +test('BrowserStack refuses a directory typed before any upload request on every route', async () => { + const tempDir = await mkdtempForTest('agent-device-browserstack-directory-'); + const bundlePath = path.join(tempDir, 'App.app'); + try { + await fs.mkdir(bundlePath); + const fetchSpy = vi.fn(); + globalThis.fetch = fetchSpy; + const refusedTyped = (error: unknown) => { + assert.ok(error instanceof AppError); + assert.equal(error.code, 'INVALID_ARGS'); + assert.equal(error.message, `BrowserStack can only upload a regular app file: ${bundlePath}`); + assert.equal(error.details?.provider, 'browserstack'); + return true; + }; + + await assert.rejects(uploadBrowserStackApp(bundlePath, upload), refusedTyped); + await assert.rejects( + resolveBrowserStackAppReference('App.app', { ...upload, cwd: tempDir }), + refusedTyped, + ); + assert.equal(fetchSpy.mock.calls.length, 0); + } finally { + await fs.rm(tempDir, { recursive: true, force: true }); + } +}); + test('BrowserStack session details lookup has a deadline and fails typed', async () => { const lookup = async () => await listBrowserStackCloudArtifacts('browserstack', 'SESSION1', upload); diff --git a/packages/provider-webdriver/src/browserstack.ts b/packages/provider-webdriver/src/browserstack.ts index 0f942d132b..08f50fad98 100644 --- a/packages/provider-webdriver/src/browserstack.ts +++ b/packages/provider-webdriver/src/browserstack.ts @@ -1,6 +1,7 @@ import type { CloudArtifact, CloudArtifactsResult } from '@agent-device/contracts/observability'; import type { CloudWebDriverCapabilityOverrides } from './capabilities.ts'; import type { CloudWebDriverUploadApp } from './runtime.ts'; +import { CLOUD_WEBDRIVER_PROVIDERS } from './providers.ts'; import { cloudArtifactsReadyOrPending, urlArtifactFromDetails } from './artifact-results.ts'; import { appFileUploadForm, @@ -80,7 +81,10 @@ export async function uploadBrowserStackApp( ): Promise { signal?.throwIfAborted(); return await postHubAppUpload( - await appFileUploadForm(appPath, 'file'), + await appFileUploadForm(appPath, 'file', { + provider: CLOUD_WEBDRIVER_PROVIDERS.browserStack, + service: 'BrowserStack', + }), { service: 'BrowserStack', endpoint: options.endpoint ?? BROWSERSTACK_APP_UPLOAD_ENDPOINT, diff --git a/packages/provider-webdriver/src/runtime-deployment.test.ts b/packages/provider-webdriver/src/runtime-deployment.test.ts index d1f4430876..ce8b5f4c93 100644 --- a/packages/provider-webdriver/src/runtime-deployment.test.ts +++ b/packages/provider-webdriver/src/runtime-deployment.test.ts @@ -1,8 +1,9 @@ import { promises as fs } from 'node:fs'; import path from 'node:path'; -import { expect, test, vi } from 'vitest'; +import { afterEach, expect, test, vi } from 'vitest'; import { createCloudWebDriverCapabilities } from './capabilities.ts'; import type { DeviceInfo } from '@agent-device/kernel/device'; +import { createBrowserStackUploadApp } from './browserstack.ts'; import { mkdtempForTest } from './tmp-dir.fixtures.ts'; import { createWebDriverDeploymentRuntime } from './runtime-deployment.ts'; import type { WebDriverProviderSession } from './runtime-session.ts'; @@ -18,6 +19,11 @@ const device: DeviceInfo = { }; const iosDevice: DeviceInfo = { ...device, platform: 'apple', id: 'webdriver:ios' }; +const realFetch = globalThis.fetch; + +afterEach(() => { + globalThis.fetch = realFetch; +}); test('keeps a stale WebDriver owner unavailable before any deployment attempt', () => { const deployment = createWebDriverDeploymentRuntime({ @@ -119,6 +125,43 @@ test('a hosted upload sends the file the materializer names, else the installabl expect(installApp).toHaveBeenNthCalledWith(1, 'hub://1', expect.any(AbortSignal)); }); +test('a hosted upload refuses a directory typed before any upload request', async () => { + const tempDir = await mkdtempForTest('agent-device-materialized-directory-'); + try { + const installablePath = path.join(tempDir, 'extracted', 'App.app'); + await fs.mkdir(installablePath, { recursive: true }); + const fetchSpy = vi.fn(); + globalThis.fetch = fetchSpy; + const installApp = vi.fn(async () => undefined); + const deployment = createWebDriverDeploymentRuntime({ + provider: 'browserstack', + uploadApp: createBrowserStackUploadApp({ + clientVersion: '0.0.0-test', + username: 'user', + accessKey: 'key', + endpoint: 'https://upload.example.test/app', + }), + findSessionForDevice: () => activeSession(installApp), + }); + + await expect( + deployment.deployMaterializedApp( + iosDevice, + { artifact: { installablePath, cleanup: async () => {} } }, + new AbortController().signal, + ), + ).rejects.toMatchObject({ + code: 'INVALID_ARGS', + message: `BrowserStack can only upload a regular app file: ${installablePath}`, + details: expect.objectContaining({ provider: 'browserstack' }), + }); + expect(fetchSpy).not.toHaveBeenCalled(); + expect(installApp).not.toHaveBeenCalled(); + } finally { + await fs.rm(tempDir, { recursive: true, force: true }); + } +}); + test('a provider without an uploader still installs the materialized bundle path', async () => { const installApp = vi.fn(async () => undefined); const deployment = createWebDriverDeploymentRuntime({ diff --git a/packages/provider-webdriver/src/webdriver-utils.test.ts b/packages/provider-webdriver/src/webdriver-utils.test.ts index 05c9e140f9..090071d755 100644 --- a/packages/provider-webdriver/src/webdriver-utils.test.ts +++ b/packages/provider-webdriver/src/webdriver-utils.test.ts @@ -5,6 +5,7 @@ import { afterEach, test, vi } from 'vitest'; import { AppError } from '@agent-device/kernel/errors'; import { asOptionalRecord } from '@agent-device/kernel/record'; import { + appFileUploadForm, createHubUploadApp, postHubAppUpload, resolveHubAppReference, @@ -120,3 +121,32 @@ test('the hub app resolver passes references through, uploads local files, and p await fs.rm(tempDir, { recursive: true, force: true }); } }); + +test('appFileUploadForm carries a regular app file and refuses anything else typed', async () => { + const tempDir = await mkdtempForTest('agent-device-upload-form-'); + try { + const appPath = path.join(tempDir, 'App.ipa'); + const bundlePath = path.join(tempDir, 'App.app'); + const missingPath = path.join(tempDir, 'Missing.ipa'); + await fs.writeFile(appPath, 'ipa bytes'); + await fs.mkdir(bundlePath); + const hub = { provider: 'hub', service: 'Hub' }; + + const file = (await appFileUploadForm(appPath, 'file', hub)).get('file') as File; + assert.equal(file.name, 'App.ipa'); + assert.equal(await file.text(), 'ipa bytes'); + + for (const refusedPath of [bundlePath, missingPath]) { + await assert.rejects(appFileUploadForm(refusedPath, 'file', hub), (error: unknown) => { + assert.ok(error instanceof AppError); + assert.equal(error.code, 'INVALID_ARGS'); + assert.equal(error.message, `Hub can only upload a regular app file: ${refusedPath}`); + assert.equal(error.details?.provider, 'hub'); + assert.equal(error.details?.appPath, refusedPath); + return true; + }); + } + } finally { + await fs.rm(tempDir, { recursive: true, force: true }); + } +}); diff --git a/packages/provider-webdriver/src/webdriver-utils.ts b/packages/provider-webdriver/src/webdriver-utils.ts index 747a557779..df7fddae3f 100644 --- a/packages/provider-webdriver/src/webdriver-utils.ts +++ b/packages/provider-webdriver/src/webdriver-utils.ts @@ -1,5 +1,5 @@ import fs from 'node:fs'; -import { readFile } from 'node:fs/promises'; +import { readFile, stat } from 'node:fs/promises'; import path from 'node:path'; import type { DeviceLease, @@ -65,8 +65,28 @@ export function withTrailingSlash(url: URL): URL { type HubCredentials = { username: string; accessKey: string }; -/** A multipart form carrying the local app file under the hub's field name. */ -export async function appFileUploadForm(appPath: string, fileField: string): Promise { +/** + * A multipart form carrying the local app file under the hub's field name. Upload APIs take one + * regular file, so anything else (an extracted `.app` directory, a missing path) is refused here, + * before any request. + */ +export async function appFileUploadForm( + appPath: string, + fileField: string, + hub: { provider: string; service: string }, +): Promise { + const entry = await stat(appPath).catch(() => undefined); + if (!entry?.isFile()) { + throw new AppError( + 'INVALID_ARGS', + `${hub.service} can only upload a regular app file: ${appPath}`, + { + provider: hub.provider, + appPath, + hint: 'Use an existing .ipa, .apk, or .aab file, or a .zip of the iOS simulator .app bundle.', + }, + ); + } const form = new FormData(); form.set(fileField, new Blob([await readFile(appPath)]), path.basename(appPath)); return form; From cfb5a4ded70eb5908b684b85560e728ce657c731 Mon Sep 17 00:00:00 2001 From: amankansal-lt Date: Sat, 3 Oct 2026 10:32:34 +0530 Subject: [PATCH 5/9] fix(provider-webdriver): keep a query on the BrowserStack session-details endpoint The session-details URL was built by string concatenation, so a query on BROWSERSTACK_SESSION_DETAILS_ENDPOINT swallowed the `.json` route. Routes are now appended to the URL path with appendUrlPath, keeping the query. A whitespace-only artifact URL in the session details was reported as a ready artifact; it is now treated as absent, and URLs are trimmed. Co-Authored-By: Claude Opus 5.5 --- .../provider-webdriver/src/artifact-results.ts | 5 +++-- .../provider-webdriver/src/browserstack.test.ts | 16 ++++++++++++++++ packages/provider-webdriver/src/browserstack.ts | 7 ++++--- .../src/webdriver-utils.test.ts | 12 ++++++++++++ .../provider-webdriver/src/webdriver-utils.ts | 7 +++++++ 5 files changed, 42 insertions(+), 5 deletions(-) diff --git a/packages/provider-webdriver/src/artifact-results.ts b/packages/provider-webdriver/src/artifact-results.ts index de29b4ab3a..3aaabc69b5 100644 --- a/packages/provider-webdriver/src/artifact-results.ts +++ b/packages/provider-webdriver/src/artifact-results.ts @@ -38,7 +38,8 @@ export function urlArtifactFromDetails( kind: CloudArtifact['kind'], name: string, ): CloudArtifact | undefined { - const url = details[field]; - if (typeof url !== 'string' || url.length === 0) return undefined; + const value = details[field]; + const url = typeof value === 'string' ? value.trim() : ''; + if (url.length === 0) return undefined; return { provider, providerSessionId, kind, name, url, availability: 'ready' }; } diff --git a/packages/provider-webdriver/src/browserstack.test.ts b/packages/provider-webdriver/src/browserstack.test.ts index d487c0c74f..93030524af 100644 --- a/packages/provider-webdriver/src/browserstack.test.ts +++ b/packages/provider-webdriver/src/browserstack.test.ts @@ -184,3 +184,19 @@ test('BrowserStack session details lookup has a deadline and fails typed', async }); } }); + +test('BrowserStack session details keep a query on the endpoint override', async () => { + const calls: string[] = []; + globalThis.fetch = async (input) => { + calls.push(String(input)); + return new Response(JSON.stringify({ automation_session: { video_url: ' ' } }), { + status: 200, + }); + }; + const result = await listBrowserStackCloudArtifacts('browserstack', 'SESSION1', { + ...upload, + endpoint: 'https://api.example.test/sessions?region=eu', + }); + assert.deepEqual(calls, ['https://api.example.test/sessions/SESSION1.json?region=eu']); + assert.equal(result?.status, 'pending'); +}); diff --git a/packages/provider-webdriver/src/browserstack.ts b/packages/provider-webdriver/src/browserstack.ts index 08f50fad98..bd6d979bc4 100644 --- a/packages/provider-webdriver/src/browserstack.ts +++ b/packages/provider-webdriver/src/browserstack.ts @@ -4,12 +4,12 @@ import type { CloudWebDriverUploadApp } from './runtime.ts'; import { CLOUD_WEBDRIVER_PROVIDERS } from './providers.ts'; import { cloudArtifactsReadyOrPending, urlArtifactFromDetails } from './artifact-results.ts'; import { + appendUrlPath, appFileUploadForm, createHubUploadApp, fetchProviderSessionDetails, postHubAppUpload, resolveHubAppReference, - trimTrailingSlash, } from './webdriver-utils.ts'; export const BROWSERSTACK_APP_AUTOMATE_ENDPOINT = 'https://hub-cloud.browserstack.com/wd/hub/'; @@ -160,8 +160,9 @@ async function fetchBrowserStackSessionDetails( sessionId: string, options: BrowserStackSessionDetailsOptions, ): Promise> { - const endpoint = new URL( - `${trimTrailingSlash(String(options.endpoint ?? BROWSERSTACK_SESSION_DETAILS_ENDPOINT))}/${sessionId}.json`, + const endpoint = appendUrlPath( + options.endpoint ?? BROWSERSTACK_SESSION_DETAILS_ENDPOINT, + `${sessionId}.json`, ); const json = await fetchProviderSessionDetails(endpoint, { clientVersion: options.clientVersion, diff --git a/packages/provider-webdriver/src/webdriver-utils.test.ts b/packages/provider-webdriver/src/webdriver-utils.test.ts index 090071d755..a7dd3fd933 100644 --- a/packages/provider-webdriver/src/webdriver-utils.test.ts +++ b/packages/provider-webdriver/src/webdriver-utils.test.ts @@ -5,6 +5,7 @@ import { afterEach, test, vi } from 'vitest'; import { AppError } from '@agent-device/kernel/errors'; import { asOptionalRecord } from '@agent-device/kernel/record'; import { + appendUrlPath, appFileUploadForm, createHubUploadApp, postHubAppUpload, @@ -150,3 +151,14 @@ test('appFileUploadForm carries a regular app file and refuses anything else typ await fs.rm(tempDir, { recursive: true, force: true }); } }); + +test('appending a route keeps a query on the base endpoint', () => { + assert.equal( + appendUrlPath('https://api.example.test/v1/?region=eu', 'sessions/S%201').toString(), + 'https://api.example.test/v1/sessions/S%201?region=eu', + ); + assert.equal( + appendUrlPath('https://api.example.test/v1', 'sessions/S1').toString(), + 'https://api.example.test/v1/sessions/S1', + ); +}); diff --git a/packages/provider-webdriver/src/webdriver-utils.ts b/packages/provider-webdriver/src/webdriver-utils.ts index df7fddae3f..95077f72fb 100644 --- a/packages/provider-webdriver/src/webdriver-utils.ts +++ b/packages/provider-webdriver/src/webdriver-utils.ts @@ -56,6 +56,13 @@ export function trimTrailingSlash(value: string): string { return lastNonSlash === value.length - 1 ? value : value.slice(0, lastNonSlash + 1); } +/** Appends `route` to the base's path; a query on the base is kept rather than swallowing the route. */ +export function appendUrlPath(base: string | URL, route: string): URL { + const url = new URL(base); + url.pathname = `${trimTrailingSlash(url.pathname)}/${route}`; + return url; +} + export function withTrailingSlash(url: URL): URL { if (url.pathname.endsWith('/')) return url; const copy = new URL(url); From 2eca2e77fcceec31db2dd1c431c827d0ef90ad9e Mon Sep 17 00:00:00 2001 From: amankansal-lt Date: Sat, 3 Oct 2026 10:32:52 +0530 Subject: [PATCH 6/9] fix(provider-webdriver): type a non-JSON connection-verification answer A 2xx connection-verification answer that is not JSON, such as a maintenance page, threw from response.json() and was reported as a network failure with a network-access hint. It is now COMMAND_FAILED with the HTTP status and the service-status hint that a non-2xx answer already carries. Co-Authored-By: Claude Opus 5.5 --- .../src/connection-verification.test.ts | 17 ++++++++++++++++ .../provider-webdriver/src/webdriver-utils.ts | 20 +++++++++++++------ 2 files changed, 31 insertions(+), 6 deletions(-) diff --git a/packages/provider-webdriver/src/connection-verification.test.ts b/packages/provider-webdriver/src/connection-verification.test.ts index c93e40109e..0185d64500 100644 --- a/packages/provider-webdriver/src/connection-verification.test.ts +++ b/packages/provider-webdriver/src/connection-verification.test.ts @@ -1,5 +1,6 @@ import assert from 'node:assert/strict'; import { afterEach, test, vi } from 'vitest'; +import { AppError } from '@agent-device/kernel/errors'; import { createProviderWebDriver } from './index.ts'; import type { RunHostCommand } from './dependencies.ts'; @@ -109,6 +110,22 @@ test('BrowserStack points HTTP failures at its service status and transport fail }); }); +test('BrowserStack reports a non-JSON verification answer typed, with its status', async () => { + vi.stubGlobal( + 'fetch', + vi.fn(async () => new Response('maintenance', { status: 200 })), + ); + + await assert.rejects(createProvider().verifyConnection(browserStackOptions), (error: unknown) => { + assert.ok(error instanceof AppError); + assert.equal(error.code, 'COMMAND_FAILED'); + assert.equal(error.message, 'BrowserStack connection verification answer was not JSON.'); + assert.equal(error.details?.status, 200); + assert.equal(error.details?.hint, 'Retry connect or check the BrowserStack service status.'); + return true; + }); +}); + test('BrowserStack defers a bs app reference outside the recent upload window', async () => { vi.stubGlobal( 'fetch', diff --git a/packages/provider-webdriver/src/webdriver-utils.ts b/packages/provider-webdriver/src/webdriver-utils.ts index 95077f72fb..3564c0427f 100644 --- a/packages/provider-webdriver/src/webdriver-utils.ts +++ b/packages/provider-webdriver/src/webdriver-utils.ts @@ -188,8 +188,9 @@ type ProviderJsonFailureHints = { /** * Fetches JSON from a hosted provider's API during connection verification. A 401/403 is - * `UNAUTHORIZED` with a credential hint, any other non-2xx is `COMMAND_FAILED`, and a transport - * failure is wrapped so its cause survives without leaking the credentials. + * `UNAUTHORIZED` with a credential hint, any other non-2xx or a body that is not JSON is + * `COMMAND_FAILED` with the status, and a transport failure is wrapped so its cause survives + * without leaking the credentials. */ export async function fetchProviderVerificationJson( endpoint: string | URL, @@ -200,6 +201,7 @@ export async function fetchProviderVerificationJson( }, ): Promise { const { service, unauthorizedHint, networkHint } = options.hints; + const serviceHint = `Retry connect or check the ${service} service status.`; try { const response = await fetch(endpoint, { headers: { @@ -215,13 +217,19 @@ export async function fetchProviderVerificationJson( `${service} rejected connection verification.`, { status: response.status, - hint: unauthorized - ? unauthorizedHint - : `Retry connect or check the ${service} service status.`, + hint: unauthorized ? unauthorizedHint : serviceHint, }, ); } - return (await response.json()) as unknown; + const json = await readProviderJsonBody(response); + if (json === undefined) { + throw new AppError( + 'COMMAND_FAILED', + `${service} connection verification answer was not JSON.`, + { status: response.status, hint: serviceHint }, + ); + } + return json; } catch (error) { if (error instanceof AppError) throw error; throw new AppError( From 7e079497cd1af5411dbdc666d5cc75dca24142b4 Mon Sep 17 00:00:00 2001 From: amankansal-lt Date: Sat, 3 Oct 2026 10:34:09 +0530 Subject: [PATCH 7/9] fix(provider-webdriver): validate and canonicalize bs:// app references `BS://id` was treated as a file path, and `bs://`, `bs://a b` or `bs://a/b` passed connect and session preparation and failed only when BrowserStack created the session. The light providers module now owns one canonicalizer, which lower-cases the scheme, and the bs:// id grammar. Connect, connection verification and the shared hub app resolver all use them, and a malformed reference is INVALID_ARGS with one message at each point. Connect verified the raw spelling typed on the command line: the generated profile stored `bs://id` for `BS://id`, but the flags handed to verification were the CLI flags laid over the profile, so the recent-apps lookup missed and reported a local artifact. The canonical reference now wins in the flags verification reads, and verification canonicalizes on its own for a hand-authored --remote-config profile. Co-Authored-By: Claude Opus 5.5 --- .../browserstack-connection-verification.ts | 18 ++++++-- .../src/browserstack.test.ts | 35 ++++++++++++++ .../provider-webdriver/src/browserstack.ts | 9 +++- .../src/connection-verification.test.ts | 29 ++++++++++++ packages/provider-webdriver/src/providers.ts | 18 ++++++++ .../src/webdriver-utils.test.ts | 39 +++++++++++++++- .../provider-webdriver/src/webdriver-utils.ts | 20 ++++++-- src/__tests__/cloud-connect-profile.test.ts | 46 +++++++++++++++++++ src/cli/connection/cloud-webdriver-profile.ts | 20 +++++++- 9 files changed, 223 insertions(+), 11 deletions(-) diff --git a/packages/provider-webdriver/src/browserstack-connection-verification.ts b/packages/provider-webdriver/src/browserstack-connection-verification.ts index ce44ddb459..02b77591df 100644 --- a/packages/provider-webdriver/src/browserstack-connection-verification.ts +++ b/packages/provider-webdriver/src/browserstack-connection-verification.ts @@ -1,5 +1,6 @@ import path from 'node:path'; import { AppError } from '@agent-device/kernel/errors'; +import { canonicalBrowserStackAppReference, isBrowserStackAppReference } from './providers.ts'; import { fetchProviderVerificationJson, sameOsVersion } from './webdriver-utils.ts'; import type { CloudWebDriverConnectionVerification, @@ -21,6 +22,7 @@ export async function verifyBrowserStackConnection( options: BrowserStackOptions, clientVersion: string, ): Promise { + const providerApp = readBrowserStackAppOption(options.app); const auth = { username: options.username, accessKey: options.accessKey }; const devices = await fetchBrowserStackJson( options.devicesEndpoint ?? BROWSERSTACK_DEVICES_ENDPOINT, @@ -43,7 +45,7 @@ export async function verifyBrowserStackConnection( ); } - const app = await verifyBrowserStackApp(options, auth, clientVersion); + const app = await verifyBrowserStackApp(providerApp, options, auth, clientVersion); return { provider: 'browserstack', service: 'BrowserStack', @@ -61,13 +63,23 @@ export async function verifyBrowserStackConnection( }; } +/** Hand-authored remote configs reach verification without passing through connect's normalization. */ +function readBrowserStackAppOption(app: string): string { + const reference = canonicalBrowserStackAppReference(app); + if (reference === undefined) return app; + if (isBrowserStackAppReference(reference)) return reference; + throw new AppError('INVALID_ARGS', `BrowserStack --provider-app ${app} is not a bs:// app id.`, { + providerApp: app, + }); +} + async function verifyBrowserStackApp( + app: string, options: BrowserStackOptions, auth: { username: string; accessKey: string }, clientVersion: string, ): Promise { - const { app } = options; - if (app.startsWith('bs://')) { + if (isBrowserStackAppReference(app)) { const apps = await fetchBrowserStackJson( options.appsEndpoint ?? BROWSERSTACK_APPS_ENDPOINT, auth, diff --git a/packages/provider-webdriver/src/browserstack.test.ts b/packages/provider-webdriver/src/browserstack.test.ts index 93030524af..73c8567e9f 100644 --- a/packages/provider-webdriver/src/browserstack.test.ts +++ b/packages/provider-webdriver/src/browserstack.test.ts @@ -200,3 +200,38 @@ test('BrowserStack session details keep a query on the endpoint override', async assert.deepEqual(calls, ['https://api.example.test/sessions/SESSION1.json?region=eu']); assert.equal(result?.status, 'pending'); }); + +test('BrowserStack canonicalizes the bs:// scheme and refuses an id outside its grammar or a directory', async () => { + const tempDir = await mkdtempForTest('agent-device-browserstack-ref-'); + const fetched: string[] = []; + globalThis.fetch = async (input) => { + fetched.push(String(input)); + return new Response(JSON.stringify({ app_url: 'bs://uploaded' }), { status: 200 }); + }; + const resolve = async (app: string) => + await resolveBrowserStackAppReference(app, { ...upload, cwd: tempDir }); + try { + await fs.mkdir(path.join(tempDir, 'App.app')); + assert.equal(await resolve('BS://app-id'), 'bs://app-id'); + assert.equal(await resolve('HTTPS://builds.example/App.apk'), 'HTTPS://builds.example/App.apk'); + for (const app of ['bs://', 'bs://a b', 'Bs://a/b']) { + await assert.rejects( + resolve(app), + (error: unknown) => + error instanceof AppError && + error.code === 'INVALID_ARGS' && + error.message === `BrowserStack --provider-app ${app} is not a bs:// app id.`, + ); + } + await assert.rejects( + resolve('App.app'), + (error: unknown) => + error instanceof AppError && + error.code === 'INVALID_ARGS' && + /can only upload a regular app file: .*App\.app$/.test(error.message), + ); + assert.deepEqual(fetched, []); + } finally { + await fs.rm(tempDir, { recursive: true, force: true }); + } +}); diff --git a/packages/provider-webdriver/src/browserstack.ts b/packages/provider-webdriver/src/browserstack.ts index bd6d979bc4..db664a797b 100644 --- a/packages/provider-webdriver/src/browserstack.ts +++ b/packages/provider-webdriver/src/browserstack.ts @@ -1,8 +1,12 @@ import type { CloudArtifact, CloudArtifactsResult } from '@agent-device/contracts/observability'; import type { CloudWebDriverCapabilityOverrides } from './capabilities.ts'; import type { CloudWebDriverUploadApp } from './runtime.ts'; -import { CLOUD_WEBDRIVER_PROVIDERS } from './providers.ts'; import { cloudArtifactsReadyOrPending, urlArtifactFromDetails } from './artifact-results.ts'; +import { + canonicalBrowserStackAppReference, + CLOUD_WEBDRIVER_PROVIDERS, + isBrowserStackAppReference, +} from './providers.ts'; import { appendUrlPath, appFileUploadForm, @@ -113,8 +117,9 @@ export async function resolveBrowserStackAppReference( service: 'BrowserStack', app, cwd: options.cwd, - referenceScheme: 'bs://', referenceLabel: 'a bs:// app id', + canonicalReference: canonicalBrowserStackAppReference, + isReference: isBrowserStackAppReference, uploadFile: async (appPath, signal) => await uploadBrowserStackApp(appPath, options, signal), signal: options.signal, }); diff --git a/packages/provider-webdriver/src/connection-verification.test.ts b/packages/provider-webdriver/src/connection-verification.test.ts index 0185d64500..2cbef2a89e 100644 --- a/packages/provider-webdriver/src/connection-verification.test.ts +++ b/packages/provider-webdriver/src/connection-verification.test.ts @@ -126,6 +126,35 @@ test('BrowserStack reports a non-JSON verification answer typed, with its status }); }); +test('BrowserStack verification canonicalizes the bs:// scheme and refuses an id outside its grammar', async () => { + const fetchMock = vi.fn(async (input) => + String(input).includes('devices') + ? jsonResponse([{ os: 'android', os_version: '14.0', device: 'Google Pixel 8' }]) + : jsonResponse([{ app_name: 'sample.apk', app_url: 'bs://app-id' }]), + ); + vi.stubGlobal('fetch', fetchMock); + + const result = await createProvider().verifyConnection({ + ...browserStackOptions, + app: 'BS://app-id', + }); + assert.deepEqual(result.app, { + status: 'verified', + name: 'sample.apk', + reference: 'bs://app-id', + }); + + fetchMock.mockClear(); + await assert.rejects( + createProvider().verifyConnection({ ...browserStackOptions, app: 'bs://a b' }), + (error: unknown) => + error instanceof AppError && + error.code === 'INVALID_ARGS' && + error.message === 'BrowserStack --provider-app bs://a b is not a bs:// app id.', + ); + assert.equal(fetchMock.mock.calls.length, 0); +}); + test('BrowserStack defers a bs app reference outside the recent upload window', async () => { vi.stubGlobal( 'fetch', diff --git a/packages/provider-webdriver/src/providers.ts b/packages/provider-webdriver/src/providers.ts index 814a0c19ae..d0d9ab6e4f 100644 --- a/packages/provider-webdriver/src/providers.ts +++ b/packages/provider-webdriver/src/providers.ts @@ -13,3 +13,21 @@ export function isCloudWebDriverProviderName( ): provider is CloudWebDriverKnownProviderName { return provider !== undefined && CLOUD_WEBDRIVER_KNOWN_PROVIDERS.has(provider); } + +const BROWSERSTACK_APP_SCHEME = 'bs://'; + +/** + * URI schemes are case-insensitive, but BrowserStack only matches the lower-case spelling, so + * `BS://id` is returned as `bs://id`. Anything without the scheme returns undefined. + */ +export function canonicalBrowserStackAppReference(app: string): string | undefined { + if (app.slice(0, BROWSERSTACK_APP_SCHEME.length).toLowerCase() !== BROWSERSTACK_APP_SCHEME) { + return undefined; + } + return `${BROWSERSTACK_APP_SCHEME}${app.slice(BROWSERSTACK_APP_SCHEME.length)}`; +} + +/** An id outside this grammar would pass every local check and fail only at session creation. */ +export function isBrowserStackAppReference(reference: string): boolean { + return /^bs:\/\/[\w.-]+$/.test(reference); +} diff --git a/packages/provider-webdriver/src/webdriver-utils.test.ts b/packages/provider-webdriver/src/webdriver-utils.test.ts index a7dd3fd933..40e2d2ad90 100644 --- a/packages/provider-webdriver/src/webdriver-utils.test.ts +++ b/packages/provider-webdriver/src/webdriver-utils.test.ts @@ -90,6 +90,12 @@ test('the hub install adapter uploads the build and launches the hinted app', as }); }); +const hubReferenceGrammar = { + canonicalReference: (app: string) => + app.slice(0, 6).toLowerCase() === 'hub://' ? `hub://${app.slice(6)}` : undefined, + isReference: (reference: string) => /^hub:\/\/\w+$/.test(reference), +}; + test('the hub app resolver passes references through, uploads local files, and passes URLs through', async () => { const tempDir = await mkdtempForTest('agent-device-hub-resolve-'); try { @@ -100,12 +106,13 @@ test('the hub app resolver passes references through, uploads local files, and p service: 'Hub', app, cwd: tempDir, - referenceScheme: 'hub://', referenceLabel: 'a hub:// app id', + ...hubReferenceGrammar, uploadFile, }); assert.equal(await resolve('hub://APP3'), 'hub://APP3'); + assert.equal(await resolve('HUB://APP3'), 'hub://APP3'); assert.equal(await resolve('https://builds.example/App.apk'), 'https://builds.example/App.apk'); assert.equal(await resolve('App.apk'), 'hub://App.apk'); assert.deepEqual(uploadFile.mock.calls, [[path.join(tempDir, 'App.apk'), undefined]]); @@ -162,3 +169,33 @@ test('appending a route keeps a query on the base endpoint', () => { 'https://api.example.test/v1/sessions/S1', ); }); + +test('the hub app resolver refuses a malformed reference, typed', async () => { + const tempDir = await mkdtempForTest('agent-device-hub-resolve-invalid-'); + try { + const uploadFile = vi.fn(async () => 'hub://never'); + const resolve = (app: string) => + resolveHubAppReference({ + service: 'Hub', + app, + cwd: tempDir, + referenceLabel: 'a hub:// app id', + ...hubReferenceGrammar, + uploadFile, + }); + + for (const [app, message] of [ + ['hub://', /^Hub --provider-app hub:\/\/ is not a hub:\/\/ app id\.$/], + ['HUB://a b', /is not a hub:\/\/ app id/], + ] as const) { + await assert.rejects( + resolve(app), + (error: unknown) => + error instanceof AppError && error.code === 'INVALID_ARGS' && message.test(error.message), + ); + } + assert.equal(uploadFile.mock.calls.length, 0); + } finally { + await fs.rm(tempDir, { recursive: true, force: true }); + } +}); diff --git a/packages/provider-webdriver/src/webdriver-utils.ts b/packages/provider-webdriver/src/webdriver-utils.ts index 3564c0427f..1c814ad7ea 100644 --- a/packages/provider-webdriver/src/webdriver-utils.ts +++ b/packages/provider-webdriver/src/webdriver-utils.ts @@ -151,21 +151,33 @@ export function createHubUploadApp( } /** - * Turns `--provider-app` into a reference the hub accepts: its own reference scheme and public - * URLs pass through, and anything else must be a local file to upload. + * Turns `--provider-app` into a reference the hub accepts: its own reference passes through in + * canonical form when it fits the hub's grammar, public URLs pass through, and anything else must + * be a local file to upload. */ export async function resolveHubAppReference(options: { service: string; app: string; cwd?: string; - referenceScheme: string; /** How the scheme reads in the error message, e.g. `a bs:// app id`. */ referenceLabel: string; + /** The canonical spelling of the hub's own reference, or undefined when `app` is not one. */ + canonicalReference: (app: string) => string | undefined; + isReference: (reference: string) => boolean; uploadFile: (appPath: string, signal?: AbortSignal) => Promise; signal?: AbortSignal; }): Promise { const { app } = options; - if (app.startsWith(options.referenceScheme) || /^https?:\/\//i.test(app)) return app; + const reference = options.canonicalReference(app); + if (reference !== undefined) { + if (options.isReference(reference)) return reference; + throw new AppError( + 'INVALID_ARGS', + `${options.service} --provider-app ${app} is not ${options.referenceLabel}.`, + { providerApp: app }, + ); + } + if (/^https?:\/\//i.test(app)) return app; const appPath = path.resolve(options.cwd ?? process.cwd(), app); if (!fs.existsSync(appPath)) { throw new AppError( diff --git a/src/__tests__/cloud-connect-profile.test.ts b/src/__tests__/cloud-connect-profile.test.ts index fb3f486168..85f0e9d367 100644 --- a/src/__tests__/cloud-connect-profile.test.ts +++ b/src/__tests__/cloud-connect-profile.test.ts @@ -416,6 +416,52 @@ test('connect browserstack generates local provider profile without credentials' } }); +test('connect browserstack canonicalizes the app scheme and refuses a malformed bs:// id', async () => { + const tempRoot = mkdtempForTestSync('agent-device-connect-browserstack-ref-'); + const stateDir = path.join(tempRoot, '.state'); + vi.stubEnv('BROWSERSTACK_USERNAME', 'browser-user'); + vi.stubEnv('BROWSERSTACK_ACCESS_KEY', 'browser-key'); + const flags = { + platform: 'android' as const, + device: 'Google Pixel 8', + providerOsVersion: '14.0', + }; + + try { + await connectWithGeneratedProviderProfile({ + stateDir, + positionals: ['browserstack'], + flags: { ...flags, providerApp: 'Bs://app-id' }, + }); + assert.equal( + readGeneratedConfig(readRequiredActiveState(stateDir).remoteConfigPath).providerApp, + 'bs://app-id', + ); + // Verification must look up the reference the profile saved, not the spelling typed. + const verified = mockedVerifyWebDriverConnection.mock.calls[0]?.[0]; + assert.equal(verified?.provider === 'browserstack' ? verified.app : undefined, 'bs://app-id'); + + for (const app of ['bs://', 'bs://a b', 'BS://a/b']) { + assert.throws( + () => + resolveCloudWebDriverConnectProfile({ + provider: 'browserstack', + stateDir, + cwd: tempRoot, + env: { BROWSERSTACK_USERNAME: 'browser-user', BROWSERSTACK_ACCESS_KEY: 'browser-key' }, + flags: { json: false, help: false, version: false, ...flags, providerApp: app }, + }), + (error: unknown) => + error instanceof AppError && + error.code === 'INVALID_ARGS' && + error.message === `BrowserStack --provider-app ${app} is not a bs:// app id.`, + ); + } + } finally { + fs.rmSync(tempRoot, { recursive: true, force: true }); + } +}); + test('connect --remote-config verifies a direct provider profile before saving state', async () => { const tempRoot = mkdtempForTestSync('agent-device-connect-provider-config-'); const stateDir = path.join(tempRoot, '.state'); diff --git a/src/cli/connection/cloud-webdriver-profile.ts b/src/cli/connection/cloud-webdriver-profile.ts index ffa64093af..6af5bd7607 100644 --- a/src/cli/connection/cloud-webdriver-profile.ts +++ b/src/cli/connection/cloud-webdriver-profile.ts @@ -4,6 +4,10 @@ import { rejectBrowserStackOnlyDeviceFeatures, type CloudWebDriverKnownProviderName, } from '@agent-device/provider-webdriver'; +import { + canonicalBrowserStackAppReference, + isBrowserStackAppReference, +} from '@agent-device/provider-webdriver/providers'; import type { RemoteConfigProfile } from '../../remote/remote-config-schema.ts'; import { AppError } from '@agent-device/kernel/errors'; import type { PlatformSelector } from '@agent-device/kernel/device'; @@ -49,6 +53,11 @@ export function resolveCloudWebDriverConnectProfile(options: { cwd: options.cwd, env: options.env, flags: options.flags, + // Verification reads these flags; it must see the canonical reference the profile saved, + // not the spelling typed on the command line. + ...(providerConfig.providerApp + ? { extraFlags: { providerApp: providerConfig.providerApp } } + : {}), }); } @@ -121,7 +130,16 @@ function browserStackProfileFields(options: { } function normalizeBrowserStackAppReference(app: string, cwd: string): string { - if (app.startsWith('bs://') || /^https?:\/\//i.test(app)) return app; + if (/^https?:\/\//i.test(app)) return app; + const reference = canonicalBrowserStackAppReference(app); + if (reference !== undefined) { + if (isBrowserStackAppReference(reference)) return reference; + throw new AppError( + 'INVALID_ARGS', + `BrowserStack --provider-app ${app} is not a bs:// app id.`, + { hint: 'Pass .' }, + ); + } const resolvedPath = path.resolve(cwd, app); try { if (fs.statSync(resolvedPath).isFile()) return resolvedPath; From 4e635cc93111a49ccbbea1e9586f98d7d91367f3 Mon Sep 17 00:00:00 2001 From: amankansal-lt Date: Sat, 3 Oct 2026 18:25:34 +0530 Subject: [PATCH 8/9] fix(provider-webdriver): refuse a whitespace-only app reference from a hub upload A 2xx upload reply whose reference was only whitespace passed the truthiness check and reached session creation. The reference is now trimmed first, so that reply is COMMAND_FAILED with the status. Co-Authored-By: Claude Opus 5.5 --- packages/provider-webdriver/src/webdriver-utils.test.ts | 1 + packages/provider-webdriver/src/webdriver-utils.ts | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/packages/provider-webdriver/src/webdriver-utils.test.ts b/packages/provider-webdriver/src/webdriver-utils.test.ts index 40e2d2ad90..34d4dd95c3 100644 --- a/packages/provider-webdriver/src/webdriver-utils.test.ts +++ b/packages/provider-webdriver/src/webdriver-utils.test.ts @@ -61,6 +61,7 @@ test('the hub upload helper fails typed with the status on an error page or a mi for (const response of [ new Response('502 Bad Gateway', { status: 502 }), new Response(JSON.stringify({ message: 'ok' }), { status: 200 }), + new Response(JSON.stringify({ ref: ' ' }), { status: 200 }), ]) { globalThis.fetch = async () => response; await assert.rejects(postHubAppUpload(new FormData(), hub), (error: unknown) => { diff --git a/packages/provider-webdriver/src/webdriver-utils.ts b/packages/provider-webdriver/src/webdriver-utils.ts index 1c814ad7ea..46ca8478b6 100644 --- a/packages/provider-webdriver/src/webdriver-utils.ts +++ b/packages/provider-webdriver/src/webdriver-utils.ts @@ -124,7 +124,7 @@ export async function postHubAppUpload( signal, }); const json = await readProviderJsonBody(response); - const appReference = options.readAppReference(json); + const appReference = options.readAppReference(json)?.trim(); if (!response.ok || !appReference) { throw new AppError('COMMAND_FAILED', `${options.service} app upload failed.`, { status: response.status, From 42fbf852e37ee14cbe2cbd1d4bd1deb514cbef5d Mon Sep 17 00:00:00 2001 From: amankansal-lt Date: Sat, 3 Oct 2026 18:26:58 +0530 Subject: [PATCH 9/9] refactor(provider-webdriver): build the malformed bs:// rejection in one place Connect, connection verification and the runtime resolver each built "BrowserStack --provider-app is not a bs:// app id." with different details, so a hand-authored remote config got no hint. parseBrowserStackAppReference now owns the grammar check and the INVALID_ARGS it throws, with the same providerApp and hint on every path. resolveHubAppReference takes that parser instead of a canonicalizer and a predicate. It lives in browserstack.ts rather than providers.ts because the providers subpath evaluates no other module, and every caller already loads browserstack.ts. Co-Authored-By: Claude Opus 5.5 --- .../browserstack-connection-verification.ts | 10 +++------- .../src/browserstack.test.ts | 17 ++++++++++------- .../provider-webdriver/src/browserstack.ts | 17 +++++++++++++++-- .../src/connection-verification.test.ts | 14 ++++++++++---- packages/provider-webdriver/src/index.ts | 1 + .../src/webdriver-utils.test.ts | 11 +++++++---- .../provider-webdriver/src/webdriver-utils.ts | 19 +++++++------------ src/__tests__/cloud-connect-profile.test.ts | 14 ++++++++++---- src/cli/connection/cloud-webdriver-profile.ts | 16 +++------------- 9 files changed, 66 insertions(+), 53 deletions(-) diff --git a/packages/provider-webdriver/src/browserstack-connection-verification.ts b/packages/provider-webdriver/src/browserstack-connection-verification.ts index 02b77591df..69e2fa870c 100644 --- a/packages/provider-webdriver/src/browserstack-connection-verification.ts +++ b/packages/provider-webdriver/src/browserstack-connection-verification.ts @@ -1,6 +1,7 @@ import path from 'node:path'; import { AppError } from '@agent-device/kernel/errors'; -import { canonicalBrowserStackAppReference, isBrowserStackAppReference } from './providers.ts'; +import { parseBrowserStackAppReference } from './browserstack.ts'; +import { isBrowserStackAppReference } from './providers.ts'; import { fetchProviderVerificationJson, sameOsVersion } from './webdriver-utils.ts'; import type { CloudWebDriverConnectionVerification, @@ -65,12 +66,7 @@ export async function verifyBrowserStackConnection( /** Hand-authored remote configs reach verification without passing through connect's normalization. */ function readBrowserStackAppOption(app: string): string { - const reference = canonicalBrowserStackAppReference(app); - if (reference === undefined) return app; - if (isBrowserStackAppReference(reference)) return reference; - throw new AppError('INVALID_ARGS', `BrowserStack --provider-app ${app} is not a bs:// app id.`, { - providerApp: app, - }); + return parseBrowserStackAppReference(app) ?? app; } async function verifyBrowserStackApp( diff --git a/packages/provider-webdriver/src/browserstack.test.ts b/packages/provider-webdriver/src/browserstack.test.ts index 73c8567e9f..845387d06f 100644 --- a/packages/provider-webdriver/src/browserstack.test.ts +++ b/packages/provider-webdriver/src/browserstack.test.ts @@ -215,13 +215,16 @@ test('BrowserStack canonicalizes the bs:// scheme and refuses an id outside its assert.equal(await resolve('BS://app-id'), 'bs://app-id'); assert.equal(await resolve('HTTPS://builds.example/App.apk'), 'HTTPS://builds.example/App.apk'); for (const app of ['bs://', 'bs://a b', 'Bs://a/b']) { - await assert.rejects( - resolve(app), - (error: unknown) => - error instanceof AppError && - error.code === 'INVALID_ARGS' && - error.message === `BrowserStack --provider-app ${app} is not a bs:// app id.`, - ); + await assert.rejects(resolve(app), (error: unknown) => { + assert.ok(error instanceof AppError); + assert.equal(error.code, 'INVALID_ARGS'); + assert.equal(error.message, `BrowserStack --provider-app ${app} is not a bs:// app id.`); + assert.deepEqual(error.details, { + providerApp: app, + hint: 'Pass .', + }); + return true; + }); } await assert.rejects( resolve('App.app'), diff --git a/packages/provider-webdriver/src/browserstack.ts b/packages/provider-webdriver/src/browserstack.ts index db664a797b..b7d18f533a 100644 --- a/packages/provider-webdriver/src/browserstack.ts +++ b/packages/provider-webdriver/src/browserstack.ts @@ -1,4 +1,5 @@ import type { CloudArtifact, CloudArtifactsResult } from '@agent-device/contracts/observability'; +import { AppError } from '@agent-device/kernel/errors'; import type { CloudWebDriverCapabilityOverrides } from './capabilities.ts'; import type { CloudWebDriverUploadApp } from './runtime.ts'; import { cloudArtifactsReadyOrPending, urlArtifactFromDetails } from './artifact-results.ts'; @@ -108,6 +109,19 @@ export function createBrowserStackUploadApp( ); } +/** + * The canonical `bs://` reference for `app`, or undefined when `app` does not use the scheme. A + * `bs://` value outside the id grammar is `INVALID_ARGS`, worded the same on every path. + */ +export function parseBrowserStackAppReference(app: string): string | undefined { + const reference = canonicalBrowserStackAppReference(app); + if (reference === undefined || isBrowserStackAppReference(reference)) return reference; + throw new AppError('INVALID_ARGS', `BrowserStack --provider-app ${app} is not a bs:// app id.`, { + providerApp: app, + hint: 'Pass .', + }); +} + /** The hub fetches a public URL itself, so only a local path is uploaded. */ export async function resolveBrowserStackAppReference( app: string, @@ -118,8 +132,7 @@ export async function resolveBrowserStackAppReference( app, cwd: options.cwd, referenceLabel: 'a bs:// app id', - canonicalReference: canonicalBrowserStackAppReference, - isReference: isBrowserStackAppReference, + parseReference: parseBrowserStackAppReference, uploadFile: async (appPath, signal) => await uploadBrowserStackApp(appPath, options, signal), signal: options.signal, }); diff --git a/packages/provider-webdriver/src/connection-verification.test.ts b/packages/provider-webdriver/src/connection-verification.test.ts index 2cbef2a89e..d3133dba53 100644 --- a/packages/provider-webdriver/src/connection-verification.test.ts +++ b/packages/provider-webdriver/src/connection-verification.test.ts @@ -147,10 +147,16 @@ test('BrowserStack verification canonicalizes the bs:// scheme and refuses an id fetchMock.mockClear(); await assert.rejects( createProvider().verifyConnection({ ...browserStackOptions, app: 'bs://a b' }), - (error: unknown) => - error instanceof AppError && - error.code === 'INVALID_ARGS' && - error.message === 'BrowserStack --provider-app bs://a b is not a bs:// app id.', + (error: unknown) => { + assert.ok(error instanceof AppError); + assert.equal(error.code, 'INVALID_ARGS'); + assert.equal(error.message, 'BrowserStack --provider-app bs://a b is not a bs:// app id.'); + assert.deepEqual(error.details, { + providerApp: 'bs://a b', + hint: 'Pass .', + }); + return true; + }, ); assert.equal(fetchMock.mock.calls.length, 0); }); diff --git a/packages/provider-webdriver/src/index.ts b/packages/provider-webdriver/src/index.ts index cba755ce82..8410ab710f 100644 --- a/packages/provider-webdriver/src/index.ts +++ b/packages/provider-webdriver/src/index.ts @@ -19,6 +19,7 @@ import type { CloudWebDriverRuntime } from './runtime.ts'; export { CLOUD_WEBDRIVER_PROVIDERS }; export { readAwsDeviceFarmRegionFromArn }; +export { parseBrowserStackAppReference } from './browserstack.ts'; export { rejectBrowserStackOnlyDeviceFeatures } from './browserstack-device-features.ts'; export type { CloudWebDriverKnownProviderName } from './providers.ts'; export type { ProviderWebDriverDependencies, RunHostCommand } from './dependencies.ts'; diff --git a/packages/provider-webdriver/src/webdriver-utils.test.ts b/packages/provider-webdriver/src/webdriver-utils.test.ts index 34d4dd95c3..ca026630ec 100644 --- a/packages/provider-webdriver/src/webdriver-utils.test.ts +++ b/packages/provider-webdriver/src/webdriver-utils.test.ts @@ -92,9 +92,12 @@ test('the hub install adapter uploads the build and launches the hinted app', as }); const hubReferenceGrammar = { - canonicalReference: (app: string) => - app.slice(0, 6).toLowerCase() === 'hub://' ? `hub://${app.slice(6)}` : undefined, - isReference: (reference: string) => /^hub:\/\/\w+$/.test(reference), + parseReference: (app: string) => { + if (app.slice(0, 6).toLowerCase() !== 'hub://') return undefined; + const reference = `hub://${app.slice(6)}`; + if (/^hub:\/\/\w+$/.test(reference)) return reference; + throw new AppError('INVALID_ARGS', `Hub --provider-app ${app} is not a hub:// app id.`); + }, }; test('the hub app resolver passes references through, uploads local files, and passes URLs through', async () => { @@ -171,7 +174,7 @@ test('appending a route keeps a query on the base endpoint', () => { ); }); -test('the hub app resolver refuses a malformed reference, typed', async () => { +test('the hub app resolver surfaces the grammar rejection of a malformed reference without uploading', async () => { const tempDir = await mkdtempForTest('agent-device-hub-resolve-invalid-'); try { const uploadFile = vi.fn(async () => 'hub://never'); diff --git a/packages/provider-webdriver/src/webdriver-utils.ts b/packages/provider-webdriver/src/webdriver-utils.ts index 46ca8478b6..6d5cb139aa 100644 --- a/packages/provider-webdriver/src/webdriver-utils.ts +++ b/packages/provider-webdriver/src/webdriver-utils.ts @@ -161,22 +161,17 @@ export async function resolveHubAppReference(options: { cwd?: string; /** How the scheme reads in the error message, e.g. `a bs:// app id`. */ referenceLabel: string; - /** The canonical spelling of the hub's own reference, or undefined when `app` is not one. */ - canonicalReference: (app: string) => string | undefined; - isReference: (reference: string) => boolean; + /** + * The canonical spelling of the hub's own reference, or undefined when `app` is not one. Throws + * when `app` uses the hub's scheme outside its grammar. + */ + parseReference: (app: string) => string | undefined; uploadFile: (appPath: string, signal?: AbortSignal) => Promise; signal?: AbortSignal; }): Promise { const { app } = options; - const reference = options.canonicalReference(app); - if (reference !== undefined) { - if (options.isReference(reference)) return reference; - throw new AppError( - 'INVALID_ARGS', - `${options.service} --provider-app ${app} is not ${options.referenceLabel}.`, - { providerApp: app }, - ); - } + const reference = options.parseReference(app); + if (reference !== undefined) return reference; if (/^https?:\/\//i.test(app)) return app; const appPath = path.resolve(options.cwd ?? process.cwd(), app); if (!fs.existsSync(appPath)) { diff --git a/src/__tests__/cloud-connect-profile.test.ts b/src/__tests__/cloud-connect-profile.test.ts index 85f0e9d367..45436b420c 100644 --- a/src/__tests__/cloud-connect-profile.test.ts +++ b/src/__tests__/cloud-connect-profile.test.ts @@ -451,10 +451,16 @@ test('connect browserstack canonicalizes the app scheme and refuses a malformed env: { BROWSERSTACK_USERNAME: 'browser-user', BROWSERSTACK_ACCESS_KEY: 'browser-key' }, flags: { json: false, help: false, version: false, ...flags, providerApp: app }, }), - (error: unknown) => - error instanceof AppError && - error.code === 'INVALID_ARGS' && - error.message === `BrowserStack --provider-app ${app} is not a bs:// app id.`, + (error: unknown) => { + assert.ok(error instanceof AppError); + assert.equal(error.code, 'INVALID_ARGS'); + assert.equal(error.message, `BrowserStack --provider-app ${app} is not a bs:// app id.`); + assert.deepEqual(error.details, { + providerApp: app, + hint: 'Pass .', + }); + return true; + }, ); } } finally { diff --git a/src/cli/connection/cloud-webdriver-profile.ts b/src/cli/connection/cloud-webdriver-profile.ts index 6af5bd7607..2d3d5d799f 100644 --- a/src/cli/connection/cloud-webdriver-profile.ts +++ b/src/cli/connection/cloud-webdriver-profile.ts @@ -1,13 +1,10 @@ import { CLOUD_WEBDRIVER_PROVIDERS, + parseBrowserStackAppReference, readAwsDeviceFarmRegionFromArn, rejectBrowserStackOnlyDeviceFeatures, type CloudWebDriverKnownProviderName, } from '@agent-device/provider-webdriver'; -import { - canonicalBrowserStackAppReference, - isBrowserStackAppReference, -} from '@agent-device/provider-webdriver/providers'; import type { RemoteConfigProfile } from '../../remote/remote-config-schema.ts'; import { AppError } from '@agent-device/kernel/errors'; import type { PlatformSelector } from '@agent-device/kernel/device'; @@ -131,15 +128,8 @@ function browserStackProfileFields(options: { function normalizeBrowserStackAppReference(app: string, cwd: string): string { if (/^https?:\/\//i.test(app)) return app; - const reference = canonicalBrowserStackAppReference(app); - if (reference !== undefined) { - if (isBrowserStackAppReference(reference)) return reference; - throw new AppError( - 'INVALID_ARGS', - `BrowserStack --provider-app ${app} is not a bs:// app id.`, - { hint: 'Pass .' }, - ); - } + const reference = parseBrowserStackAppReference(app); + if (reference !== undefined) return reference; const resolvedPath = path.resolve(cwd, app); try { if (fs.statSync(resolvedPath).isFile()) return resolvedPath;