feat: classify adb failures with actionable hints at the executor level

[thymikee]

What

Adds a central adb-stderr → hint classifier in src/platforms/android/adb-executor.ts and attaches the classified hint (plus retriable for clearly transient families) to AppError details at the executor level, so all ~65 Android COMMAND_FAILED sites surface actionable guidance instead of raw adb stderr — with no per-site changes.

Why

Previously the only adb stderr classification lived in snapshot.ts (RETRYABLE_ADB_STDERR_PATTERNS), was snapshot-local, and was retry-only. Every other adb failure reached the agent as a raw stderr line with the generic "retry with --debug" hint — a wasted round-trip for failures with a well-known fix (accept the USB-debugging prompt, pass --serial, uninstall before install, etc.).

How

• classifyAdbFailure(stderr, stdout) recognizes the common failure families, each mapped to an actionable hint and a machine-readable adbFailure reason:
  • device offline, device unauthorized (USB-debugging authorization prompt), device not found
  • more than one device/emulator (points at --serial), no devices/emulators found
  • adb server version mismatch, connection drops (transport error / connection reset / broken pipe / protocol fault)
  • INSTALL_FAILED_*: insufficient storage, update-incompatible (uninstall first), version downgrade, plus a generic fallback
  • Clearly transient families (offline, not-found, connection drops, server mismatch) also get retriable: true.
  • Install verdicts are matched on stdout too (where pm puts them); transport families match stderr only, so arbitrary adb shell output can't be misread as a transport failure.
• Enrichment is attached in both executor funnels — the local serial executor and the provider-scoped exec installed by withAndroidAdbProvider (tunnel-backed adb gets the same hints). An existing site-provided hint is never overwritten. The direct adb devices -l discovery call is wrapped too, where server-mismatch/no-devices failures actually surface.
• snapshot.ts consumes the shared classifier for its retry decision, keeping only its snapshot-specific dump-race patterns (timed out, no such file or directory). Retry behavior unchanged.
• retriable reaches the wire: normalizeError lifts details.retriable to the top-level error (mirroring the details.hint lift, stripped from details), and the daemon's Phase-2 typed-error graft lets a throw-site classification win over the conservative code-level policy. Wire shape is unchanged when nothing classified ('retriable' in error stays false).
• Fixes a bare Error in the port-reverse removal path that surfaced as UNKNOWN — now a classified COMMAND_FAILED AppError with stdout/stderr/exitCode details.

Notes for reviewers

• docs/adr/0010-error-system.md is referenced by the July error audit but was never committed to any branch; this follows the kernel's observable conventions instead (details-hint lift, additive typed-error signals).
• The known-flaky fillAndroid process-spawn tests tripped once under load during verification; per-test timings against a clean tree are equivalent and five consecutive runs of that file are green — no regression.

Testing

• pnpm check:quick (oxlint + tsc) passes.
• Android unit suite + errors + typed-error router tests: 275/275 across 19 files, including 8 new tests covering each classifier family, executor-level enrichment, hint preservation, stdout-vs-stderr matching, the provider-scope path, and the port-reverse fix.

[github-actions]

Size Report

Metric	Base	Current	Diff
JS raw	1.5 MB	1.5 MB	-196 B
JS gzip	483.0 kB	482.7 kB	-269 B
npm tarball	585.2 kB	584.8 kB	-415 B
npm unpacked	2.1 MB	2.1 MB	-860 B

Startup median (7 runs, lower is better):

Scenario	Base	Current	Diff
CLI --version	26.8 ms	26.5 ms	-0.4 ms
CLI --help	47.6 ms	47.7 ms	+0.2 ms

Top changed chunks:

Chunk	Raw diff	Gzip diff
dist/src/3340.js	+3.3 kB	+1.1 kB
dist/src/9722.js	-652 B	-437 B
dist/src/2426.js	-1.1 kB	-386 B
dist/src/2948.js	-1.1 kB	-275 B
dist/src/interaction.js	-151 B	-41 B

[thymikee]

CI needs attention before review.

• Lint & Format fails pnpm format:check on:
  • src/platforms/android/__tests__/adb-executor.test.ts
  • src/platforms/android/adb-executor.ts
