fix(webdriver-utils): clear error for native app sessions on Percy on Automate (PPLT-6148) by Shivanshu-07 · Pull Request #2456 · percy/cli

Shivanshu-07 · 2026-10-07T03:41:50Z

Summary

Jira: PPLT-6148 — Appium quarterly upgrade + shift default to Appium 2.x

Percy on Automate capture builds the comparison tag and regions through JavaScript (window.screen size, devicePixelRatio, DOM lookups). A native App Automate session has no JS context, so every snapshot failed with an opaque 405 Method Not Allowed — reproduced on Appium 1.22.0, 2.19.0 and 3.5.2 — and most SDKs swallow it, leaving users with no snapshots and no explanation.

Change

AutomateProvider.createDriver detects a native session (no browserName, but app / appium:app / appPackage / bundleId / bundleID on the caps or desired) and throws:

Percy on Automate captures browser sessions; this is a native app session. Use App Percy for native apps (an app project token with percy app:exec).

Browser sessions (desktop and mobile web, including Android Chrome which also reports appPackage) always carry browserName and are unaffected. iOS native sessions report browserName: "", which is treated as absent.

Verification

webdriver-utils: 250 specs pass; forcing isNativeAppSession to false fails both native specs. ESLint clean.
Live on BrowserStack (Appium 2.19.0): native Android and iOS sessions now log the new error instead of 405; Android Chrome Percy on Automate still captures 3/3 snapshots.

🤖 Generated with Claude Code

Summary by CodeRabbit

Updates
- Native-app sessions through Automate are now rejected with guidance to use App Percy. Browser sessions continue to be accepted.

coderabbitai · 2026-10-07T03:42:09Z

Important

Review skipped

Review was skipped as selected files did not have any reviewable changes.

⚙️ Run configuration

Configuration used: Repository YAML (base), Central YAML (inherited), Organization UI (inherited), Workspace UI (inherited)
Review profile: ASSERTIVE
Plan: Enterprise
Run ID: 87e80374-07d6-4fbe-bcee-cd932b5f1152

📥 Commits

Reviewing files that changed from the base of the PR and between 1906964 and 54bf956.

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

🔍 Trigger review

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Repository YAML (base), Central YAML (inherited), Organization UI (inherited), Workspace UI (inherited)
Review profile: ASSERTIVE
Plan: Enterprise
Run ID: 3e690610-fd35-40ba-8c0b-98a1739cf361

📥 Commits

Reviewing files that changed from the base of the PR and between 80a7983 and 1906964.

📒 Files selected for processing (2)

packages/webdriver-utils/src/providers/automateProvider.js
packages/webdriver-utils/test/providers/automateProvider.test.js

🔗 Linked repositories identified

CodeRabbit considers these linked repositories for cross-repo context during reviews:

percy/percy-selenium-dotnet (auto-detected)

Included review availability: This review used your included allowance. Your plan provides up to 12 included reviews per hour; 7 remain after this review.

📜 Recent review details

⏰ Context from checks skipped due to timeout. (8)

GitHub Check: Build & verify executable
GitHub Check: Lint
GitHub Check: Build
GitHub Check: semgrep/ci
GitHub Check: Build
GitHub Check: Typecheck
GitHub Check: Analyze (javascript-typescript)
GitHub Check: Analyze (actions)

🧰 Additional context used

📚 Code guidelines (8)

8 cross-repository guideline sources

📓 Path-based instructions (8)

Source excerpt: **`PERCY_TOKEN`** — read only from env vars or explicit flags; never log it, never write it to `package.json`, `.percy.yml`, or any committed file.

📄 CodeRabbit inference engine (percy/browserstack-ai-harness-percy:stacks/stack-domain-percy-cli/rules/security.md)

Files:

packages/webdriver-utils/test/providers/automateProvider.test.js
packages/webdriver-utils/src/providers/automateProvider.js

Source excerpt: Read `../rules/security.md` first.

📄 CodeRabbit inference engine (percy/browserstack-ai-harness-percy:stacks/stack-domain-percy-cli/knowledge/DEPENDENCIES.md)

Files:

packages/webdriver-utils/test/providers/automateProvider.test.js
packages/webdriver-utils/src/providers/automateProvider.js

Source excerpt: GraalJS scripts must not use Node globals.

📄 CodeRabbit inference engine (percy/browserstack-ai-harness-percy:stacks/stack-domain-percy-app-sdk/agents/stack-percy-app-sdk-code-reviewer.md)

Files:

packages/webdriver-utils/test/providers/automateProvider.test.js
packages/webdriver-utils/src/providers/automateProvider.js

Source excerpt: Match the dominant style for the area you're touching.

📄 CodeRabbit inference engine (percy/browserstack-ai-harness-percy:stacks/stack-domain-percy-cli/rules/commit-conventions.md)

Files:

packages/webdriver-utils/test/providers/automateProvider.test.js
packages/webdriver-utils/src/providers/automateProvider.js

Source excerpt: Known user-facing errors, their meaning, and remediation.

