fix(daemon): reconcile device claims held by a replaced daemon (#2057)

A daemon that is replaced but keeps running keeps the device claim it
took. Its sessions are only reachable through the `daemon.json` its state
dir publishes, which now names the successor, so `DEVICE_IN_USE` named a
session `session list` could not report and no `close` could release.

Claim classification now proves that case: a live owner that is not the
daemon published for its own state dir is `owner-daemon-superseded`, and
reconciliation settles it exactly like a dead owner — after the same
exact-owner durable-resource recovery.

Proof runs one way only, on `ownerIdentityDiffers`: the mirror of
`ownerIdentityMatches`, which treats equal pids with an unreadable start
time as unproven rather than different. An absent or unreadable
registration keeps the claim blocking, and the process reading one never
supersedes itself: a caller had to reach it to ask. The unattended
daemon-startup sweep settles a still running owner only inside the state
dir it now serves; a superseded owner elsewhere is left to the
acquisition path, at the one device a caller actually asked for.

`daemon-registration.ts` owns reading the published identity and returns
the canonical `OwnerIdentity`, so `daemon-stop.ts` drops its private copy
of that parser rather than a third one being added.

Closes #2031

Author: thymikee

src/__tests__/cli-device-status.test.ts

@@ -4,6 +4,7 @@ import path from 'node:path';
 import { test, vi } from 'vitest';
 import { readCurrentOwnerIdentity } from '../utils/owner-identity.ts';
 import { runCliCapture } from './cli-capture.ts';
+import { publishDaemonRegistration } from './test-utils/device-claim-store.ts';
 import { mkdtempForTestSync } from './test-utils/tmp-dir.ts';
 
 vi.mock('../utils/host-process.ts', async (importOriginal) =>
@@ -193,3 +194,50 @@ test('states that nothing is claimed when every claim is stale', async () => {
     fs.rmSync(claimsDir, { recursive: true, force: true });
   }
 });
+
+test('names a replaced-but-running daemon owner as stale rather than a live holder', async () => {
+  const claimsDir = mkdtempForTestSync('agent-device-cli-claims-');
+  const stateDir = path.join(claimsDir, 'state');
+  try {
+    // #2031: the recorded owner still runs, but we are the daemon published for
+    // its state dir, so its session is one `session list` cannot report.
+    publishDaemonRegistration(stateDir, readCurrentOwnerIdentity());
+    fs.writeFileSync(
+      path.join(claimsDir, 'superseded.json'),
+      JSON.stringify({
+        schemaVersion: 1,
+        deviceKey: 'local:web:none:agent-browser-chrome',
+        device: {
+          platform: 'web',
+          id: 'agent-browser-chrome',
+          name: 'Chrome',
+          kind: 'device',
+        },
+        session: 'cwd:/w:default',
+        workspace: '/w',
+        stateDir,
+        ownerPid: process.ppid,
+        ownerStartTime: null,
+        ownerToken: 'superseded-token',
+        createdAtMs: 1,
+        updatedAtMs: 1,
+      }),
+    );
+
+    const normal = await runCliCapture(['device', 'status'], {
+      env: { AGENT_DEVICE_CLAIMS_DIR: claimsDir },
+    });
+    assert.match(normal.stdout, /No live local device claims found\./);
+    assert.match(normal.stdout, /1 stale claim hidden/);
+
+    const stale = await runCliCapture(['device', 'status', '--stale', '--json'], {
+      env: { AGENT_DEVICE_CLAIMS_DIR: claimsDir },
+    });
+    const payload = JSON.parse(stale.stdout);
+    assert.equal(payload.data.claims.length, 1);
+    assert.equal(payload.data.claims[0].classification, 'owner-daemon-superseded');
+    assert.equal(payload.data.claims[0].owner.session, 'cwd:/w:default');
+  } finally {
+    fs.rmSync(claimsDir, { recursive: true, force: true });
+  }
+});

src/__tests__/test-utils/device-claim-store.ts

@@ -3,6 +3,7 @@ import path from 'node:path';
 import { afterEach } from 'vitest';
 import { mkdtempForTestSync } from './tmp-dir.ts';
 import type { DeviceClaimReconciler } from '../../daemon/device-claims.ts';
+import type { OwnerIdentity } from '../../utils/owner-identity.ts';
 
 export type IsolatedDeviceClaimStore = {
   /** Temporary root holding both the claim store and the daemon state dir. */
@@ -37,3 +38,23 @@ export const retainOrphanedDeviceClaims: DeviceClaimReconciler = async () => ({
   status: 'retained',
   reason: 'test-live-owner',
 });
+
+/**
+ * Publishes the daemon registration a running daemon writes for its state dir.
+ * Claim ownership is only reachable through the daemon named there, so tests
+ * that exercise reachability have to stand one in.
+ */
+export function publishDaemonRegistration(stateDir: string, owner: OwnerIdentity): void {
+  fs.mkdirSync(stateDir, { recursive: true });
+  fs.writeFileSync(
+    path.join(stateDir, 'daemon.json'),
+    JSON.stringify({
+      port: 1,
+      transport: 'socket',
+      token: 'test-token',
+      pid: owner.pid,
+      ...(owner.startTime === null ? {} : { processStartTime: owner.startTime }),
+      stateDir,
+    }),
+  );
+}

src/daemon/__tests__/daemon-registration.test.ts

@@ -0,0 +1,86 @@
+import assert from 'node:assert/strict';
+import fs from 'node:fs';
+import path from 'node:path';
+import { afterEach, test } from 'vitest';
+import { isSupersededDaemonOwner, readRegisteredDaemonIdentity } from '../daemon-registration.ts';
+import { writeInfo } from '../server/server-lifecycle.ts';
+import { publishDaemonRegistration } from '../../__tests__/test-utils/device-claim-store.ts';
+import { mkdtempForTestSync } from '../../__tests__/test-utils/tmp-dir.ts';
+
+const roots: string[] = [];
+
+afterEach(() => {
+  for (const root of roots.splice(0)) fs.rmSync(root, { recursive: true, force: true });
+});
+
+function useStateDir(): string {
+  const root = mkdtempForTestSync('agent-device-daemon-registration-');
+  roots.push(root);
+  return root;
+}
+
+function infoPathOf(stateDir: string): string {
+  return path.join(stateDir, 'daemon.json');
+}
+
+test('reads back the identity a running daemon publishes for its state dir', () => {
+  const stateDir = useStateDir();
+  writeInfo(stateDir, infoPathOf(stateDir), path.join(stateDir, 'daemon.log'), {
+    socketPort: 1234,
+    token: 'token',
+    version: '0.0.0-test',
+    codeSignature: 'signature',
+    processStartTime: 'published-start',
+  });
+
+  assert.deepEqual(readRegisteredDaemonIdentity(infoPathOf(stateDir)), {
+    pid: process.pid,
+    startTime: 'published-start',
+  });
+});
+
+test('reads no identity from an absent, corrupt, or pid-less registration', () => {
+  const stateDir = useStateDir();
+  assert.equal(readRegisteredDaemonIdentity(infoPathOf(stateDir)), null);
+
+  fs.writeFileSync(infoPathOf(stateDir), '{not json');
+  assert.equal(readRegisteredDaemonIdentity(infoPathOf(stateDir)), null);
+
+  for (const pid of [0, -1, 1.5, '7', null]) {
+    fs.writeFileSync(infoPathOf(stateDir), JSON.stringify({ pid, token: 'token' }));
+    assert.equal(readRegisteredDaemonIdentity(infoPathOf(stateDir)), null);
+  }
+});
+
+test('a different published pid proves the owner no longer serves its state dir', () => {
+  const stateDir = useStateDir();
+  publishDaemonRegistration(stateDir, { pid: 4242, startTime: 'successor-start' });
+
+  assert.equal(isSupersededDaemonOwner({ stateDir, pid: 4141, startTime: 'owner-start' }), true);
+});
+
+test('a published start time proves supersession only when both sides are readable', () => {
+  const stateDir = useStateDir();
+  publishDaemonRegistration(stateDir, { pid: 4242, startTime: 'successor-start' });
+  assert.equal(isSupersededDaemonOwner({ stateDir, pid: 4242, startTime: 'owner-start' }), true);
+  // An unreadable start time on either side leaves the identity unproven.
+  assert.equal(isSupersededDaemonOwner({ stateDir, pid: 4242, startTime: null }), false);
+  publishDaemonRegistration(stateDir, { pid: 4242, startTime: null });
+  assert.equal(isSupersededDaemonOwner({ stateDir, pid: 4242, startTime: 'owner-start' }), false);
+});
+
+test('the published owner itself is never superseded, and absence is not proof', () => {
+  const stateDir = useStateDir();
+  publishDaemonRegistration(stateDir, { pid: 4242, startTime: 'owner-start' });
+  assert.equal(isSupersededDaemonOwner({ stateDir, pid: 4242, startTime: 'owner-start' }), false);
+
+  fs.rmSync(infoPathOf(stateDir));
+  assert.equal(isSupersededDaemonOwner({ stateDir, pid: 4242, startTime: 'owner-start' }), false);
+});
+
+test('the reading process is never superseded by a registration naming someone else', () => {
+  const stateDir = useStateDir();
+  publishDaemonRegistration(stateDir, { pid: 4242, startTime: 'successor-start' });
+
+  assert.equal(isSupersededDaemonOwner({ stateDir, pid: process.pid, startTime: 'ours' }), false);
+});

src/daemon/__tests__/device-claim-prune.test.ts

@@ -7,6 +7,7 @@ import { reconcileOrphanedDeviceClaims } from '../device-claims.ts';
 import { resolveDeviceClaimPath } from '../device-claim-paths.ts';
 import { acquireProcessLock } from '../../utils/process-lock.ts';
 import { readCurrentOwnerIdentity } from '../../utils/owner-identity.ts';
+import { publishDaemonRegistration } from '../../__tests__/test-utils/device-claim-store.ts';
 import { mkdtempForTestSync } from '../../__tests__/test-utils/tmp-dir.ts';
 
 vi.mock('../../utils/host-process.ts', async (importOriginal) =>
@@ -63,7 +64,10 @@ test('reconciles claims whose owner is gone and keeps every other claim', async
     });
     fs.writeFileSync(path.join(claimsDir, 'garbage.json'), 'not json');
 
-    const summary = await reconcileOrphanedDeviceClaims(async () => ({ status: 'reconciled' }));
+    const summary = await reconcileOrphanedDeviceClaims(
+      async () => ({ status: 'reconciled' }),
+      process.cwd(),
+    );
 
     const claimPathFor = (name: string) => resolveDeviceClaimPath(`local:android:none:${name}`);
     assert.deepEqual(summary, { examined: 1, reconciled: 1, retained: 0, changed: 0 });
@@ -81,12 +85,15 @@ test('reconciles nothing when the claim store does not exist', async () => {
     os.tmpdir(),
     'agent-device-reconciliation-absent-store',
   );
-  assert.deepEqual(await reconcileOrphanedDeviceClaims(async () => ({ status: 'reconciled' })), {
-    examined: 0,
-    reconciled: 0,
-    retained: 0,
-    changed: 0,
-  });
+  assert.deepEqual(
+    await reconcileOrphanedDeviceClaims(async () => ({ status: 'reconciled' }), process.cwd()),
+    {
+      examined: 0,
+      reconciled: 0,
+      retained: 0,
+      changed: 0,
+    },
+  );
 });
 
 test('leaves a live successor written while the reconciliation waited for the claim lock', async () => {
@@ -128,7 +135,10 @@ test('leaves a live successor written while the reconciliation waited for the cl
       description: 'test-held device claim lock',
     });
 
-    const reconciliation = reconcileOrphanedDeviceClaims(async () => ({ status: 'reconciled' }));
+    const reconciliation = reconcileOrphanedDeviceClaims(
+      async () => ({ status: 'reconciled' }),
+      process.cwd(),
+    );
     // The reconciliation is now blocked on the lock; stand in the live successor.
     writeContested(owner.pid, owner.startTime, 'live-token');
     await release();
@@ -145,3 +155,61 @@ test('leaves a live successor written while the reconciliation waited for the cl
     fs.rmSync(claimsDir, { recursive: true, force: true });
   }
 });