• Unit Tests and Coverage both fail the same assertion:
  • src/daemon/handlers/__tests__/session-appstate-input-perf.test.ts
  • test: perf preserves successful metrics and normalizes per-metric Android sampling failures
  • expected the normalized hint to match /retry with --debug/i
  • received: The device is connected but offline — wait for it to finish booting or run adb reconnect, then retry.

Please run pnpm format, then fix the test/behavior mismatch. If the new offline-specific hint is intended, update the regression to assert that explicit recovery contract; otherwise preserve the debug-retry hint when wrapping this per-metric failure.

[thymikee]

I found two classifier coverage gaps that still leave common adb failures on the generic path.

1. allowFailure: true adb paths still bypass the classifier. The executor wrapper only classifies thrown errors, but many Android flows return nonzero results and later throw fresh AppErrors from stdout/stderr, so common failures like device offline still get the generic hint. Please centralize enrichment for nonzero-result-to-AppError paths or call attachAdbFailureHint at those manual throw sites, with a regression for one allowFailure path.

2. Provider-scoped enrichment only wraps exec, but pullAndroidAdbFile and installAndroidAdbPackage prefer semantic provider methods. A provider implementing install that throws an AppError with INSTALL_FAILED_* output will still surface the generic hint. Please wrap/catch semantic provider methods too, or classify in the transfer helpers, and add provider-method coverage.

Refs:

• src/platforms/android/adb-executor.ts
• src/platforms/android/app-lifecycle.ts

[thymikee]

Addressed both classifier coverage gaps in fa3d70d:

1. allowFailure paths. There was no existing common construction point for the nonzero-result-to-AppError conversions, so I created one: androidAdbResultError(message, result, details?) in adb-executor.ts builds the COMMAND_FAILED error from a tolerated adb result and runs it through attachAdbFailureHint. The result-inspecting throw sites now route through it — app launch/uninstall (app-lifecycle.ts), appearance read (settings.ts), keyboard/clipboard queries (device-input-state.ts), package listing (app-helpers.ts), logcat capture (logcat.ts), helper APK installs (snapshot-helper-install.ts, multitouch-helper.ts), heap-dump pull (perf.ts), and the uiautomator dump fallback (snapshot.ts). Hint strings stay in the central matcher table; a site-specific hint (heap-dump pull) still wins because enrichment never overwrites an existing hint.

2. Semantic provider methods. Enrichment moved into normalizeAndroidAdbProvider — the single funnel every provider passes through, for scope installation and explicitly passed providers alike. It now wraps exec plus pull/install/installBundle, so a provider whose install rejects with INSTALL_FAILED_* output gets the classified hint. A WeakSet keeps repeated normalization from stacking wrappers; the local provider needs no wrap since its methods delegate to the already-enriched serial executor.

Tests added:

• androidAdbResultError classifies tolerated nonzero results like thrown executor failures
• androidAdbResultError keeps a site hint over the classified one
• semantic provider install failures carry classified hints (scoped provider, INSTALL_FAILED_UPDATE_INCOMPATIBLE)
• explicitly passed provider pull failures carry classified hints (device offline)
• getAndroidKeyboardStatusWithAdb classifies tolerated adb failures with actionable hints (allowFailure regression on a real production path)

Verified: format/typecheck/lint clean, src/platforms/android (269 tests), the perf handler suite, and test/integration (87 tests) all green.

[thymikee]

Rechecked fa3d70df8. The two adb classifier blockers I called out look addressed now: androidAdbResultError centralizes the allowFailure result-to-AppError enrichment, and provider normalization now wraps exec plus semantic pull/install/installBundle, including explicitly passed providers.

Focused validation passed locally:

pnpm exec vitest run src/platforms/android/__tests__/adb-executor.test.ts src/platforms/android/__tests__/device-input-state.test.ts

Still not ready for a final ready-for-human label: GitHub currently reports mergeable_state=dirty, and the status rollup is only showing CodeQL checks. Please rebase or merge current main, resolve conflicts, and refresh the full CI matrix before the final readiness pass.

[thymikee]

Final refresh on ff4556629: the branch is clean now and all 21 reported checks are green. The reviewed adb classifier fixes are still the PR surface against current main, so I added ready-for-human.

[github-actions]

PR Preview Action v1.8.1
Preview removed because the pull request was closed.
2026-07-04 10:20 UTC