📄 CodeRabbit inference engine (percy/browserstack-ai-harness-percy:stacks/stack-domain-percy-cli/knowledge/ERROR-CATALOG.md)

Files:

packages/webdriver-utils/test/providers/automateProvider.test.js
packages/webdriver-utils/src/providers/automateProvider.js

Source excerpt: | Symptom | Cause | |---|---| | Tests pass but exit hangs | Asset discovery did not finish; raise `discovery.networkIdleTimeout` in `.percy.yml`.

📄 CodeRabbit inference engine (percy/browserstack-ai-harness-percy:stacks/stack-domain-percy-web-sdks/knowledge/flow-percy-exec-lifecycle.md)

Files:

packages/webdriver-utils/test/providers/automateProvider.test.js
packages/webdriver-utils/src/providers/automateProvider.js

Source excerpt: Healthcheck (same as web SDKs).

📄 CodeRabbit inference engine (percy/browserstack-ai-harness-percy:stacks/stack-domain-percy-web-sdks/knowledge/flow-snapshot-appium.md)

Files:

packages/webdriver-utils/test/providers/automateProvider.test.js
packages/webdriver-utils/src/providers/automateProvider.js

Source excerpt: Add an entry to this file.

📄 CodeRabbit inference engine (percy/browserstack-ai-harness-percy:stacks/stack-domain-percy-web-sdks/knowledge/FEATURE-FLAGS.md)

Files:

packages/webdriver-utils/test/providers/automateProvider.test.js
packages/webdriver-utils/src/providers/automateProvider.js

🔇 Additional comments (2)

packages/webdriver-utils/test/providers/automateProvider.test.js (1)

73-106: LGTM!

packages/webdriver-utils/src/providers/automateProvider.js (1)

19-24: 🎯 Functional Correctness

caps.desired being a truthy string does not cause the claimed TypeError; indexed property lookups return undefined. The remaining claim depends on BrowserStack returning native-app identifiers inside appium:options or under the cited prefixed names. The available BrowserStack documentation establishes that an app capability selects the app, but does not establish that returned shape. The proposed extra checks are therefore unsupported.

📝 Walkthrough

Walkthrough

AutomateProvider now identifies native-app capabilities and rejects those sessions with App Percy guidance before metadata resolution. Tests cover Android and iOS rejection, plus accepted browser and minimally specified Android sessions.

Changes

Native-app session handling

Layer / File(s)	Summary
Detect and reject native-app sessions `packages/webdriver-utils/src/providers/automateProvider.js`, `packages/webdriver-utils/test/providers/automateProvider.test.js`	AutomateProvider checks top-level and `desired` capabilities for native-app identifiers and rejects matching sessions before metadata resolution. Tests cover native-app rejection and accepted browser and minimally specified sessions.

Priority: ⬇️ Low

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Bug fix

Merge Risk: ⚪ Minimal · up to 19069

No actionable issue remains on the available evidence; the change is ready for normal merge checks.

🚥 Pre-merge checks | ✅ 4

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title includes the Jira ID PPLT-6148 and clearly describes the native-app session error change for Percy on Automate.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

_{Comment @coderabbitai help to get the list of available commands.}

…sions on Percy on Automate (PPLT-6148) Automate capture builds the comparison tag and regions through JavaScript (window.screen size, devicePixelRatio, DOM lookups). A native App Automate session has no JS context, so every snapshot failed with an opaque "405 Method Not Allowed" on every Appium version (1.22.0, 2.19.0 and 3.5.2), which most SDKs then swallow. Detect a native session in createDriver (no browserName but an app identifier) and throw an actionable error pointing at App Percy instead. Verified live on BrowserStack with Appium 2.19.0: native Android and iOS sessions now report the new error; Android Chrome Percy on Automate still captures 3/3 snapshots. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

Shivanshu-07 · 2026-10-16T11:35:09Z

Claude Code PR Review

PR: #2456 • Head: 54bf956 • Reviewers: code-review (built-in fallback; no in-repo reviewers found)

Summary

Adds AutomateProvider.isNativeAppSession(caps) and makes createDriver fail fast with a clear "use App Percy" error. Before this, a native app session sent to Percy on Automate surfaced as an opaque 405 from the JS executor. The PR also adds 4 unit tests.

Review Table

