fix(daemon): report the device claim retained by a failed close

[thymikee]

Summary

When close cannot confirm the device was released — the platform close threw, or a best-effort cleanup step failed — the daemon deliberately keeps the advisory device claim. That is correct: handing a device in an unknown state to the next session would be worse.

But the session record is deleted on the very next line regardless, and nothing said so. The claim was left owned by a session name the daemon no longer knows about, reclaimable only by reopening under that exact name or by the daemon dying — pruneDeadDeviceClaims only reaps claims whose owning process is gone, and the daemon is long-lived by design.

It now emits a device_claim_close_effects_unconfirmed warn diagnostic carrying the device key and session name, mirroring rollbackNewSessionClaim on the open path, which already handles the structurally identical situation. Retention policy is unchanged — this makes an existing deliberate behaviour observable.

Validation

Two new tests using the real acquireAdvisoryDeviceClaim/inspectDeviceClaims with per-test temp claim dirs — no DI seam, which CI forbids. Proven red without the fix:

× a failed platform close retains the device claim and reports it
Error: ENOENT: no such file or directory, open '.../diagnostics.ndjson'
Tests  1 failed | 27 passed (28)

The diagnostics file is never created because zero events are emitted without the change — a maximally strong failure signal.

Step 2 of the plan (extending the thrown error's hint) was deliberately skipped: deviceClaimBlockingError is either a rethrown platform error or an aggregate built in session-teardown.ts, neither constructed in this file, and AGENTS.md requires preserving hint/diagnosticId/logPath when wrapping rather than inventing a wrapper.

Residual risk — draft until live evidence

No live run of a genuinely failed close yet (needs a device disconnected mid-close), so the diagnostic's real-world shape is unverified.

Follow-up

The retained claim is still only recoverable by reopening under the same session name or by daemon death. A device release --stale/--force surface would close that, and the payload added here (device key + session name) is exactly what it would consume. Related: #1320, whose read side already ships.

Scope

2 files. Split out of #1639 per review. Local gates are currently unreliable on this machine due to a competing test run — pushed on GitHub CI's authority with maintainer agreement.

[github-actions]

Size Report

Metric	Base	Current	Diff
JS raw	1.99 MB	1.99 MB	+130 B
JS gzip	635.8 kB	635.8 kB	+24 B
npm tarball	769.2 kB	769.3 kB	+23 B
npm unpacked	2.69 MB	2.69 MB	+130 B

Startup median (7 runs, lower is better):

Scenario	Base	Current	Diff
CLI --version	26.0 ms	26.1 ms	+0.1 ms
CLI --help	63.1 ms	63.1 ms	+0.1 ms

Top changed chunks:

Chunk	Raw diff	Gzip diff
dist/src/session.js	+130 B	+24 B

[thymikee]

Reviewed exact head 6b6bc013. The code path is sound: a close whose platform/cleanup effects are unconfirmed retains the advisory device claim, emits device_claim_close_effects_unconfirmed, deletes the session, and rethrows the original error; the new real-claim regression is non-vacuous and pins both retention and successful clearing.

Not ready yet: this device-facing failed-close path has no required exact-head live evidence. Please demonstrate a disconnect/unavailable-target close where the daemon log contains device_claim_close_effects_unconfirmed, device status still reports the retained claim, and cleanup/recovery is shown. The separate best-effort-cleanup-failure branch remains an untested residual. Current required checks are still pending; no code blocker or conflict found.

[thymikee]

Untested residual closed at 037d4dc2b. The live-evidence requirement is not addressed — see below.

The residual. The retention decision reads platformCloseError ?? cleanupAggregate, and both committed tests drove only the first input. A best-effort cleanup failure — a wedged perfetto stop, a dead snapshot helper — reaches the same retention through a different value, and is the branch operators hit more often. The new test drives it through cleanupAndroidNativePerfSession rejecting while the platform close itself succeeds, then asserts the same three facts: the claim stays live and still names the deleted session, the session record is gone, and the warn diagnostic carries the retained claim's device key and session.

Verified non-vacuous, and isolated to that branch. Gating the emit on platformCloseError:

   × a failing best-effort cleanup also retains the device claim and reports it
      Tests  1 failed | 28 passed (29)

One test, not three — I first tried the blunter mutation (dropping ?? cleanupAggregate entirely) and it took down two pre-existing cleanup tests as well, which would have proven much less about this branch specifically.

Still outstanding, and I cannot close it here: the live disconnect/unavailable-target reproduction showing device_claim_close_effects_unconfirmed in the daemon log with device status reporting the retained claim. I have no device or simulator access in this environment, so that evidence has to come from a run with hardware. Flagging it rather than letting the residual fix read as readiness — this stays not-ready on that requirement alone.

---

Generated by Claude Code

[thymikee]

Live evidence — failed close retains and now reports the claim (exact head 037d4dc2b)

Pixel 7 CI emulator, serial emulator-5556, isolated --state-dir /tmp/ad-evidence-1647.

Method correction worth recording, because the obvious approach silently proves nothing: my first attempt was open Settings → adb emu kill → close --session claimtest with no app positional. It succeeded and retained nothing. Reading session-close.ts explains why — with zero positionals shouldDispatchPlatformClose short-circuits and the Android platform close is never called, and the one adb call that does run (the best-effort IME-restore settings get secure …) is issued allowFailure: true, so a missing device is swallowed as a benign no-record outcome. Passing the app positional is what routes through closeAndroidApp → am force-stop, which is not allowFailure and genuinely throws. Two attempts for this step.

1. Open, claim acquired

$ agent-device open Settings --platform android --device "Pixel 7 CI" --session claimtest --debug
Opened: Settings
$ agent-device device status
android Pixel 7 CI: live session=claimtest workspace=…/agent-ae5ccfbd4d8f47535

2. Target made unavailable

$ adb -s emulator-5556 emu kill
OK: killing emulator, bye bye
$ adb devices
List of devices attached

3. Close fails

$ agent-device close Settings --session claimtest --debug
Error (COMMAND_FAILED): device 'emulator-5556' not found
Hint: The device disconnected or is restarting — verify it is listed in adb devices, then retry.
{"args":["-s","emulator-5556","shell","am","force-stop","com.android.settings"],
 "adbFailure":"device_not_found","exitCode":1}

4. Diagnostic emitted, claim retained — from sessions/claimtest/requests/f7ce4bf7d77f19dc.ndjson:

{"level":"error","phase":"platform_command","command":"close","data":{"platform":"android","error":"adb exited with code 1"}}
{"level":"warn","phase":"device_claim_close_effects_unconfirmed","session":"claimtest",
 "data":{"deviceKey":"local:android:none:emulator-5556","session":"claimtest"}}
{"level":"error","phase":"request_failed","command":"close","data":{"error":"adb exited with code 1"}}

$ agent-device device status
android Pixel 7 CI: live session=claimtest workspace=…/agent-ae5ccfbd4d8f47535

The claim survives keyed by session name, while the daemon's own session record is gone — confirmed because a subsequent open under claimtest created a fresh session rather than reporting a conflict. That is precisely the state this PR makes visible: previously the retained claim was owned by a session name nothing else knew about, and nothing said so.

5. Recovery — rebooted the emulator (same serial returned), then reopened under the same session name; it reclaimed silently with no device_claim_advisory_conflict, passing isCurrentClaimOwner (device-claims.ts:111-125) on session name + workspace + stateDir + owner pid/start-time. A subsequent clean close Settings with the device alive released it properly:

$ agent-device device status
No local advisory device claims found.

Recovery via daemon shutdown was not separately exercised — the reopen path already demonstrates recovery and was the first option listed. Happy to run that variant if you want both pinned.

Residual you flagged: the best-effort-cleanup-failure branch is now covered by 037d4dc2b on this branch; this run exercised the platformCloseError branch specifically.

Cleanup: daemon stopped gracefully, my emulator killed, worktrees clean.

[thymikee]

Re-reviewed exact head 037d4dc2b. No code findings. Both unconfirmed-close inputs (platformCloseError and cleanupAggregate) retain the real advisory claim and emit device_claim_close_effects_unconfirmed; the tests independently pin both branches plus successful clearing. Exact-head Pixel 7 evidence proves the real failed am force-stop route, retained claim visibility, same-session recovery, and final release. Coordinator validation: pnpm check:affected --run passed (93 files, 563 tests). Mergeable with green CodeQL and no confirmed failing check. Clean readiness verdict; the coordinator is applying ready-for-human even though the author left it draft.

[github-actions]

PR Preview Action v1.8.1
Preview removed because the pull request was closed.
2026-08-07 05:50 UTC