From 3908559fe2f00e0f144fa3f9805bca4c6705da57 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Pierzcha=C5=82a?= Date: Tue, 18 Aug 2026 14:32:57 +0200 Subject: [PATCH] fix: report real claim results from daemon stop (#1818) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix: report real claim results from daemon stop `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. * fix: classify daemon stop claim results from the clear outcome `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. --- src/cli/commands/__tests__/daemon.test.ts | 12 ++ src/cli/commands/daemon.ts | 19 ++- .../__tests__/daemon-shutdown-report.test.ts | 34 +++++- src/daemon/__tests__/device-claims.test.ts | 20 +++- src/daemon/daemon-shutdown-report.ts | 73 ++++++++++- src/daemon/daemon-stop.ts | 16 ++- src/daemon/device-claims.ts | 30 +++-- src/daemon/server/daemon-runtime.ts | 68 ++++++----- .../server/daemon-shutdown-claims.test.ts | 113 ++++++++++++++++++ src/daemon/server/daemon-shutdown-claims.ts | 79 ++++++++++++ 10 files changed, 415 insertions(+), 49 deletions(-) create mode 100644 src/daemon/server/daemon-shutdown-claims.test.ts create mode 100644 src/daemon/server/daemon-shutdown-claims.ts diff --git a/src/cli/commands/__tests__/daemon.test.ts b/src/cli/commands/__tests__/daemon.test.ts index a0822364c..a6012f677 100644 --- a/src/cli/commands/__tests__/daemon.test.ts +++ b/src/cli/commands/__tests__/daemon.test.ts @@ -35,6 +35,7 @@ const GRACEFUL_RESULT: DaemonStopResult = { cleanupConfidence: 'known', claimsReleased: [], claimsOrphaned: [], + claimsSuperseded: [], providerReleases: { status: 'completed', released: [], pending: [] }, warnings: [], }; @@ -68,11 +69,18 @@ test('merges a graceful shutdown report and cleans runner leases with the start- const stateDir = mkdtempForTestSync('agent-device-daemon-command-'); mocks.readDaemonStopIdentity.mockReturnValue({ pid: 123, processStartTime: 'start-time' }); mocks.stopDaemon.mockResolvedValue(GRACEFUL_RESULT); + const claim = { + deviceKey: 'local:android:none:emulator-5554', + session: 'default', + platform: 'android', + deviceId: 'emulator-5554', + }; mocks.readDaemonShutdownReport.mockReturnValue({ providerReleases: { released: [{ leaseId: 'lease-1', provider: 'limrun' }], pending: [], }, + claims: { released: [claim], orphaned: [], superseded: [] }, }); try { @@ -95,6 +103,10 @@ test('merges a graceful shutdown report and cleans runner leases with the start- released: [{ leaseId: 'lease-1', provider: 'limrun' }], pending: [], }, + // #1799: a graceful stop reports the claims it actually released. + claimsReleased: [claim], + claimsOrphaned: [], + claimsSuperseded: [], }), expect.any(Function), ); diff --git a/src/cli/commands/daemon.ts b/src/cli/commands/daemon.ts index 53af4c08a..ee6b9e91a 100644 --- a/src/cli/commands/daemon.ts +++ b/src/cli/commands/daemon.ts @@ -41,7 +41,14 @@ function mergeShutdownReport( ): DaemonStopResult { if (stopped.mode !== 'graceful' || report) { return report - ? { ...stopped, providerReleases: { status: 'completed', ...report.providerReleases } } + ? { + ...stopped, + providerReleases: { status: 'completed', ...report.providerReleases }, + claimsReleased: report.claims.released, + claimsOrphaned: report.claims.orphaned, + claimsSuperseded: report.claims.superseded, + warnings: [...stopped.warnings, ...supersededClaimWarnings(report.claims.superseded)], + } : stopped; } return { @@ -55,6 +62,16 @@ function mergeShutdownReport( }; } +/** A superseded claim is not a failure to report as one, but the operator's + * device is now owned elsewhere, so it must not pass silently. */ +function supersededClaimWarnings(superseded: DaemonStopResult['claimsSuperseded']): string[] { + if (superseded.length === 0) return []; + const devices = superseded.map((claim) => claim.deviceId).join(', '); + return [ + `Another owner had already claimed ${devices} before this daemon released it, so those devices are now owned elsewhere.`, + ]; +} + function renderDaemonStop( result: Pick & { clean: boolean; diff --git a/src/daemon/__tests__/daemon-shutdown-report.test.ts b/src/daemon/__tests__/daemon-shutdown-report.test.ts index 7dd3ff5a0..428fb642c 100644 --- a/src/daemon/__tests__/daemon-shutdown-report.test.ts +++ b/src/daemon/__tests__/daemon-shutdown-report.test.ts @@ -9,7 +9,14 @@ import { import { LeaseRegistry } from '../lease-registry.ts'; import { mkdtempForTestSync } from '../../__tests__/test-utils/tmp-dir.ts'; -test('round-trips provider release records without persisting lease credentials', () => { +const claim = { + deviceKey: 'local:android:none:emulator-5554', + session: 'default', + platform: 'android', + deviceId: 'emulator-5554', +}; + +test('round-trips provider release and device claim records without lease credentials', () => { const stateDir = mkdtempForTestSync('agent-device-shutdown-report-'); const lease = new LeaseRegistry().allocateLease({ tenantId: 'tenant-a', @@ -18,13 +25,36 @@ test('round-trips provider release records without persisting lease credentials' }); try { - writeDaemonShutdownReport(stateDir, { released: [lease], pending: [lease] }); + writeDaemonShutdownReport(stateDir, { + providerReleases: { released: [lease], pending: [lease] }, + claims: { released: [claim], orphaned: [], superseded: [claim] }, + }); expect(readDaemonShutdownReport(stateDir)).toEqual({ providerReleases: { released: [{ leaseId: lease.leaseId, provider: 'limrun' }], pending: [{ leaseId: lease.leaseId, provider: 'limrun' }], }, + claims: { released: [claim], orphaned: [], superseded: [claim] }, + }); + } finally { + fs.rmSync(stateDir, { recursive: true, force: true }); + } +}); + +test('a report written before claim reporting still reads its provider releases', () => { + const stateDir = mkdtempForTestSync('agent-device-shutdown-report-'); + const reportPath = path.join(stateDir, 'daemon-shutdown.json'); + + try { + fs.writeFileSync( + reportPath, + JSON.stringify({ providerReleases: { released: [], pending: [] } }), + ); + + expect(readDaemonShutdownReport(stateDir)).toEqual({ + providerReleases: { released: [], pending: [] }, + claims: { released: [], orphaned: [], superseded: [] }, }); } finally { fs.rmSync(stateDir, { recursive: true, force: true }); diff --git a/src/daemon/__tests__/device-claims.test.ts b/src/daemon/__tests__/device-claims.test.ts index 7cb2ead57..c4c66ee30 100644 --- a/src/daemon/__tests__/device-claims.test.ts +++ b/src/daemon/__tests__/device-claims.test.ts @@ -254,10 +254,28 @@ test('clears only the exact owner token and identity, never a successor claim', claimPath(root), JSON.stringify({ ...stored, ownerToken: 'successor-token', session: 'second' }), ); - await clearDeviceClaim(acquired.ownership); + // Resolving is not releasing: the outcome is what a caller reporting + // ownership must read, since the successor's claim is deliberately kept. + assert.equal(await clearDeviceClaim(acquired.ownership), 'ownership-changed'); assert.equal(inspectDeviceClaims({ serial: device.id })[0]?.claim?.session, 'second'); }); +test('reports the exact outcome of clearing an owned, missing, and unowned claim', async () => { + const root = useClaimsRoot(); + const acquired = await acquireDeviceClaim({ + device, + session: 'owner', + workspace: '/worktrees/owner', + stateDir: root, + }); + assert.equal(acquired.status, 'acquired'); + if (acquired.status !== 'acquired') return; + + assert.equal(await clearDeviceClaim(acquired.ownership), 'deleted'); + assert.equal(await clearDeviceClaim(acquired.ownership), 'absent'); + assert.equal(await clearDeviceClaim(undefined), 'absent'); +}); + test('keeps corrupt records visible and classifies dead owners without reclaiming either', () => { const root = useClaimsRoot(); fs.writeFileSync(path.join(root, 'corrupt.json'), '{bad json'); diff --git a/src/daemon/daemon-shutdown-report.ts b/src/daemon/daemon-shutdown-report.ts index 53128eb98..cf1e503bd 100644 --- a/src/daemon/daemon-shutdown-report.ts +++ b/src/daemon/daemon-shutdown-report.ts @@ -9,21 +9,53 @@ export type ProviderReleaseRecord = { provider?: string; }; +/** + * #1320: what happened to one session's device claim during graceful teardown. + * `released` means the claim was confirmed gone after the session reached a safe + * terminal state; `orphaned` means teardown left it in place, so the exiting + * daemon's dead owner identity is what later proves it reclaimable; `superseded` + * means another owner had already replaced it, so this daemon released nothing + * and left nothing to reconcile. + */ +export type DeviceClaimRecord = { + deviceKey: string; + session: string; + platform: string; + deviceId: string; +}; + export type DaemonShutdownReport = { providerReleases: { released: ProviderReleaseRecord[]; pending: ProviderReleaseRecord[]; }; + claims: { + released: DeviceClaimRecord[]; + orphaned: DeviceClaimRecord[]; + superseded: DeviceClaimRecord[]; + }; }; export function writeDaemonShutdownReport( stateDir: string, - providerReleases: { released: readonly DeviceLease[]; pending: readonly DeviceLease[] }, + outcome: { + providerReleases: { released: readonly DeviceLease[]; pending: readonly DeviceLease[] }; + claims: { + released: readonly DeviceClaimRecord[]; + orphaned: readonly DeviceClaimRecord[]; + superseded: readonly DeviceClaimRecord[]; + }; + }, ): void { const report: DaemonShutdownReport = { providerReleases: { - released: providerReleases.released.map(toProviderReleaseRecord), - pending: providerReleases.pending.map(toProviderReleaseRecord), + released: outcome.providerReleases.released.map(toProviderReleaseRecord), + pending: outcome.providerReleases.pending.map(toProviderReleaseRecord), + }, + claims: { + released: [...outcome.claims.released], + orphaned: [...outcome.claims.orphaned], + superseded: [...outcome.claims.superseded], }, }; const filePath = shutdownReportPath(stateDir); @@ -42,7 +74,10 @@ export function writeDaemonShutdownReport( export function readDaemonShutdownReport(stateDir: string): DaemonShutdownReport | null { try { const parsed = JSON.parse(fs.readFileSync(shutdownReportPath(stateDir), 'utf8')) as unknown; - return isDaemonShutdownReport(parsed) ? parsed : null; + if (!isProviderReleaseReport(parsed)) return null; + // A report left behind by a daemon that predates claim reporting still + // describes its provider releases honestly; it just knows nothing of claims. + return { ...parsed, claims: readClaimSection(parsed) }; } catch { return null; } @@ -65,7 +100,9 @@ function toProviderReleaseRecord(lease: DeviceLease): ProviderReleaseRecord { }; } -function isDaemonShutdownReport(value: unknown): value is DaemonShutdownReport { +function isProviderReleaseReport( + value: unknown, +): value is Omit & { claims?: unknown } { if (!value || typeof value !== 'object') return false; const releases = (value as { providerReleases?: unknown }).providerReleases; if (!releases || typeof releases !== 'object') return false; @@ -78,6 +115,32 @@ function isDaemonShutdownReport(value: unknown): value is DaemonShutdownReport { ); } +function readClaimSection(value: { claims?: unknown }): DaemonShutdownReport['claims'] { + const claims = value.claims; + if (!claims || typeof claims !== 'object') return { released: [], orphaned: [], superseded: [] }; + const records = claims as { released?: unknown; orphaned?: unknown; superseded?: unknown }; + return { + released: readClaimRecords(records.released), + orphaned: readClaimRecords(records.orphaned), + superseded: readClaimRecords(records.superseded), + }; +} + +function readClaimRecords(value: unknown): DeviceClaimRecord[] { + return Array.isArray(value) ? value.filter(isDeviceClaimRecord) : []; +} + +function isDeviceClaimRecord(value: unknown): value is DeviceClaimRecord { + if (!value || typeof value !== 'object') return false; + const record = value as Partial>; + return ( + typeof record.deviceKey === 'string' && + typeof record.session === 'string' && + typeof record.platform === 'string' && + typeof record.deviceId === 'string' + ); +} + function isProviderReleaseRecord(value: unknown): value is ProviderReleaseRecord { if (!value || typeof value !== 'object') return false; const record = value as { leaseId?: unknown; provider?: unknown }; diff --git a/src/daemon/daemon-stop.ts b/src/daemon/daemon-stop.ts index 6505f60e1..5814a71b3 100644 --- a/src/daemon/daemon-stop.ts +++ b/src/daemon/daemon-stop.ts @@ -4,7 +4,7 @@ import { isAgentDeviceDaemonProcess, trySignalProcess } from './daemon-process.t import { isProcessAlive, waitForProcessExit } from '../utils/host-process.ts'; import { sleep } from '../utils/timeouts.ts'; import type { DaemonPaths } from './config.ts'; -import type { ProviderReleaseRecord } from './daemon-shutdown-report.ts'; +import type { DeviceClaimRecord, ProviderReleaseRecord } from './daemon-shutdown-report.ts'; const DAEMON_STOP_GRACE_TIMEOUT_MS = 10_000; const DAEMON_STOP_KILL_TIMEOUT_MS = 2_000; @@ -19,8 +19,15 @@ export type DaemonStopResult = { stopped: boolean; mode: 'graceful' | 'forced' | 'not-running'; cleanupConfidence: 'known' | 'unknown'; - claimsReleased: []; - claimsOrphaned: []; + /** + * #1320 claim results. Only a graceful stop can carry values: they come from + * the shutdown report the exiting daemon wrote, so a forced kill or a daemon + * that was not running reports none rather than claiming certainty. + */ + claimsReleased: DeviceClaimRecord[]; + claimsOrphaned: DeviceClaimRecord[]; + /** Claims another owner had already taken over; this daemon released nothing. */ + claimsSuperseded: DeviceClaimRecord[]; providerReleases: { status: 'completed' | 'unknown'; released: ProviderReleaseRecord[]; @@ -66,6 +73,7 @@ export async function stopDaemon(params: { cleanupConfidence: 'known', claimsReleased: [], claimsOrphaned: [], + claimsSuperseded: [], providerReleases: { status: 'completed', released: [], pending: [] }, warnings: [], }; @@ -89,6 +97,7 @@ export async function stopDaemon(params: { cleanupConfidence: 'unknown', claimsReleased: [], claimsOrphaned: [], + claimsSuperseded: [], providerReleases: { status: 'unknown', released: [], pending: null }, warnings: [ 'The daemon was force-killed before provider lease state could be finalized. Provider allocations may remain active.', @@ -148,6 +157,7 @@ function notRunningResult(): DaemonStopResult { cleanupConfidence: 'known', claimsReleased: [], claimsOrphaned: [], + claimsSuperseded: [], providerReleases: { status: 'completed', released: [], pending: [] }, warnings: [], }; diff --git a/src/daemon/device-claims.ts b/src/daemon/device-claims.ts index 0330100f4..d09886d1d 100644 --- a/src/daemon/device-claims.ts +++ b/src/daemon/device-claims.ts @@ -231,28 +231,44 @@ function isCurrentClaimOwner( ); } +/** + * What releasing a claim actually did. Resolving is not the same as releasing: + * clearing deliberately leaves a claim it does not own in place, so a caller + * that reports ownership must read this rather than the absence of a throw. + * + * - `deleted` — the claim this ownership acquired was removed. + * - `absent` — no claim remains for the device; nothing to remove. + * - `ownership-changed`— a claim remains, but it is not the one we acquired + * (a successor owner, or a record we cannot attribute). + */ +export type DeviceClaimClearOutcome = 'deleted' | 'absent' | 'ownership-changed'; + export async function clearDeviceClaim( ownership: DeviceClaimSessionOwnership | undefined, -): Promise { - if (!ownership) return; - await withDeviceClaimLock(ownership.deviceKey, async () => { - const inspected = inspectDeviceClaimFile(resolveDeviceClaimPath(ownership.deviceKey)); - if (!inspected?.claim) return; +): Promise { + if (!ownership) return 'absent'; + return await withDeviceClaimLock(ownership.deviceKey, async () => { + const claimPath = resolveDeviceClaimPath(ownership.deviceKey); + const inspected = inspectDeviceClaimFile(claimPath); + if (!inspected) return 'absent'; const claim = inspected.claim; if ( + !claim || claim.ownerToken !== ownership.ownerToken || !ownerIdentityMatches( { pid: claim.ownerPid, startTime: claim.ownerStartTime }, { pid: ownership.ownerPid, startTime: ownership.ownerStartTime }, ) ) { - return; + return 'ownership-changed'; } try { - fs.unlinkSync(resolveDeviceClaimPath(ownership.deviceKey)); + fs.unlinkSync(claimPath); } catch (error) { if ((error as NodeJS.ErrnoException).code !== 'ENOENT') throw error; + return 'absent'; } + return 'deleted'; }); } diff --git a/src/daemon/server/daemon-runtime.ts b/src/daemon/server/daemon-runtime.ts index ac4a6e387..dbae9aa28 100644 --- a/src/daemon/server/daemon-runtime.ts +++ b/src/daemon/server/daemon-runtime.ts @@ -24,12 +24,9 @@ import { closeDaemonServers } from './server-shutdown.ts'; import type { DaemonInvokeFn, SessionState } from '../types.ts'; import { createDaemonIdleReap } from './daemon-idle-reap.ts'; import { finalizeDaemonSessionLease } from './daemon-session-lease-finalizer.ts'; -import { - clearDeviceClaim, - reconcileOrphanedDeviceClaims, - type DeviceClaimReconciler, -} from '../device-claims.ts'; +import { reconcileOrphanedDeviceClaims, type DeviceClaimReconciler } from '../device-claims.ts'; import { createDeviceClaimReconciler } from '../device-claim-reconciliation.ts'; +import { createDaemonShutdownClaimLedger } from './daemon-shutdown-claims.ts'; import { emitDiagnostic, flushDiagnosticsToSessionFile, @@ -323,30 +320,36 @@ export async function startDaemonRuntime( ); }; - const teardownDaemonSession = async (session: SessionState): Promise => - await teardownDaemonSessionForShutdown({ - session, - sessionStore, - stderr, - finalizeApplicationLifecycle: async (sessionToFinalize) => - await finalizeDaemonSessionApplicationLifecycle({ - gateway: deviceRuntimeGateway, - scope: createDaemonRecoveryPlatformScope(), - session: sessionToFinalize, - stateDir: baseDir, - runtimeHints: runtimeHintValues(sessionStore.getRuntimeHints(sessionToFinalize.name)), - }), - beforeDelete: async (sessionToFinalize) => { - await finalizeDaemonSessionLease({ - session: sessionToFinalize, - leaseRegistry, - expiredProviderLeaseReleaser, - timeoutMs: DAEMON_SESSION_LEASE_RELEASE_TIMEOUT_MS, - }); - }, - afterSuccessfulTeardown: async (sessionToFinalize) => - await clearDeviceClaim(sessionToFinalize.deviceClaim), - }); + const shutdownClaimLedger = createDaemonShutdownClaimLedger(); + + const teardownDaemonSession = async (session: SessionState): Promise => { + try { + await teardownDaemonSessionForShutdown({ + session, + sessionStore, + stderr, + finalizeApplicationLifecycle: async (sessionToFinalize) => + await finalizeDaemonSessionApplicationLifecycle({ + gateway: deviceRuntimeGateway, + scope: createDaemonRecoveryPlatformScope(), + session: sessionToFinalize, + stateDir: baseDir, + runtimeHints: runtimeHintValues(sessionStore.getRuntimeHints(sessionToFinalize.name)), + }), + beforeDelete: async (sessionToFinalize) => { + await finalizeDaemonSessionLease({ + session: sessionToFinalize, + leaseRegistry, + expiredProviderLeaseReleaser, + timeoutMs: DAEMON_SESSION_LEASE_RELEASE_TIMEOUT_MS, + }); + }, + afterSuccessfulTeardown: shutdownClaimLedger.releaseClaim, + }); + } finally { + shutdownClaimLedger.finalize(session); + } + }; const teardownDaemonSessions = async (): Promise => { const sessionsToStop = sessionStore.toArray(); @@ -545,13 +548,18 @@ export async function startDaemonRuntime( const providerReleaseDrain = await expiredProviderLeaseReleaser.drain( DAEMON_PROVIDER_RELEASE_DRAIN_TIMEOUT_MS, ); - writeDaemonShutdownReport(baseDir, providerReleaseDrain); + writeDaemonShutdownReport(baseDir, { + providerReleases: providerReleaseDrain, + claims: shutdownClaimLedger.claims, + }); emitDiagnostic({ level: providerReleaseDrain.pending.length === 0 ? 'info' : 'warn', phase: 'daemon_shutdown_provider_release_drain', data: { releasedLeaseIds: providerReleaseDrain.released.map((lease) => lease.leaseId), pendingLeaseIds: providerReleaseDrain.pending.map((lease) => lease.leaseId), + releasedDeviceKeys: shutdownClaimLedger.claims.released.map((claim) => claim.deviceKey), + orphanedDeviceKeys: shutdownClaimLedger.claims.orphaned.map((claim) => claim.deviceKey), }, }); expiredProviderLeaseReleaser.shutdown(); diff --git a/src/daemon/server/daemon-shutdown-claims.test.ts b/src/daemon/server/daemon-shutdown-claims.test.ts new file mode 100644 index 000000000..36796f0ec --- /dev/null +++ b/src/daemon/server/daemon-shutdown-claims.test.ts @@ -0,0 +1,113 @@ +import { expect, test } from 'vitest'; +import { ANDROID_EMULATOR } from '../../__tests__/test-utils/device-fixtures.ts'; +import { + isolatedDeviceClaimStores, + retainOrphanedDeviceClaims, +} from '../../__tests__/test-utils/device-claim-store.ts'; +import fs from 'node:fs'; +import { acquireDeviceClaim } from '../device-claims.ts'; +import { resolveDeviceClaimPath } from '../device-claim-paths.ts'; +import { inspectDeviceClaims } from '../device-claim-inspection.ts'; +import { createDaemonShutdownClaimLedger } from './daemon-shutdown-claims.ts'; +import type { SessionState } from '../types.ts'; + +const setup = isolatedDeviceClaimStores('agent-device-shutdown-claim-ledger-'); + +function claimRecord(session: SessionState, name: string) { + return { + deviceKey: session.deviceClaim?.deviceKey, + session: name, + platform: 'android', + deviceId: 'emulator-5554', + }; +} + +async function claimedSession(name: string): Promise { + const { stateDir } = setup(); + const acquired = await acquireDeviceClaim({ + device: ANDROID_EMULATOR, + session: name, + workspace: stateDir, + stateDir, + reconcileOrphanedDeviceClaim: retainOrphanedDeviceClaims, + }); + if (acquired.status !== 'acquired') throw new Error('expected an acquired claim'); + return { + stateDir, + name, + device: ANDROID_EMULATOR, + deviceClaim: acquired.ownership, + createdAt: Date.now(), + actions: [], + }; +} + +test('a claim cleared after clean teardown is reported released', async () => { + const session = await claimedSession('default'); + const ledger = createDaemonShutdownClaimLedger(); + + await ledger.releaseClaim(session); + ledger.finalize(session); + + expect(ledger.claims).toEqual({ + released: [claimRecord(session, 'default')], + orphaned: [], + superseded: [], + }); + expect(inspectDeviceClaims({})).toEqual([]); +}); + +test('a claim left behind by a failed teardown is reported orphaned', async () => { + const session = await claimedSession('stuck'); + const ledger = createDaemonShutdownClaimLedger(); + + // Teardown never reached a safe terminal state, so `releaseClaim` never runs. + ledger.finalize(session); + + expect(ledger.claims.released).toEqual([]); + expect(ledger.claims.orphaned).toEqual([claimRecord(session, 'stuck')]); + expect(inspectDeviceClaims({}).map((entry) => entry.claim?.session)).toEqual(['stuck']); +}); + +test('a claim replaced by a successor owner is reported superseded, never released', async () => { + const session = await claimedSession('replaced'); + const deviceKey = session.deviceClaim?.deviceKey ?? ''; + // The shape recovery leaves behind: this daemon's claim file is removed out + // from under it, and another owner claims the same device before this daemon + // reaches teardown. `clearDeviceClaim` finds a claim it does not own and + // deliberately leaves it alone, so "the call resolved" cannot mean "released". + fs.rmSync(resolveDeviceClaimPath(deviceKey)); + const successor = await acquireDeviceClaim({ + device: ANDROID_EMULATOR, + session: 'successor', + workspace: '/worktrees/successor', + stateDir: `${session.stateDir}-successor`, + reconcileOrphanedDeviceClaim: retainOrphanedDeviceClaims, + }); + expect(successor.status).toBe('acquired'); + + const ledger = createDaemonShutdownClaimLedger(); + await ledger.releaseClaim(session); + ledger.finalize(session); + + expect(ledger.claims.released).toEqual([]); + expect(ledger.claims.orphaned).toEqual([]); + expect(ledger.claims.superseded).toEqual([claimRecord(session, 'replaced')]); + // The successor keeps its device: teardown must never delete a foreign claim. + expect(inspectDeviceClaims({}).map((entry) => entry.claim?.session)).toEqual(['successor']); +}); + +test('a session that never held a claim contributes nothing', async () => { + const ledger = createDaemonShutdownClaimLedger(); + const session: SessionState = { + name: 'remote', + device: ANDROID_EMULATOR, + createdAt: Date.now(), + actions: [], + }; + + await ledger.releaseClaim(session); + ledger.finalize(session); + + expect(ledger.claims).toEqual({ released: [], orphaned: [], superseded: [] }); +}); diff --git a/src/daemon/server/daemon-shutdown-claims.ts b/src/daemon/server/daemon-shutdown-claims.ts new file mode 100644 index 000000000..ffa7cb260 --- /dev/null +++ b/src/daemon/server/daemon-shutdown-claims.ts @@ -0,0 +1,79 @@ +import { publicPlatformString } from '@agent-device/kernel/device'; +import { emitDiagnostic } from '../../utils/diagnostics.ts'; +import { clearDeviceClaim, type DeviceClaimClearOutcome } from '../device-claims.ts'; +import type { DeviceClaimRecord } from '../daemon-shutdown-report.ts'; +import type { SessionState } from '../types.ts'; + +export type DaemonShutdownClaims = { + released: DeviceClaimRecord[]; + orphaned: DeviceClaimRecord[]; + superseded: DeviceClaimRecord[]; +}; + +export type DaemonShutdownClaimLedger = Readonly<{ + claims: DaemonShutdownClaims; + /** Runs only once a session's teardown reached a safe terminal state. */ + releaseClaim(session: SessionState): Promise; + /** Classifies the session's claim once its teardown has finished either way. */ + finalize(session: SessionState): void; +}>; + +/** + * #1320 claim results for `daemon stop`, classified from what clearing actually + * did rather than from whether it threw: + * + * - `released` — the claim was confirmed gone after a clean teardown. + * - `orphaned` — teardown left our claim in place. The exiting daemon's owner + * identity dies with the process, so this is the + * cleanup-pending state proof-based reconciliation resolves. + * - `superseded` — our claim was already replaced by another owner. It is + * neither released (we released nothing) nor orphaned (no + * claim of ours remains to reconcile), so it gets its own + * bucket instead of being folded into a list whose meaning it + * would break. + */ +export function createDaemonShutdownClaimLedger(): DaemonShutdownClaimLedger { + const claims: DaemonShutdownClaims = { released: [], orphaned: [], superseded: [] }; + const outcomes = new Map(); + return { + claims, + releaseClaim: async (session) => { + if (!session.deviceClaim) return; + try { + outcomes.set(session.name, await clearDeviceClaim(session.deviceClaim)); + } catch (error) { + // An unrecorded outcome stays orphaned: the claim may still be on disk. + emitDiagnostic({ + level: 'warn', + phase: 'daemon_shutdown_device_claim_release_failed', + data: { + session: session.name, + deviceKey: session.deviceClaim.deviceKey, + error: error instanceof Error ? error.message : String(error), + }, + }); + } + }, + finalize: (session) => { + const claim = session.deviceClaim; + if (!claim) return; + const record: DeviceClaimRecord = { + deviceKey: claim.deviceKey, + session: session.name, + platform: publicPlatformString(session.device), + deviceId: session.device.id, + }; + switch (outcomes.get(session.name)) { + case 'deleted': + case 'absent': + claims.released.push(record); + return; + case 'ownership-changed': + claims.superseded.push(record); + return; + default: + claims.orphaned.push(record); + } + }, + }; +}