Priority	Category	Check	Status	Notes
High	Security	No hardcoded secrets or credentials	Pass	None introduced
High	Security	Authentication/authorization checks present	N/A	No auth surface touched
High	Security	Input validation and sanitization	N/A	Reads session caps only
High	Security	No IDOR — resource ownership validated	N/A	—
High	Security	No SQL injection (parameterized queries)	N/A	—
High	Correctness	Logic is correct, handles edge cases	Pass	Core path is correct. Two edge-case gaps in detection (Medium findings below): browserName is only checked at top level, and the key list has no `appium:`-prefixed variants
High	Correctness	Error handling is explicit, no swallowed exceptions	Pass	The throw propagates through `captureScreenshot`, which logs and rethrows
High	Correctness	No race conditions or concurrency issues	N/A	Synchronous check on cached caps
Medium	Testing	New code has corresponding tests	Pass	4 cases: Android native, iOS empty browserName, mobile browser, bare session
Medium	Testing	Error paths and edge cases tested	Fail	`app`, `appium:app`, `bundleID`, a desired-only browserName and the `getCapabilites` fallback are untested
Medium	Testing	Existing tests still pass (no regressions)	Pass	Additive change; existing createDriver tests untouched
Medium	Performance	No N+1 queries or unbounded data fetching	Pass	Reuses caps that are already fetched and cached
Medium	Performance	Long-running tasks use background jobs	N/A	—
Medium	Quality	Follows existing codebase patterns	Pass	Static helper on the provider, matching `supports()`
Medium	Quality	Changes are focused (single concern)	Pass	2 files, one concern
Low	Quality	Meaningful names, no dead code	Pass	—
Low	Quality	Comments explain why, not what	Pass	The comment explains the 405 and the browserName/app-id heuristic
Low	Quality	No unnecessary dependencies added	Pass	None added

Findings

File: packages/webdriver-utils/src/providers/automateProvider.js:19-23
Severity: Medium
Reviewer: code-review
Issue: The browser signal is read only from top-level caps.browserName, but app identifiers are read from both caps and caps.desired. Take a legacy or JSONWP mobile-web session that reports {desired: {browserName: 'chrome', appPackage: 'com.android.chrome'}} with no top-level browserName. It is classified as native and every snapshot hard-fails. Before this PR, the same session captured normally.
Suggestion: Treat caps.browserName || desired.browserName || caps.browser as a browser signal, and add a test for the desired-only case.
File: packages/webdriver-utils/src/providers/automateProvider.js:21
Severity: Medium
Reviewer: code-review
Issue: The key list has no appium:appPackage or appium:bundleId. getCapabilites falls back to the SDK-passed capabilities when GET /session fails, and those can carry W3C-prefixed keys. In that case a native session is not detected and the user still gets the opaque 405.
Suggestion: Add appium:appPackage, appium:bundleId and appium:appActivity, or strip the appium: prefix before checking.
File: packages/webdriver-utils/test/providers/automateProvider.test.js:73-106
Severity: Medium
Reviewer: code-review
Issue: Only top-level appPackage and desired.bundleId are exercised. You could remove appium:app or bundleID from the list, or drop the desired[key] lookup, and the suite would still pass.
Suggestion: Parameterize a native-key case over every entry in the list, and add a desired-only browserName case that must be accepted.
File: packages/webdriver-utils/src/providers/automateProvider.js:31-34
Severity: Low
Reviewer: code-review
Issue: The detection lives only in AutomateProvider. GenericProvider (non-BrowserStack grids) runs the same JS-based capture and still fails opaquely on native sessions.
Suggestion: This is optional and fine as a follow-up: lift the check into GenericProvider.createDriver or MetaDataResolver.resolve.

Verdict: PASS

Shivanshu-07 requested a review from a team as a code owner October 7, 2026 03:41

Shivanshu-07 requested review from Sriram567 and ninadbstack October 7, 2026 03:41

coderabbitai Bot approved these changes Oct 7, 2026

View reviewed changes

rishigupta1599 approved these changes Oct 7, 2026

View reviewed changes

rishigupta1599 approved these changes Oct 9, 2026

View reviewed changes

Shivanshu-07 force-pushed the fix/poa-native-app-session-error branch from 1906964 to 54bf956 Compare October 10, 2026 16:10

Shivanshu-07 merged commit dcc0a33 into master Oct 16, 2026
52 checks passed

Shivanshu-07 deleted the fix/poa-native-app-session-error branch October 16, 2026 11:39

Shivanshu-07 added the 🐛 bug Something isn't working label Oct 16, 2026

Navigation Menu

Repository navigation

fix(webdriver-utils): clear error for native app sessions on Percy on Automate (PPLT-6148) - #2456

fix(webdriver-utils): clear error for native app sessions on Percy on Automate (PPLT-6148)#2456
Shivanshu-07 merged 1 commit into
masterfrom
fix/poa-native-app-session-error

Shivanshu-07 commented Oct 7, 2026 •

edited by coderabbitai Bot

Loading

Uh oh!

coderabbitai Bot commented Oct 7, 2026 •

edited

Loading

Review skipped

Walkthrough

Changes

Uh oh!

Shivanshu-07 commented Oct 16, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

Shivanshu-07 commented Oct 7, 2026 • edited by coderabbitai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Change

Verification

Summary by CodeRabbit

Uh oh!

coderabbitai Bot commented Oct 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Review skipped

Walkthrough

Changes

Uh oh!

Shivanshu-07 commented Oct 16, 2026

Claude Code PR Review

Summary

Review Table

Findings

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Shivanshu-07 commented Oct 7, 2026 •

edited by coderabbitai Bot

Loading

coderabbitai Bot commented Oct 7, 2026 •

edited

Loading