Skip to content

Commit 0ea71a5

Browse files
committed
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.
1 parent 2b6d04a commit 0ea71a5

8 files changed

Lines changed: 294 additions & 41 deletions

File tree

‎src/cli/commands/__tests__/daemon.test.ts‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -68,11 +68,18 @@ test('merges a graceful shutdown report and cleans runner leases with the start-
6868
const stateDir = mkdtempForTestSync('agent-device-daemon-command-');
6969
mocks.readDaemonStopIdentity.mockReturnValue({ pid: 123, processStartTime: 'start-time' });
7070
mocks.stopDaemon.mockResolvedValue(GRACEFUL_RESULT);
71+
const claim = {
72+
deviceKey: 'local:android:none:emulator-5554',
73+
session: 'default',
74+
platform: 'android',
75+
deviceId: 'emulator-5554',
76+
};
7177
mocks.readDaemonShutdownReport.mockReturnValue({
7278
providerReleases: {
7379
released: [{ leaseId: 'lease-1', provider: 'limrun' }],
7480
pending: [],
7581
},
82+
claims: { released: [claim], orphaned: [] },
7683
});
7784

7885
try {
@@ -95,6 +102,9 @@ test('merges a graceful shutdown report and cleans runner leases with the start-
95102
released: [{ leaseId: 'lease-1', provider: 'limrun' }],
96103
pending: [],
97104
},
105+
// #1799: a graceful stop reports the claims it actually released.
106+
claimsReleased: [claim],
107+
claimsOrphaned: [],
98108
}),
99109
expect.any(Function),
100110
);

