Skip to content

Add Android adapter with emulator and installed-device navigation (QUA-2204) - #180

Merged
Desperado merged 7 commits into
mainfrom
Desperado/qua-2202-epic-remaining
Oct 28, 2026
Merged

Desperado merged 7 commits into
mainfrom
Desperado/qua-2202-epic-remaining

Conversation

@Desperado

@Desperado Desperado commented Oct 24, 2026 •

Copy link
Copy Markdown
Contributor

Adds an opt-in android:<app-alias> target using pinned Appium 3.8.0 / UiAutomator2 8.7.0, with an owned local emulator adapter and experimental attachment to an installed app on a physical Android device. Deterministic actions use fresh observed controls and text/state assertions; receipts bind the APK and runtime identity to the active test. This implements the M2 foundation for QUA-2204, within QUA-2202.

Physical attachment verifies the installed APK digest, leases the device exclusively on this host, and rejects ambiguous controls before mutation. It preserves the installed app and its data, disables reset, and restores owned automation state during normal cleanup. A private installed-app smoke checks both the selected destination and heading across four navigation destinations. It does not submit changes.

Android dependencies remain separate from the ordinary SDK. The driver defers agent-device 0.21.31 pending privacy qualification of its automatic artifact paths. Screenshots, page source, video, logcat and raw device serials are not retained in receipts. Existing Playwright behavior stays opt-in, and native mutations get one attempt even with --attempts 3.

Validation:

  • Real physical device, Android 17/API 37: a private installed app completed all four navigation destinations with eight worker-reported assertions, matched provenance and a validated receipt. A wrong-heading response control failed the expected text assertion after successful navigation.
  • Physical cleanup checks confirmed the installed APK was unchanged, the app remained running, automation helpers stopped, owned forwards were removed, the lease was released, hidden-API policies were restored and the input method was preserved.
  • Android 15/API 35 arm64 emulator: the final 12-case matrix qualified every case on the reviewed head. Healthy navigation executed five target assertions; wrong amount/state failed the intended assertions; missing/ambiguous IDs rejected before mutation. Mid-action cancellation and timeout each dispatched exactly one tap, terminalized with the expected reason and stopped the owned emulator. Disconnect, failed install, unavailable AVD, bad APK digest and unsupported versions rejected at their expected stages. The emulator set was preserved throughout.
  • Real qualification exposed a transport/deadline race. The fix enforces the absolute deadline on completion as well as in the timer. Both late-success and late-error regressions failed before the fix and pass after it; the real blocked-click control now reports deadline with cleanup.
  • npm ci, Chromium installation, npm test (199 passed, no skips), ordinary SDK smoke, shared-target smoke and packed-consumer smoke passed. These include business failures and owned browser/worker timeout/cancellation.
  • Focused Go tests, strict pinned Python contract/assessment compatibility checks in go test -race ./... (both required fixture flags enabled), go vet ./... and diff whitespace checks passed.
  • Connected-code review covered lifecycle ownership, cleanup, control grounding, receipt binding and callers. No confirmed blocking findings remain in the documented experimental scope. A high-signal secret scan found no matches in the physical-device source files or deadline-fix additions. All required GitHub checks passed for the final head; optional Windows arm64 and QualityMax checks were still running at merge review.

Qualification is limited to these runtimes and recorded cases. Physical-device disconnect and outer-process hard-kill recovery, keyboard/fold transitions and other phones remain unqualified. Private device configuration and execution evidence stay local under .context/.

verifiedAssertions remains 0 and source assertion coverage remains unknown. Semantic screen checks (M3), the live reporter (M4), the shared finance demo (M5), epic-wide real-run qualification (M6), iOS, goals and replay remain open. The later explicit semantic run is configured by user choice to use the existing Codex CLI login without API fallback; it has not run.

No SDK publication, deployment, shared adb shutdown or user AVD configuration change is part of this PR.

@vercel

vercel Bot commented Oct 24, 2026 •

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

1 Skipped Deployment
Project Deployment Actions Updated
9lives-run Ignored Ignored Preview Oct 28, 2026 10:07pm UTC

@qualitymaxapp

