Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
34 changes: 34 additions & 0 deletions src/__tests__/cli-device-status.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -132,3 +132,37 @@ test('keeps corrupt and state-dir-gone claims visible in normal status', async (
fs.rmSync(claimsDir, { recursive: true, force: true });
}
});

test('states that nothing is claimed when every claim is stale', async () => {
// Regression: the all-stale case rendered only the hidden-claim notice, so a
// user asking what holds a device saw a maintenance warning and no answer.
const claimsDir = fs.mkdtempSync(path.join(os.tmpdir(), 'agent-device-cli-claims-'));
try {
fs.writeFileSync(
path.join(claimsDir, 'stale.json'),
JSON.stringify({
schemaVersion: 1,
deviceKey: 'local:apple:ios:stale-phone',
device: { platform: 'ios', id: 'stale-phone', name: 'Dead iPhone', kind: 'device' },
session: 'dead-session',
workspace: '/worktrees/dead',
stateDir: process.cwd(),
ownerPid: 999_999_999,
ownerStartTime: 'old-start-time',
ownerToken: 'stale-token',
createdAtMs: 1,
updatedAtMs: 1,
}),
);

const result = await runCliCapture(['device', 'status'], {
env: { AGENT_DEVICE_CLAIMS_DIR: claimsDir },
});

assert.match(result.stdout, /No live local advisory device claims found\./);
assert.match(result.stdout, /1 stale claim hidden/);
assert.doesNotMatch(result.stdout, /Dead iPhone/);
} finally {
fs.rmSync(claimsDir, { recursive: true, force: true });
}
});
65 changes: 65 additions & 0 deletions src/__tests__/daemon-entrypoint.test.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import assert from 'node:assert/strict';
import crypto from 'node:crypto';
import fs from 'node:fs';
import http from 'node:http';
import os from 'node:os';
Expand Down Expand Up @@ -294,3 +295,67 @@ test('daemon entrypoint publishes HTTP metadata and cleans up on shutdown', asyn
fs.rmSync(stateDir, { recursive: true, force: true });
}
});