+
+test('reconciles a claim whose owning daemon was replaced while still running', async () => {
+  const claimsDir = mkdtempForTestSync('agent-device-reconciliation-superseded-');
+  process.env.AGENT_DEVICE_CLAIMS_DIR = claimsDir;
+  const stateDir = path.join(claimsDir, 'state');
+  try {
+    // #2031: the daemon-startup scan a replaced daemon leaves behind. Our own
+    // registration serves the state dir, so the still-running owner recorded in
+    // the claim can never be asked to release it.
+    publishDaemonRegistration(stateDir, readCurrentOwnerIdentity());
+    writeClaim(claimsDir, 'superseded.json', {
+      ownerPid: process.ppid,
+      ownerStartTime: null,
+      stateDir,
+    });
+
+    const summary = await reconcileOrphanedDeviceClaims(
+      async () => ({ status: 'reconciled' }),
+      stateDir,
+    );
+
+    assert.deepEqual(summary, { examined: 1, reconciled: 1, retained: 0, changed: 0 });
+    assert.equal(
+      fs.existsSync(resolveDeviceClaimPath('local:android:none:superseded.json')),
+      false,
+    );
+  } finally {
+    fs.rmSync(claimsDir, { recursive: true, force: true });
+  }
+});
+
+test('leaves a superseded owner in another state dir to the device that asks for it', async () => {
+  const claimsDir = mkdtempForTestSync('agent-device-reconciliation-foreign-superseded-');
+  process.env.AGENT_DEVICE_CLAIMS_DIR = claimsDir;
+  const stateDir = path.join(claimsDir, 'other-state');
+  try {
+    // The owner still runs, so finalizing the durable resources attributed to it
+    // is only the business of the daemon that now serves ITS state dir.
+    publishDaemonRegistration(stateDir, readCurrentOwnerIdentity());
+    writeClaim(claimsDir, 'foreign.json', {
+      ownerPid: process.ppid,
+      ownerStartTime: null,
+      stateDir,
+    });
+    const reconcile = vi.fn(async () => ({ status: 'reconciled' as const }));
+
+    const summary = await reconcileOrphanedDeviceClaims(
+      reconcile,
+      path.join(claimsDir, 'our-state'),
+    );
+
+    assert.deepEqual(summary, { examined: 0, reconciled: 0, retained: 0, changed: 0 });
+    assert.equal(reconcile.mock.calls.length, 0);
+    assert.equal(fs.existsSync(resolveDeviceClaimPath('local:android:none:foreign.json')), true);
+  } finally {
+    fs.rmSync(claimsDir, { recursive: true, force: true });
+  }
+});