diff --git a/src/__tests__/cli-device-status.test.ts b/src/__tests__/cli-device-status.test.ts index abea55afd..188aae2dc 100644 --- a/src/__tests__/cli-device-status.test.ts +++ b/src/__tests__/cli-device-status.test.ts @@ -2,10 +2,38 @@ import assert from 'node:assert/strict'; import fs from 'node:fs'; import os from 'node:os'; import path from 'node:path'; -import { test } from 'vitest'; +import { test, vi } from 'vitest'; import { readCurrentOwnerIdentity } from '../utils/owner-identity.ts'; import { runCliCapture } from './cli-capture.ts'; +// readProcessStartTime shells out to `ps` with a 1s timeout (see +// utils/host-process.ts). This file's fixtures read our own start time once +// to build a "live" claim, then production re-reads it during classification +// to confirm identity; under full-suite CPU contention that second `ps` call +// can miss its deadline and return null, mismatching the cached value and +// flipping a genuinely-live claim to 'owner-process-dead'. Cache our own +// pid's start time after the first (real) read so every later read is +// self-consistent with the fixtures instead of racing a fresh `ps` call. +// isProcessAlive is left real (a plain `kill(pid, 0)` syscall, not a +// subprocess), so the fabricated-dead-pid fixtures below still classify as +// stale through that real, non-flaky check. +vi.mock('../utils/host-process.ts', async () => { + const actual = await vi.importActual( + '../utils/host-process.ts', + ); + let cachedOwnStartTime: string | null | undefined; + return { + ...actual, + readProcessStartTime: (pid: number) => { + if (pid !== process.pid) return null; + if (cachedOwnStartTime === undefined) { + cachedOwnStartTime = actual.readProcessStartTime(process.pid); + } + return cachedOwnStartTime; + }, + }; +}); + test('device status is daemonless and does not send a daemon request', async () => { const result = await runCliCapture(['device', 'status', '--json']); assert.equal(result.code, null); diff --git a/src/platforms/apple/core/__tests__/runner-session.test.ts b/src/platforms/apple/core/__tests__/runner-session.test.ts index a401e900c..c29d31541 100644 --- a/src/platforms/apple/core/__tests__/runner-session.test.ts +++ b/src/platforms/apple/core/__tests__/runner-session.test.ts @@ -24,6 +24,7 @@ const { mockIsProcessAlive, mockIsProcessGroupAlive, mockPrepareXctestrunWithEnv, + mockReadProcessStartTime, mockResolveExpectedRunnerCacheMetadata, mockResolveRunnerDerivedPath, mockRunAppleToolCommand, @@ -40,6 +41,12 @@ const { mockIsProcessAlive: vi.fn(), mockIsProcessGroupAlive: vi.fn(), mockPrepareXctestrunWithEnv: vi.fn(), + // Non-empty default: RUNNER_OWNER_START_TIME below is computed at module + // load (before beforeEach), and readProcessStartTime's real implementation + // shells out to `ps` with a 1s timeout that can miss under CPU contention, + // flipping a live owner to 'owner-process-dead'. Deterministic value, no + // shell-out; identity is still enforced by pid in beforeEach below. + mockReadProcessStartTime: vi.fn((_pid: number) => 'fixed-test-owner-start-time' as string | null), mockResolveExpectedRunnerCacheMetadata: vi.fn(), mockResolveRunnerDerivedPath: vi.fn(), mockRunAppleToolCommand: vi.fn(), @@ -68,6 +75,7 @@ vi.mock('../../../../utils/host-process.ts', async () => { ...actual, isProcessAlive: mockIsProcessAlive, isProcessGroupAlive: mockIsProcessGroupAlive, + readProcessStartTime: mockReadProcessStartTime, }; }); @@ -156,6 +164,12 @@ beforeEach(async () => { mockRunAppleToolCommand.mockResolvedValue({ exitCode: 0, stdout: '', stderr: '' }); mockIsProcessAlive.mockReturnValue(true); mockIsProcessGroupAlive.mockReturnValue(false); + // Our pid reads back its fixed start time; any other pid reads as + // not-found, same as a real `ps` miss. Dead-lease tests use fabricated + // pids already rejected by mockIsProcessAlive before this is consulted. + mockReadProcessStartTime.mockImplementation((pid: number) => + pid === process.pid ? RUNNER_OWNER_START_TIME : null, + ); mockWaitForRunner.mockResolvedValue(runnerResponse({ uptimeMs: 1 })); }); diff --git a/src/utils/__tests__/daemon-client.test.ts b/src/utils/__tests__/daemon-client.test.ts index 1594d31b7..3b67f90af 100644 --- a/src/utils/__tests__/daemon-client.test.ts +++ b/src/utils/__tests__/daemon-client.test.ts @@ -1,4 +1,4 @@ -import { test } from 'vitest'; +import { test, vi } from 'vitest'; import assert from 'node:assert/strict'; import http from 'node:http'; import net from 'node:net'; @@ -36,6 +36,36 @@ import { import { stopProcessForTakeover } from '../../daemon/daemon-process.ts'; import { findProjectRoot, readVersion } from '../version.ts'; +// readProcessStartTime/readProcessCommand shell out to `ps` with a 1s +// timeout (see host-process.ts). isAgentDeviceDaemonProcess re-reads both for +// every liveness check, so a spawned-daemon fixture that is proven live once +// (a real read, right after the process starts) can still be misclassified +// as dead later if a *subsequent* `ps` call happens to miss its deadline +// under full-suite CPU contention. mockReadProcessStartTime/mockReadProcessCommand +// default to `undefined`, which falls through to the real implementation for +// every pid in every test in this file; only the one test below that needs a +// stable answer for its spawned pid configures an override, and clears it +// afterward. +const { mockReadProcessStartTime, mockReadProcessCommand } = vi.hoisted(() => ({ + mockReadProcessStartTime: vi.fn<(pid: number) => string | null | undefined>(), + mockReadProcessCommand: vi.fn<(pid: number) => string | null | undefined>(), +})); + +vi.mock('../host-process.ts', async () => { + const actual = await vi.importActual('../host-process.ts'); + return { + ...actual, + readProcessStartTime: (pid: number) => { + const overridden = mockReadProcessStartTime(pid); + return overridden !== undefined ? overridden : actual.readProcessStartTime(pid); + }, + readProcessCommand: (pid: number) => { + const overridden = mockReadProcessCommand(pid); + return overridden !== undefined ? overridden : actual.readProcessCommand(pid); + }, + }; +}); + type MockHttpResponse = EventEmitter & { headers?: Record; statusCode?: number; @@ -413,11 +443,25 @@ test('cleanupFailedDaemonStartupMetadata retains live startup daemon on timeout' try { await new Promise((resolve) => setTimeout(resolve, 50)); + // Read the spawned daemon's real identity once (ground truth: it is + // genuinely alive, with this real start time and command line), then + // pin readProcessStartTime/readProcessCommand to keep returning these + // same proven-real values for this pid. isAgentDeviceDaemonProcess reads + // both again internally on every call inside cleanupFailedDaemonStartupMetadata; + // without pinning, a second real `ps` call could miss its 1s timeout + // under load and misclassify this genuinely-live daemon as dead. const processStartTime = readProcessStartTime(pid) ?? undefined; - if (readProcessCommand(pid) === null || processStartTime === undefined) { + const command = readProcessCommand(pid); + if (command === null || processStartTime === undefined) { t.skip('process command/start inspection is unavailable in this environment'); return; } + mockReadProcessStartTime.mockImplementation((queriedPid: number) => + queriedPid === pid ? processStartTime : undefined, + ); + mockReadProcessCommand.mockImplementation((queriedPid: number) => + queriedPid === pid ? command : undefined, + ); const paths = resolveDaemonPaths(stateDir); fs.mkdirSync(paths.baseDir, { recursive: true }); @@ -450,6 +494,8 @@ test('cleanupFailedDaemonStartupMetadata retains live startup daemon on timeout' assert.equal(fs.existsSync(paths.infoPath), true); assert.equal(fs.existsSync(paths.lockPath), true); } finally { + mockReadProcessStartTime.mockReset(); + mockReadProcessCommand.mockReset(); if (isProcessAlive(pid)) { process.kill(pid, 'SIGKILL'); await waitForProcessExit(pid, 1_500);