test('daemon runtime records the startup device-claim prune in daemon.log', async () => {
// Regression: the prune ran before publishDaemonInfo, which truncates
// daemon.log — so the event was written and then wiped, leaving nothing in
// the log users are told to inspect. Asserted against a real runtime start
// rather than the prune in isolation, since the ordering is the bug.
const stateDir = fs.mkdtempSync(path.join(os.tmpdir(), 'agent-device-daemon-prune-log-'));
const claimsDir = fs.mkdtempSync(path.join(os.tmpdir(), 'agent-device-daemon-prune-claims-'));
const paths = resolveDaemonPaths(stateDir);
const deviceKey = 'local:android:none:prune-log';
// resolveDeviceClaimRoot reads process.env directly, not the env passed to
// startDaemonRuntime, so the store has to be redirected on the process.
const previousClaimsDir = process.env.AGENT_DEVICE_CLAIMS_DIR;
process.env.AGENT_DEVICE_CLAIMS_DIR = claimsDir;
let exitCode: number | undefined;

try {
fs.writeFileSync(
path.join(claimsDir, `${crypto.createHash('sha256').update(deviceKey).digest('hex')}.json`),
JSON.stringify({
schemaVersion: 1,
deviceKey,
device: { platform: 'android', id: 'prune-log', name: 'Prune Log', kind: 'emulator' },
session: 'dead-session',
workspace: '/worktrees/dead',
stateDir: process.cwd(),
ownerPid: 999_999_999,
ownerStartTime: 'old-start-time',
ownerToken: 'dead-token',
createdAtMs: 1,
updatedAtMs: 1,
}),
);

const runtime = await startDaemonRuntime({
env: {
...process.env,
AGENT_DEVICE_STATE_DIR: stateDir,
AGENT_DEVICE_DAEMON_SERVER_MODE: 'http',
AGENT_DEVICE_CLAIMS_DIR: claimsDir,
},
exit: (code) => {
exitCode = code;
},
registerProcessHandlers: false,
stderr: { write: () => {} },
stdout: { write: () => {} },
});

assert.notEqual(runtime, null);
assert.deepEqual(fs.readdirSync(claimsDir), [], 'the dead claim should be pruned');

const log = fs.readFileSync(paths.logPath, 'utf8');
assert.match(log, /"phase":"device_claim_prune"/);
assert.match(log, /"pruned":1/);

await runtime?.shutdown();
assert.equal(exitCode, 0);
} finally {
process.env.AGENT_DEVICE_CLAIMS_DIR = previousClaimsDir;
fs.rmSync(stateDir, { recursive: true, force: true });
fs.rmSync(claimsDir, { recursive: true, force: true });
}
});
4 changes: 3 additions & 1 deletion src/cli/commands/device.ts
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,9 @@ function renderDeviceStatus(
if (options.hiddenStaleClaims === 0) return 'No local advisory device claims found.';
}
return [
...claimLines,
// Without this the all-stale case renders only the hidden-claim notice, so
// the answer to "what holds this device" reads as a maintenance warning.
...(claimLines.length === 0 ? ['No live local advisory device claims found.'] : claimLines),
!options.staleOnly && options.hiddenStaleClaims > 0
? `${options.hiddenStaleClaims} stale ${options.hiddenStaleClaims === 1 ? 'claim' : 'claims'} hidden; inspect with: ${options.staleCommand}`
: null,
Expand Down
127 changes: 127 additions & 0 deletions src/daemon/__tests__/device-claim-prune.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,127 @@
import assert from 'node:assert/strict';
import fs from 'node:fs';
import os from 'node:os';
import path from 'node:path';
import { afterEach, test } from 'vitest';
import { pruneDeadDeviceClaims } 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';

const previousClaimsDir = process.env.AGENT_DEVICE_CLAIMS_DIR;

afterEach(() => {
process.env.AGENT_DEVICE_CLAIMS_DIR = previousClaimsDir;
});

// Claims live at the hash of their device key; the prune only touches a file
// that is the canonical path for the key it contains.
function writeClaim(
_claimsDir: string,
fileName: string,
overrides: { ownerPid: number; ownerStartTime?: string | null; stateDir?: string },
): void {
const deviceKey = `local:android:none:${fileName}`;
fs.writeFileSync(
resolveDeviceClaimPath(deviceKey),
JSON.stringify({
schemaVersion: 1,
deviceKey,
device: { platform: 'android', id: fileName, name: fileName, kind: 'emulator' },
session: `${fileName}-session`,
workspace: '/worktrees/x',
stateDir: overrides.stateDir ?? process.cwd(),
ownerPid: overrides.ownerPid,
ownerStartTime: overrides.ownerStartTime ?? undefined,
ownerToken: `${fileName}-token`,
createdAtMs: 1,
updatedAtMs: 1,
}),
);
}

test('prunes claims whose owner is gone and keeps every other claim', async () => {
const claimsDir = fs.mkdtempSync(path.join(os.tmpdir(), 'agent-device-prune-claims-'));
process.env.AGENT_DEVICE_CLAIMS_DIR = claimsDir;
const owner = readCurrentOwnerIdentity();
try {
writeClaim(claimsDir, 'dead.json', { ownerPid: 999_999_999, ownerStartTime: 'old' });
writeClaim(claimsDir, 'live.json', { ownerPid: owner.pid, ownerStartTime: owner.startTime });
// Live process whose state dir vanished: pruning this could hand its device
// to a second session, so it is reported stale but never deleted.
writeClaim(claimsDir, 'state-dir-gone.json', {
ownerPid: owner.pid,
ownerStartTime: owner.startTime,
stateDir: path.join(claimsDir, 'missing-state-dir'),
});
fs.writeFileSync(path.join(claimsDir, 'garbage.json'), 'not json');

const { pruned } = await pruneDeadDeviceClaims();

const claimPathFor = (name: string) => resolveDeviceClaimPath(`local:android:none:${name}`);
assert.equal(pruned, 1);
assert.equal(fs.existsSync(claimPathFor('dead.json')), false);
assert.equal(fs.existsSync(claimPathFor('live.json')), true);
assert.equal(fs.existsSync(claimPathFor('state-dir-gone.json')), true);
assert.equal(fs.existsSync(path.join(claimsDir, 'garbage.json')), true);
} finally {
fs.rmSync(claimsDir, { recursive: true, force: true });
}
});

test('prunes nothing when the claim store does not exist', async () => {
process.env.AGENT_DEVICE_CLAIMS_DIR = path.join(os.tmpdir(), 'agent-device-prune-absent-store');
assert.deepEqual(await pruneDeadDeviceClaims(), { pruned: 0 });
});

test('leaves a live successor written while the prune waited for the claim lock', async () => {
// Claim paths are derived from the device key, so a concurrent daemon can
// prune the same dead claim and a new session can write its live successor
// to that exact path. Holding the lock here reproduces that window: the scan
// sees a dead claim, then the file is replaced before the prune can unlink.
const claimsDir = fs.mkdtempSync(path.join(os.tmpdir(), 'agent-device-prune-race-'));
process.env.AGENT_DEVICE_CLAIMS_DIR = claimsDir;
const owner = readCurrentOwnerIdentity();
const deviceKey = 'local:android:none:contested';
const claimPath = resolveDeviceClaimPath(deviceKey);
const writeContested = (ownerPid: number, ownerStartTime: string | null, token: string) => {
fs.writeFileSync(
claimPath,
JSON.stringify({
schemaVersion: 1,
deviceKey,
device: { platform: 'android', id: 'contested', name: 'Contested', kind: 'emulator' },
session: 'contested-session',
workspace: '/worktrees/x',
stateDir: process.cwd(),
ownerPid,
ownerStartTime: ownerStartTime ?? undefined,
ownerToken: token,
createdAtMs: 1,
updatedAtMs: 1,
}),
);
};

try {
writeContested(999_999_999, 'old', 'dead-token');

const release = await acquireProcessLock({
lockDirPath: `${claimPath}.lock`,
owner: { pid: owner.pid, startTime: owner.startTime, acquiredAtMs: Date.now() },
timeoutMs: 5_000,
description: 'test-held device claim lock',
});

const pruning = pruneDeadDeviceClaims();
// The prune is now blocked on the lock; stand in the live successor.
writeContested(owner.pid, owner.startTime, 'live-token');
await release();

assert.deepEqual(await pruning, { pruned: 0 });
assert.equal(fs.existsSync(claimPath), true);
assert.equal(JSON.parse(fs.readFileSync(claimPath, 'utf8')).ownerToken, 'live-token');
} finally {
fs.rmSync(claimsDir, { recursive: true, force: true });
}
});
51 changes: 50 additions & 1 deletion src/daemon/device-claims.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import { emitDiagnostic } from '../utils/diagnostics.ts';
import { acquireProcessLock } from '../utils/process-lock.ts';
import { ownerIdentityMatches, readCurrentOwnerIdentity } from '../utils/owner-identity.ts';
import { inspectDeviceClaimFile, type InspectedDeviceClaim } from './device-claim-inspection.ts';
import { resolveDeviceClaimPath } from './device-claim-paths.ts';
import { resolveDeviceClaimPath, resolveDeviceClaimRoot } from './device-claim-paths.ts';

const DEVICE_CLAIM_SCHEMA_VERSION = 1;
const DEVICE_CLAIM_LOCK_TIMEOUT_MS = 30_000;
Expand Down Expand Up @@ -149,6 +149,55 @@ export async function clearAdvisoryDeviceClaim(
});
}

