Skip to content
Open
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
Original file line number Diff line number Diff line change
Expand Up @@ -59,3 +59,11 @@ test('a missing entry in a completed process snapshot stays fail-closed without
assert.equal(mockIsProcessZombie.mock.calls.length, 0);
assert.equal(mockReadProcessStartTime.mock.calls.length, 0);
});

test('a pid outside the native range is unknown without a liveness probe', () => {
assert.equal(
classifyOwnerLiveness({ owner: { pid: 2_147_483_648, startTime: 'start-a' } }),
'unknown',
);
assert.equal(mockIsProcessAlive.mock.calls.length, 0);
Comment thread
thymikee marked this conversation as resolved.
});
11 changes: 10 additions & 1 deletion packages/host-kit/src/internal/owner-identity.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,11 @@ export type OwnerIdentity = {
startTime: string | null;
};

/** A process id accepted by Node's native signal API. */
export function isProcessPid(value: unknown): value is number {
return typeof value === 'number' && Number.isInteger(value) && value > 0 && value <= 0x7fff_ffff;
}

export type OwnerLiveness =
| 'live'
| 'owner-process-dead'
Expand Down Expand Up @@ -75,6 +80,7 @@ export function classifyOwnerLivenessFromObservation(
observation?: HostProcessIdentityObservation | null,
): OwnerLiveness {
const { owner, stateDir } = params;
if (!isProcessPid(owner.pid)) return 'unknown';
if (!isProcessAlive(owner.pid)) return 'owner-process-dead';
if (observation !== undefined ? observation?.state.startsWith('Z') : isProcessZombie(owner.pid)) {
return 'owner-process-dead';
Expand All @@ -88,7 +94,10 @@ export function classifyOwnerLivenessFromObservation(
return 'owner-process-reused';
}
}
if (!stateDir) return 'live';
return stateDir ? classifyOwnerStateDirectory(stateDir) : 'live';
}