qualitymaxapp Bot commented Oct 24, 2026 •

Copy link
Copy Markdown
Contributor

QualityMax Review

Reviewing 76a6dc786cf7 now.

Max's current mission: Max is reviewing the diff from his tiny laptop.

 /\_/\     _________
( o.o )   /_______/|
 > ^ <    | qmax  | |
 /| |\   |_______|/
(_| |_)

The completed review will replace this message.

@qualitymaxapp qualitymaxapp Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

QualityMax Review — canonical overview updated; no inline findings were created for this review.

😸 2 finding(s) auto-dismissed by the Finding Verifier
  • llm-path-traversal packages/playwright/src/android.ts:240 — false positive: The file's SHA256 must exactly match the provided hash, preventing arbitrary file reads, and this is a local developer tool.
  • llm-command-injection packages/playwright/src/android.ts:286 — false positive: The variable is passed via an argv array to spawn (preventing shell injection) and used as an HTTP base path for a local server, not a filesystem path, making path traversal and command injection inapplicable.

Auto-dismissals are advisory. If a dismissal looks wrong, treat the finding as blocking and request a verifier review.

@qualitymaxapp

qualitymaxapp Bot commented Oct 24, 2026 •

Copy link
Copy Markdown
Contributor

✅ QualityMax Pipeline

Gate Result
🔍 AI diff review ✅ Clean · qwen3.8-max · completed · 35 eligible / 35 reviewed · qwen3.8-max
🔍 SAST completed · 39 eligible / 39 reviewed · claude-haiku-4-5-20251001
🔍 Canonical PR review delivery completed · 0 eligible / 0 reviewed · exact-head review #5690209173 and overview #6387916647 confirmed

Powered by QualityMax — AI-Powered Test Automation

@qualitymaxapp qualitymaxapp Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

QualityMax Review — canonical overview updated; no inline findings were created for this review.

😸 2 finding(s) auto-dismissed by the Finding Verifier
  • llm-path-traversal packages/playwright/src/android.ts:240 — false positive: The file content is strictly validated against a SHA256 hash before any use, preventing arbitrary file reads via path traversal.
  • llm-command-injection packages/playwright/src/android.ts:286 — false positive: The subprocess uses an argv list preventing shell injection, and the URL path is for local IPC to a locally spawned server, not a file system path.

Auto-dismissals are advisory. If a dismissal looks wrong, treat the finding as blocking and request a verifier review.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔵 Needs a closer look

Native cancellation and privacy-sensitive cleanup require fixes and human review of the revision-specific qualification evidence.

2 open issues
What changed in this PR

Adds an opt-in local Android adapter to the shared target framework, with deterministic resource-ID actions, fresh assertions, and runtime attribution in Go-managed receipts.

Changes:

  • Introduces an owned emulator/Appium lifecycle with separately installed Android dependencies.
  • Extends target selection, evidence validation, receipt models, and schemas for Android.
  • Adds synthetic fixtures, qualification controls, polling regressions, and setup documentation.

Review verification was limited to targeted source inspection; Android qualification and project tests were not rerun.