/**
* Deletes claim files whose owning process is provably gone.
*
* Claims are released on session close and daemon shutdown, but a process that
* dies abruptly leaves its file behind forever and nothing else reaps them.
*
* The liveness check is repeated under the per-device lock immediately before
* unlinking: claim paths are derived from the device key, so a concurrent
* daemon can prune the same dead claim and a new session can write its live
* successor to that exact path while this scan is still running. Deleting on
* the first read would take the successor with it.
*
* Deliberately narrower than the CLI's stale filter: `owner-state-dir-gone`
* describes a LIVE process whose state dir vanished, and deleting that claim
* could hand its device to a second session.
*/
export async function pruneDeadDeviceClaims(): Promise<{ pruned: number }> {
const root = resolveDeviceClaimRoot();
let entries: fs.Dirent[];
try {
entries = fs.readdirSync(root, { withFileTypes: true });
} catch {
return { pruned: 0 };
}
let pruned = 0;
for (const entry of entries) {
if (!entry.isFile() || !entry.name.endsWith('.json')) continue;
const filePath = path.join(root, entry.name);
const scanned = inspectDeviceClaimFile(filePath);
if (scanned?.classification !== 'owner-process-dead' || !scanned.claim) continue;
const { deviceKey, ownerToken } = scanned.claim;
// A file whose name is not the hash of its own device key is not the file
// the lock protects, so leave it rather than unlink something else.
if (resolveDeviceClaimPath(deviceKey) !== filePath) continue;
await withDeviceClaimLock(deviceKey, async () => {
const current = inspectDeviceClaimFile(filePath);
if (current?.classification !== 'owner-process-dead') return;
if (current.claim?.ownerToken !== ownerToken) return;
try {
fs.unlinkSync(filePath);
pruned += 1;
} catch (error) {
if ((error as NodeJS.ErrnoException).code !== 'ENOENT') throw error;
}
});
}
return { pruned };
}

