[airowe]

Summary

The blocking-dialog and foreground observations ran adb shell dumpsys window … and dumpsys activity … with no timeout around every Android interaction. A hung read waited for the client envelope, which resets the daemon. Now every variant one observation reads (createDumpReader, packages/platform-android/src/observation.ts) shares one 10 s budget, counted from its first dump read.

• A timed-out read or a spent budget counts as unanswered (unknown / {}), like an empty dump. The command proceeds: "a failed probe has never been a refusal".
• Other failures still propagate.
• AndroidObservationHost.runAdb options gain timeoutMs. tap and wm size are unchanged.

3 files. This unblocks the #3326 ledger (#3937).

Validation

• pnpm check:affected --run --head cc4dd4a2f: passed, exactCommit=true (base c48d27a81).
• The new tests fail on base. The budget tests also fail against the per-read version, defd554a9.
• Live at 8a582e78c (before the budget-start commit) on a Pixel 7 API 36 emulator (android-helper 0.21.32-dev, AGENT_DEVICE_EXEC_TRACE=1):
  • open --relaunch, press 'label="Network & internet"', back and longpress 540 1200 1500 passed 3 of 3 runs.
  • Every observation read carried the budget.
  • One press request also logged one adb exec without timeoutMs. It settled at 4.6 s after android_touch_helper_install, outside the observation reads. The trace has no args, so I could not identify it.
  • An earlier run failed press because Clock came to the foreground after boot. It passed on rerun.
• I did not reproduce a hung dumpsys live.

🤖 Generated with Claude Code

[thymikee]

This is ready at defd554: the dumpsys reads behind the dialog and foreground checks are now bounded, and a timeout maps to an unanswered result as intended. All 11 checks pass at this head, and there are no conflicts. I read the code and did not run the tests locally; the emulator run and its exec trace come from the PR body only. Not blocking, and you can take or leave these: readScreenSize in observation.ts still runs adb shell wm size without a timeout and is called on the touch route, so #3326 should bound it together with tap; the timedOutAdb fixture in observation.test.ts builds the timeout error by hand, so building it with the exec layer's own timeout constructor would prove the real error shape passes isCommandTimeoutError; and the bound is per observation, so waitForAndroidAppFocus and the foreground check can each add one on a device that stays hung, which the #3326 ledger should count per route.

[airowe]

Thanks. On the three non-blocking points:

• Budget is now per observation (8a582e78c, from review on fix(timeouts): prove the longpress request envelope with a worst-case ledger #3937). All variants of one observation share one 10 s budget, and the fix(timeouts): prove the longpress request envelope with a worst-case ledger #3937 ledger walks every variant. That counts each observation the route makes, including the foreground check's second read. Live evidence and the gate are re-recorded at 8a582e78c.
• readScreenSize / tap left as they are. The fix(timeouts): prove the longpress request envelope with a worst-case ledger #3937 ledger fails on any call without a bound, and neither one shows up on the longpress route. Bounding them belongs with the route that calls them.
• timedOutAdb fixture kept. packages/host-kit/src/internal/exec.test.ts:257 already checks that a real runCmd timeout passes isCommandTimeoutError. A fixture checked against that same predicate covers the error shape here.

🤖 Addressed by Claude Code

[thymikee]

This PR is ready. At 8a582e7 the code looks correct to me, and the 11 checks are all green. I have no other concerns with the change since the earlier review. I did not run the tests or a device; the regression judgment comes from reading the diff, and the emulator run and exec trace come from the PR body. I also did not trace the timeout error shape of provider-backed adb executors, only the local path in exec.ts. Not blocking, and you can take or leave it: in observation.ts the dump deadline starts when the reader is created, so in readBlockingDialog up to 2 s of the helper focus call comes out of the 10 s budget, and a shared reader in readAppFocus pays for the second focus call the same way. You could create the reader after the focus call, or word the comment as "budget runs from when the observation starts". No conflicts.

[airowe]

Taken in 31676642a: the dump budget now starts at an observation's first dump read, not when the reader is created. So the helper focus call in readBlockingDialog, and both focus calls on a shared reader in readAppFocus, no longer spend it. A new test spends half the budget in a mocked focus call and checks that the first dump still gets the full bound. It fails at 8a582e78c (5000 vs 10000). check:affected --head 31676642a passed. The #3937 ledger still measures 289,989 ms.

🤖 Addressed by Claude Code

[thymikee]

This PR is ready at f02edea. The change since the earlier review only moves where the deadline starts, and I found one small edge case, described below. CI is green with 11 checks and none failing, and there are no conflicts. I did not run the tests. I judged the regression from the pre-change code path and the virtual-clock mechanics. The live emulator evidence in the PR body is from 8a582e7, not f02edea, and no hung dumpsys was reproduced live. The earlier review accepted that evidence. Not blocking, and you can take or leave it: observation.ts now calls deadline.isExpired() and then deadline.remainingMs(), which read the clock twice. If the millisecond rolls over between the two calls, remainingMs() returns 0, normalizeTimeoutMs in exec.ts maps that to no timeout, and that one dump read runs unbounded. Reading once avoids it: const remainingMs = deadline.remainingMs(); if (remainingMs <= 0) return Promise.resolve('');, then pass timeoutMs: remainingMs.

[airowe]

Good catch. Fixed in cc4dd4a2f. The reader now reads deadline.remainingMs() once and uses that value for both the expiry check and timeoutMs, so a rollover can no longer hand normalizeTimeoutMs a 0 that it turns into "no timeout". A new test uses a clock that moves 1 ms on every read, with the budget left at 1–2 ms. It fails at f02edea8e ("expected 0 to be greater than 0") and passes now. check:affected --head cc4dd4a2f passed. #3937 merged it in at 376fddd56, and its gate passed too.

🤖 Addressed by Claude Code