test: make pid-liveness fixtures deterministic under load

Merged
TH
2mo ago
+91-3

Summary

Three tests fail intermittently with assertions (not timeouts) during full-suite runs while passing in isolation. All three share one root cause: their fixture represents a live owner via a real pid, and the production liveness check re-reads that owner's process start time (or command line) with a second, independent ps shell-out (utils/host-process.ts, PS_TIMEOUT_MS = 1000). Under full-suite CPU contention, that second ps call can miss its 1s deadline and return null, which mismatches the value captured earlier and flips a genuinely-live owner to owner-process-dead. This is exactly the "liveness can race under CPU load" shape the repo's timeouts-only flake taxonomy doesn't cover, since the failure surfaces as a wrong assertion, not a hang.

Per-test root cause, fix, and counterfactual

1. runner-session.test.ts — "runner session startup rejects live foreign runner lease"

Root cause: The fixture uses ownerPid: process.pid, ownerStartTime: RUNNER_OWNER_START_TIME (runner-lease.ts:123, classifyRunnerLease → isRunnerLeaseOwnerProcessAlive → classifyOwnerLiveness). RUNNER_OWNER_START_TIME is captured once via a real ps call at module import; classifyOwnerLiveness re-reads readProcessStartTime(pid) at classification time. Under load the second call can time out and return null, mismatching the cached value and flipping live → stale('owner-process-dead').

Fix: Added mockReadProcessStartTime to the file's existing vi.mock('.../host-process.ts', ...). It returns a fixed deterministic string for process.pid (used both at module-import time, so RUNNER_OWNER_START_TIME itself becomes deterministic, and during classification) and null for any other pid — removing the real subprocess call entirely while still requiring an exact pid match. isProcessAlive stays mocked as before (unchanged), so the existing dead-lease test (fabricated huge pid, short-circuits before readProcessStartTime is ever consulted) is untouched and still proves the dead-owner path.

Counterfactual: Inverted owner-identity.ts's if (!isProcessAlive(owner.pid)) to if (isProcessAlive(owner.pid)):

AssertionError: Missing expected rejection.
 ❯ src/platforms/apple/core/__tests__/runner-session.test.ts:873:5

Restored, test passes again.

2. cli-device-status.test.ts — "keeps normal status compact while retaining proven-stale claims for explicit inspection"

Root cause: This is a black-box CLI integration test (runCliCapture runs the real runCli in-process, no mocks). readCurrentOwnerIdentity() captures the test's own pid+startTime once via a real ps call to build a "live" claim fixture; device-claim-inspection.ts → classifyOwnerLiveness re-reads readProcessStartTime(pid) again when classifying claims for device status. Under load, the second real ps call can time out, misclassifying the live claim as stale — inflating the stale count from the expected 1 (the deliberately-fabricated dead claim) to 2.

Fix: Added a scoped vi.mock('../utils/host-process.ts', ...) that keeps everything real except readProcessStartTime, which now caches the first real read for our own pid and returns that same value on every subsequent call (self-consistent with the fixture regardless of whether that first ps call itself succeeds or times out — if it times out, both the fixture and the classifier consistently see null, which classifyOwnerLiveness treats as "skip the freshness check", not as a mismatch). isProcessAlive is untouched/real, so the fabricated-dead-pid (999_999_999) fixtures in this file still classify as stale through a real, non-flaky syscall check.

Counterfactual: Same inversion in owner-identity.ts:

AssertionError: The input did not match the regular expression /Live Pixel: live/. Input:
'No live local advisory device claims found.\n' +
  '2 stale claims hidden; inspect with: agent-device device status --stale\n'

Restored, test passes again.

3. daemon-client.test.ts — "cleanupFailedDaemonStartupMetadata retains live startup daemon on timeout" (third sighting, same mechanism, in scope)

Root cause: This test spawns a real child process to stand in for a daemon (needed because isAgentDeviceDaemonProcess also checks the command line against daemon patterns, so process.pid alone won't do). It reads the child's real start time/command once to build the fixture, then cleanupFailedDaemonStartupMetadata → isAgentDeviceDaemonProcess (daemon-process.ts:20) re-reads both via real ps calls (once per info-file check, once per lock-file check) to confirm liveness. Any of those re-reads can time out under load and return null/mismatch, flipping the live daemon to "not live" and failing retainedInfoProcess/retainedLockProcess.

Fix: Added a file-scoped vi.mock('../host-process.ts', ...) for readProcessStartTime/readProcessCommand that defaults to the real implementation for every pid in every other test in the file (pure passthrough, zero behavior change elsewhere), and only this one test configures a pinned override for its spawned pid — using the real values it already captured — clearing the override in finally.

Counterfactual: Inverted daemon-process.ts's identity check (if (actualStartTime === expectedStartTime) return false;):

AssertionError: Expected values to be strictly equal:
+ actual - expected
+ undefined
- true
 ❯ src/utils/__tests__/daemon-client.test.ts:489:12

Restored, test passes again.

Validation

  • pnpm typecheck && pnpm lint && pnpm format:check — all clean.
  • npx vitest run on all three touched files — 107/107 pass.
  • 10x loop of the three touched files — 107/107 pass every run, zero flakes.
  • Same three files run once while 4 node -e 'while(true){}' CPU-load processes ran concurrently in the same foreground command — 107/107 pass; load processes killed within the same command.

Generated by Claude Code