[vivek-aucert]

Summary

Android app close treats an unknown final process probe as proof that the app is gone. Require two consecutive explicit gone results 150 ms apart within the existing two-second deadline. Bound probes by remaining time and report android-app-close-unconfirmed when closure cannot be proved, preventing relaunch through uncertainty.

Four files: Android lifecycle, regression tests, CLI help and command reference. Kill behavior and ownership are unchanged. Probe diagnostics contain timing/state, not raw output. Related: Aucert/aucert#5653. No scope expansion.

Validation

Independent review passed for base e6ac165961def2f7b80336d2c04586a3ace32ce0 to head 0b8affe21707bc232d376c8f1ab9d060ee8c42f3.

pnpm check:affected --run --base e6ac165961def2f7b80336d2c04586a3ace32ce0 --head 0b8affe21707bc232d376c8f1ab9d060ee8c42f3: passed, exactCommit=true. All runnable selected checks passed: format, lint, typecheck, layering, fallow, build, 343 related tests, unit check (17,398 passed, one skipped), and documentation checks.

The first head run had two snapshot-helper test failures. After competing local suites ended, unchanged base pnpm run check:unit passed, followed by the full head gate above. The earlier failure remains recorded; its cause is unproven.

The unknown-probe regression fails on original base e5676488d1005d1db7ffa8b673e5f4b6f9a09216 and passes on original fixed 5d6128147c691e542890d38466dfd3d1958366af; this port preserves that Android production/test patch.

Draft: live Android proof at the rebased head remains pending. Coverage, platform CI and parked device lanes were skipped, not passed. Existing device sessions were untouched.

[thymikee]

At 0b8affe, the Android close fence has one correctness problem and no live device proof yet. Checks have not run: the PR is a draft from a fork, so no device lanes ran, and the local check:affected failure in snapshot-helper-exit-result.test.ts looks unrelated because that test does not import app-lifecycle.ts, though it was not compared at base. Before merge, the state bug below needs a fix and the live Android run needs to be attached.

In app-lifecycle.ts:833, observation.state = stateProbe?.value ?? 'unknown' overwrites the last answered state when probeBeforeDeadline discards a probe that was still running at the deadline. Each probe now gets timeoutMs = remainingMs, so the probe that spans the deadline always ends as unknown. Take a process that is still running after force-stop, such as the launcher, with 20-100 ms per pidof. If the 2 s window ends during the 50 ms poll sleep, the result is android-app-close-timeout/process. If it ends during a pidof, the result is android-app-close-unconfirmed/process, although every answered probe said running. So an app that is clearly still running is reported at random as "could not confirm". That contradicts commands.md:105, and a caller that branches on details.reason picks the wrong recovery. The returned state should be the last state a probe answered within the window, and a discarded or timed-out probe must never replace an answered one. Please assign the state only from probes that answered, and make sure an unknown probe does not overwrite an answered running. The test closeAndroidApp refuses an exhausted process fence passes only because its probes take zero time. Please add a regression where each pidof answers 67890 after a non-zero delay (for example 30 ms with a 50 ms poll) and assert android-app-close-timeout with phase process.

This is a device-facing change to the close fence, relaunch gating and probe timeouts (app-lifecycle.ts:714), and the PR body says live proof is pending. Please run these on an Android emulator at 0b8affe. (a) open <pkg> then close <pkg> must succeed, and the request diagnostics must show two android.close.process-probe events with state gone, ordinals 1 and 2, and elapsedMs well under 2000. (b) open <pkg> --relaunch on a running app must succeed and show a new pid. (c) close on an app that is not running must succeed. (d) close com.android.launcher, or another app Android restarts at once, should show the reported details.reason and phase.

Could the fence be simpler? Keep the tri-state loop and the bounded probe, but return 'gone' | 'running' | 'unanswered' instead of throwing inline, and map it in one place next to ANDROID_CLOSE_TIMEOUTS. running maps to android-app-close-timeout/process as today. unanswered or no stable pair maps to android-app-close-unconfirmed/process, or reuses androidPidofUnavailableError when every probe in the window was unknown. Dropping the per-probe diagnostics and the cadence prose in help would leave about 20 production lines instead of 52 with one error constructor, and it fixes the state bug above by construction because the result comes from answered probes.

Not blocking, and fine to take or leave: build the new close error through the same constructor as the other fence failures instead of inline in waitForAndroidPackageStopped (:717); rename the dotted android.close.process-probe phase to snake_case like the other platform-android phases (:835); use a plain let lastState instead of the one-field observation object (:825); keep only the observable contract in the close help (packages/command-surface/src/management/app.ts:96) and leave cadence and diagnostics fields to the docs page; add a patch changeset for the new android-app-close-unconfirmed reason; and split the per-probe diagnostics (:834) from this fix, or assert them in their own test.

I found the state bug by reading the code, not by running a test with non-zero probe latency, so how often it shows up depends on adb latency on the device. Does the regression above fail at this head as expected?

[thymikee]

0b8affe now conflicts with main. Please rebase when you address the earlier review; the findings there still apply.

[thymikee]

Rebased onto main at c96a2d5 to resolve the conflict. New head da10b5a. Conflicts in:

• packages/command-surface/src/management/app.ts: kept the PR's close help text and re-added main's new sentence about retrying close after unconfirmed cleanup.
• website/docs/docs/commands.md: kept both bullets, the PR's Android process-check bullet and main's bullet about close on a session with a device claim.

No other changes.

[thymikee]

da10b5a is the rebase onto main. The conflict from the earlier review (#4252 (comment)) is resolved and there are no conflicts now, but one defect from that review remains and live validation is missing.

The close state can still be wrong at app-lifecycle.ts:833. observation.state = stateProbe?.value ?? 'unknown' overwrites the last answered state when probeBeforeDeadline discards the probe that spans the deadline. An app that stays running (for example the launcher, where pidof takes 20-100ms) ends as android-app-close-unconfirmed/process or android-app-close-timeout/process, depending on where the 2s window lands. A caller that branches on details.reason then picks the wrong recovery, and this contradicts commands.md, which says a surviving process is a timeout. Please assign the state only from probes that answered, so the last answered state is kept. Please also add a regression where each pidof answers a pid after a non-zero delay, with a 50ms poll, and assert android-app-close-timeout/process.

The close fence, relaunch gating and probe timeouts at app-lifecycle.ts:714 are device-facing, and I see no live emulator run at this head. Please attach one that shows: (a) open then close emits two android.close.process-probe events with gone ordinals 1 and 2 and elapsedMs well under 2000; (b) open --relaunch on a running app succeeds with a new pid; (c) close on a non-running app succeeds; (d) close com.android.launcher reports the documented reason and phase.

The one check on this head is green. It is a draft fork PR, so the device lanes did not run, and that is not evidence for the Android path. I did not run tests locally, so the regression gap in the first finding comes from reading the code. I did not review the upstream main commits in the rebase. Before merge, the state overwrite needs the fix and regression above, and the emulator run needs to be attached.