‎src/cli/commands/daemon.ts‎

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,12 @@ function mergeShutdownReport(
4141
): DaemonStopResult {
4242
if (stopped.mode !== 'graceful' || report) {
4343
return report
44-
? { ...stopped, providerReleases: { status: 'completed', ...report.providerReleases } }
44+
? {
45+
...stopped,
46+
providerReleases: { status: 'completed', ...report.providerReleases },
47+
claimsReleased: report.claims.released,
48+
claimsOrphaned: report.claims.orphaned,
49+
}
4550
: stopped;
4651
}
4752
return {

‎src/daemon/__tests__/daemon-shutdown-report.test.ts‎

Lines changed: 32 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,14 @@ import {
99
import { LeaseRegistry } from '../lease-registry.ts';
1010
import { mkdtempForTestSync } from '../../__tests__/test-utils/tmp-dir.ts';
1111

12-
test('round-trips provider release records without persisting lease credentials', () => {
12+
const claim = {
13+
deviceKey: 'local:android:none:emulator-5554',
14+
session: 'default',
15+
platform: 'android',
16+
deviceId: 'emulator-5554',
17+
};
18+
19+
test('round-trips provider release and device claim records without lease credentials', () => {
1320
const stateDir = mkdtempForTestSync('agent-device-shutdown-report-');
1421
const lease = new LeaseRegistry().allocateLease({
1522
tenantId: 'tenant-a',
@@ -18,13 +25,36 @@ test('round-trips provider release records without persisting lease credentials'
1825
});
1926

2027
try {
21-
writeDaemonShutdownReport(stateDir, { released: [lease], pending: [lease] });
28+
writeDaemonShutdownReport(stateDir, {
29+
providerReleases: { released: [lease], pending: [lease] },
30+
claims: { released: [claim], orphaned: [] },
31+
});
2232

2333
expect(readDaemonShutdownReport(stateDir)).toEqual({
2434
providerReleases: {
2535
released: [{ leaseId: lease.leaseId, provider: 'limrun' }],
2636
pending: [{ leaseId: lease.leaseId, provider: 'limrun' }],
2737
},
38+
claims: { released: [claim], orphaned: [] },
39+
});
40+
} finally {
41+
fs.rmSync(stateDir, { recursive: true, force: true });
42+
}
43+
});
44+
45+
test('a report written before claim reporting still reads its provider releases', () => {
46+
const stateDir = mkdtempForTestSync('agent-device-shutdown-report-');
47+
const reportPath = path.join(stateDir, 'daemon-shutdown.json');
48+
49+
try {
50+
fs.writeFileSync(
51+
reportPath,
52+
JSON.stringify({ providerReleases: { released: [], pending: [] } }),
53+
);
54+
55+
expect(readDaemonShutdownReport(stateDir)).toEqual({
56+
providerReleases: { released: [], pending: [] },
57+
claims: { released: [], orphaned: [] },
2858
});
2959
} finally {
3060
fs.rmSync(stateDir, { recursive: true, force: true });

‎src/daemon/daemon-shutdown-report.ts‎

Lines changed: 59 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -9,21 +9,45 @@ export type ProviderReleaseRecord = {
99
provider?: string;
1010
};
1111

12+
/**
13+
* #1320: what happened to one session's device claim during graceful teardown.
14+
* `released` means the claim was cleared after the session reached a safe
15+
* terminal state; `orphaned` means teardown left it in place, so the exiting
16+
* daemon's dead owner identity is what later proves it reclaimable.
17+
*/
18+
export type DeviceClaimRecord = {
19+
deviceKey: string;
20+
session: string;
21+
platform: string;
22+
deviceId: string;
23+
};
24+
1225
export type DaemonShutdownReport = {
1326
providerReleases: {
1427
released: ProviderReleaseRecord[];
1528
pending: ProviderReleaseRecord[];
1629
};
30+
claims: {
31+
released: DeviceClaimRecord[];
32+
orphaned: DeviceClaimRecord[];
33+
};
1734
};
1835

1936
export function writeDaemonShutdownReport(
2037
stateDir: string,
21-
providerReleases: { released: readonly DeviceLease[]; pending: readonly DeviceLease[] },
38+
outcome: {
39+
providerReleases: { released: readonly DeviceLease[]; pending: readonly DeviceLease[] };
40+
claims: { released: readonly DeviceClaimRecord[]; orphaned: readonly DeviceClaimRecord[] };
41+
},
2242
): void {
2343
const report: DaemonShutdownReport = {
2444
providerReleases: {
25-
released: providerReleases.released.map(toProviderReleaseRecord),
26-
pending: providerReleases.pending.map(toProviderReleaseRecord),
45+
released: outcome.providerReleases.released.map(toProviderReleaseRecord),
46+
pending: outcome.providerReleases.pending.map(toProviderReleaseRecord),
47+
},
48+
claims: {
49+
released: [...outcome.claims.released],
50+
orphaned: [...outcome.claims.orphaned],
2751
},
2852
};
2953
const filePath = shutdownReportPath(stateDir);
@@ -42,7 +66,10 @@ export function writeDaemonShutdownReport(
4266
export function readDaemonShutdownReport(stateDir: string): DaemonShutdownReport | null {
4367
try {
4468
const parsed = JSON.parse(fs.readFileSync(shutdownReportPath(stateDir), 'utf8')) as unknown;
45-
return isDaemonShutdownReport(parsed) ? parsed : null;
69+
if (!isProviderReleaseReport(parsed)) return null;
70+
// A report left behind by a daemon that predates claim reporting still
71+
// describes its provider releases honestly; it just knows nothing of claims.
72+
return { ...parsed, claims: readClaimSection(parsed) };
4673
} catch {
4774
return null;
4875
}
@@ -65,7 +92,9 @@ function toProviderReleaseRecord(lease: DeviceLease): ProviderReleaseRecord {
6592
};
6693
}
6794

68-
function isDaemonShutdownReport(value: unknown): value is DaemonShutdownReport {
95+
function isProviderReleaseReport(
96+
value: unknown,
97+
): value is Omit<DaemonShutdownReport, 'claims'> & { claims?: unknown } {
6998
if (!value || typeof value !== 'object') return false;
7099
const releases = (value as { providerReleases?: unknown }).providerReleases;
71100
if (!releases || typeof releases !== 'object') return false;
@@ -78,6 +107,31 @@ function isDaemonShutdownReport(value: unknown): value is DaemonShutdownReport {
78107
);
79108
}
80109

110+
function readClaimSection(value: { claims?: unknown }): DaemonShutdownReport['claims'] {
111+
const claims = value.claims;
112+
if (!claims || typeof claims !== 'object') return { released: [], orphaned: [] };
113+
const records = claims as { released?: unknown; orphaned?: unknown };
114+
return {
115+
released: readClaimRecords(records.released),
116+
orphaned: readClaimRecords(records.orphaned),
117+
};
118+
}
119+
120+
function readClaimRecords(value: unknown): DeviceClaimRecord[] {
121+
return Array.isArray(value) ? value.filter(isDeviceClaimRecord) : [];
122+
}
123+
124+
function isDeviceClaimRecord(value: unknown): value is DeviceClaimRecord {
125+
if (!value || typeof value !== 'object') return false;
126+
const record = value as Partial<Record<keyof DeviceClaimRecord, unknown>>;
127+
return (
128+
typeof record.deviceKey === 'string' &&
129+
typeof record.session === 'string' &&
130+
typeof record.platform === 'string' &&
131+
typeof record.deviceId === 'string'
132+
);
133+
}
134+
81135
function isProviderReleaseRecord(value: unknown): value is ProviderReleaseRecord {
82136
if (!value || typeof value !== 'object') return false;
83137
const record = value as { leaseId?: unknown; provider?: unknown };

‎src/daemon/daemon-stop.ts‎

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ import { isAgentDeviceDaemonProcess, trySignalProcess } from './daemon-process.t
44
import { isProcessAlive, waitForProcessExit } from '../utils/host-process.ts';
55
import { sleep } from '../utils/timeouts.ts';
66
import type { DaemonPaths } from './config.ts';
7-
import type { ProviderReleaseRecord } from './daemon-shutdown-report.ts';
7+
import type { DeviceClaimRecord, ProviderReleaseRecord } from './daemon-shutdown-report.ts';
88

99
const DAEMON_STOP_GRACE_TIMEOUT_MS = 10_000;
1010
const DAEMON_STOP_KILL_TIMEOUT_MS = 2_000;
@@ -19,8 +19,13 @@ export type DaemonStopResult = {
1919
stopped: boolean;
2020
mode: 'graceful' | 'forced' | 'not-running';
2121
cleanupConfidence: 'known' | 'unknown';
22-
claimsReleased: [];
23-
claimsOrphaned: [];
22+
/**
23+
* #1320 claim results. Only a graceful stop can carry values: they come from
24+
* the shutdown report the exiting daemon wrote, so a forced kill or a daemon
25+
* that was not running reports none rather than claiming certainty.
26+
*/
27+
claimsReleased: DeviceClaimRecord[];
28+
claimsOrphaned: DeviceClaimRecord[];
2429
providerReleases: {
2530
status: 'completed' | 'unknown';
2631
released: ProviderReleaseRecord[];

‎src/daemon/server/daemon-runtime.ts‎

Lines changed: 38 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -24,12 +24,9 @@ import { closeDaemonServers } from './server-shutdown.ts';
2424
import type { DaemonInvokeFn, SessionState } from '../types.ts';
2525
import { createDaemonIdleReap } from './daemon-idle-reap.ts';
2626
import { finalizeDaemonSessionLease } from './daemon-session-lease-finalizer.ts';
27-
import {
28-
clearDeviceClaim,
29-
reconcileOrphanedDeviceClaims,
30-
type DeviceClaimReconciler,
31-
} from '../device-claims.ts';
27+
import { reconcileOrphanedDeviceClaims, type DeviceClaimReconciler } from '../device-claims.ts';
3228
import { createDeviceClaimReconciler } from '../device-claim-reconciliation.ts';
29+
import { createDaemonShutdownClaimLedger } from './daemon-shutdown-claims.ts';
3330
import {
3431
emitDiagnostic,
3532
flushDiagnosticsToSessionFile,
@@ -323,30 +320,36 @@ export async function startDaemonRuntime(
323320
);
324321
};
325322

326-
const teardownDaemonSession = async (session: SessionState): Promise<void> =>
327-
await teardownDaemonSessionForShutdown({
328-
session,
329-
sessionStore,
330-
stderr,
331-
finalizeApplicationLifecycle: async (sessionToFinalize) =>
332-
await finalizeDaemonSessionApplicationLifecycle({
333-
gateway: deviceRuntimeGateway,
334-
scope: createDaemonRecoveryPlatformScope(),
335-
session: sessionToFinalize,
336-
stateDir: baseDir,
337-
runtimeHints: runtimeHintValues(sessionStore.getRuntimeHints(sessionToFinalize.name)),
338-
}),
339-
beforeDelete: async (sessionToFinalize) => {
340-
await finalizeDaemonSessionLease({
341-
session: sessionToFinalize,
342-
leaseRegistry,
343-
expiredProviderLeaseReleaser,
344-
timeoutMs: DAEMON_SESSION_LEASE_RELEASE_TIMEOUT_MS,
345-
});
346-
},
347-
afterSuccessfulTeardown: async (sessionToFinalize) =>
348-
await clearDeviceClaim(sessionToFinalize.deviceClaim),
349-
});
323+
const shutdownClaimLedger = createDaemonShutdownClaimLedger();
324+
325+
const teardownDaemonSession = async (session: SessionState): Promise<void> => {
326+
try {
327+
await teardownDaemonSessionForShutdown({
328+
session,
329+
sessionStore,
330+
stderr,
331+
finalizeApplicationLifecycle: async (sessionToFinalize) =>
332+
await finalizeDaemonSessionApplicationLifecycle({
333+
gateway: deviceRuntimeGateway,
334+
scope: createDaemonRecoveryPlatformScope(),
335+
session: sessionToFinalize,
336+
stateDir: baseDir,
337+
runtimeHints: runtimeHintValues(sessionStore.getRuntimeHints(sessionToFinalize.name)),
338+
}),
339+
beforeDelete: async (sessionToFinalize) => {
340+
await finalizeDaemonSessionLease({
341+
session: sessionToFinalize,
342+
leaseRegistry,
343+
expiredProviderLeaseReleaser,
344+
timeoutMs: DAEMON_SESSION_LEASE_RELEASE_TIMEOUT_MS,
345+
});
346+
},
347+
afterSuccessfulTeardown: shutdownClaimLedger.releaseClaim,
348+
});
349+
} finally {
350+
shutdownClaimLedger.finalize(session);
351+
}
352+
};
350353

351354
const teardownDaemonSessions = async (): Promise<void> => {
352355
const sessionsToStop = sessionStore.toArray();
@@ -545,13 +548,18 @@ export async function startDaemonRuntime(
545548
const providerReleaseDrain = await expiredProviderLeaseReleaser.drain(
546549
DAEMON_PROVIDER_RELEASE_DRAIN_TIMEOUT_MS,
547550
);
548-
writeDaemonShutdownReport(baseDir, providerReleaseDrain);
551+
writeDaemonShutdownReport(baseDir, {
552+
providerReleases: providerReleaseDrain,
553+
claims: shutdownClaimLedger.claims,
554+
});
549555
emitDiagnostic({
550556
level: providerReleaseDrain.pending.length === 0 ? 'info' : 'warn',
551557
phase: 'daemon_shutdown_provider_release_drain',
552558
data: {
553559
releasedLeaseIds: providerReleaseDrain.released.map((lease) => lease.leaseId),
554560
pendingLeaseIds: providerReleaseDrain.pending.map((lease) => lease.leaseId),
561+
releasedDeviceKeys: shutdownClaimLedger.claims.released.map((claim) => claim.deviceKey),
562+
orphanedDeviceKeys: shutdownClaimLedger.claims.orphaned.map((claim) => claim.deviceKey),
555563
},
556564
});
557565
expiredProviderLeaseReleaser.shutdown();

0 commit comments

Comments
 (0)