fix(provider-webdriver): harden BrowserStack app references and endpoints by amankansal-lt · Pull Request #3169 · callstack/agent-device

amankansal-lt · 2026-10-03T12:34:23Z

Summary

Split out of #3118 and stacked on #3167. Please review only the top three commits. This PR has no TestMu code.

A query string on the session-details endpoint override is kept at the end of the URL (appendUrlPath). Before, the route was appended after the query.
Whitespace-only artifact URLs now count as absent.
If connection verification gets a 2xx answer that is not JSON, it now returns a typed COMMAND_FAILED with the status and hint. Before, this was reported as a network failure.
Review nit [2]: there is now a single bs:// canonicalizer and grammar (/^bs:\/\/[\w.-]+$/) in providers.ts. It is used at connect, on the remote-config connect route, in verification and when preparing a session.
- BS://id is now treated as bs://id.
- A malformed id is now INVALID_ARGS.

Behaviour change: a bs:// id with characters outside [\w.-] is now refused. BrowserStack's documented app_url (bs://<hex hash>) fits it. A bare custom_id is still not accepted through --provider-app; that is unchanged.

Validation

pnpm check:affected --run passes, and eager-closure-budgets passes.
Not run live against BrowserStack.

🤖 Generated with Claude Code

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 <noreply@anthropic.com>

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 <noreply@anthropic.com>

… 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 <noreply@anthropic.com>

…y 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 <noreply@anthropic.com>

…ails endpoint The session-details URL was built by string concatenation, so a query on BROWSERSTACK_SESSION_DETAILS_ENDPOINT swallowed the `<id>.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 <noreply@anthropic.com>

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 <noreply@anthropic.com>

`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 <noreply@anthropic.com>

cubic-dev-ai

All reported issues were addressed across 19 files

_{Reply with feedback, questions, or to request a fix.

Re-trigger cubic}

…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 <noreply@anthropic.com>

…one place Connect, connection verification and the runtime resolver each built "BrowserStack --provider-app <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 <noreply@anthropic.com>

thymikee · 2026-10-03T13:35:53Z

This PR is ready at 42fbf85. The code looks correct, and the single reported check passes. I reviewed only the commits beyond #3167 (744a5ff..42fbf85). The endpoint scheme and host checks in the earlier stacked commits are outside this delta, so I did not re-review them. I did not run the tests. I judged that they would fail on the old code by reading the pre-change paths. I did not run it against live BrowserStack. These changes are input validation and URL building, the unit tests mock fetch, and the default session-details URL is byte-identical to before. So I did not treat a live run as required.

Not blocking: in browserstack-connection-verification.ts line 78, the check isBrowserStackAppReference(app) re-checks a grammar that readBrowserStackAppOption already enforced, so it can never reject. You could pass the result of parseBrowserStackAppReference into verifyBrowserStackApp and branch on whether it is defined. Also, website/docs/docs/browserstack.md could say that the bs:// id may use only letters, digits, "_", "." and "-", and that the scheme is case-insensitive. Take or leave both.

Both inline threads from the other review are fixed at this commit, so please resolve them: the whitespace-only app reference fix (#3169 (comment)) and the single rejection builder (#3169 (comment)).

Nothing in this change holds up the merge. It can merge once #3167 and #3164 below it have merged.

…token-attach-c41aff * commit 'd396f3b509d9ed7cddaf170351ea6cf34e02ac16': fix(provider-webdriver): harden BrowserStack app references and endpoints (#3169) fix(android): back off a timed-out snapshot helper session and bound content re-captures (#3160) fix: centralize confirmed daemon retirement (#3126) fix: bind daemon registration writes to the acquired owner (#3125) fix: return confirmed daemon termination outcomes (#3124) refactor(move): share daemon registration and shutdown report modules (#3123) fix: preserve process lock exclusion across publication and reclaim (#3122) fix(cli): refuse a non-URL install-from-source source up front (#3166) fix(daemon): start a lease's TTL when its allocation completes (#3165) fix(android): fail doctor when adb is the Windows binary on a POSIX host (#3157) 0.21.20 0.21.19 # Conflicts: # src/daemon/server/daemon-runtime-metadata-ownership.test.ts

Brings feat/testmu-provider-plugin (915e572) up to date with main, which since landed the split-out callstack#3165 (lease TTL), callstack#3166 (install-from-source URL refusal), callstack#3169 (BrowserStack app references, hub helpers, materialized upload) and callstack#3173 (Limrun attached instances). Main's reviewed versions win for the split-out code: appFileUploadForm with {provider, service}, resolveHubAppReference with parseReference, parseBrowserStackAppReference, isHttpUrl in install, the lease work pass only when a provider allocates, and the upload file check in appFileUploadForm rather than runtime-deployment. Re-applied on top only what the migration needs: provider profile-field refusal (connect, WebDriver and Limrun allocation, refused repeat allocation keeps its lease), optional auth on fetchProviderVerificationJson for TestMu's public catalog, and the provider-webdriver/plugin barrel without asRecord. The TestMu plugin now uses asOptionalRecord from @agent-device/kernel/record, parses lt:// references itself and uploads a public URL before delegating to resolveHubAppReference, and passes {provider, service} to appFileUploadForm. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

…merge (#3221) * test(provider-webdriver): cover an uppercase URL scheme passing through unchanged Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * test(provider-webdriver): assert the code of a verification transport failure Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * test(provider-webdriver): cover a named pipe refused before any upload read Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * test(provider-webdriver): create the FIFO with runCmd Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>

amankansal-lt and others added 7 commits October 3, 2026 11:09

amankansal-lt mentioned this pull request Oct 3, 2026

feat(provider-webdriver): add TestMu AI device cloud provider #3118

Open

cubic-dev-ai Bot reviewed Oct 3, 2026

View reviewed changes

Comment thread packages/provider-webdriver/src/webdriver-utils.ts Outdated

Comment thread packages/provider-webdriver/src/webdriver-utils.ts Outdated

amankansal-lt and others added 2 commits October 3, 2026 18:25

thymikee added the ready-for-human Valid work that needs human implementation, judgment, or maintainer merge label Oct 3, 2026

thymikee merged commit d396f3b into callstack:main Oct 3, 2026
13 of 14 checks passed

Navigation Menu

Repository navigation

fix(provider-webdriver): harden BrowserStack app references and endpoints - #3169

fix(provider-webdriver): harden BrowserStack app references and endpoints#3169
thymikee merged 9 commits into
callstack:mainfrom
LambdaTest:split/browserstack-hardening

amankansal-lt commented Oct 3, 2026 •

edited by cubic-dev-ai Bot

Loading

Uh oh!

cubic-dev-ai Bot left a comment •

edited

Loading

Uh oh!

Uh oh!

Uh oh!

thymikee commented Oct 3, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

amankansal-lt commented Oct 3, 2026 • edited by cubic-dev-ai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Validation

Uh oh!

cubic-dev-ai Bot left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

thymikee commented Oct 3, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

amankansal-lt commented Oct 3, 2026 •

edited by cubic-dev-ai Bot

Loading

cubic-dev-ai Bot left a comment •

edited

Loading