From 0ea71a5bf7d7b438d78fee0ad85b589cd38ecbc6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Pierzcha=C5=82a?= Date: Tue, 18 Aug 2026 12:20:35 +0200 Subject: [PATCH 1/2] fix: report real claim results from daemon stop MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `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. --- src/cli/commands/__tests__/daemon.test.ts | 10 +++ src/cli/commands/daemon.ts | 7 +- .../__tests__/daemon-shutdown-report.test.ts | 34 +++++++- src/daemon/daemon-shutdown-report.ts | 64 ++++++++++++-- src/daemon/daemon-stop.ts | 11 ++- src/daemon/server/daemon-runtime.ts | 68 ++++++++------- .../server/daemon-shutdown-claims.test.ts | 86 +++++++++++++++++++ src/daemon/server/daemon-shutdown-claims.ts | 55 ++++++++++++ 8 files changed, 294 insertions(+), 41 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 a0822364c9..34a98a435d 100644 --- a/src/cli/commands/__tests__/daemon.test.ts +++ b/src/cli/commands/__tests__/daemon.test.ts @@ -68,11 +68,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: [] }, }); try { @@ -95,6 +102,9 @@ 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: [], }), expect.any(Function), ); diff --git a/src/cli/commands/daemon.ts b/src/cli/commands/daemon.ts index 53af4c08ac..06a7f49f08 100644 --- a/src/cli/commands/daemon.ts +++ b/src/cli/commands/daemon.ts @@ -41,7 +41,12 @@ 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, + } : stopped; } return { diff --git a/src/daemon/__tests__/daemon-shutdown-report.test.ts b/src/daemon/__tests__/daemon-shutdown-report.test.ts index 7dd3ff5a06..4d20b1be35 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: [] }, + }); expect(readDaemonShutdownReport(stateDir)).toEqual({ providerReleases: { released: [{ leaseId: lease.leaseId, provider: 'limrun' }], pending: [{ leaseId: lease.leaseId, provider: 'limrun' }], }, + claims: { released: [claim], orphaned: [] }, + }); + } 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: [] }, }); } finally { fs.rmSync(stateDir, { recursive: true, force: true }); diff --git a/src/daemon/daemon-shutdown-report.ts b/src/daemon/daemon-shutdown-report.ts index 53128eb98c..6272015d10 100644 --- a/src/daemon/daemon-shutdown-report.ts +++ b/src/daemon/daemon-shutdown-report.ts @@ -9,21 +9,45 @@ export type ProviderReleaseRecord = { provider?: string; }; +/** + * #1320: what happened to one session's device claim during graceful teardown. + * `released` means the claim was cleared 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. + */ +export type DeviceClaimRecord = { + deviceKey: string; + session: string; + platform: string; + deviceId: string; +}; + export type DaemonShutdownReport = { providerReleases: { released: ProviderReleaseRecord[]; pending: ProviderReleaseRecord[]; }; + claims: { + released: DeviceClaimRecord[]; + orphaned: 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[] }; + }, ): 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], }, }; const filePath = shutdownReportPath(stateDir); @@ -42,7 +66,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 +92,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 +107,31 @@ 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: [] }; + const records = claims as { released?: unknown; orphaned?: unknown }; + return { + released: readClaimRecords(records.released), + orphaned: readClaimRecords(records.orphaned), + }; +} + +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 6505f60e11..f5e0ad5904 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,13 @@ 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[]; providerReleases: { status: 'completed' | 'unknown'; released: ProviderReleaseRecord[]; diff --git a/src/daemon/server/daemon-runtime.ts b/src/daemon/server/daemon-runtime.ts index ac4a6e387d..dbae9aa285 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 0000000000..19eb731b2a --- /dev/null +++ b/src/daemon/server/daemon-shutdown-claims.test.ts @@ -0,0 +1,86 @@ +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 { acquireDeviceClaim } from '../device-claims.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-'); + +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 { + 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: [ + { + deviceKey: session.deviceClaim?.deviceKey, + session: 'default', + platform: 'android', + deviceId: 'emulator-5554', + }, + ], + orphaned: [], + }); + 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([ + { + deviceKey: session.deviceClaim?.deviceKey, + session: 'stuck', + platform: 'android', + deviceId: 'emulator-5554', + }, + ]); + expect(inspectDeviceClaims({}).map((entry) => entry.claim?.session)).toEqual(['stuck']); +}); + +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: [] }); +}); diff --git a/src/daemon/server/daemon-shutdown-claims.ts b/src/daemon/server/daemon-shutdown-claims.ts new file mode 100644 index 0000000000..a906c4b443 --- /dev/null +++ b/src/daemon/server/daemon-shutdown-claims.ts @@ -0,0 +1,55 @@ +import { publicPlatformString } from '@agent-device/kernel/device'; +import { emitDiagnostic } from '../../utils/diagnostics.ts'; +import { clearDeviceClaim } from '../device-claims.ts'; +import type { DeviceClaimRecord } from '../daemon-shutdown-report.ts'; +import type { SessionState } from '../types.ts'; + +export type DaemonShutdownClaimLedger = Readonly<{ + claims: { released: DeviceClaimRecord[]; orphaned: DeviceClaimRecord[] }; + /** 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`: `released` is a claim cleared after a + * clean teardown, `orphaned` is one this shutdown left in place. The exiting + * daemon's owner identity dies with the process, so an orphaned claim is exactly + * the cleanup-pending state proof-based reconciliation later resolves. + */ +export function createDaemonShutdownClaimLedger(): DaemonShutdownClaimLedger { + const released: DeviceClaimRecord[] = []; + const orphaned: DeviceClaimRecord[] = []; + const releasedSessions = new Set(); + return { + claims: { released, orphaned }, + releaseClaim: async (session) => { + if (!session.deviceClaim) return; + try { + await clearDeviceClaim(session.deviceClaim); + releasedSessions.add(session.name); + } catch (error) { + 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; + (releasedSessions.has(session.name) ? released : orphaned).push({ + deviceKey: claim.deviceKey, + session: session.name, + platform: publicPlatformString(session.device), + deviceId: session.device.id, + }); + }, + }; +} From 7b3291ec929a7e52130fc7b7448194f2248a9840 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Pierzcha=C5=82a?= Date: Tue, 18 Aug 2026 13:38:38 +0200 Subject: [PATCH 2/2] fix: classify daemon stop claim results from the clear outcome MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `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 | 4 +- src/cli/commands/daemon.ts | 12 ++++ .../__tests__/daemon-shutdown-report.test.ts | 6 +- src/daemon/__tests__/device-claims.test.ts | 20 +++++- src/daemon/daemon-shutdown-report.ts | 19 ++++-- src/daemon/daemon-stop.ts | 5 ++ src/daemon/device-claims.ts | 30 ++++++--- .../server/daemon-shutdown-claims.test.ts | 63 +++++++++++++------ src/daemon/server/daemon-shutdown-claims.ts | 52 ++++++++++----- 9 files changed, 162 insertions(+), 49 deletions(-) diff --git a/src/cli/commands/__tests__/daemon.test.ts b/src/cli/commands/__tests__/daemon.test.ts index 34a98a435d..a6012f6779 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: [], }; @@ -79,7 +80,7 @@ test('merges a graceful shutdown report and cleans runner leases with the start- released: [{ leaseId: 'lease-1', provider: 'limrun' }], pending: [], }, - claims: { released: [claim], orphaned: [] }, + claims: { released: [claim], orphaned: [], superseded: [] }, }); try { @@ -105,6 +106,7 @@ test('merges a graceful shutdown report and cleans runner leases with the start- // #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 06a7f49f08..ee6b9e91af 100644 --- a/src/cli/commands/daemon.ts +++ b/src/cli/commands/daemon.ts @@ -46,6 +46,8 @@ function mergeShutdownReport( 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; } @@ -60,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 4d20b1be35..428fb642c0 100644 --- a/src/daemon/__tests__/daemon-shutdown-report.test.ts +++ b/src/daemon/__tests__/daemon-shutdown-report.test.ts @@ -27,7 +27,7 @@ test('round-trips provider release and device claim records without lease creden try { writeDaemonShutdownReport(stateDir, { providerReleases: { released: [lease], pending: [lease] }, - claims: { released: [claim], orphaned: [] }, + claims: { released: [claim], orphaned: [], superseded: [claim] }, }); expect(readDaemonShutdownReport(stateDir)).toEqual({ @@ -35,7 +35,7 @@ test('round-trips provider release and device claim records without lease creden released: [{ leaseId: lease.leaseId, provider: 'limrun' }], pending: [{ leaseId: lease.leaseId, provider: 'limrun' }], }, - claims: { released: [claim], orphaned: [] }, + claims: { released: [claim], orphaned: [], superseded: [claim] }, }); } finally { fs.rmSync(stateDir, { recursive: true, force: true }); @@ -54,7 +54,7 @@ test('a report written before claim reporting still reads its provider releases' expect(readDaemonShutdownReport(stateDir)).toEqual({ providerReleases: { released: [], pending: [] }, - claims: { released: [], orphaned: [] }, + 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 7cb2ead579..c4c66ee30e 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 6272015d10..cf1e503bd5 100644 --- a/src/daemon/daemon-shutdown-report.ts +++ b/src/daemon/daemon-shutdown-report.ts @@ -11,9 +11,11 @@ export type ProviderReleaseRecord = { /** * #1320: what happened to one session's device claim during graceful teardown. - * `released` means the claim was cleared after the session reached a safe + * `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. + * 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; @@ -30,6 +32,7 @@ export type DaemonShutdownReport = { claims: { released: DeviceClaimRecord[]; orphaned: DeviceClaimRecord[]; + superseded: DeviceClaimRecord[]; }; }; @@ -37,7 +40,11 @@ export function writeDaemonShutdownReport( stateDir: string, outcome: { providerReleases: { released: readonly DeviceLease[]; pending: readonly DeviceLease[] }; - claims: { released: readonly DeviceClaimRecord[]; orphaned: readonly DeviceClaimRecord[] }; + claims: { + released: readonly DeviceClaimRecord[]; + orphaned: readonly DeviceClaimRecord[]; + superseded: readonly DeviceClaimRecord[]; + }; }, ): void { const report: DaemonShutdownReport = { @@ -48,6 +55,7 @@ export function writeDaemonShutdownReport( claims: { released: [...outcome.claims.released], orphaned: [...outcome.claims.orphaned], + superseded: [...outcome.claims.superseded], }, }; const filePath = shutdownReportPath(stateDir); @@ -109,11 +117,12 @@ function isProviderReleaseReport( function readClaimSection(value: { claims?: unknown }): DaemonShutdownReport['claims'] { const claims = value.claims; - if (!claims || typeof claims !== 'object') return { released: [], orphaned: [] }; - const records = claims as { released?: unknown; orphaned?: unknown }; + 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), }; } diff --git a/src/daemon/daemon-stop.ts b/src/daemon/daemon-stop.ts index f5e0ad5904..5814a71b37 100644 --- a/src/daemon/daemon-stop.ts +++ b/src/daemon/daemon-stop.ts @@ -26,6 +26,8 @@ export type DaemonStopResult = { */ claimsReleased: DeviceClaimRecord[]; claimsOrphaned: DeviceClaimRecord[]; + /** Claims another owner had already taken over; this daemon released nothing. */ + claimsSuperseded: DeviceClaimRecord[]; providerReleases: { status: 'completed' | 'unknown'; released: ProviderReleaseRecord[]; @@ -71,6 +73,7 @@ export async function stopDaemon(params: { cleanupConfidence: 'known', claimsReleased: [], claimsOrphaned: [], + claimsSuperseded: [], providerReleases: { status: 'completed', released: [], pending: [] }, warnings: [], }; @@ -94,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.', @@ -153,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 0330100f40..d09886d1d2 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-shutdown-claims.test.ts b/src/daemon/server/daemon-shutdown-claims.test.ts index 19eb731b2a..36796f0ecb 100644 --- a/src/daemon/server/daemon-shutdown-claims.test.ts +++ b/src/daemon/server/daemon-shutdown-claims.test.ts @@ -4,14 +4,25 @@ 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-'); -async function claimedSession(name: string): Promise { +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, @@ -22,6 +33,7 @@ async function claimedSession(name: string): Promise { }); if (acquired.status !== 'acquired') throw new Error('expected an acquired claim'); return { + stateDir, name, device: ANDROID_EMULATOR, deviceClaim: acquired.ownership, @@ -38,15 +50,9 @@ test('a claim cleared after clean teardown is reported released', async () => { ledger.finalize(session); expect(ledger.claims).toEqual({ - released: [ - { - deviceKey: session.deviceClaim?.deviceKey, - session: 'default', - platform: 'android', - deviceId: 'emulator-5554', - }, - ], + released: [claimRecord(session, 'default')], orphaned: [], + superseded: [], }); expect(inspectDeviceClaims({})).toEqual([]); }); @@ -59,17 +65,38 @@ test('a claim left behind by a failed teardown is reported orphaned', async () = ledger.finalize(session); expect(ledger.claims.released).toEqual([]); - expect(ledger.claims.orphaned).toEqual([ - { - deviceKey: session.deviceClaim?.deviceKey, - session: 'stuck', - platform: 'android', - deviceId: 'emulator-5554', - }, - ]); + 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 = { @@ -82,5 +109,5 @@ test('a session that never held a claim contributes nothing', async () => { await ledger.releaseClaim(session); ledger.finalize(session); - expect(ledger.claims).toEqual({ released: [], orphaned: [] }); + 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 index a906c4b443..ffa7cb260f 100644 --- a/src/daemon/server/daemon-shutdown-claims.ts +++ b/src/daemon/server/daemon-shutdown-claims.ts @@ -1,11 +1,17 @@ import { publicPlatformString } from '@agent-device/kernel/device'; import { emitDiagnostic } from '../../utils/diagnostics.ts'; -import { clearDeviceClaim } from '../device-claims.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: { released: DeviceClaimRecord[]; orphaned: DeviceClaimRecord[] }; + 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. */ @@ -13,23 +19,30 @@ export type DaemonShutdownClaimLedger = Readonly<{ }>; /** - * #1320 claim results for `daemon stop`: `released` is a claim cleared after a - * clean teardown, `orphaned` is one this shutdown left in place. The exiting - * daemon's owner identity dies with the process, so an orphaned claim is exactly - * the cleanup-pending state proof-based reconciliation later resolves. + * #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 released: DeviceClaimRecord[] = []; - const orphaned: DeviceClaimRecord[] = []; - const releasedSessions = new Set(); + const claims: DaemonShutdownClaims = { released: [], orphaned: [], superseded: [] }; + const outcomes = new Map(); return { - claims: { released, orphaned }, + claims, releaseClaim: async (session) => { if (!session.deviceClaim) return; try { - await clearDeviceClaim(session.deviceClaim); - releasedSessions.add(session.name); + 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', @@ -44,12 +57,23 @@ export function createDaemonShutdownClaimLedger(): DaemonShutdownClaimLedger { finalize: (session) => { const claim = session.deviceClaim; if (!claim) return; - (releasedSessions.has(session.name) ? released : orphaned).push({ + 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); + } }, }; }