function writeClaim(claim: DeviceClaim): void {
const claimPath = resolveDeviceClaimPath(claim.deviceKey);
fs.mkdirSync(path.dirname(claimPath), { recursive: true, mode: 0o700 });
Expand Down
29 changes: 28 additions & 1 deletion src/daemon/server/daemon-runtime.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ 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 { clearAdvisoryDeviceClaim } from '../device-claims.ts';
import { clearAdvisoryDeviceClaim, pruneDeadDeviceClaims } from '../device-claims.ts';
import {
emitDiagnostic,
flushDiagnosticsToSessionFile,
Expand Down Expand Up @@ -342,6 +342,9 @@ export async function startDaemonRuntime(
socketPort = opened.socketPort;
httpPort = opened.httpPort;
publishDaemonInfo(socketPort, httpPort);
// After publication: publishDaemonInfo truncates daemon.log, so anything
// written before it is lost — including the prune's own diagnostic.
await pruneDeviceClaimsForDaemonStartup(logPath);
// Arms the initial idle-reap timer: a daemon that starts and never
// receives a request must still be able to reap itself.
idleReap.noteActivity();
Expand Down Expand Up @@ -448,6 +451,30 @@ export async function startDaemonRuntime(
};
}

async function pruneDeviceClaimsForDaemonStartup(logPath: string): Promise<void> {
// Startup runs outside any diagnostics scope, where emitDiagnostic is a no-op,
// so the prune has to open one of its own for its events to be recorded.
await withDiagnosticsScope(
{ command: 'daemon', session: 'daemon', logPath, debug: true },
async () => {
try {
const { pruned } = await pruneDeadDeviceClaims();
if (pruned > 0) {
emitDiagnostic({ phase: 'device_claim_prune', data: { pruned } });
flushDiagnosticsToSessionFile({ force: true });
}
} catch (error) {
emitDiagnostic({
level: 'warn',
phase: 'device_claim_prune_failed',
data: { error: error instanceof Error ? error.message : String(error) },
});
flushDiagnosticsToSessionFile({ force: true });
}
},
);
}

export async function cleanupWebBrowserOrphansForDaemonStartup(params: {
stateDir: string;
sessionStore: SessionStore;
Expand Down
Loading