File Description
tools/​android/​package.json Pins optional Android tooling.
testdata/​sdk/​target-assertion/​observation-errors.spec.ts Tests unavailable-read classification.
testdata/​sdk/​target-assertion/​deadline.spec.ts Tests polling deadlines and cancellation.
testdata/​sdk/​target-assertion.config.ts Configures polling regression tests.
testdata/​sdk/​android/​native.spec.ts Exercises native assertions and failure controls.
testdata/​sdk/​android.config.ts Configures the Android qualification project.
testdata/​android/​res/​values/​ids.xml Defines deterministic resource IDs.
testdata/​android/​requirements.json Maps expected native outcomes.
testdata/​android/​MainActivity.java Implements the synthetic transfer fixture.
testdata/​android/​AndroidManifest.xml Declares the fixture application.
scripts/​package-smoke.cjs Checks Android packaging and optional imports.
scripts/​build-android-fixture.cjs Builds isolated fixture APKs.
scripts/​android-smoke.cjs Runs the native qualification matrix.
scripts/​android-preservation.cjs Checks preexisting-session preservation.
packages/​playwright/​test/​android.test.cjs Tests runtime metadata validation.
packages/​playwright/​src/​targets.ts Adds Android support and polling classification.
packages/​playwright/​src/​reporter.ts Emits bounded Android runtime evidence.
packages/​playwright/​src/​protocol.ts Accepts Android target identifiers.
packages/​playwright/​src/​android.ts Implements the owned local Android driver.
packages/​playwright/​src/​android-runtime.ts Defines and validates runtime metadata.
packages/​playwright/​package.json Exposes the optional Android subpath.
package.json Adds the Android smoke command.
internal/​runner/​model.go Adds Android runtime receipt fields.
internal/​runner/​fake_adapter_test.go Tests scratch cleanup after worker failure.
internal/​runner/​execute.go Owns native scratch and records runtime metadata.
internal/​runner/​adapter.go Extends adapter validation results.
internal/​adapters/​playwrightsdk/​protocol.go Validates runtime evidence and test binding.
internal/​adapters/​playwrightsdk/​android_test.go Tests rejection of invalid runtime evidence.
internal/​adapters/​playwrightsdk/​adapter.go Enables Android target planning.
docs/​targets.md Documents Android and polling semantics.
docs/​contracts/​run-status.schema.json Adds runtime metadata to status schemas.
docs/​contracts/​run-result.schema.json Adds runtime metadata to result schemas.
docs/​contracts/​receipt.schema.json Adds runtime metadata to receipt schemas.
docs/​android-driver.md Documents setup, ownership, and qualification limits.
cmd/​9l/​main.go Updates target help and validation messages.
cmd/​9l/​android_contract_test.go Checks required schema test attribution.

Review effort: Balanced