function classifyOwnerStateDirectory(stateDir: string): OwnerLiveness {
try {
fs.statSync(stateDir);
return 'live';
Expand Down
3 changes: 2 additions & 1 deletion packages/host-kit/src/internal/process-lock.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import { publishFileSync } from './atomic-file.ts';
import { emitDiagnostic } from './diagnostics.ts';
import {
classifyOwnerLiveness,
isProcessPid,
ownerIdentityMatches,
type OwnerLiveness,
} from './owner-identity.ts';
Expand Down Expand Up @@ -439,7 +440,7 @@ function parseProcessLockOwner(contents: string): ProcessLockOwnerRecord | null

const PROCESS_LOCK_OWNER_FIELD_SHAPES: Record<keyof ProcessLockOwner, (value: unknown) => boolean> =
{
pid: (value) => typeof value === 'number' && Number.isInteger(value) && value > 0,
pid: isProcessPid,
acquiredAtMs: (value) => typeof value === 'number' && Number.isFinite(value),
startTime: (value) => value === undefined || value === null || typeof value === 'string',
};
Expand Down
1 change: 1 addition & 0 deletions packages/host-kit/src/process.ts
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ export {
export {
classifyOwnerLiveness,
classifyOwnerLivenessFromObservation,
isProcessPid,
type OwnerIdentity,
ownerIdentityDiffers,
ownerIdentityMatches,
Expand Down
10 changes: 10 additions & 0 deletions src/__tests__/daemon-exit-wait.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,16 @@ afterEach(() => {
vi.restoreAllMocks();
});

test.each([0, -1, 1.5, 2_147_483_648, Number.MAX_SAFE_INTEGER])(
'an invalid native pid %s cannot prove exit during recovery',
async (pid) => {
expect(await waitForDaemonExit({ pid, startTime: OURS }, { timeoutMs: 0 })).toEqual({
exited: false,
elapsedMs: 0,
});
},
);

test('waitForDaemonExit reports a pid recycled mid-wait as exited, without burning the deadline', async () => {
setTimeout(() => state.starts.set(PID, RECYCLED), 20);
const wait = await waitForDaemonExit(
Expand Down
13 changes: 13 additions & 0 deletions src/__tests__/daemon-process-takeover.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,19 @@ const TAKEOVER_TIMEOUTS = { termTimeoutMs: 5_000, killTimeoutMs: 2_000 };
const spawnedChildren: { child: ChildProcess; exited: Promise<void> }[] = [];
const spawnedRoots: string[] = [];

test.each([0, -1, 1.5, Number.NaN, '123', 2_147_483_648, Number.MAX_SAFE_INTEGER])(
'an invalid daemon pid %s cannot prove exit',
async (pid) => {
assert.deepEqual(
await stopDaemonProcess(
{ pid: pid as number, startTime: 'captured-birth' },
{ mode: 'force', termTimeoutMs: 0, killTimeoutMs: 0 },
),
{ status: 'retained', reason: 'identity-unverified' },
);
},
);

afterEach(async () => {
for (const { child, exited } of spawnedChildren.splice(0)) {
if (child.exitCode === null && child.signalCode === null) child.kill('SIGKILL');
Expand Down
3 changes: 2 additions & 1 deletion src/__tests__/test-utils/daemon-http-fixture.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ export type HttpDaemonFixture = {

export async function startHttpDaemonFixture(
responseData: Record<string, unknown>,
options: { ready?: () => boolean } = {},
): Promise<HttpDaemonFixture> {
const seenPaths: string[] = [];
const rpcRequests: Record<string, any>[] = [];
Expand All @@ -24,7 +25,7 @@ export async function startHttpDaemonFixture(
seenPaths.push(`${req.method ?? 'GET'} ${url.pathname}`);

if (req.method === 'GET' && url.pathname === '/health') {
res.writeHead(200);
res.writeHead(options.ready?.() === false ? 503 : 200);
res.end('ok');
return;
}
Expand Down
21 changes: 20 additions & 1 deletion src/__tests__/test-utils/registered-daemon-fixture.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,8 @@ import assert from 'node:assert/strict';
import fs from 'node:fs';
import path from 'node:path';
import { vi } from 'vitest';
import type { runCmdDetachedMonitored } from '@agent-device/host-kit/command';
import type { runCmdDetachedMonitored, ExecDetachedExit } from '@agent-device/host-kit/command';
import { readDaemonInfo, type DaemonInfo } from '../../daemon-client/daemon-client-metadata.ts';
import { readProcessStartTime } from '@agent-device/host-kit/process';
import { stopDaemonProcess } from '../../daemon-process.ts';
import type { DaemonPaths } from '../../daemon-resolution.ts';
Expand Down Expand Up @@ -76,6 +77,24 @@ export function spawnRegisteredDaemonFixture(
return child;
}

export async function waitForRegisteredDaemonFixture(
paths: DaemonPaths,
child: ReturnType<typeof runCmdDetachedMonitored>,
): Promise<DaemonInfo> {
let exit: ExecDetachedExit | undefined;
void child.exited.then((result) => {
exit = result;
});
for (let attempt = 0; attempt < 400; attempt += 1) {
if (exit)
throw new Error(`Registered child exited before publication: ${JSON.stringify(exit)}`);
const info = readDaemonInfo(paths.infoPath);
if (info?.pid === child.pid) return info;
Comment thread
thymikee marked this conversation as resolved.
await new Promise((resolve) => setTimeout(resolve, 10));
}
throw new Error(`Registered child ${child.pid} did not publish ${paths.infoPath} within 4s`);
}

export async function finishRegisteredDaemonFixture(stateDir: string): Promise<void> {
for (const owned of children.get(stateDir) ?? []) {
const child = owned.launch;
Expand Down
9 changes: 2 additions & 7 deletions src/daemon-client/__tests__/daemon-client-lifecycle.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import { afterEach, test, vi } from 'vitest';
import { mkdtempForTestSync } from '../../__tests__/test-utils/tmp-dir.ts';
import {
spawnRegisteredDaemonFixture,
waitForRegisteredDaemonFixture,
finishRegisteredDaemonFixture,
finishRegisteredDaemonFixtures,
} from '../../__tests__/test-utils/registered-daemon-fixture.ts';
Expand All @@ -27,7 +28,6 @@ import { resolveDaemonPaths, type DaemonPaths } from '../../daemon-resolution.ts
import { sendToDaemon, type DaemonRequest, type DaemonResponse } from '../daemon-client.ts';
import { attachActiveSessionAddressHint } from '../daemon-client-lifecycle.ts';
import { sendRequest } from '../daemon-client-transport.ts';
import { readDaemonInfo } from '../daemon-client-metadata.ts';
import type { DaemonRetirementResult } from '../../daemon-registration-owner.ts';
import {
closeLoopbackServer,
Expand Down Expand Up @@ -646,12 +646,7 @@ test('sendRequest timeout cleanup uses resolved daemon paths instead of request
};

try {
let info = readDaemonInfo(daemonPaths.infoPath);
for (let attempt = 0; !info && attempt < 200; attempt += 1) {
await actualRetry.sleep(10);
info = readDaemonInfo(daemonPaths.infoPath);
}
assert.ok(info);
const info = await waitForRegisteredDaemonFixture(daemonPaths, child);
let thrown: unknown;
try {
await sendRequest(info, request, 'http', daemonPaths, 50);
Expand Down
59 changes: 55 additions & 4 deletions src/daemon-client/__tests__/daemon-client-startup-race.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -203,6 +203,46 @@ for (const exit of [
});
}

test('a joined busy contender waits for a published winner to become ready without signaling it', async (t) => {
if (!(await supportsLoopbackBind())) return t.skip('loopback unavailable');
const paths = resolveDaemonPaths(mkdtempForTestSync('daemon-start-delayed-ready-'));
let probes = 0;
const http = await startHttpDaemonFixture({ devices: [] }, { ready: () => ++probes >= 12 });
const deferred = path.join(paths.baseDir, 'defer-publication');
fs.writeFileSync(deferred, 'wait');
const winner = spawnRegisteredDaemonFixture(paths, fields(http.port), { stdio: 'ignore' });
await awaitFile(path.join(paths.baseDir, 'registration-held'));
let joined = false;
spawn.mockImplementation((_command, _args, options) => {
const child = spawnRegisteredDaemonFixture(paths, fields(http.port), options);
void child.exited.then((exit) => {
assert.equal(exit.exitCode, DAEMON_STARTUP_EXIT_CODES.busy);
Comment thread
thymikee marked this conversation as resolved.
joined = true;
});
return child;
});
pause.mockImplementation(async () => {
if (joined) fs.rmSync(deferred, { force: true });
await actualRetry.sleep(10);
});
const signal = vi.spyOn(process, 'kill');
try {
assert.equal((await sendToDaemon(request(paths))).ok, true);
assert.equal(joined, true);
assert.ok(probes >= 12);
assert.equal(spawn.mock.calls.length, 1);
assert.equal(http.rpcRequests.length, 1);
assert.equal(isProcessAlive(winner.pid), true);
assert.equal(
signal.mock.calls.some(([pid, kind]) => pid === winner.pid && kind !== 0),
false,
);
} finally {
signal.mockRestore();
await closeLoopbackServer(http.server);
}
});

for (const held of [true, false]) {
test(`startup uses one deadline when the claim is ${held ? 'held' : 'released for relaunch'}`, async () => {
const paths = resolveDaemonPaths(mkdtempForTestSync('daemon-start-budget-'));
Expand All @@ -215,6 +255,7 @@ for (const held of [true, false]) {
let now = Date.now();
const started = now;
let released = false;
let advanced = false;
let finishPending: () => void = () => {};
const nativeTimeout = globalThis.setTimeout;
vi.spyOn(globalThis, 'setTimeout').mockImplementation((handler, ms, ...args) =>
Expand All @@ -231,9 +272,12 @@ for (const held of [true, false]) {
}),
}));
pause.mockImplementation(async (ms) => {
if (!held && !released) {
await claim.acquisition.release();
released = true;
if (!advanced) {
if (!held) {
await claim.acquisition.release();
released = true;
}
advanced = true;
now += 14_750;
} else now += ms;
});
Expand Down Expand Up @@ -291,7 +335,14 @@ test.for([
const notice = vi.spyOn(process.stderr, 'write').mockImplementation(() => true);
try {
const pending = sendToDaemon(request(paths));
if (expected.alive) await assert.rejects(pending);
if (expected.alive)
await assert.rejects(pending, (error: unknown) => {
assert.ok(error instanceof AppError);
assert.equal(error.details?.kind, 'daemon_startup_failed');
assert.equal(error.details?.startupAttempts, 1);
assert.equal(error.details?.startupTimeoutMs, 15_000);
return true;
});
else {
assert.equal((await pending).ok, true);
await winner.exited;
Expand Down
Loading
Loading