From 1416cf681ebc01978e26a9ed2171382f4b4117f2 Mon Sep 17 00:00:00 2001 From: Kris Zyp Date: Thu, 8 Oct 2026 17:32:28 -0600 Subject: [PATCH 1/4] fix(lifecycle): include setup in the startup deadline Count validation and setup after an address claim against startupMaxMs, refuse expired launches, and give boot only the remaining budget. Clean up newly acquired resources after failures while preserving reused resources and unconfirmed children. Co-Authored-By: GPT-6.1 Codex --- CONTRIBUTING.md | 2 + README.md | 12 +- src/harperLifecycle.ts | 205 +++++++++++-------- src/loopbackAddressPool.ts | 17 +- test/harperStartupDeadline.test.ts | 304 +++++++++++++++++++++++++++++ 5 files changed, 447 insertions(+), 93 deletions(-) create mode 100644 test/harperStartupDeadline.test.ts diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 5d22b90..94bbb36 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -29,6 +29,8 @@ Tests live in `test/` (separate from `src/`) and run on the built-in Node.js tes Internal-only helpers may be exported from their modules for testing (e.g. `runHarperCommand` in `harperLifecycle.ts`) but are deliberately **not** re-exported from `src/index.ts`, keeping them out of the public API. +The internal `reserveLoopbackAddress` returns the claim time recorded immediately before publishing the slot; the public `getNextAvailableLoopbackAddress` still returns a string. `launchHarper` passes that origin to `runHarperCommand`, whose absolute watchdog includes validation, log setup, script resolution and boot, but excludes pre-claim pool waits. It checks expiry immediately before spawn and when readiness is observed. Failed launches reap the captured child before releasing a newly claimed slot or removing an owned install; unconfirmed exit keeps those resources parked. Reused addresses and caller-owned directories survive failure. `test/harperStartupDeadline.test.ts` enforces these boundaries, while the existing lifecycle tests keep idle timing and post-readiness registration outside the ceiling. + `loopbackAddressPool.ts` reads its pool file location, address range, and canary ports from the environment once, when it is imported, so `test/loopbackAddressPool.test.ts` sets them before a dynamic import: a private pool file in its own temp directory, and `127.0.0.1`, the one address every OS has without aliases. Its foreign-listener cases need an OS that lets an exclusive bind of that address succeed beside a listener on all interfaces, as macOS and Windows do; where the bind conflicts instead (Linux), the conflict canary refuses the address first and those cases skip themselves. That is why CI includes a macOS job alongside Linux and Windows. ## Releases diff --git a/README.md b/README.md index 5bcd190..5e249e1 100644 --- a/README.md +++ b/README.md @@ -109,7 +109,7 @@ The Harper binary is resolved in the following order: ```ts interface StartHarperOptions { startupTimeoutMs?: number; // Idle timeout: max gap between startup output chunks. Default: 60000 (150000 under CI) or HARPER_INTEGRATION_TEST_STARTUP_TIMEOUT_MS - startupMaxMs?: number; // Absolute startup ceiling regardless of output. Default: 120000 (300000 under CI) or HARPER_INTEGRATION_TEST_STARTUP_MAX_MS + startupMaxMs?: number; // Ceiling from address reservation through readiness, including setup. Default: 120000 (300000 under CI) or HARPER_INTEGRATION_TEST_STARTUP_MAX_MS config?: object; // Harper config overrides (passed via HARPER_SET_CONFIG) env?: object; // Additional environment variables for the Harper process harperBinPath?: string; // Explicit path to dist/bin/harper.js @@ -118,12 +118,16 @@ interface StartHarperOptions { Startup readiness is detected by Harper printing `successfully started`. Rather than a single wall-clock deadline (which makes a slow-but-healthy boot indistinguishable from a hang), the watchdog uses an **idle timeout** that resets on every chunk of output — so the limit is time-since-last-progress — plus a generous absolute ceiling as a backstop. This is why a slow CI boot that keeps logging no longer trips the timeout. +`startupMaxMs` starts when the pool slot is claimed, before address validation and log setup. Waiting for a pool slot (including quarantine) and preparing an install or fixture before the claim do not consume it. Setup after reservation leaves only the remaining budget for boot; at or after the deadline, an unlaunched start is refused and late readiness is rejected with `HarperStartupError`. A restart using an existing hostname gets a fresh budget for that call. Readiness ends startup timing; monitor registration can finish afterward. + +> **Behavior change:** the absolute ceiling previously began after spawning Harper. Slow setup now consumes that ceiling. Failed starts release addresses and remove install directories acquired by that call once child exit is confirmed; caller-supplied directories and reused addresses are preserved. Captured stdout/stderr remain on `HarperStartupError`, and configured `HARPER_INTEGRATION_TEST_LOG_DIR` logs are kept, but logs inside a removed install directory are deleted. If exit cannot be confirmed, resources stay in place with a warning. The deadline uses the wall clock, so forward clock adjustments can expire startup; a suspended or blocked runner cannot enforce it until JavaScript resumes. + > **Behavior change:** `startupTimeoutMs` (and `HARPER_INTEGRATION_TEST_STARTUP_TIMEOUT_MS`) previously meant an absolute startup deadline. It now means the **idle / no-output window**. If you relied on it as a hard ceiling to fail slow boots quickly, set `startupMaxMs` (or `HARPER_INTEGRATION_TEST_STARTUP_MAX_MS`) instead. **Environment Variables:** - `HARPER_INTEGRATION_TEST_STARTUP_TIMEOUT_MS` - Idle startup timeout: max time between chunks of startup output before Harper is treated as hung (resets on output). Default `60000` (`150000` under CI). -- `HARPER_INTEGRATION_TEST_STARTUP_MAX_MS` - Absolute ceiling on total startup time, regardless of ongoing output. Default `120000` (`300000` under CI). +- `HARPER_INTEGRATION_TEST_STARTUP_MAX_MS` - Absolute ceiling from address reservation through readiness, including setup after reservation, regardless of ongoing output. Default `120000` (`300000` under CI). - `HARPER_INTEGRATION_TEST_INSTALL_PARENT_DIR` - Parent directory for temp Harper install dirs (default: OS tmpdir) - `HARPER_INTEGRATION_TEST_INSTALL_SCRIPT` - Path to Harper CLI script - `HARPER_INTEGRATION_TEST_ALLOW_FOREIGN_LISTENERS` - Set to `1` or `true` to start a node on an address whose operations or HTTP port another process already accepts connections on, or that could not be checked for one, with a warning instead of an error (see [Other services listening on Harper's ports](#other-services-listening-on-harpers-ports)). @@ -300,11 +304,11 @@ The pool is one file shared by every process on the machine: `${TMPDIR}/harper-i A pool file that is unparseable, not an array, or an empty array can still be produced by a writer that doesn't publish by rename (an older version of this package killed mid-write) or by an outside edit. Such a file is either about to be completed or has lost reservations whose holders may not have bound their address yet. Allocation does refuse an address that a Harper node is already listening on, but that check cannot see a reservation nothing is listening on yet. So the file is quarantined rather than reset: -- `getNextAvailableLoopbackAddress()` waits, and both release functions leave the file untouched, until it has gone 6 minutes without modification. That outlasts `startHarper`'s default startup ceiling (2 minutes, 5 under CI), so a `startHarper` that reserved an address before the file broke has bound it or given up by then. +- `getNextAvailableLoopbackAddress()` waits, and both release functions leave the file untouched, until it has gone 6 minutes without modification. That outlasts `startHarper`'s default reservation-to-readiness ceiling (2 minutes, 5 under CI), including setup after the claim. Expired starts are refused before spawn or killed during boot; this reduces the chance of reissuing their unbound address. - After that, the pool is reinitialized with every address free and a warning is logged. From then on, an address still in use is protected only by that listening check. - Deleting the file skips the wait, but only do that once every integration-test run on the machine — including ones using older versions of this package — has stopped. -The quarantine bounds, rather than eliminates, the risk of handing out an address twice: a caller that holds an address for longer than that without binding it (for example, with a `startupMaxMs` above 5 minutes) is not covered. +The quarantine bounds, rather than eliminates, the risk of handing out an address twice: longer custom ceilings, direct or older-version callers, reservations kept unbound between kill and restart, clock changes, and suspended or blocked runners can outlast it. The five-minute default leaves a minute for failure cleanup; the package cannot guarantee that cleanup completes during a host stall. ## Node.js Test Runner diff --git a/src/harperLifecycle.ts b/src/harperLifecycle.ts index 292473d..e9e6821 100644 --- a/src/harperLifecycle.ts +++ b/src/harperLifecycle.ts @@ -4,7 +4,7 @@ import { join, basename, dirname } from 'node:path'; import { tmpdir } from 'node:os'; import { mkdtemp, mkdir, rm, cp } from 'node:fs/promises'; import { type SuiteContext, type TestContext } from 'node:test'; -import { getNextAvailableLoopbackAddress, releaseLoopbackAddress } from './loopbackAddressPool.ts'; +import { reserveLoopbackAddress, releaseLoopbackAddress } from './loopbackAddressPool.ts'; import { waitForPortsFree } from './portUtils.ts'; import { ok, equal } from 'node:assert'; import { createRequire } from 'node:module'; @@ -71,7 +71,8 @@ export const DEFAULT_STARTUP_TIMEOUT_MS = parseInt(process.env.HARPER_INTEGRATION_TEST_STARTUP_TIMEOUT_MS || '', 10) || (IS_CI ? 150000 : 60000); /** - * Absolute ceiling on total startup time, regardless of ongoing output — a generous backstop + * Absolute ceiling from address reservation through readiness, including setup after reservation, + * regardless of ongoing output — a generous backstop * so a process that chatters forever without ever reporting ready still fails. Higher under CI, * where shared/contended runners boot more slowly. * @@ -126,7 +127,8 @@ export interface StartHarperOptions { */ startupTimeoutMs?: number; /** - * Absolute ceiling (ms) on total startup time, regardless of ongoing output. + * Absolute ceiling (ms) from address reservation through readiness, including setup after + * reservation. Pool waits before the claim are excluded; reused hostnames get a new per-call ceiling. * Falls back to {@link DEFAULT_STARTUP_MAX_MS} (120s locally, 300s under CI). */ startupMaxMs?: number; @@ -359,6 +361,8 @@ interface RunHarperCommandOptions { timeoutMs?: number; /** Absolute timeout (ms): ceiling on total time regardless of output. Falls back to DEFAULT_STARTUP_MAX_MS. */ maxMs?: number; + reservedAt?: number; + onSpawn?: (proc: ChildProcess) => void; /** Loopback address this instance is bound to; recorded with the instance monitor for diagnostics. */ hostname?: string; } @@ -389,8 +393,13 @@ export function runHarperCommand({ harperBinPath, timeoutMs, maxMs, + reservedAt = Date.now(), + onSpawn, hostname, }: RunHarperCommandOptions): Promise { + const maxTimeoutMs = maxMs ?? DEFAULT_STARTUP_MAX_MS; + const startupDeadline = reservedAt + maxTimeoutMs; + const maxTimeoutMessage = `Harper did not report ready within the maximum startup time of ${maxTimeoutMs}ms`; const harperScript = getHarperScript(harperBinPath); const runtime = HARPER_RUNTIME; const runtimeArgs = @@ -398,7 +407,7 @@ export function runHarperCommand({ ? [harperScript, ...args] : ['--trace-warnings', '--force-node-api-uncaught-exceptions-policy=true', harperScript, ...args]; const instanceId = nextInstanceId(); - const proc = spawn(runtime, runtimeArgs, { + const spawnOptions = { env: { ...process.env, ...env, ...buildInstanceEnv(instanceId) }, // On POSIX, run Harper as its own process-group leader so both teardown and the shared // instance monitor can signal the whole group (parent + any worker children), not just the @@ -406,7 +415,9 @@ export function runHarperCommand({ // stdio stays piped (not detached), and we never unref, so output capture and lifetime // management are unchanged. detached: process.platform !== 'win32', - }); + }; + if (Date.now() >= startupDeadline) throw new HarperStartupError(maxTimeoutMessage, '', ''); + const proc = spawn(runtime, runtimeArgs, spawnOptions); // Publishes the instance to the shared monitor, which reaps it if this runner dies without // running cleanup, and installs the runner-side handlers covering cooperative exits. @@ -420,7 +431,6 @@ export function runHarperCommand({ } const idleTimeoutMs = timeoutMs ?? DEFAULT_STARTUP_TIMEOUT_MS; - const maxTimeoutMs = maxMs ?? DEFAULT_STARTUP_MAX_MS; return new Promise((resolve, reject) => { let stdout = ''; @@ -446,6 +456,10 @@ export function runHarperCommand({ const succeed = () => { if (settled || readinessDetected) return; + if (Date.now() >= startupDeadline) { + failStartup(maxTimeoutMessage); + return; + } readinessDetected = true; // Left armed across registration, these would let registry-lock contention time out — and // kill — an instance that already booted successfully. @@ -477,8 +491,8 @@ export function runHarperCommand({ // Absolute backstop, regardless of ongoing output. maxTimer = setTimeout( - () => failStartup(`Harper did not report ready within the maximum startup time of ${maxTimeoutMs}ms`), - maxTimeoutMs + () => failStartup(maxTimeoutMessage), + Math.max(0, startupDeadline - Date.now()) ); resetIdleTimer(); @@ -516,6 +530,7 @@ export function runHarperCommand({ reject(error); }); proc.on('exit', (statusCode, signal) => { + if (!startupFinished() && Date.now() >= startupDeadline) failStartup(maxTimeoutMessage); if (!settled) { settled = true; clearTimers(); @@ -530,6 +545,7 @@ export function runHarperCommand({ stdoutStream?.end(); stderrStream?.end(); }); + onSpawn?.(proc); }); } @@ -598,8 +614,8 @@ export async function startHarper(ctx: HarperTestContext, options?: StartHarperO * @param ctx - The test context to populate with Harper instance details * @param options - Optional configuration for the setup process * @param ownsDataRootDir - Whether the install directory was created for this start. If allocating a - * loopback address throws, an owned directory is removed, and `ctx.harper` cleared if it names that - * directory, before the error propagates; a caller-supplied directory is left for the caller. + * loopback address or starting throws, an owned directory is removed once the child is gone, and + * `ctx.harper` cleared if it names that directory; a caller-supplied directory is left for the caller. * @returns The context with the `harper` property populated */ async function launchHarper( @@ -607,94 +623,115 @@ async function launchHarper( options: StartHarperOptions | undefined, ownsDataRootDir: boolean ): Promise { + let reservedAt = Date.now(); const dataRootDirPrefix = join( process.env.HARPER_INTEGRATION_TEST_INSTALL_PARENT_DIR || tmpdir(), `harper-integration-test-` ); const dataRootDir = ctx.harper?.dataRootDir ?? (await mkdtemp(dataRootDirPrefix)); - let loopbackAddress: string; + let loopbackAddress = ctx.harper?.hostname; + const ownsLoopbackAddress = loopbackAddress == null; + let spawnedProcess: ChildProcess | undefined; try { - loopbackAddress = ctx.harper?.hostname ?? (await getNextAvailableLoopbackAddress()); - } catch (error) { - if (ownsDataRootDir) { - await rm(dataRootDir, { recursive: true, force: true }).catch(() => {}); - if (ctx.harper?.dataRootDir === dataRootDir) ctx.harper = undefined; + if (loopbackAddress == null) { + ({ loopbackAddress, reservedAt } = await reserveLoopbackAddress()); } - throw error; - } - // Set up per-suite log directory when HARPER_INTEGRATION_TEST_LOG_DIR is configured - const logDirEnv = process.env.HARPER_INTEGRATION_TEST_LOG_DIR; - let logDir: string | undefined; - if (logDirEnv) { - const suiteName = sanitizeForFilesystem(ctx.name || 'unknown'); - const timestamp = new Date().toISOString().replace(/[:.]/g, '-'); - logDir = join(logDirEnv, `${suiteName}-${sanitizeForFilesystem(loopbackAddress)}-${timestamp}`); - await mkdir(logDir, { recursive: true }); - - // Output for the test runner (e.g. run.ts) to map this log dir to the current test file - process.stdout.write(`${LOG_DIR_MARKER_PREFIX} ${logDir}\n`); - } + // Set up per-suite log directory when HARPER_INTEGRATION_TEST_LOG_DIR is configured + const logDirEnv = process.env.HARPER_INTEGRATION_TEST_LOG_DIR; + let logDir: string | undefined; + if (logDirEnv) { + const suiteName = sanitizeForFilesystem(ctx.name || 'unknown'); + const timestamp = new Date().toISOString().replace(/[:.]/g, '-'); + logDir = join(logDirEnv, `${suiteName}-${sanitizeForFilesystem(loopbackAddress)}-${timestamp}`); + await mkdir(logDir, { recursive: true }); + + // Output for the test runner (e.g. run.ts) to map this log dir to the current test file + process.stdout.write(`${LOG_DIR_MARKER_PREFIX} ${logDir}\n`); + } - // Point Harper's log directory to the suite log dir so hdb.log is preserved for upload - const config = { ...options?.config }; - if (logDir) { - config.logging = { ...config.logging, root: logDir }; - } + // Point Harper's log directory to the suite log dir so hdb.log is preserved for upload + const config = { ...options?.config }; + if (logDir) { + config.logging = { ...config.logging, root: logDir }; + } - const args = [ - `--ROOTPATH=${dataRootDir}`, - `--AUTHENTICATION_AUTHORIZELOCAL=true`, - `--HDB_ADMIN_USERNAME=${DEFAULT_ADMIN_USERNAME}`, - `--HDB_ADMIN_PASSWORD=${DEFAULT_ADMIN_PASSWORD}`, - '--THREADS_COUNT=1', - '--THREADS_DEBUG=false', - `--NODE_HOSTNAME=${loopbackAddress}`, - `--HTTP_PORT=${loopbackAddress}:${HTTP_PORT}`, - `--OPERATIONSAPI_NETWORK_PORT=${loopbackAddress}:${OPERATIONS_API_PORT}`, - `--MQTT_NETWORK_PORT=${loopbackAddress}:${MQTT_PORT}`, - `--MQTT_NETWORK_SECUREPORT=${loopbackAddress}:${MQTTS_PORT}`, - '--LOGGING_LEVEL=debug', - '--LOGGING_STDSTREAMS=false', - ]; - - // Bind secure port if HTTPS is needed (mTLS or other TLS config present) - if (options?.config?.http?.mtls !== undefined || options?.config?.tls !== undefined) { - args.push(`--HTTP_SECUREPORT=${loopbackAddress}:${HTTPS_PORT}`); - } + const args = [ + `--ROOTPATH=${dataRootDir}`, + `--AUTHENTICATION_AUTHORIZELOCAL=true`, + `--HDB_ADMIN_USERNAME=${DEFAULT_ADMIN_USERNAME}`, + `--HDB_ADMIN_PASSWORD=${DEFAULT_ADMIN_PASSWORD}`, + '--THREADS_COUNT=1', + '--THREADS_DEBUG=false', + `--NODE_HOSTNAME=${loopbackAddress}`, + `--HTTP_PORT=${loopbackAddress}:${HTTP_PORT}`, + `--OPERATIONSAPI_NETWORK_PORT=${loopbackAddress}:${OPERATIONS_API_PORT}`, + `--MQTT_NETWORK_PORT=${loopbackAddress}:${MQTT_PORT}`, + `--MQTT_NETWORK_SECUREPORT=${loopbackAddress}:${MQTTS_PORT}`, + '--LOGGING_LEVEL=debug', + '--LOGGING_STDSTREAMS=false', + ]; + + // Bind secure port if HTTPS is needed (mTLS or other TLS config present) + if (options?.config?.http?.mtls !== undefined || options?.config?.tls !== undefined) { + args.push(`--HTTP_SECUREPORT=${loopbackAddress}:${HTTPS_PORT}`); + } - // HARPER_SET_CONFIG must be passed as an environment variable, not a CLI arg, because - // applyRuntimeEnvVarConfig reads from process.env.HARPER_SET_CONFIG. buildHarperChildEnv also isolates the - // child's HOME into dataRootDir so Harper's global boot pointer never lands in the developer's real home. - const harperEnv = buildHarperChildEnv(dataRootDir, config, options?.env); - - const result = await runHarperCommand({ - args, - env: harperEnv, - completionMessage: 'successfully started', - logDir, - harperBinPath: options?.harperBinPath, - timeoutMs: options?.startupTimeoutMs, - maxMs: options?.startupMaxMs, - hostname: loopbackAddress, - }); + // HARPER_SET_CONFIG must be passed as an environment variable, not a CLI arg, because + // applyRuntimeEnvVarConfig reads from process.env.HARPER_SET_CONFIG. buildHarperChildEnv also isolates the + // child's HOME into dataRootDir so Harper's global boot pointer never lands in the developer's real home. + const harperEnv = buildHarperChildEnv(dataRootDir, config, options?.env); + + const result = await runHarperCommand({ + args, + env: harperEnv, + completionMessage: 'successfully started', + logDir, + harperBinPath: options?.harperBinPath, + timeoutMs: options?.startupTimeoutMs, + maxMs: options?.startupMaxMs, + reservedAt, + onSpawn: (proc) => { spawnedProcess = proc; }, + hostname: loopbackAddress, + }); - publishHarperNode(ctx, { - dataRootDir, - admin: { - username: DEFAULT_ADMIN_USERNAME, - password: DEFAULT_ADMIN_PASSWORD, - }, - httpURL: `http://${loopbackAddress}:${HTTP_PORT}`, - operationsAPIURL: `http://${loopbackAddress}:${OPERATIONS_API_PORT}`, - hostname: loopbackAddress, - process: result.process, - logDir, - startupOutput: { stdout: result.stdout, stderr: result.stderr }, - }); + publishHarperNode(ctx, { + dataRootDir, + admin: { + username: DEFAULT_ADMIN_USERNAME, + password: DEFAULT_ADMIN_PASSWORD, + }, + httpURL: `http://${loopbackAddress}:${HTTP_PORT}`, + operationsAPIURL: `http://${loopbackAddress}:${OPERATIONS_API_PORT}`, + hostname: loopbackAddress, + process: result.process, + logDir, + startupOutput: { stdout: result.stdout, stderr: result.stderr }, + }); - return ctx as StartedHarperTestContext; + return ctx as StartedHarperTestContext; + } catch (error) { + if (spawnedProcess?.pid !== undefined) { + await killHarper({ harper: { process: spawnedProcess } } as StartedHarperTestContext, { graceMs: 0 }); + if (spawnedProcess.exitCode === null && spawnedProcess.signalCode === null) { + console.warn(`[integration-testing] Could not confirm startup process exit; keeping ${loopbackAddress} and ${dataRootDir}`); + throw error; + } + } + if (ownsLoopbackAddress && loopbackAddress) { + await releaseLoopbackAddress(loopbackAddress).catch((cleanupError) => { + console.warn(`[integration-testing] Could not release ${loopbackAddress} after startup failed: ${cleanupError}`); + }); + } + if (ownsDataRootDir) { + await rm(dataRootDir, { recursive: true, force: true, maxRetries: 10 }).catch((cleanupError) => { + console.warn(`[integration-testing] Could not remove ${dataRootDir} after startup failed: ${cleanupError}`); + }); + if (ctx.harper?.dataRootDir === dataRootDir) ctx.harper = undefined; + } + throw error; + } } function signalWindowsProcessTree(pid: number, signal: 'SIGTERM' | 'SIGKILL'): void { diff --git a/src/loopbackAddressPool.ts b/src/loopbackAddressPool.ts index b7c107e..56926d2 100644 --- a/src/loopbackAddressPool.ts +++ b/src/loopbackAddressPool.ts @@ -32,7 +32,7 @@ const RETRY_DELAY_MS = 1000; // How long an unusable pool file must go unmodified before it is reinitialized. The reservations it // lost may not be bound yet, which the conflict canary can't see, so this must outlast startHarper's -// reserve-to-bind time: its startup ceiling (DEFAULT_STARTUP_MAX_MS) plus the setup before it. +// reserve-to-readiness ceiling (DEFAULT_STARTUP_MAX_MS, including setup after reservation). const UNUSABLE_POOL_QUARANTINE_MS = 360000; // Port used as a conflict canary when allocating an address. This MUST be a port that @@ -467,6 +467,10 @@ export async function validateLoopbackAddressPool(): Promise<{ * ports and `HARPER_INTEGRATION_TEST_ALLOW_FOREIGN_LISTENERS` is not set (the slot is released) */ export async function getNextAvailableLoopbackAddress(): Promise { + return (await reserveLoopbackAddress()).loopbackAddress; +} + +export async function reserveLoopbackAddress(): Promise<{ loopbackAddress: string; reservedAt: number }> { // Each index maps to a different loopback address (index 0 -> 127.0.0.2, index 1 -> 127.0.0.3, etc.) // So if the first test process number is 42, it would be assigned to index 0 associated with address 127.0.0.2 // [42, null, null, ...]; @@ -490,6 +494,7 @@ export async function getNextAvailableLoopbackAddress(): Promise { // the lingering node has had time to exit. const triedIndices = new Set(); while (true) { + let reservedAt = 0; const assignedIndex = await withLock(async () => { // Read the pool file const loopbackPool = await readPoolFile(); @@ -512,6 +517,7 @@ export async function getNextAvailableLoopbackAddress(): Promise { loopbackPool[index] = process.pid; } // Write the updated pool back to the file + if (index !== null) reservedAt = Date.now(); await writePoolFile(loopbackPool); return index; @@ -520,6 +526,7 @@ export async function getNextAvailableLoopbackAddress(): Promise { // If we got an index, validate and return the address if (assignedIndex !== null) { const loopbackAddress = `127.0.0.${assignedIndex + HARPER_LOOPBACK_POOL_START}`; + const reservation = { loopbackAddress, reservedAt }; try { await validateLoopbackAddress(loopbackAddress); } catch (error) { @@ -544,7 +551,7 @@ export async function getNextAvailableLoopbackAddress(): Promise { if (conflictingPort === null) { // macOS and Windows let the canary's exclusive bind succeed beside a listener on all interfaces; Linux refuses it. if (process.platform === 'linux') { - return loopbackAddress; + return reservation; } let shadowedPort: number | null; try { @@ -555,7 +562,7 @@ export async function getNextAvailableLoopbackAddress(): Promise { console.warn( `[loopback-pool] Could not check ${loopbackAddress} for another process's listener (${reason}); continuing because ${ALLOW_FOREIGN_LISTENERS_ENV} is set.` ); - return loopbackAddress; + return reservation; } await releaseRefusedAddress(loopbackAddress); throw new LoopbackAddressValidationError( @@ -565,13 +572,13 @@ export async function getNextAvailableLoopbackAddress(): Promise { ); } if (shadowedPort === null) { - return loopbackAddress; + return reservation; } if (foreignListenersAllowed()) { console.warn( `[loopback-pool] Another process accepts connections on ${loopbackAddress}:${shadowedPort}; continuing because ${ALLOW_FOREIGN_LISTENERS_ENV} is set. This node's clients reach that process whenever the node has no listener of its own there.` ); - return loopbackAddress; + return reservation; } await releaseRefusedAddress(loopbackAddress); throw new ForeignListenerError(loopbackAddress, shadowedPort); diff --git a/test/harperStartupDeadline.test.ts b/test/harperStartupDeadline.test.ts new file mode 100644 index 0000000..3c322c1 --- /dev/null +++ b/test/harperStartupDeadline.test.ts @@ -0,0 +1,304 @@ +import { after, afterEach, beforeEach, mock, test } from 'node:test'; +import { deepStrictEqual, match, ok, rejects, strictEqual } from 'node:assert'; +import * as fsPromises from 'node:fs/promises'; +import * as childProcess from 'node:child_process'; +import * as timersPromises from 'node:timers/promises'; +import { createServer, type AddressInfo } from 'node:net'; +import { EventEmitter } from 'node:events'; +import { PassThrough } from 'node:stream'; +import { tmpdir } from 'node:os'; +import { join } from 'node:path'; +import type { HarperTestContext, StartedHarperTestContext } from '../src/harperLifecycle.ts'; + +const root = await fsPromises.mkdtemp(join(tmpdir(), 'harper-startup-deadline-')); +const poolPath = join(root, 'harper-integration-test-loopback-pool.json'); +const logRoot = join(root, 'logs'); +const readyScript = join(root, 'ready.cjs'); +const chattyScript = join(root, 'chatty.cjs'); +await fsPromises.writeFile(readyScript, "process.stdout.write('successfully started\\n'); setInterval(() => {}, 1000);\n"); +await fsPromises.writeFile(chattyScript, "setInterval(() => process.stdout.write('booting\\n'), 10);\n"); + +async function freePort(): Promise { + const server = createServer(); + await new Promise((resolve, reject) => { + server.once('error', reject); + server.listen(0, '127.0.0.1', resolve); + }); + const port = (server.address() as AddressInfo).port; + await new Promise((resolve) => server.close(() => resolve())); + return port; +} + +const overrides = { + TMPDIR: root, + TMP: root, + TEMP: root, + HARPER_RUNTIME: 'node', + HARPER_INTEGRATION_TEST_MONITOR: 'off', + HARPER_INTEGRATION_TEST_INSTALL_PARENT_DIR: root, + HARPER_INTEGRATION_TEST_LOG_DIR: logRoot, + HARPER_INTEGRATION_TEST_LOOPBACK_POOL_START: '1', + HARPER_INTEGRATION_TEST_LOOPBACK_POOL_COUNT: '1', + HARPER_INTEGRATION_TEST_CONFLICT_PROBE_PORT: String(await freePort()), + HARPER_INTEGRATION_TEST_HTTP_CONFLICT_PROBE_PORT: String(await freePort()), +}; +const savedEnv = Object.fromEntries(Object.keys(overrides).map((key) => [key, process.env[key]])); +Object.assign(process.env, overrides); + +let clock: number; +let setupElapsedMs = 0; +let publishElapsedMs = 0; +let poolWaitElapsedMs = 0; +let readinessElapsedMs = 0; +let setupError: Error | undefined; +let corruptDuringSetup = false; +let unconfirmedChild: childProcess.ChildProcess | undefined; +const children: childProcess.ChildProcess[] = []; +let restoreClock: () => void; + +mock.module('node:fs/promises', { + namedExports: { + ...fsPromises, + mkdir: async (...args: Parameters) => { + clock += setupElapsedMs; + if (corruptDuringSetup) await fsPromises.writeFile(poolPath, '[torn'); + if (setupError) throw setupError; + return fsPromises.mkdir(...args); + }, + rename: async (...args: Parameters) => { + await fsPromises.rename(...args); + if (args[1] === poolPath) { + clock += publishElapsedMs; + publishElapsedMs = 0; + } + }, + }, +}); +mock.module('node:timers/promises', { + namedExports: { + ...timersPromises, + setTimeout: async (...args: Parameters) => { + if (poolWaitElapsedMs) { + clock += poolWaitElapsedMs; + poolWaitElapsedMs = 0; + await fsPromises.writeFile(poolPath, '[null]'); + return; + } + return timersPromises.setTimeout(...args); + }, + }, +}); +mock.module('node:child_process', { + namedExports: { + ...childProcess, + spawn: (...args: Parameters) => { + if (unconfirmedChild && args[0] !== 'taskkill') return unconfirmedChild; + const child = childProcess.spawn(...args); + if (args[0] !== 'taskkill') { + children.push(child); + child.stdout?.on('data', (chunk: Buffer) => { + if (chunk.toString().includes('successfully started')) clock += readinessElapsedMs; + }); + } + return child; + }, + }, +}); + +const { startHarper, setupHarperWithFixture, killHarper, teardownHarper, HarperStartupError, DEFAULT_STARTUP_MAX_MS } = + await import('../src/harperLifecycle.ts'); +const { readPoolFile } = await import('../src/loopbackAddressPool.ts'); + +beforeEach(async () => { + clock = Date.now(); + const clockMock = mock.method(Date, 'now', () => clock); + restoreClock = () => clockMock.mock.restore(); + setupElapsedMs = publishElapsedMs = poolWaitElapsedMs = readinessElapsedMs = 0; + setupError = undefined; + corruptDuringSetup = false; + unconfirmedChild = undefined; + children.length = 0; + await fsPromises.writeFile(poolPath, '[null]'); +}); + +afterEach(async () => { + try { + for (const child of children) { + await killHarper({ harper: { process: child } } as StartedHarperTestContext, { graceMs: 0 }); + } + } finally { + restoreClock(); + } +}); + +after(async () => { + for (const [key, value] of Object.entries(savedEnv)) { + if (value === undefined) delete process.env[key]; + else process.env[key] = value; + } + await fsPromises.rm(root, { recursive: true, force: true }); +}); + +async function assertCleanFailure(ctx: HarperTestContext): Promise { + deepStrictEqual(JSON.parse(await fsPromises.readFile(poolPath, 'utf-8')), [null], 'the claimed slot must be released'); + strictEqual(ctx.harper, undefined, 'an owned fixture install must be unpublished'); + deepStrictEqual((await fsPromises.readdir(root)).filter((name) => name.startsWith('harper-integration-test-') && !name.endsWith('.json')), [], 'owned installs must be removed'); +} + +function startupError(maxMs: number): (error: Error) => boolean { + return (error) => { + ok(error instanceof HarperStartupError); + match(error.message, new RegExp(`maximum startup time of ${maxMs}ms`)); + return true; + }; +} + +for (const maxMs of [0, 1000, DEFAULT_STARTUP_MAX_MS]) { + test(`setup at the ${maxMs}ms ceiling refuses spawn and cleans up`, async () => { + setupElapsedMs = maxMs; + const ctx: HarperTestContext = {}; + await rejects(startHarper(ctx, { + harperBinPath: readyScript, + startupMaxMs: maxMs === DEFAULT_STARTUP_MAX_MS ? undefined : maxMs, + }), startupError(maxMs)); + strictEqual(children.length, 0, 'an expired start must never spawn'); + await assertCleanFailure(ctx); + }); +} + +test('fixture startup expiry removes and unpublishes its owned install', async () => { + const fixture = join(root, 'component'); + await fsPromises.mkdir(fixture); + await fsPromises.writeFile(join(fixture, 'config.yaml'), 'name: fixture'); + setupElapsedMs = 1000; + const ctx: HarperTestContext = {}; + await rejects(setupHarperWithFixture(ctx, fixture, { startupMaxMs: 1000, harperBinPath: readyScript }), startupError(1000)); + strictEqual(children.length, 0); + await assertCleanFailure(ctx); +}); + +test('pool waits before the claim do not consume the startup ceiling', async () => { + await fsPromises.writeFile(poolPath, `[${process.pid}]`); + poolWaitElapsedMs = 10000; + const ctx: HarperTestContext = {}; + try { + await startHarper(ctx, { startupMaxMs: 1000, harperBinPath: readyScript }); + strictEqual(ctx.harper?.hostname, '127.0.0.1'); + match(ctx.harper?.startupOutput?.stdout || '', /successfully started/); + } finally { + await teardownHarper(ctx as StartedHarperTestContext); + } +}); + +test('time spent publishing and validating an already claimed slot consumes the ceiling', async () => { + publishElapsedMs = 1000; + const ctx: HarperTestContext = {}; + await rejects(startHarper(ctx, { startupMaxMs: 1000, harperBinPath: readyScript }), startupError(1000)); + strictEqual(children.length, 0); + await assertCleanFailure(ctx); +}); + +test('boot receives only the budget left after setup, and failure reaps before cleanup', async (t) => { + setupElapsedMs = 900; + const setTimer = t.mock.method(globalThis, 'setTimeout'); + const ctx: HarperTestContext = {}; + await rejects(startHarper(ctx, { startupMaxMs: 1000, startupTimeoutMs: 3000, harperBinPath: chattyScript }), startupError(1000)); + strictEqual(setTimer.mock.calls[0].arguments[1], 100, 'only the remaining 100ms is available for boot'); + strictEqual(children.length, 1); + ok(children[0].exitCode !== null || children[0].signalCode !== null, 'child exit must precede resource cleanup'); + await assertCleanFailure(ctx); +}); + +test('readiness observed at the deadline is rejected even before the timer fires', async () => { + readinessElapsedMs = 1000; + const ctx: HarperTestContext = {}; + await rejects(startHarper(ctx, { startupMaxMs: 1000, harperBinPath: readyScript }), startupError(1000)); + ok(children[0].exitCode !== null || children[0].signalCode !== null); + await assertCleanFailure(ctx); +}); + +test('script resolution cannot admit a spawn after the deadline', async (t) => { + const script = join(root, 'resolution-ready.cjs'); + await fsPromises.copyFile(readyScript, script); + t.mock.method(console, 'log', () => { clock += 1000; }); + const ctx: HarperTestContext = {}; + await rejects(startHarper(ctx, { startupMaxMs: 1000, harperBinPath: script }), startupError(1000)); + strictEqual(children.length, 0); + await assertCleanFailure(ctx); +}); + +test('a setup error releases the new reservation and removes its install', async () => { + setupError = new Error('log mkdir failed'); + const ctx: HarperTestContext = {}; + await rejects(startHarper(ctx, { harperBinPath: readyScript }), (error) => error === setupError); + await assertCleanFailure(ctx); +}); + +test('failed spawn cleans up without waiting for an exit event that never fires', async () => { + const script = join(root, 'spawn-error.cjs'); + await fsPromises.copyFile(readyScript, script); + await rejects(startHarper({}, { harperBinPath: script, env: { PATH: join(root, 'missing-bin') } }), { code: 'ENOENT' }); + strictEqual(children[0].pid, undefined); + await assertCleanFailure({}); +}); + +test('failure preserves a caller directory while releasing a newly claimed address', async () => { + const dataRootDir = await fsPromises.mkdtemp(join(root, 'caller-install-')); + const ctx: HarperTestContext = { harper: { dataRootDir } }; + setupElapsedMs = 1000; + await rejects(startHarper(ctx, { startupMaxMs: 1000, harperBinPath: readyScript }), startupError(1000)); + deepStrictEqual(ctx.harper, { dataRootDir }); + ok((await fsPromises.stat(dataRootDir)).isDirectory()); + deepStrictEqual(JSON.parse(await fsPromises.readFile(poolPath, 'utf-8')), [null]); +}); + +test('a reused hostname has a fresh per-call budget and survives a failed restart', async () => { + const dataRootDir = await fsPromises.mkdtemp(join(root, 'restart-install-')); + const original = { dataRootDir, hostname: '127.0.0.1' }; + const ctx: HarperTestContext = { harper: original }; + await fsPromises.writeFile(poolPath, `[${process.pid}]`); + setupElapsedMs = 1000; + await rejects(startHarper(ctx, { startupMaxMs: 1000, harperBinPath: readyScript }), startupError(1000)); + strictEqual(ctx.harper, original); + deepStrictEqual(JSON.parse(await fsPromises.readFile(poolPath, 'utf-8')), [process.pid]); + setupElapsedMs = 500; + try { + const started = await startHarper(ctx, { startupMaxMs: 1000, harperBinPath: readyScript }); + match(started.harper.startupOutput?.stdout || '', /successfully started/); + } finally { + await teardownHarper(ctx as StartedHarperTestContext); + } +}); + +test('an expired start leaves a pool corrupted after its claim quarantined and unchanged', async () => { + setupElapsedMs = 1000; + corruptDuringSetup = true; + await rejects(startHarper({}, { startupMaxMs: 1000, harperBinPath: readyScript }), startupError(1000)); + strictEqual(children.length, 0); + strictEqual(await fsPromises.readFile(poolPath, 'utf-8'), '[torn'); + strictEqual(await readPoolFile(), null, 'a lost claim must remain quarantined'); +}); + +test('an unconfirmed child exit keeps its reservation and install parked', async (t) => { + const child = Object.assign(new EventEmitter(), { + pid: 2147483647, + exitCode: null, + signalCode: null, + stdout: new PassThrough(), + stderr: new PassThrough(), + kill: () => true, + }) as unknown as childProcess.ChildProcess; + unconfirmedChild = child; + const warn = t.mock.method(console, 'warn', () => {}); + try { + await rejects(startHarper({}, { startupMaxMs: 10, harperBinPath: readyScript }), startupError(10)); + deepStrictEqual(JSON.parse(await fsPromises.readFile(poolPath, 'utf-8')), [process.pid]); + ok((await fsPromises.readdir(root)).some((name) => name.startsWith('harper-integration-test-') && !name.endsWith('.json'))); + ok(warn.mock.calls.some((call) => String(call.arguments[0]).includes('Could not confirm startup process exit'))); + } finally { + child.emit('exit', null, 'SIGKILL'); + for (const name of await fsPromises.readdir(root)) { + if (name.startsWith('harper-integration-test-') && !name.endsWith('.json')) await fsPromises.rm(join(root, name), { recursive: true, force: true }); + } + } +}); From 64800f4e5015e43c7739af82cbab10e29dd81767 Mon Sep 17 00:00:00 2001 From: Kris Zyp Date: Thu, 8 Oct 2026 17:59:53 -0600 Subject: [PATCH 2/4] fix(lifecycle): protect reservations during failed-start cleanup Reap failed process groups and verify ports before cleanup. Keep expired claims untouched after quarantine can replace them, preserve exit diagnostics, and isolate the startup tests from occupied host ports. Co-Authored-By: GPT-6.1 Codex --- CONTRIBUTING.md | 2 +- README.md | 4 +- src/harperLifecycle.ts | 15 +++- src/loopbackAddressPool.ts | 10 ++- test/harperStartupDeadline.test.ts | 119 +++++++++++++++++++++++++++-- 5 files changed, 137 insertions(+), 13 deletions(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 94bbb36..936a9bd 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -29,7 +29,7 @@ Tests live in `test/` (separate from `src/`) and run on the built-in Node.js tes Internal-only helpers may be exported from their modules for testing (e.g. `runHarperCommand` in `harperLifecycle.ts`) but are deliberately **not** re-exported from `src/index.ts`, keeping them out of the public API. -The internal `reserveLoopbackAddress` returns the claim time recorded immediately before publishing the slot; the public `getNextAvailableLoopbackAddress` still returns a string. `launchHarper` passes that origin to `runHarperCommand`, whose absolute watchdog includes validation, log setup, script resolution and boot, but excludes pre-claim pool waits. It checks expiry immediately before spawn and when readiness is observed. Failed launches reap the captured child before releasing a newly claimed slot or removing an owned install; unconfirmed exit keeps those resources parked. Reused addresses and caller-owned directories survive failure. `test/harperStartupDeadline.test.ts` enforces these boundaries, while the existing lifecycle tests keep idle timing and post-readiness registration outside the ceiling. +The internal `reserveLoopbackAddress` returns the claim time recorded immediately before publishing the slot; the public `getNextAvailableLoopbackAddress` still returns a string. Startup's absolute ceiling includes everything after the claim through observed readiness, excluding pre-claim pool waits and post-readiness registration. Failure cleanup keeps resources parked unless the captured child has exited and Harper's ports are free; reused addresses and caller-owned directories survive failure. Automatic failure release also checks claim age inside the pool lock: a claim at least as old as the quarantine may have been replaced by another start with the same PID, so `releaseLoopbackReservation` leaves it untouched. `test/harperStartupDeadline.test.ts` enforces these boundaries. `loopbackAddressPool.ts` reads its pool file location, address range, and canary ports from the environment once, when it is imported, so `test/loopbackAddressPool.test.ts` sets them before a dynamic import: a private pool file in its own temp directory, and `127.0.0.1`, the one address every OS has without aliases. Its foreign-listener cases need an OS that lets an exclusive bind of that address succeed beside a listener on all interfaces, as macOS and Windows do; where the bind conflicts instead (Linux), the conflict canary refuses the address first and those cases skip themselves. That is why CI includes a macOS job alongside Linux and Windows. diff --git a/README.md b/README.md index 5e249e1..812383f 100644 --- a/README.md +++ b/README.md @@ -120,7 +120,7 @@ Startup readiness is detected by Harper printing `successfully started`. Rather `startupMaxMs` starts when the pool slot is claimed, before address validation and log setup. Waiting for a pool slot (including quarantine) and preparing an install or fixture before the claim do not consume it. Setup after reservation leaves only the remaining budget for boot; at or after the deadline, an unlaunched start is refused and late readiness is rejected with `HarperStartupError`. A restart using an existing hostname gets a fresh budget for that call. Readiness ends startup timing; monitor registration can finish afterward. -> **Behavior change:** the absolute ceiling previously began after spawning Harper. Slow setup now consumes that ceiling. Failed starts release addresses and remove install directories acquired by that call once child exit is confirmed; caller-supplied directories and reused addresses are preserved. Captured stdout/stderr remain on `HarperStartupError`, and configured `HARPER_INTEGRATION_TEST_LOG_DIR` logs are kept, but logs inside a removed install directory are deleted. If exit cannot be confirmed, resources stay in place with a warning. The deadline uses the wall clock, so forward clock adjustments can expire startup; a suspended or blocked runner cannot enforce it until JavaScript resumes. +> **Behavior change:** the absolute ceiling previously began after spawning Harper. Slow setup now consumes that ceiling. After allocation succeeds, failed starts normally release addresses and remove install directories acquired by that call once child exit is confirmed; caller-supplied directories and reused addresses are preserved. Captured stdout/stderr remain on `HarperStartupError`, and configured `HARPER_INTEGRATION_TEST_LOG_DIR` logs are kept, but logs inside a removed install directory are deleted. If exit or release of Harper's ports cannot be confirmed, resources stay in place with a warning. Automatic failure cleanup also leaves a claim at least 6 minutes old untouched: recovery may already have reissued that address to a newer start under the same PID. An unreleased slot can stay reserved until its runner exits. The deadline uses the wall clock, so forward clock adjustments can expire startup; a suspended or blocked runner cannot enforce it until JavaScript resumes. > **Behavior change:** `startupTimeoutMs` (and `HARPER_INTEGRATION_TEST_STARTUP_TIMEOUT_MS`) previously meant an absolute startup deadline. It now means the **idle / no-output window**. If you relied on it as a hard ceiling to fail slow boots quickly, set `startupMaxMs` (or `HARPER_INTEGRATION_TEST_STARTUP_MAX_MS`) instead. @@ -308,7 +308,7 @@ A pool file that is unparseable, not an array, or an empty array can still be pr - After that, the pool is reinitialized with every address free and a warning is logged. From then on, an address still in use is protected only by that listening check. - Deleting the file skips the wait, but only do that once every integration-test run on the machine — including ones using older versions of this package — has stopped. -The quarantine bounds, rather than eliminates, the risk of handing out an address twice: longer custom ceilings, direct or older-version callers, reservations kept unbound between kill and restart, clock changes, and suspended or blocked runners can outlast it. The five-minute default leaves a minute for failure cleanup; the package cannot guarantee that cleanup completes during a host stall. +The quarantine bounds, rather than eliminates, the risk of handing out an address twice: longer custom ceilings, direct or older-version callers, reservations kept unbound between kill and restart, clock changes, and suspended or blocked runners can outlast it. The default ceiling (2 minutes, 5 under CI) leaves at least a minute for failure cleanup; the package cannot guarantee that cleanup completes during a host stall. ## Node.js Test Runner diff --git a/src/harperLifecycle.ts b/src/harperLifecycle.ts index e9e6821..800531a 100644 --- a/src/harperLifecycle.ts +++ b/src/harperLifecycle.ts @@ -4,7 +4,7 @@ import { join, basename, dirname } from 'node:path'; import { tmpdir } from 'node:os'; import { mkdtemp, mkdir, rm, cp } from 'node:fs/promises'; import { type SuiteContext, type TestContext } from 'node:test'; -import { reserveLoopbackAddress, releaseLoopbackAddress } from './loopbackAddressPool.ts'; +import { reserveLoopbackAddress, releaseLoopbackAddress, releaseLoopbackReservation } from './loopbackAddressPool.ts'; import { waitForPortsFree } from './portUtils.ts'; import { ok, equal } from 'node:assert'; import { createRequire } from 'node:module'; @@ -359,9 +359,11 @@ interface RunHarperCommandOptions { harperBinPath?: string; /** Idle timeout (ms): max time between output chunks before treating the process as hung. Resets on output. Falls back to DEFAULT_STARTUP_TIMEOUT_MS. */ timeoutMs?: number; - /** Absolute timeout (ms): ceiling on total time regardless of output. Falls back to DEFAULT_STARTUP_MAX_MS. */ + /** Absolute ceiling (ms) from reservedAt (or this call's entry) through readiness. Falls back to DEFAULT_STARTUP_MAX_MS. */ maxMs?: number; + /** Claim time, or launch entry when reusing a hostname. */ reservedAt?: number; + /** Called after listeners are attached so lifecycle cleanup can reap a failed child. */ onSpawn?: (proc: ChildProcess) => void; /** Loopback address this instance is bound to; recorded with the instance monitor for diagnostics. */ hostname?: string; @@ -530,7 +532,7 @@ export function runHarperCommand({ reject(error); }); proc.on('exit', (statusCode, signal) => { - if (!startupFinished() && Date.now() >= startupDeadline) failStartup(maxTimeoutMessage); + if (statusCode === 0 && !startupFinished() && Date.now() >= startupDeadline) failStartup(maxTimeoutMessage); if (!settled) { settled = true; clearTimers(); @@ -713,14 +715,19 @@ async function launchHarper( return ctx as StartedHarperTestContext; } catch (error) { if (spawnedProcess?.pid !== undefined) { + signalHarperTree(spawnedProcess, 'SIGKILL'); await killHarper({ harper: { process: spawnedProcess } } as StartedHarperTestContext, { graceMs: 0 }); if (spawnedProcess.exitCode === null && spawnedProcess.signalCode === null) { console.warn(`[integration-testing] Could not confirm startup process exit; keeping ${loopbackAddress} and ${dataRootDir}`); throw error; } + if (loopbackAddress && !(await waitForPortsFree(loopbackAddress, ALL_HARPER_PORTS, DEFAULT_PORT_RELEASE_TIMEOUT_MS))) { + console.warn(`[integration-testing] Harper ports on ${loopbackAddress} still in use after startup failed; keeping ${dataRootDir} and the reservation`); + throw error; + } } if (ownsLoopbackAddress && loopbackAddress) { - await releaseLoopbackAddress(loopbackAddress).catch((cleanupError) => { + await releaseLoopbackReservation(loopbackAddress, reservedAt).catch((cleanupError) => { console.warn(`[integration-testing] Could not release ${loopbackAddress} after startup failed: ${cleanupError}`); }); } diff --git a/src/loopbackAddressPool.ts b/src/loopbackAddressPool.ts index 56926d2..c9ba5da 100644 --- a/src/loopbackAddressPool.ts +++ b/src/loopbackAddressPool.ts @@ -515,9 +515,9 @@ export async function reserveLoopbackAddress(): Promise<{ loopbackAddress: strin } else { // Assign the process PID to that index to mark it as used loopbackPool[index] = process.pid; + reservedAt = Date.now(); } // Write the updated pool back to the file - if (index !== null) reservedAt = Date.now(); await writePoolFile(loopbackPool); return index; @@ -628,6 +628,10 @@ function removeDeadProcessesFromPool(loopbackPool: LoopbackPool) { * @throws InvalidLoopbackAddressError if the address format is invalid */ export async function releaseLoopbackAddress(address: string): Promise { + await releaseLoopbackReservation(address); +} + +export async function releaseLoopbackReservation(address: string, reservedAt?: number): Promise { // Validate and parse the address const index = parseLoopbackAddress(address); @@ -635,6 +639,10 @@ export async function releaseLoopbackAddress(address: string): Promise { // Read the pool file const loopbackPool = await readPoolFile(); if (!loopbackPool || loopbackPool[index] !== process.pid) return; + if (reservedAt !== undefined && Date.now() - reservedAt >= UNUSABLE_POOL_QUARANTINE_MS) { + console.warn(`[loopback-pool] Skipping automatic release of ${address}: its old claim may have been replaced during quarantine recovery.`); + return; + } // Release the address by setting it to null loopbackPool[index] = null; diff --git a/test/harperStartupDeadline.test.ts b/test/harperStartupDeadline.test.ts index 3c322c1..38becb6 100644 --- a/test/harperStartupDeadline.test.ts +++ b/test/harperStartupDeadline.test.ts @@ -9,14 +9,28 @@ import { PassThrough } from 'node:stream'; import { tmpdir } from 'node:os'; import { join } from 'node:path'; import type { HarperTestContext, StartedHarperTestContext } from '../src/harperLifecycle.ts'; +import * as portUtils from '../src/portUtils.ts'; const root = await fsPromises.mkdtemp(join(tmpdir(), 'harper-startup-deadline-')); const poolPath = join(root, 'harper-integration-test-loopback-pool.json'); const logRoot = join(root, 'logs'); const readyScript = join(root, 'ready.cjs'); const chattyScript = join(root, 'chatty.cjs'); +const exitScript = join(root, 'exit.cjs'); +const cleanExitScript = join(root, 'clean-exit.cjs'); +const descendantScript = join(root, 'descendant.cjs'); await fsPromises.writeFile(readyScript, "process.stdout.write('successfully started\\n'); setInterval(() => {}, 1000);\n"); await fsPromises.writeFile(chattyScript, "setInterval(() => process.stdout.write('booting\\n'), 10);\n"); +await fsPromises.writeFile(exitScript, "process.stderr.write('boot failed\\n'); process.exit(1);\n"); +await fsPromises.writeFile(cleanExitScript, "process.exit(0);\n"); +await fsPromises.writeFile(descendantScript, ` +const { spawn } = require('node:child_process'); +const child = spawn(process.execPath, ['-e', "require('node:net').createServer().listen(Number(process.env.TEST_PORT), '127.0.0.1', () => process.stdout.write('listening'))"], { stdio: ['ignore', 'pipe', 'inherit'] }); +child.stdout.once('data', () => { + process.stderr.write('descendant:' + child.pid + '\\n'); + process.exit(1); +}); +`); async function freePort(): Promise { const server = createServer(); @@ -44,17 +58,23 @@ const overrides = { }; const savedEnv = Object.fromEntries(Object.keys(overrides).map((key) => [key, process.env[key]])); Object.assign(process.env, overrides); +const testPort = await freePort(); let clock: number; let setupElapsedMs = 0; let publishElapsedMs = 0; let poolWaitElapsedMs = 0; let readinessElapsedMs = 0; +let exitElapsedMs = 0; let setupError: Error | undefined; let corruptDuringSetup = false; +let replaceDuringSetup = false; let unconfirmedChild: childProcess.ChildProcess | undefined; +let portsRemainHeld = false; +let observeRelease = false; +let releasedWhileChildAlive = false; const children: childProcess.ChildProcess[] = []; -let restoreClock: () => void; +let restoreClock: (() => void) | undefined; mock.module('node:fs/promises', { namedExports: { @@ -62,6 +82,12 @@ mock.module('node:fs/promises', { mkdir: async (...args: Parameters) => { clock += setupElapsedMs; if (corruptDuringSetup) await fsPromises.writeFile(poolPath, '[torn'); + if (replaceDuringSetup) { + await fsPromises.writeFile(poolPath, '[torn'); + const oldMtime = new Date(clock - 360001); + await fsPromises.utimes(poolPath, oldMtime, oldMtime); + await getNextAvailableLoopbackAddress(); + } if (setupError) throw setupError; return fsPromises.mkdir(...args); }, @@ -70,10 +96,23 @@ mock.module('node:fs/promises', { if (args[1] === poolPath) { clock += publishElapsedMs; publishElapsedMs = 0; + if (observeRelease && await fsPromises.readFile(poolPath, 'utf-8') === '[null]') { + releasedWhileChildAlive = !(await portUtils.isPortFree('127.0.0.1', testPort)); + } } }, }, }); +mock.module('../src/portUtils.ts', { + namedExports: { + ...portUtils, + waitForPortsFree: async (host: string, ports: number[], timeoutMs: number) => { + deepStrictEqual(ports, [9925, 9926, 9927, 1883, 8883]); + if (portsRemainHeld) return false; + return portUtils.waitForPortsFree(host, [testPort], timeoutMs); + }, + }, +}); mock.module('node:timers/promises', { namedExports: { ...timersPromises, @@ -99,6 +138,7 @@ mock.module('node:child_process', { child.stdout?.on('data', (chunk: Buffer) => { if (chunk.toString().includes('successfully started')) clock += readinessElapsedMs; }); + child.on('exit', () => { clock += exitElapsedMs; }); } return child; }, @@ -107,16 +147,19 @@ mock.module('node:child_process', { const { startHarper, setupHarperWithFixture, killHarper, teardownHarper, HarperStartupError, DEFAULT_STARTUP_MAX_MS } = await import('../src/harperLifecycle.ts'); -const { readPoolFile } = await import('../src/loopbackAddressPool.ts'); +const { readPoolFile, getNextAvailableLoopbackAddress } = await import('../src/loopbackAddressPool.ts'); beforeEach(async () => { + restoreClock = undefined; clock = Date.now(); const clockMock = mock.method(Date, 'now', () => clock); restoreClock = () => clockMock.mock.restore(); - setupElapsedMs = publishElapsedMs = poolWaitElapsedMs = readinessElapsedMs = 0; + setupElapsedMs = publishElapsedMs = poolWaitElapsedMs = readinessElapsedMs = exitElapsedMs = 0; setupError = undefined; corruptDuringSetup = false; + replaceDuringSetup = false; unconfirmedChild = undefined; + portsRemainHeld = observeRelease = releasedWhileChildAlive = false; children.length = 0; await fsPromises.writeFile(poolPath, '[null]'); }); @@ -127,7 +170,7 @@ afterEach(async () => { await killHarper({ harper: { process: child } } as StartedHarperTestContext, { graceMs: 0 }); } } finally { - restoreClock(); + restoreClock?.(); } }); @@ -203,7 +246,7 @@ test('boot receives only the budget left after setup, and failure reaps before c const setTimer = t.mock.method(globalThis, 'setTimeout'); const ctx: HarperTestContext = {}; await rejects(startHarper(ctx, { startupMaxMs: 1000, startupTimeoutMs: 3000, harperBinPath: chattyScript }), startupError(1000)); - strictEqual(setTimer.mock.calls[0].arguments[1], 100, 'only the remaining 100ms is available for boot'); + ok(setTimer.mock.calls.some((call) => call.arguments[1] === 100), 'only the remaining 100ms is available for boot'); strictEqual(children.length, 1); ok(children[0].exitCode !== null || children[0].signalCode !== null, 'child exit must precede resource cleanup'); await assertCleanFailure(ctx); @@ -217,6 +260,61 @@ test('readiness observed at the deadline is rejected even before the timer fires await assertCleanFailure(ctx); }); +test('a failed exit retains its status and diagnostics even when observed at the deadline', async () => { + exitElapsedMs = 1000; + const ctx: HarperTestContext = {}; + await rejects(startHarper(ctx, { startupMaxMs: 1000, harperBinPath: exitScript }), (error: Error) => { + ok(error instanceof HarperStartupError); + match(error.message, /failed with exit code\/signal 1/); + match(error.stderr, /boot failed/); + return true; + }); + await assertCleanFailure(ctx); +}); + +test('a normal exit observed at the deadline cannot bypass the startup ceiling', async () => { + exitElapsedMs = 1000; + const ctx: HarperTestContext = {}; + await rejects(startHarper(ctx, { startupMaxMs: 1000, harperBinPath: cleanExitScript }), startupError(1000)); + await assertCleanFailure(ctx); +}); + +test('a failed leader is reaped with its descendant before the slot is released', { skip: process.platform === 'win32' }, async () => { + observeRelease = true; + let descendantPid: number | undefined; + const ctx: HarperTestContext = {}; + try { + await rejects(startHarper(ctx, { startupMaxMs: 5000, harperBinPath: descendantScript, env: { TEST_PORT: String(testPort) } }), (error: Error) => { + ok(error instanceof HarperStartupError); + descendantPid = Number(error.stderr.match(/descendant:(\d+)/)?.[1]); + ok(descendantPid); + return true; + }); + strictEqual(releasedWhileChildAlive, false, 'the pool must not be released while the descendant holds its port'); + ok(await portUtils.isPortFree('127.0.0.1', testPort), 'the descendant must have been killed'); + await assertCleanFailure(ctx); + } finally { + if (descendantPid) { + try { process.kill(descendantPid, 'SIGKILL'); } catch {} + } + } +}); + +test('ports still held after failed startup keep the slot and owned directory parked', async (t) => { + portsRemainHeld = true; + const warn = t.mock.method(console, 'warn', () => {}); + try { + await rejects(startHarper({}, { startupMaxMs: 1000, harperBinPath: exitScript }), /failed with exit code\/signal 1/); + deepStrictEqual(JSON.parse(await fsPromises.readFile(poolPath, 'utf-8')), [process.pid]); + ok((await fsPromises.readdir(root)).some((name) => name.startsWith('harper-integration-test-') && !name.endsWith('.json'))); + ok(warn.mock.calls.some((call) => String(call.arguments[0]).includes('still in use after startup failed'))); + } finally { + for (const name of await fsPromises.readdir(root)) { + if (name.startsWith('harper-integration-test-') && !name.endsWith('.json')) await fsPromises.rm(join(root, name), { recursive: true, force: true }); + } + } +}); + test('script resolution cannot admit a spawn after the deadline', async (t) => { const script = join(root, 'resolution-ready.cjs'); await fsPromises.copyFile(readyScript, script); @@ -279,6 +377,17 @@ test('an expired start leaves a pool corrupted after its claim quarantined and u strictEqual(await readPoolFile(), null, 'a lost claim must remain quarantined'); }); +test('cleanup after quarantine recovery cannot clear a newer reservation with the same PID', async (t) => { + setupElapsedMs = 360001; + replaceDuringSetup = true; + const warn = t.mock.method(console, 'warn', () => {}); + const ctx: HarperTestContext = {}; + await rejects(startHarper(ctx, { startupMaxMs: 1000, harperBinPath: readyScript }), startupError(1000)); + strictEqual(children.length, 0); + deepStrictEqual(JSON.parse(await fsPromises.readFile(poolPath, 'utf-8')), [process.pid], 'cleanup must preserve the newer claim'); + ok(warn.mock.calls.some((call) => String(call.arguments[0]).includes('Skipping automatic release'))); +}); + test('an unconfirmed child exit keeps its reservation and install parked', async (t) => { const child = Object.assign(new EventEmitter(), { pid: 2147483647, From eb55cf1265bcc8caa038a95ba5fff3c22ce18bef Mon Sep 17 00:00:00 2001 From: Kris Zyp Date: Thu, 8 Oct 2026 18:19:18 -0600 Subject: [PATCH 3/4] fix(lifecycle): avoid signalling exited Windows leaders Skip taskkill once a Windows leader has exited, keep regression port waits bounded independently of the startup clock, and document the internal reservation helpers. Co-Authored-By: GPT-6.1 Codex --- CONTRIBUTING.md | 2 +- src/harperLifecycle.ts | 6 ++-- src/loopbackAddressPool.ts | 2 ++ test/harperStartupDeadline.test.ts | 44 ++++++++++++++++++++++++++---- 4 files changed, 46 insertions(+), 8 deletions(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 936a9bd..d79022e 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -29,7 +29,7 @@ Tests live in `test/` (separate from `src/`) and run on the built-in Node.js tes Internal-only helpers may be exported from their modules for testing (e.g. `runHarperCommand` in `harperLifecycle.ts`) but are deliberately **not** re-exported from `src/index.ts`, keeping them out of the public API. -The internal `reserveLoopbackAddress` returns the claim time recorded immediately before publishing the slot; the public `getNextAvailableLoopbackAddress` still returns a string. Startup's absolute ceiling includes everything after the claim through observed readiness, excluding pre-claim pool waits and post-readiness registration. Failure cleanup keeps resources parked unless the captured child has exited and Harper's ports are free; reused addresses and caller-owned directories survive failure. Automatic failure release also checks claim age inside the pool lock: a claim at least as old as the quarantine may have been replaced by another start with the same PID, so `releaseLoopbackReservation` leaves it untouched. `test/harperStartupDeadline.test.ts` enforces these boundaries. +The internal `reserveLoopbackAddress` returns the claim time recorded immediately before publishing the slot; the public `getNextAvailableLoopbackAddress` still returns a string. Startup's absolute ceiling includes everything after the claim through observed readiness, excluding pre-claim pool waits and post-readiness registration. Failure cleanup keeps resources parked unless the captured child has exited and Harper's ports are free; reused addresses and caller-owned directories survive failure. Windows cleanup does not signal an already-exited leader PID, which may have been reused; occupied ports still keep its resources parked. Automatic failure release also checks claim age inside the pool lock: a claim at least as old as the quarantine may have been replaced by another start with the same PID, so `releaseLoopbackReservation` leaves it untouched. `test/harperStartupDeadline.test.ts` enforces these boundaries. `loopbackAddressPool.ts` reads its pool file location, address range, and canary ports from the environment once, when it is imported, so `test/loopbackAddressPool.test.ts` sets them before a dynamic import: a private pool file in its own temp directory, and `127.0.0.1`, the one address every OS has without aliases. Its foreign-listener cases need an OS that lets an exclusive bind of that address succeed beside a listener on all interfaces, as macOS and Windows do; where the bind conflicts instead (Linux), the conflict canary refuses the address first and those cases skip themselves. That is why CI includes a macOS job alongside Linux and Windows. diff --git a/src/harperLifecycle.ts b/src/harperLifecycle.ts index 800531a..aebb318 100644 --- a/src/harperLifecycle.ts +++ b/src/harperLifecycle.ts @@ -72,8 +72,8 @@ export const DEFAULT_STARTUP_TIMEOUT_MS = /** * Absolute ceiling from address reservation through readiness, including setup after reservation, - * regardless of ongoing output — a generous backstop - * so a process that chatters forever without ever reporting ready still fails. Higher under CI, + * regardless of ongoing output — a generous backstop so a process that chatters forever without + * ever reporting ready still fails. Higher under CI, * where shared/contended runners boot more slowly. * * Override with `HARPER_INTEGRATION_TEST_STARTUP_MAX_MS`. Default 120s (300s under CI). @@ -758,6 +758,8 @@ function signalHarperTree(proc: ChildProcess, signal: 'SIGTERM' | 'SIGKILL'): vo const pid = proc.pid; if (pid === undefined) return; if (process.platform === 'win32') { + // Once the leader exits, taskkill can target a reused PID but cannot follow the old tree. + if (proc.exitCode !== null || proc.signalCode !== null) return; try { signalWindowsProcessTree(pid, signal); } catch { diff --git a/src/loopbackAddressPool.ts b/src/loopbackAddressPool.ts index c9ba5da..4900221 100644 --- a/src/loopbackAddressPool.ts +++ b/src/loopbackAddressPool.ts @@ -470,6 +470,7 @@ export async function getNextAvailableLoopbackAddress(): Promise { return (await reserveLoopbackAddress()).loopbackAddress; } +/** Internal claim-time handoff for startup; intentionally not re-exported from index.ts. */ export async function reserveLoopbackAddress(): Promise<{ loopbackAddress: string; reservedAt: number }> { // Each index maps to a different loopback address (index 0 -> 127.0.0.2, index 1 -> 127.0.0.3, etc.) // So if the first test process number is 42, it would be assigned to index 0 associated with address 127.0.0.2 @@ -631,6 +632,7 @@ export async function releaseLoopbackAddress(address: string): Promise { await releaseLoopbackReservation(address); } +/** Internal release leaves a supplied claim untouched after quarantine age; not re-exported from index.ts. */ export async function releaseLoopbackReservation(address: string, reservedAt?: number): Promise { // Validate and parse the address const index = parseLoopbackAddress(address); diff --git a/test/harperStartupDeadline.test.ts b/test/harperStartupDeadline.test.ts index 38becb6..3bc2a3b 100644 --- a/test/harperStartupDeadline.test.ts +++ b/test/harperStartupDeadline.test.ts @@ -69,7 +69,8 @@ let exitElapsedMs = 0; let setupError: Error | undefined; let corruptDuringSetup = false; let replaceDuringSetup = false; -let unconfirmedChild: childProcess.ChildProcess | undefined; +let injectedChild: childProcess.ChildProcess | undefined; +let taskkillCalls = 0; let portsRemainHeld = false; let observeRelease = false; let releasedWhileChildAlive = false; @@ -109,7 +110,13 @@ mock.module('../src/portUtils.ts', { waitForPortsFree: async (host: string, ports: number[], timeoutMs: number) => { deepStrictEqual(ports, [9925, 9926, 9927, 1883, 8883]); if (portsRemainHeld) return false; - return portUtils.waitForPortsFree(host, [testPort], timeoutMs); + // A failed group reap must time out even while the startup clock is frozen. + const deadline = performance.now() + timeoutMs; + while (!(await portUtils.isPortFree(host, testPort))) { + if (performance.now() >= deadline) return false; + await timersPromises.setTimeout(100); + } + return true; }, }, }); @@ -131,7 +138,12 @@ mock.module('node:child_process', { namedExports: { ...childProcess, spawn: (...args: Parameters) => { - if (unconfirmedChild && args[0] !== 'taskkill') return unconfirmedChild; + if (args[0] === 'taskkill') taskkillCalls++; + if (injectedChild && args[0] !== 'taskkill') { + const child = injectedChild; + if (child.exitCode !== null) queueMicrotask(() => child.emit('exit', child.exitCode)); + return child; + } const child = childProcess.spawn(...args); if (args[0] !== 'taskkill') { children.push(child); @@ -158,7 +170,8 @@ beforeEach(async () => { setupError = undefined; corruptDuringSetup = false; replaceDuringSetup = false; - unconfirmedChild = undefined; + injectedChild = undefined; + taskkillCalls = 0; portsRemainHeld = observeRelease = releasedWhileChildAlive = false; children.length = 0; await fsPromises.writeFile(poolPath, '[null]'); @@ -279,6 +292,27 @@ test('a normal exit observed at the deadline cannot bypass the startup ceiling', await assertCleanFailure(ctx); }); +test('Windows cleanup never sends taskkill to an already exited leader PID', async () => { + injectedChild = Object.assign(new EventEmitter(), { + pid: 2147483647, + exitCode: 1, + signalCode: null, + stdout: new PassThrough(), + stderr: new PassThrough(), + kill: () => true, + }) as unknown as childProcess.ChildProcess; + const originalPlatform = Object.getOwnPropertyDescriptor(process, 'platform')!; + const ctx: HarperTestContext = {}; + Object.defineProperty(process, 'platform', { value: 'win32' }); + try { + await rejects(startHarper(ctx, { startupMaxMs: 1000, harperBinPath: readyScript }), /failed with exit code\/signal 1/); + strictEqual(taskkillCalls, 0, 'an exited PID may now belong to an unrelated process'); + await assertCleanFailure(ctx); + } finally { + Object.defineProperty(process, 'platform', originalPlatform); + } +}); + test('a failed leader is reaped with its descendant before the slot is released', { skip: process.platform === 'win32' }, async () => { observeRelease = true; let descendantPid: number | undefined; @@ -397,7 +431,7 @@ test('an unconfirmed child exit keeps its reservation and install parked', async stderr: new PassThrough(), kill: () => true, }) as unknown as childProcess.ChildProcess; - unconfirmedChild = child; + injectedChild = child; const warn = t.mock.method(console, 'warn', () => {}); try { await rejects(startHarper({}, { startupMaxMs: 10, harperBinPath: readyScript }), startupError(10)); From 4acac9a2265784f335de441e1bbd1b77aad1dad0 Mon Sep 17 00:00:00 2001 From: Kris Zyp Date: Thu, 8 Oct 2026 18:35:45 -0600 Subject: [PATCH 4/4] fix(lifecycle): reject expired command promises consistently Return a rejected Promise before spawn so a chained catch receives the same startup timeout as the watchdog path. Co-Authored-By: GPT-6.1 Codex --- CONTRIBUTING.md | 2 +- src/harperLifecycle.ts | 2 +- test/harperStartupDeadline.test.ts | 12 +++++++++++- 3 files changed, 13 insertions(+), 3 deletions(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index d79022e..d6696b9 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -29,7 +29,7 @@ Tests live in `test/` (separate from `src/`) and run on the built-in Node.js tes Internal-only helpers may be exported from their modules for testing (e.g. `runHarperCommand` in `harperLifecycle.ts`) but are deliberately **not** re-exported from `src/index.ts`, keeping them out of the public API. -The internal `reserveLoopbackAddress` returns the claim time recorded immediately before publishing the slot; the public `getNextAvailableLoopbackAddress` still returns a string. Startup's absolute ceiling includes everything after the claim through observed readiness, excluding pre-claim pool waits and post-readiness registration. Failure cleanup keeps resources parked unless the captured child has exited and Harper's ports are free; reused addresses and caller-owned directories survive failure. Windows cleanup does not signal an already-exited leader PID, which may have been reused; occupied ports still keep its resources parked. Automatic failure release also checks claim age inside the pool lock: a claim at least as old as the quarantine may have been replaced by another start with the same PID, so `releaseLoopbackReservation` leaves it untouched. `test/harperStartupDeadline.test.ts` enforces these boundaries. +The internal `reserveLoopbackAddress` returns the claim time recorded immediately before publishing the slot; the public `getNextAvailableLoopbackAddress` still returns a string. Startup's absolute ceiling includes everything after the claim through observed readiness, excluding pre-claim pool waits and post-readiness registration. Expired `runHarperCommand` calls return a rejected Promise before spawning. Failure cleanup keeps resources parked unless the captured child has exited and Harper's ports are free; reused addresses and caller-owned directories survive failure. Windows cleanup does not signal an already-exited leader PID, which may have been reused; occupied ports still keep its resources parked. Automatic failure release also checks claim age inside the pool lock: a claim at least as old as the quarantine may have been replaced by another start with the same PID, so `releaseLoopbackReservation` leaves it untouched. `test/harperStartupDeadline.test.ts` enforces these boundaries. `loopbackAddressPool.ts` reads its pool file location, address range, and canary ports from the environment once, when it is imported, so `test/loopbackAddressPool.test.ts` sets them before a dynamic import: a private pool file in its own temp directory, and `127.0.0.1`, the one address every OS has without aliases. Its foreign-listener cases need an OS that lets an exclusive bind of that address succeed beside a listener on all interfaces, as macOS and Windows do; where the bind conflicts instead (Linux), the conflict canary refuses the address first and those cases skip themselves. That is why CI includes a macOS job alongside Linux and Windows. diff --git a/src/harperLifecycle.ts b/src/harperLifecycle.ts index aebb318..81010a0 100644 --- a/src/harperLifecycle.ts +++ b/src/harperLifecycle.ts @@ -418,7 +418,7 @@ export function runHarperCommand({ // management are unchanged. detached: process.platform !== 'win32', }; - if (Date.now() >= startupDeadline) throw new HarperStartupError(maxTimeoutMessage, '', ''); + if (Date.now() >= startupDeadline) return Promise.reject(new HarperStartupError(maxTimeoutMessage, '', '')); const proc = spawn(runtime, runtimeArgs, spawnOptions); // Publishes the instance to the shared monitor, which reaps it if this runner dies without diff --git a/test/harperStartupDeadline.test.ts b/test/harperStartupDeadline.test.ts index 3bc2a3b..158e9c0 100644 --- a/test/harperStartupDeadline.test.ts +++ b/test/harperStartupDeadline.test.ts @@ -157,7 +157,7 @@ mock.module('node:child_process', { }, }); -const { startHarper, setupHarperWithFixture, killHarper, teardownHarper, HarperStartupError, DEFAULT_STARTUP_MAX_MS } = +const { startHarper, setupHarperWithFixture, runHarperCommand, killHarper, teardownHarper, HarperStartupError, DEFAULT_STARTUP_MAX_MS } = await import('../src/harperLifecycle.ts'); const { readPoolFile, getNextAvailableLoopbackAddress } = await import('../src/loopbackAddressPool.ts'); @@ -209,6 +209,16 @@ function startupError(maxMs: number): (error: Error) => boolean { }; } +test('pre-spawn expiry returns a rejected Promise to chained catch handlers', async () => { + let capturedError: unknown; + await runHarperCommand({ args: [], env: {}, maxMs: 0, harperBinPath: readyScript }).catch((error) => { + capturedError = error; + }); + ok(capturedError instanceof HarperStartupError); + match(capturedError.message, /maximum startup time of 0ms/); + strictEqual(children.length, 0, 'an expired command must never spawn'); +}); + for (const maxMs of [0, 1000, DEFAULT_STARTUP_MAX_MS]) { test(`setup at the ${maxMs}ms ceiling refuses spawn and cleans up`, async () => { setupElapsedMs = maxMs;