Comment thread packages/playwright/src/android.ts Resolved
// grace only decides whether to close the driver, if work is still pending.
const timer = setTimeout(() => {
this.stop('deadline', false);
this.stop(deadlineMs < this.options.deadlineMs ? deadlineReason() : 'deadline', false);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Android drivers now release immediately at an operation deadline, while web drivers retain the artifact-preservation grace. Added a regression for an abort-settled transport. Commit 2528df0.

@qualitymaxapp

qualitymaxapp Bot commented Oct 28, 2026

Copy link
Copy Markdown
Contributor

⚠️ QualityMax Diff Analysis — WARN

Completed 4 bounded review batches covering the full eligible diff. The diff introduces Android runtime attribution with strict validation, schema enforcement, and scratch directory isolation. No auth-related security issues were found in the visible changes. However, a logic defect exists: the Go-side APKSHA256 and worker hash validation uses the testID regex (which permits uppercase hex) while the TypeScript equivalent strictly enforces lowercase hex, creating an inconsistency that could allow invalid hashes through the Go path. Reviewed the new Android target driver, protocol changes, reporter attachment handling, and target polling logic. The Appium server is bound to 127.0.0.1 with a random UUID base path, subprocess environments are explicitly allowlisted to exclude credentials, and adb commands use argv arrays without shell interpolation. No auth-related security defects (authentication bypass, credential exposure, or unauthorized access paths) were found in the supplied diff. Non-auth concerns such as TOCTOU on the APK hash check and unvalidated fill input exist but fall outside the requested review scope. Reviewed the added Android qualification scripts, test fixtures, and smoke tests. No auth-related security issues were found: signing credentials are generated per-run via crypto.randomBytes and passed through environment variables to subprocesses without shell interpolation or logging. The globalThis.fetch monkey-patch in the test spec intercepts Appium HTTP transport for fault injection but does not touch authentication flows or credential handling. All subprocess invocations use argv arrays (spawnSync/execFile) avoiding shell injection paths. The diff introduces Playwright-based test coverage for target assertion error handling and adds a local Android tooling package.json with pinned Appium dependencies. No authentication-related code paths are introduced or modified, so the scoped auth-security review finds no issues. The changes are limited to test fixtures and optional local tooling configuration.

🟡 Finding 1: logic

File: internal/adapters/playwrightsdk/protocol.go:470 | Severity: warning

What: validAndroidRuntime validates r.APKSHA256 and r.Worker using testID.MatchString(), but the TypeScript equivalent in packages/playwright/src/android-runtime.ts uses /^[a-f0-9]{64}$/ which only accepts lowercase hex. The testID regex (not fully visible but evidenced by the test using strings.Repeat("a", 64) and strings.Repeat("b", 64)) likely permits uppercase characters or different constraints. This means the Go validator could accept APK SHA256 hashes or worker identifiers containing uppercase hex digits that the TypeScript bridge would reject, leading to inconsistent validation behavior across the two runtimes.

Fix: Define a dedicated sha256Hex regex in Go matching ^[a-f0-9]{64}$ and use it for both APKSHA256 and Worker fields in validAndroidRuntime, mirroring the TypeScript implementation exactly.

Fix with your LLM agent
<your-llm-agent> "Fix the logic issue in internal/adapters/playwrightsdk/protocol.go at line 470: validAndroidRuntime validates r.APKSHA256 and r.Worker using testID.MatchString(), but the TypeScript equivalent in packages/playwright/src/android-runtime.ts uses /^[a-f0-9]{64}$/ which only accepts lowercase hex. The testID regex (not fully visible but evidenced by the test using strings.Repeat(\"a\", 64) and strings.Repeat(\"b\", 64)) likely permits uppercase characters or different constraints. This means the Go validator could accept APK SHA256 hashes or worker identifiers containing uppercase hex digits that the TypeScript bridge would reject, leading to inconsistent validation behavior across the two runtimes."
Fix all findings with your LLM agent
<your-llm-agent> "Fix all QualityMax review findings in this PR:\n- logic in internal/adapters/playwrightsdk/protocol.go:470: validAndroidRuntime validates r.APKSHA256 and r.Worker using testID.MatchString(), but the TypeScript equivalent in packages/playwright/src/android-runtime.ts uses /^[a-f0-9]{64}$/ which only accepts lowercase hex. The testID regex (not fully visible but evidenced by the test using strings.Repeat(\"a\", 64) and strings.Repeat(\"b\", 64)) likely permits uppercase characters or different constraints. This means the Go validator could accept APK SHA256 hashes or worker identifiers containing uppercase hex digits that the TypeScript bridge would reject, leading to inconsistent validation behavior across the two runtimes."

Change diagram — Class

classDiagram
  class TargetDriver {
    <<interface>>
    +kind: string
    +appId: string
    +open()
    +reset()
    +close()
    +observeTestId()
  }
  class androidTargetDriver {
    +appId: string
    +sdkRoot: string
    +toolsRoot: string
    +avd: string
    +apk: string
    +apkSHA256: string
  }
  class TargetApp {
    +deadlineMs: number
    +cancel()
    +getByTestId()
    +open()
    +reset()
    +close()
  }
  class createTargetTest {
    +native: TargetDriver
    +extend()
  }
  TargetDriver <|.. androidTargetDriver
  createTargetTest --> TargetApp : creates
  createTargetTest --> androidTargetDriver : uses
Loading

Analyzed commit 1956693b with qwen3.8-max (34 files, 33583 tokens) | Customize review preferences

@qualitymaxapp qualitymaxapp Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

QualityMax Review — canonical overview updated; no inline findings were created for this review.

😸 3 finding(s) auto-dismissed by the Finding Verifier
  • llm-path-traversal packages/playwright/src/android.ts:240 — false positive: The file content is strictly validated against a SHA256 hash, preventing arbitrary file reads, and the write destination is a secure temporary directory, not user-controlled.
  • llm-command-injection packages/playwright/src/android.ts:268 — false positive: The subprocess is invoked using an argv list without the shell option, which inherently prevents shell injection.
  • llm-command-injection packages/playwright/src/android.ts:286 — false positive: The code uses an argv list for spawn preventing shell injection, and owner is a UUID used in a local URL path, not a file path or shell command.

Auto-dismissals are advisory. If a dismissal looks wrong, treat the finding as blocking and request a verifier review.

@Desperado Desperado changed the title Add owned local Android adapter and M2 qualification (QUA-2204) Add Android adapter with emulator and installed-device navigation (QUA-2204) Oct 28, 2026
@Desperado
Desperado requested a balanced review from Copilot October 28, 2026 18:15

@qualitymaxapp qualitymaxapp Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

QualityMax Review — canonical overview updated; no inline findings were created for this review.

😸 4 finding(s) auto-dismissed by the Finding Verifier
  • llm-command-injection packages/playwright/src/android.ts:161 — false positive: The code at the specified location (line 161) creates an AbortSignal and does not invoke any shell commands, making the injection finding a false positive.
  • llm-path-traversal packages/playwright/src/android.ts:168 — false positive: options.appPackage is strictly validated by a regex allowing only alphanumeric characters, underscores, and dots, which prevents any path traversal characters.
  • llm-command-injection packages/playwright/src/android.ts:280 — false positive: Line 280 contains no shell command or 'owner' variable; it is a simple conditional check, making the finding entirely inapplicable.
  • llm-command-injection packages/playwright/src/android.ts:318 — false positive: Line 318 constructs a file path using a template literal and a SHA256 hash, not a shell command, so no command injection is possible.

Auto-dismissals are advisory. If a dismissal looks wrong, treat the finding as blocking and request a verifier review.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

Device leases can be bypassed by differing TMPDIR values, allowing concurrent sessions to interfere, and Android timeout handling can leave native operations running past their deadline.

4 open issues
1 resolved since last review

Review effort: Balanced

Comment thread packages/playwright/src/android.ts Resolved
Comment thread packages/playwright/src/targets.ts Resolved
Comment thread scripts/android-smoke.cjs Resolved
@Desperado
Desperado force-pushed the Desperado/qua-2202-epic-remaining branch from c368d77 to dc4402e Compare October 28, 2026 18:24

@qualitymaxapp qualitymaxapp Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

QualityMax Review — canonical overview updated; no inline findings were created for this review.

😸 4 finding(s) auto-dismissed by the Finding Verifier
  • llm-command-injection packages/playwright/src/android.ts:161 — false positive: Line 161 contains no shell command, and the shell helper is not shown to be called with user input.
  • llm-path-traversal packages/playwright/src/android.ts:168 — false positive: options.appPackage is strictly validated by a regex allowing only alphanumeric chars, underscores, and dots, which prevents any path traversal characters like slashes or dots.
  • llm-command-injection packages/playwright/src/android.ts:280 — already fixed: The flagged code describing a shell command with an owner variable at line 280 does not exist in the current revision.
  • llm-command-injection packages/playwright/src/android.ts:318 — false positive: The interpolated variables are derived from Android package names or internal scratch paths, which cannot contain shell metacharacters, preventing injection.

Auto-dismissals are advisory. If a dismissal looks wrong, treat the finding as blocking and request a verifier review.

Co-authored-by: Desperado <195594+Desperado@users.noreply.github.com>
@Desperado
Desperado marked this pull request as ready for review October 28, 2026 21:48

@qualitymaxapp qualitymaxapp Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

QualityMax Review — canonical overview updated; no inline findings were created for this review.

😸 4 finding(s) auto-dismissed by the Finding Verifier
  • llm-command-injection packages/playwright/src/android.ts:161 — false positive: Line 161 is an AbortSignal call, not a shell command, and all configuration inputs are strictly validated with regexes.
  • llm-path-traversal packages/playwright/src/android.ts:168 — false positive: The options.appPackage is strictly validated by a regex allowing only alphanumeric characters, underscores, and dots, preventing any path traversal or shell injection.
  • llm-command-injection packages/playwright/src/android.ts:176 — false positive: The appPackage is passed via an argv list to adb without a shell, and line 176 is unrelated to shell commands.
  • llm-command-injection packages/playwright/src/android.ts:318 — false positive: Line 318 only constructs a local file path using a SHA256 hash. It contains no shell commands, URL paths, or user-controlled 'name' parameters, making the finding entirely mismatched.

Auto-dismissals are advisory. If a dismissal looks wrong, treat the finding as blocking and request a verifier review.

@Desperado
Desperado merged commit 5c95b3f into main Oct 28, 2026
21 of 22 checks passed
@Desperado
Desperado deleted the Desperado/qua-2202-epic-remaining branch October 28, 2026 22:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants