fix(android): report dispatched no for adb host refusals - #3079
Conversation
There was a problem hiding this comment.
All reported issues were addressed across 96 files
Reply with feedback, questions, or to request a fix.
Fix all with cubic | Re-trigger cubic
|
[claude-fable-5-1] responding on behalf of @thymikee All three findings addressed in 1c7f3cd.
|
Size Report
Startup median (7 runs, lower is better):
|
|
This PR is ready at 1c7f3cd. The code looks correct to me, and all 19 checks pass. The PR body reports five Apple/screenshot vitest timeouts under host load. Those files do not overlap the Android adb-failure and touch-helper paths this diff touches. There are no conflicts. Not blocking, and you can take or leave these: (1) I did not run the tests or the mutation ( |
1c7f3cd to
cf00361
Compare
|
[claude-fable-5-1] responding on behalf of @thymikee Replayed onto #3071's |
|
The latest changes at cf00361 look good. The rebase left only one logical change, the table fold in the adb failure classifier, and it matches the earlier review of 1c7f3cd. The existing adb-failure and dispatch-disclosure tests cover that route. I did not run the platform-android tests locally, and I did not run a live adb device. I compared the two table versions side by side. I also did not re-review the base changes from #3071 that came in through the rebase. All 19 checks pass. There are no conflicts. Nothing more is needed from review. Merge can go ahead once #3071 lands. |
7486840 to
5717c00
Compare
…for it device_unauthorized, multiple_devices, no_devices, device_not_found and server_version_mismatch now carry hostRefusal like device_offline, so attachAdbFailureHint stamps details.adbHostRefusal for all six. The adb input producers (discloseAdbInputDispatch) and the one-shot and session helper gesture producers read that stamp instead of only TOOL_MISSING; a later step still overrides to unknown through discloseDispatchAfterSteps. The one-shot helper launch failure now runs through attachAdbFailureHint so the classifier stays the single source. Tests and the mutation that makes each fail: - adb-failure.test.ts "every host-refusal reason ...": drop hostRefusal from the no_devices matcher. - adb-failure.test.ts "reasons a device-side command ...": add hostRefusal to connection_dropped or any install_* matcher. - dispatch-disclosure.test.ts android-adb.input-tap.host-refused.* (six rows, real executor via withFakeAdb): make discloseAdbInputDispatch ignore adbHostRefusal (6 failures). - dispatch-disclosure.test.ts android-helper.gesture.host-refused: drop attachAdbFailureHint from the one-shot no-parseable-output branch. - android-adb.input-tap.failed now scripts a non-refusal stderr and stays unknown.
… a nonzero exit - dispatch-disclosure fixture: the mixed-refusal row now feeds `error: device offline\nKilled` through the real tap executor and expects unknown. Mutation: ignoring non-trailer lines in classifyHostRefusal (trailer.every -> true) makes the row report no. - touch-helper: the parsed-final/nonzero error goes through attachAdbFailureHint before disclosure. Test: a nonzero exit with `adb: device offline` keeps adbFailure, hint, retriable. Mutation: dropping attachAdbFailureHint from that branch fails the new test (verified). - adb-failure.test: the device_offline hint is a literal string instead of a value derived from the classifier. Mutation: editing the hint text in adb-failure.ts fails the assertion.
5717c00 to
a8bb438
Compare
Summary
An adb failure that adb itself prints before any device-side work now reports
dispatched: "no". The classifier marks a host refusal (device_unauthorized,device_offline,multiple_devices,no_devices,device_not_found) only when the whole adb output is the refusal: empty stdout, a first stderr line that is exactly the refusal, and only adb's fixed trailer lines after it. Mixed output,server_version_mismatch,install_*,connection_droppedand timeouts stayunknown.attachAdbFailureHintstampsdetails.adbHostRefusal.discloseAdbInputDispatchand the one-shot touch helper read that stamp, so there is no string matching at the producers.Closes #3075. Part of #3069. Stacked on #3071. Touches 6 files.
Validation
Tested commit: df909a1. Not device-facing, so no live evidence.
Every lane passed except vitest-related. There, 5 of 1324 test files failed, each with a 5000ms timeout, and each passed when rerun alone. The passing lanes were format, lint, typecheck, layering, di-seams, fallow, mcp-metadata, build, package, integration-node and macos-coverage. The 5 files are:
screenshot-density passed alone, but took 4.95s against the 5s timeout. The same file is untouched by the branch diff against 6165953, so this is a slow test under host load rather than a regression. No code changes or commits were made.
Contention reruns passed alone:
Unresolved risk: fill at zero dispatched steps (
text-input.ts) is unchanged. The one-shot helper failure now also carries the classifier'sadbFailure, hint andretriable.