Skip to content

fix: report real claim results from daemon stop - #1818

Merged
thymikee merged 2 commits into
mainfrom
fix/1799-daemon-stop-claim-report
Aug 18, 2026
Merged

fix: report real claim results from daemon stop#1818
thymikee merged 2 commits into
mainfrom
fix/1799-daemon-stop-claim-report

Conversation

@thymikee

@thymikee thymikee commented Aug 18, 2026

Copy link
Copy Markdown
Member

Summary

Follow-up to #1809 (now merged), which this was split out of so that PR stayed scoped to the enforcement seam. Targets main and stands alone.

Refs #1799 (observation 3), #1320 (the "Claim results distinguish reconciled claimsReleased from cleanup-pending claimsOrphaned" acceptance item).

Before: DaemonStopResult typed claimsReleased and claimsOrphaned as the literal [], and the graceful, forced and not-running paths each returned empty literals. The daemon-side release happened in teardownDaemonSession (clearDeviceClaim, outcome discarded) and the CLI's mergeShutdownReport overlaid only providerReleases. So a graceful stop that really did release a device claim reported none — confirmed live on 0.20.9 in #1809's validation.

After:

  • clearDeviceClaim returns a typed DeviceClaimClearOutcome ('deleted' | 'absent' | 'ownership-changed') instead of nothing. It deliberately leaves a claim it does not own in place, so resolving never meant releasing; a caller that reports ownership has to read the outcome. 'ownership-changed' also now covers an unattributable record, which the old !inspected?.claim early return silently folded in with "file gone".
  • Graceful teardown records each session's outcome through a small ledger (src/daemon/server/daemon-shutdown-claims.ts) and classifies from it:
    • released — absence confirmed (deleted or absent) after the session reached a safe terminal state.
    • orphaned — teardown left our claim in place, or clearing threw. Exactly feat: add cross-worktree device ownership and safe recovery #1320's cleanup-pending state: the exiting daemon's owner identity dies with the process, which is what later proves the claim reclaimable.
    • superseded — another owner had already replaced our claim. This is neither released (this daemon freed nothing) nor orphaned (feat: add cross-worktree device ownership and safe recovery #1320 defines orphaned as staleness proven with cleanup pending, and no claim of ours remains to reconcile), so it gets its own bucket rather than breaking the meaning of either list, and raises a daemon stop warning so a device now owned elsewhere cannot pass silently. cleanupConfidence stays known: the state is not uncertain, it is attributed elsewhere.
  • clearDeviceClaim failures during shutdown are recorded as orphaned with a diagnostic instead of propagating out of Promise.all and aborting the rest of the shutdown.
  • DaemonShutdownReport carries a claims section; DaemonStopResult types all three fields as arrays of a small claim summary (deviceKey, session, platform, deviceId); mergeShutdownReport overlays them next to provider releases.
  • Forced and not-running stops stay empty, because they cannot know.
  • A report written by a daemon that predates claim reporting still reads its provider releases and reports no claims, rather than being rejected as malformed.

No daemon wire surface change — DaemonStopResult is local CLI output.

Validation

Successor-claim regression, proven red first. Against the pre-fix ledger, the successor's claim landed in claimsReleased:

FAIL src/daemon/server/daemon-shutdown-claims.test.ts >
     a claim replaced by a successor owner is reported superseded, never released
AssertionError: expected [ { …(4) } ] to deeply equal []
  93|   expect(ledger.claims.released).toEqual([]);

New/updated tests:

  • src/daemon/server/daemon-shutdown-claims.test.ts — released / orphaned / superseded classification and the no-claim case, against the real claim store; the superseded case also asserts the successor's claim survives teardown untouched.
  • src/daemon/__tests__/device-claims.test.ts — the existing successor test now asserts the returned 'ownership-changed', plus a case pinning deleted / absent / absent for owned, already-cleared, and no-ownership.
  • src/daemon/__tests__/daemon-shutdown-report.test.ts — round-trips the claim section, and the pre-claim-reporting report still reads.
  • src/cli/commands/__tests__/daemon.test.ts — the graceful merge now asserts the claims it carries.

Live (Android, Pixel_9_Pro_XLemulator-5554), same command on both builds:

$ agent-device daemon stop --state-dir <A> --json        # 0.20.9
{ "stopped": true, "mode": "graceful", "claimsReleased": [], "claimsOrphaned": [], ... }
                                        # ...while the claim file WAS removed

$ agent-device daemon stop --state-dir <A> --json        # this branch
{
  "stopped": true, "mode": "graceful", "cleanupConfidence": "known",
  "claimsReleased": [ { "deviceKey": "local:android:none:emulator-5554", "session": "stop-claim",
                        "platform": "android", "deviceId": "emulator-5554" } ],
  "claimsOrphaned": [], ...
}

Gates: pnpm format, pnpm exec tsc --noEmit, pnpm check:affected --run (436 files / 3702 tests green, layering + fallow clean).

Scope

10 files, +415/−49. No follow-ups; device release (#1320's explicit recovery surface) remains out of scope for both PRs.

@github-actions

github-actions Bot commented Aug 18, 2026

Copy link
Copy Markdown

Size Report

Metric Base Current Diff
JS raw 2.28 MB 2.28 MB +1.9 kB
JS gzip 748.3 kB 748.9 kB +553 B
npm tarball 870.1 kB 870.6 kB +527 B
npm unpacked 3.04 MB 3.04 MB +1.9 kB

Startup median (7 runs, lower is better):

Scenario Base Current Diff
CLI --version 27.1 ms 28.4 ms +1.2 ms
CLI --help 67.8 ms 66.8 ms -1.0 ms

Top changed chunks:

Chunk Raw diff Gzip diff
dist/src/internal/daemon.js +879 B +282 B

@thymikee

Copy link
Copy Markdown
Member Author

P1: daemon-shutdown-claims.ts records a session in claimsReleased whenever clearDeviceClaim() resolves, but clearDeviceClaim() deliberately resolves without deleting when the on-disk owner token/start identity has changed. A successor claim can therefore remain while daemon stop reports it released, violating #1320’s report accuracy contract. Return an explicit clear outcome (deleted/already absent vs ownership changed), classify released only when absence is confirmed, and add the existing successor-claim shape as a shutdown-ledger regression. #1818 is also stacked on #1809 and exact-head Coverage needs a retry/classification.

@thymikee

Copy link
Copy Markdown
Member Author

P1 fixed in dfb0a90ea — you were right, and the bug was worse than a mislabel: the ledger counted a device the daemon never freed as freed.

Typed outcome, no boolean. clearDeviceClaim returns DeviceClaimClearOutcome:

export type DeviceClaimClearOutcome = 'deleted' | 'absent' | 'ownership-changed';

deleted = the claim we acquired was removed; absent = no claim remains for the device (missing file, or an ENOENT unlink race); ownership-changed = a claim remains that is not ours. That last case now also covers a record we can't attribute — the previous !inspected?.claim early return folded "file gone" and "file corrupt" into one silent path, and only the first of those is absence.

Classification. The ledger reads the outcome instead of the absence of a throw: released only for deleted/absent, and an unrecorded outcome (teardown never released, or clearing threw) still falls through to orphaned.

Where ownership-changed goes. It gets its own claimsSuperseded bucket rather than being folded into either existing list. It is not released — this daemon freed nothing. It is not orphaned either: #1320 defines orphaned as staleness proven with cleanup pending, and here no claim of ours remains to reconcile, so putting it there would send a reader looking for a stale record that isn't there. Silently dropping it from both lists loses the one fact the operator needs, so it is also surfaced as a daemon stop warning ("Another owner had already claimed <device> before this daemon released it, so those devices are now owned elsewhere"). cleanupConfidence stays known — the state is not uncertain, it is known and attributed elsewhere.

Regression, proven red first. Added a claim replaced by a successor owner is reported superseded, never released to the shutdown-ledger test, built from the exact shape device-claims.test.ts already pins (our claim file removed by recovery, another owner claiming the device before teardown). Against the pre-fix ledger:

FAIL src/daemon/server/daemon-shutdown-claims.test.ts >
     a claim replaced by a successor owner is reported superseded, never released
AssertionError: expected [ { …(4) } ] to deeply equal []
  93|   expect(ledger.claims.released).toEqual([]);

— i.e. the successor's claim sitting in claimsReleased, exactly the accuracy violation you described. Green after the fix, and the test also asserts the successor's claim survives teardown untouched.

device-claims.test.ts gained the module-level counterpart: the existing successor test now asserts the returned 'ownership-changed', plus a case pinning deleted / absent / absent for owned, already-cleared, and no-ownership.

Stacking / Coverage. #1818 stays based on fix/1799-transient-device-claims and I'll retarget it to main once #1809 merges; #1809 is green on 01fcba515. The earlier Coverage failure here was a superseded run, not a result: the job died at 1m27s with Run coverage and every later step at conclusion null (nothing after Check contention retry policy ever ran), which is the cancellation signature rather than a test failure. This push re-runs it from scratch; I'm watching it and will flag it if it fails for a real reason.

Gates on this head: pnpm format && pnpm check:affected --run green (436 files / 3702 tests).

@thymikee
thymikee force-pushed the fix/1799-daemon-stop-claim-report branch from dfb0a90 to de2df1b Compare August 18, 2026 12:01
Base automatically changed from fix/1799-transient-device-claims to main August 18, 2026 12:12
@thymikee
thymikee force-pushed the fix/1799-daemon-stop-claim-report branch from de2df1b to fe37c7a Compare August 18, 2026 12:12
`daemon stop` typed `claimsReleased`/`claimsOrphaned` as the literal `[]` and
every path hardcoded them, so a graceful stop that released a device claim still
reported none (#1799 observation 3, #1320 acceptance). Graceful teardown now
records each session's claim outcome — released after a clean teardown, orphaned
when teardown left the claim in place — into the daemon shutdown report, and the
CLI merges them alongside provider releases. Forced and not-running stops stay
empty because they cannot know, and a report written before claim reporting
still reads its provider releases.
`clearDeviceClaim` deliberately resolves without deleting when the on-disk claim
is no longer the one it acquired, so the shutdown ledger's "the call resolved"
test reported a successor's claim as released — a device the daemon never freed,
counted as freed.

`clearDeviceClaim` now returns a typed outcome (`deleted` | `absent` |
`ownership-changed`) instead of nothing, and the ledger classifies from it:
released only when absence is confirmed, and a new `superseded` bucket for a
claim another owner had already taken over. Superseded is neither released (this
daemon freed nothing) nor orphaned (no claim of ours remains to reconcile), so
folding it into either would break that list's meaning; it also raises a warning
so a device now owned elsewhere cannot pass silently.
@thymikee
thymikee force-pushed the fix/1799-daemon-stop-claim-report branch from fe37c7a to 7b3291e Compare August 18, 2026 12:25
@thymikee
thymikee marked this pull request as ready for review August 18, 2026 12:26
@thymikee

Copy link
Copy Markdown
Member Author

Re-reviewed exact head 7b3291e: the prior successor-claim reporting bug is fixed. Claim clearing now returns an explicit outcome; ownership changes are reported as superseded rather than released, the foreign successor is preserved, and the real-store regression proves the distinction. The stack blocker is gone because #1809 merged and this PR now targets main. Code review is clean and ready-for-human; Coverage, Swift compile, and Android/iOS Smoke are still running.

@thymikee thymikee added the ready-for-human Valid work that needs human implementation, judgment, or maintainer merge label Aug 18, 2026
@thymikee
thymikee merged commit 3908559 into main Aug 18, 2026
31 checks passed
@thymikee
thymikee deleted the fix/1799-daemon-stop-claim-report branch August 18, 2026 12:32
@github-actions

Copy link
Copy Markdown
PR Preview Action v1.8.1
Preview removed because the pull request was closed.
2026-08-18 12:33 UTC

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ready-for-human Valid work that needs human implementation, judgment, or maintainer merge

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant