From a1218d0d9e5c083ffa3b47ff557bf784def4b1f8 Mon Sep 17 00:00:00 2001 From: Kris Zyp Date: Tue, 15 Sep 2026 09:38:36 -0600 Subject: [PATCH 1/8] fix: make loopback pool file writes atomic and reads torn-write tolerant writePoolFile was a plain writeFile() with no temp-file-plus-rename, so a writer killed mid-flush (or one still writing when withLock's stale-lock timeout lets another process step past it) could leave a torn file behind. readPoolFile only special-cased ENOENT, so a torn/unparseable read (or any valid-JSON-but-non-array content) propagated out of getNextAvailableLoopbackAddress() and crashed every other agent's suite reading the same shared pool file on the box. - writePoolFile now publishes via write-to-temp-then-rename (same pattern as writeRegistryFile in harperInstanceRegistry.ts), so readers only ever see the old complete file or the new complete file. - readPoolFile now reinitializes (instead of throwing) on a JSON.parse failure or a parsed value that isn't an array, the same way it already does for ENOENT. - Both functions take an optional poolPath param (default: the real shared path) for test injection only; the shared tmpdir location is unchanged. Went through cross-model plan and full-diff review rounds; one round-trip worth recording: a first pass also validated pool-array *length* against HARPER_INTEGRATION_TEST_LOOPBACK_POOL_COUNT, but review traced a worse failure it introduced (two concurrently-configured-with-different-counts runs would perpetually reject and reinitialize each other's file, mutually wiping live reservations) than the pre-existing, unrelated behavior it replaced, so that check was reverted back to shape-only validation. Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_01KpUAPREP3CnnPxN1JMc4XT Co-Authored-By: GPT-5 Codex Dispatch-Task: pr-maint-e87fb81eefc34798cd5b627901e19247 --- package.json | 2 +- src/loopbackAddressPool.ts | 50 +++++++++++---- test/loopbackAddressPool.test.ts | 105 ++++++++++++++++++++++++++++++- 3 files changed, 142 insertions(+), 15 deletions(-) diff --git a/package.json b/package.json index d9aef96..c856a4c 100644 --- a/package.json +++ b/package.json @@ -39,7 +39,7 @@ "scripts": { "check": "tsc", "build": "tsc -p tsconfig.build.json", - "test": "node --test \"test/**/*.test.ts\"", + "test": "node --experimental-test-module-mocks --test \"test/**/*.test.ts\"", "prepack": "npm run build" }, "engines": { diff --git a/src/loopbackAddressPool.ts b/src/loopbackAddressPool.ts index 8a95455..4b32fda 100644 --- a/src/loopbackAddressPool.ts +++ b/src/loopbackAddressPool.ts @@ -1,7 +1,8 @@ import { setTimeout as sleep } from 'node:timers/promises'; +import { randomBytes } from 'node:crypto'; import { join } from 'node:path'; import { tmpdir } from 'node:os'; -import { open, readFile, stat, unlink, writeFile } from 'node:fs/promises'; +import { open, readFile, rename, stat, unlink, writeFile } from 'node:fs/promises'; import { createServer } from 'node:net'; import { findAcceptingPort } from './portUtils.ts'; @@ -207,31 +208,56 @@ async function withLock(callback: () => Promise): Promise { * index represents a loopback address (127.0.0.2, 127.0.0.3, etc.) and the value * is either null (available) or a process PID (in use). * - * If the file doesn't exist, creates and returns a new empty pool with all addresses - * marked as available (null). + * A missing, unparseable, or wrong-shaped file all reinitialize to an empty pool rather + * than throw. * + * @param poolPath The pool file path (overridable for tests; defaults to the shared pool file) * @returns The loopback pool array */ -async function readPoolFile(): Promise { +export async function readPoolFile(poolPath: string = HARPER_LOOPBACK_POOL_PATH): Promise { try { - const content = await readFile(HARPER_LOOPBACK_POOL_PATH, 'utf-8'); - return JSON.parse(content) as LoopbackPool; + const content = await readFile(poolPath, 'utf-8'); + const parsed: unknown = JSON.parse(content); + if (!Array.isArray(parsed)) { + throw new SyntaxError(`pool file did not contain a JSON array (got ${typeof parsed})`); + } + return parsed as LoopbackPool; } catch (error) { - if ((error as NodeJS.ErrnoException).code === 'ENOENT') { - // If the pool file doesn't exist yet, create it with null entries - return Array(HARPER_LOOPBACK_POOL_COUNT).fill(null); + if (error instanceof SyntaxError) { + console.warn(`[loopback-pool] ${poolPath} is corrupt; reinitializing. ${error.message}`); + } else if ((error as NodeJS.ErrnoException).code !== 'ENOENT') { + throw error; } - throw error; + return Array(HARPER_LOOPBACK_POOL_COUNT).fill(null); } } +let pendingWriteCounter = 0; + /** * Writes the loopback pool to the pool file as JSON. * + * Publishes by write-to-temp-then-rename, so readers only ever see the old complete file or + * the new complete file. The pending name must be unpredictable, not just unique — a fixed + * or guessable one could be pre-planted as a symlink. + * * @param pool The loopback pool array to persist + * @param poolPath The pool file path (overridable for tests; defaults to the shared pool file) */ -async function writePoolFile(pool: LoopbackPool): Promise { - await writeFile(HARPER_LOOPBACK_POOL_PATH, JSON.stringify(pool)); +export async function writePoolFile(pool: LoopbackPool, poolPath: string = HARPER_LOOPBACK_POOL_PATH): Promise { + const pendingPath = `${poolPath}.${process.pid}.${++pendingWriteCounter}.${randomBytes(8).toString('hex')}.pending`; + try { + const pendingFileHandle = await open(pendingPath, 'wx'); + try { + await pendingFileHandle.writeFile(JSON.stringify(pool)); + } finally { + await pendingFileHandle.close(); + } + await rename(pendingPath, poolPath); + } catch (error) { + await unlink(pendingPath).catch(() => {}); + throw error; + } } /** diff --git a/test/loopbackAddressPool.test.ts b/test/loopbackAddressPool.test.ts index 3ebd39d..1f0371d 100644 --- a/test/loopbackAddressPool.test.ts +++ b/test/loopbackAddressPool.test.ts @@ -1,7 +1,9 @@ -import { test, after } from 'node:test'; +import { test, after, mock } from 'node:test'; import { deepStrictEqual, match, ok, rejects, strictEqual } from 'node:assert'; import { createServer, type AddressInfo, type Server } from 'node:net'; import { existsSync, mkdirSync, mkdtempSync, readdirSync, readFileSync, rmSync, writeFileSync } from 'node:fs'; +import { mkdtemp, readdir, readFile, rm, writeFile } from 'node:fs/promises'; +import * as fsPromises from 'node:fs/promises'; import { tmpdir } from 'node:os'; import { join } from 'node:path'; import { setTimeout as sleep } from 'node:timers/promises'; @@ -41,7 +43,7 @@ process.env.HARPER_INTEGRATION_TEST_LOOPBACK_POOL_COUNT = '1'; process.env.HARPER_INTEGRATION_TEST_CONFLICT_PROBE_PORT = String(operationsPort); process.env.HARPER_INTEGRATION_TEST_HTTP_CONFLICT_PROBE_PORT = String(httpPort); delete process.env[ALLOW_ENV]; -const { getNextAvailableLoopbackAddress, releaseLoopbackAddress } = await import('../src/loopbackAddressPool.ts'); +const { getNextAvailableLoopbackAddress, releaseLoopbackAddress, readPoolFile, writePoolFile } = await import('../src/loopbackAddressPool.ts'); const { startHarper, setupHarperWithFixture, publishHarperNode, createHarperContext } = await import( '../src/harperLifecycle.ts' ); @@ -80,6 +82,15 @@ async function refusal(): Promise { throw new Error(`expected getNextAvailableLoopbackAddress to reject, but it handed out ${address}`); } +async function withTempDir(body: (dir: string) => Promise): Promise { + const dir = await mkdtemp(join(tmpdir(), 'loopback-pool-test-')); + try { + await body(dir); + } finally { + await rm(dir, { recursive: true, force: true }); + } +} + test('hands out an address when no other process listens on its ports', async () => { const address = await getNextAvailableLoopbackAddress(); strictEqual(address, HOST); @@ -208,3 +219,93 @@ test('startHarper and setupHarperWithFixture remove the install directory they c await close(listener); } }); + +test('readPoolFile reinitializes instead of throwing on a truncated/corrupt pool file', async () => { + await withTempDir(async (dir) => { + const poolPath = join(dir, 'pool.json'); + await writeFile(poolPath, '[1,2,nul'); + + const pool = await readPoolFile(poolPath); + + ok(Array.isArray(pool)); + ok(pool.length > 0); + ok(pool.every((slot) => slot === null)); + }); +}); + +test('readPoolFile reinitializes on valid JSON that is not a pool array', async () => { + await withTempDir(async (dir) => { + const poolPath = join(dir, 'pool.json'); + await writeFile(poolPath, 'null'); + + const pool = await readPoolFile(poolPath); + + ok(Array.isArray(pool)); + ok(pool.every((slot) => slot === null)); + }); +}); + +test('readPoolFile still rethrows errors unrelated to a missing/corrupt file', async () => { + await withTempDir(async (dir) => { + const poolPath = join(dir, 'pool.json'); + const m = mock.module('node:fs/promises', { + // @types/node 22.0.0 only knows `namedExports`; `exports` isn't in its types yet. + namedExports: { + ...fsPromises, + readFile: async () => { + const error = new Error('simulated EACCES') as NodeJS.ErrnoException; + error.code = 'EACCES'; + throw error; + }, + }, + }); + try { + const { readPoolFile: freshReadPoolFile } = await import(`../src/loopbackAddressPool.ts?fresh=${Date.now()}`); + await rejects(() => freshReadPoolFile(poolPath), /simulated EACCES/); + } finally { + m.restore(); + } + }); +}); + +test('writePoolFile round-trips through readPoolFile and leaves no leftover pending file', async () => { + await withTempDir(async (dir) => { + const poolPath = join(dir, 'pool.json'); + const pool = [null, 123, null, 456]; + + await writePoolFile(pool, poolPath); + + deepStrictEqual(await readPoolFile(poolPath), pool); + deepStrictEqual(await readdir(dir), ['pool.json']); + }); +}); + +test('writePoolFile never leaves a partially-written file visible at the pool path', async () => { + await withTempDir(async (dir) => { + const poolPath = join(dir, 'pool.json'); + const original = [null, null, null]; + await writeFile(poolPath, JSON.stringify(original)); + + // Faults the publish step after the pending write already completed, proving poolPath + // itself is untouched. This exercises the catchable-error branch of cleanup, not immunity + // to an actual SIGKILL — a hard kill in this window bypasses catch/finally and does orphan + // the pending file (accepted gap: the pool has no reaper, unlike the instance registry). + const m = mock.module('node:fs/promises', { + namedExports: { + ...fsPromises, + rename: async () => { + throw new Error('simulated rename failure'); + }, + }, + }); + try { + const { writePoolFile: freshWritePoolFile } = await import(`../src/loopbackAddressPool.ts?fresh=${Date.now()}`); + await rejects(() => freshWritePoolFile([1, 2, 3], poolPath), /simulated rename failure/); + } finally { + m.restore(); + } + + deepStrictEqual(JSON.parse(await readFile(poolPath, 'utf-8')), original); + deepStrictEqual(await readdir(dir), ['pool.json']); + }); +}); From f3b744c2bb278f98156e9ca596c4a0f48547d16e Mon Sep 17 00:00:00 2001 From: Kris Zyp Date: Tue, 15 Sep 2026 10:03:45 -0600 Subject: [PATCH 2/8] fix: reinitialize an empty pool array instead of leaving it unusable readPoolFile's Array.isArray check let a syntactically valid but empty array ([]) pass through untouched, silently contradicting its own "wrong-shaped file all reinitialize" contract. With no slots to find, getNextAvailableLoopbackAddress spins forever with no diagnostic (a CI timeout with no clue why) instead of reinitializing like every other corrupt-shape case. Now treated as corrupt. Length mismatches against this process's configured count are still left alone deliberately (see the comment) - that's the mutual-wipe regression from an earlier round. Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_01KpUAPREP3CnnPxN1JMc4XT --- src/loopbackAddressPool.ts | 9 +++++++-- test/loopbackAddressPool.test.ts | 18 ++++++++++++++---- 2 files changed, 21 insertions(+), 6 deletions(-) diff --git a/src/loopbackAddressPool.ts b/src/loopbackAddressPool.ts index 4b32fda..507892e 100644 --- a/src/loopbackAddressPool.ts +++ b/src/loopbackAddressPool.ts @@ -218,8 +218,13 @@ export async function readPoolFile(poolPath: string = HARPER_LOOPBACK_POOL_PATH) try { const content = await readFile(poolPath, 'utf-8'); const parsed: unknown = JSON.parse(content); - if (!Array.isArray(parsed)) { - throw new SyntaxError(`pool file did not contain a JSON array (got ${typeof parsed})`); + // An empty array has no free slots to find, so the caller would spin forever with no + // diagnostic instead of reinitializing; anything longer is left alone even if it doesn't + // match this process's configured count (see writePoolFile: a differently-configured + // concurrent process legitimately produces one, and treating that as corrupt too makes + // the two processes reinitialize each other's file back and forth). + if (!Array.isArray(parsed) || parsed.length === 0) { + throw new SyntaxError(`pool file did not contain a non-empty JSON array (got ${typeof parsed})`); } return parsed as LoopbackPool; } catch (error) { diff --git a/test/loopbackAddressPool.test.ts b/test/loopbackAddressPool.test.ts index 1f0371d..1881e60 100644 --- a/test/loopbackAddressPool.test.ts +++ b/test/loopbackAddressPool.test.ts @@ -245,6 +245,18 @@ test('readPoolFile reinitializes on valid JSON that is not a pool array', async }); }); +test('readPoolFile reinitializes on an empty pool array instead of leaving no free slots', async () => { + await withTempDir(async (dir) => { + const poolPath = join(dir, 'pool.json'); + await writeFile(poolPath, '[]'); + + const pool = await readPoolFile(poolPath); + + ok(pool.length > 0); + ok(pool.every((slot) => slot === null)); + }); +}); + test('readPoolFile still rethrows errors unrelated to a missing/corrupt file', async () => { await withTempDir(async (dir) => { const poolPath = join(dir, 'pool.json'); @@ -286,10 +298,8 @@ test('writePoolFile never leaves a partially-written file visible at the pool pa const original = [null, null, null]; await writeFile(poolPath, JSON.stringify(original)); - // Faults the publish step after the pending write already completed, proving poolPath - // itself is untouched. This exercises the catchable-error branch of cleanup, not immunity - // to an actual SIGKILL — a hard kill in this window bypasses catch/finally and does orphan - // the pending file (accepted gap: the pool has no reaper, unlike the instance registry). + // A hard kill in this same window (vs. this mocked rejection) bypasses catch/finally and + // orphans the pending file — the pool has no reaper for that, unlike the instance registry. const m = mock.module('node:fs/promises', { namedExports: { ...fsPromises, From f13d08881557d1770bd9bf998d2e72f97d0472ce Mon Sep 17 00:00:00 2001 From: Kris Zyp Date: Tue, 15 Sep 2026 10:08:50 -0600 Subject: [PATCH 3/8] fix: avoid millisecond-collision flake in fresh-import cache busting Round-4 review (Gemini + domain) caught a real flake risk: the two mock.module tests cache-busted their fresh dynamic import with Date.now(), which can collide within the same millisecond on a fast runner and silently reuse the wrong test's mocked module instance, failing with an unrelated error message instead of the expected one. Switched to a monotonic per-process counter. Also trimmed a comment that restated the empty-array branch instead of only the non-obvious mixed-configuration rationale, and fixed readPoolFile's doc comment, which had drifted out of sync with what actually reinitializes. Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_01KpUAPREP3CnnPxN1JMc4XT --- src/loopbackAddressPool.ts | 11 ++++------- test/loopbackAddressPool.test.ts | 10 ++++++++-- 2 files changed, 12 insertions(+), 9 deletions(-) diff --git a/src/loopbackAddressPool.ts b/src/loopbackAddressPool.ts index 507892e..d338195 100644 --- a/src/loopbackAddressPool.ts +++ b/src/loopbackAddressPool.ts @@ -208,8 +208,10 @@ async function withLock(callback: () => Promise): Promise { * index represents a loopback address (127.0.0.2, 127.0.0.3, etc.) and the value * is either null (available) or a process PID (in use). * - * A missing, unparseable, or wrong-shaped file all reinitialize to an empty pool rather - * than throw. + * A missing, unparseable, non-array, or empty file all reinitialize to an empty pool rather + * than throw. A non-empty array of any other length is trusted as-is, even if it doesn't + * match this process's configured count: two processes configured with different counts + * sharing this file would otherwise perpetually reinitialize each other's out of it. * * @param poolPath The pool file path (overridable for tests; defaults to the shared pool file) * @returns The loopback pool array @@ -218,11 +220,6 @@ export async function readPoolFile(poolPath: string = HARPER_LOOPBACK_POOL_PATH) try { const content = await readFile(poolPath, 'utf-8'); const parsed: unknown = JSON.parse(content); - // An empty array has no free slots to find, so the caller would spin forever with no - // diagnostic instead of reinitializing; anything longer is left alone even if it doesn't - // match this process's configured count (see writePoolFile: a differently-configured - // concurrent process legitimately produces one, and treating that as corrupt too makes - // the two processes reinitialize each other's file back and forth). if (!Array.isArray(parsed) || parsed.length === 0) { throw new SyntaxError(`pool file did not contain a non-empty JSON array (got ${typeof parsed})`); } diff --git a/test/loopbackAddressPool.test.ts b/test/loopbackAddressPool.test.ts index 1881e60..9dd21cb 100644 --- a/test/loopbackAddressPool.test.ts +++ b/test/loopbackAddressPool.test.ts @@ -82,6 +82,12 @@ async function refusal(): Promise { throw new Error(`expected getNextAvailableLoopbackAddress to reject, but it handed out ${address}`); } +let freshImportCounter = 0; +/** A cache-busting suffix that's monotonic, unlike Date.now() (can collide within a millisecond). */ +function freshModuleUrl(): string { + return `../src/loopbackAddressPool.ts?fresh=${++freshImportCounter}`; +} + async function withTempDir(body: (dir: string) => Promise): Promise { const dir = await mkdtemp(join(tmpdir(), 'loopback-pool-test-')); try { @@ -272,7 +278,7 @@ test('readPoolFile still rethrows errors unrelated to a missing/corrupt file', a }, }); try { - const { readPoolFile: freshReadPoolFile } = await import(`../src/loopbackAddressPool.ts?fresh=${Date.now()}`); + const { readPoolFile: freshReadPoolFile } = await import(freshModuleUrl()); await rejects(() => freshReadPoolFile(poolPath), /simulated EACCES/); } finally { m.restore(); @@ -309,7 +315,7 @@ test('writePoolFile never leaves a partially-written file visible at the pool pa }, }); try { - const { writePoolFile: freshWritePoolFile } = await import(`../src/loopbackAddressPool.ts?fresh=${Date.now()}`); + const { writePoolFile: freshWritePoolFile } = await import(freshModuleUrl()); await rejects(() => freshWritePoolFile([1, 2, 3], poolPath), /simulated rename failure/); } finally { m.restore(); From 179a6bb3a126792ea2d78231ffce88b20b659f3a Mon Sep 17 00:00:00 2001 From: Kris Zyp Date: Tue, 15 Sep 2026 10:13:04 -0600 Subject: [PATCH 4/8] chore: trim review-history comment on the fresh-import helper Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_01KpUAPREP3CnnPxN1JMc4XT --- test/loopbackAddressPool.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/loopbackAddressPool.test.ts b/test/loopbackAddressPool.test.ts index 9dd21cb..32f7d77 100644 --- a/test/loopbackAddressPool.test.ts +++ b/test/loopbackAddressPool.test.ts @@ -83,7 +83,7 @@ async function refusal(): Promise { } let freshImportCounter = 0; -/** A cache-busting suffix that's monotonic, unlike Date.now() (can collide within a millisecond). */ +/** Monotonic cache-busting suffix — millisecond-resolution timestamps can collide. */ function freshModuleUrl(): string { return `../src/loopbackAddressPool.ts?fresh=${++freshImportCounter}`; } From cc29eb3d402c3200b440c912fb0b6e2e6cb6ddf9 Mon Sep 17 00:00:00 2001 From: Kris Zyp Date: Thu, 1 Oct 2026 10:30:12 -0600 Subject: [PATCH 5/8] fix: quarantine an unusable loopback pool file instead of resetting it at once Resetting an unparseable, non-array or empty pool file to all-free the moment it is read can hand a runner's reserved-but-not-yet-bound address to another runner, since the conflict canary only sees bound addresses, and it turns a torn read by a still-writing legacy writer into erasing every live reservation. readPoolFile now reports such a file as quarantined (null) until it has gone 6 minutes unmodified, which outlasts startHarper's reserve-to-bind time, and only then reinitializes it. Allocation waits on its existing retry loop and both release functions skip their write while quarantined. releaseLoopbackAddress now clears only a slot this process holds, so a delayed release after a reinitialization cannot erase another runner's newer reservation. Documents the pool file, its atomic publish and this recovery policy in the README. Co-Authored-By: Claude Opus 5.5 Dispatch-Task: fix-kriszyp_integration-testing_32-b8360e96 Co-Authored-By: GPT-5 Codex Dispatch-Task: pr-maint-e87fb81eefc34798cd5b627901e19247 --- CONTRIBUTING.md | 2 +- README.md | 10 +++ src/loopbackAddressPool.ts | 80 ++++++++++++++++----- test/loopbackAddressPool.test.ts | 116 ++++++++++++++++++++++++++----- 4 files changed, 170 insertions(+), 38 deletions(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index ae2c43f..5d22b90 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -6,7 +6,7 @@ Contributors are encouraged to communicate with maintainers in issues or other c Source files are located in `src/`. These are built to the `dist/` directory. The published package includes `dist/`, `scripts/`, and the regular npm metadata and documentation files. -The `src/index.ts` is the source for the main export. This is the public re-export of all the various utilities from `src/harperLifecycle.ts`, `targz.ts`, and more. The `src/run.ts` is the source for the `harper-integration-test-run` bin script. The internal `src/harperInstanceRegistry.ts` publishes each running Harper instance to a shared on-disk registry, and `src/harperMonitor.ts` is the singleton monitor process that reads it and reaps instances whose test runner died without cleaning up (see README's *Orphaned Instance Monitor* section). A registry record covers a whole process group, so the monitor — the only half that can see when that group is finished — is also the only writer that removes one; the lifecycle side registers and never deregisters. And the `scripts/setup-loopback.sh` is the source for the `harper-integration-test-setup-loopback` bin script. +The `src/index.ts` is the source for the main export. This is the public re-export of all the various utilities from `src/harperLifecycle.ts`, `targz.ts`, and more. The `src/run.ts` is the source for the `harper-integration-test-run` bin script. The internal `src/harperInstanceRegistry.ts` publishes each running Harper instance to a shared on-disk registry, and `src/harperMonitor.ts` is the singleton monitor process that reads it and reaps instances whose test runner died without cleaning up (see README's *Orphaned Instance Monitor* section). A registry record covers a whole process group, so the monitor — the only half that can see when that group is finished — is also the only writer that removes one; the lifecycle side registers and never deregisters. The pool behind the loopback utilities, `src/loopbackAddressPool.ts`, never writes an unusable pool file back as an empty pool before its quarantine ends (see README's *Loopback Pool Utilities*): every path that rewrites the file returns early when `readPoolFile` reports it quarantined (`null`), so a new write path must do the same. And the `scripts/setup-loopback.sh` is the source for the `harper-integration-test-setup-loopback` bin script. The package is `"type": "module"` — all source files are ESM by default. diff --git a/README.md b/README.md index 04f2307..8ef3398 100644 --- a/README.md +++ b/README.md @@ -292,6 +292,16 @@ These are used internally by `startHarper` and `teardownHarper`, but are exporte - `releaseLoopbackAddress(address: string): Promise` - Returns an address to the pool - `releaseAllLoopbackAddressesForCurrentProcess(): Promise` - Releases all addresses held by this process +The pool is one file shared by every process on the machine: `${TMPDIR}/harper-integration-test-loopback-pool.json`, a JSON array with one slot per address holding the PID of the process that reserved it (or `null`), guarded by the `harper-integration-test-loopback-pool.lock` mutex beside it. Updates are published by renaming a complete `*.pending` file over it, so a process killed mid-update leaves the previous pool intact (and possibly an inert `*.pending` file). Releasing an address clears its slot only if this process holds it. + +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. +- 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. + ## Node.js Test Runner > Tests executed by this runner must use the `node:test` API. If you're using a different test framework, use the [lifecycle APIs](#api) directly and refer to the [runner source](./src/run.ts) as a reference implementation. diff --git a/src/loopbackAddressPool.ts b/src/loopbackAddressPool.ts index d338195..a0dca2a 100644 --- a/src/loopbackAddressPool.ts +++ b/src/loopbackAddressPool.ts @@ -30,6 +30,12 @@ const HARPER_LOOPBACK_POOL_LOCK_PATH = join(tmpdir(), 'harper-integration-test-l const LOCK_STALE_TIMEOUT_MS = 10000; const RETRY_DELAY_MS = 1000; +// How long an unusable pool file must go unmodified before it is reinitialized. The reservations +// it lost may belong to runners that haven't bound their address yet, which the conflict canary +// cannot see, so this has to outlast startHarper's reserve-to-bind time: DEFAULT_STARTUP_MAX_MS +// (300s under CI) in harperLifecycle.ts, plus a minute for the work before that watchdog starts. +const UNUSABLE_POOL_QUARANTINE_MS = 360000; + // Port used as a conflict canary when allocating an address. This MUST be a port that // Harper binds WITHOUT SO_REUSEPORT (the operations API — `OPERATIONS_API_PORT` in // harperLifecycle.ts — does exactly that, main-thread-only). Because that port is @@ -203,35 +209,63 @@ async function withLock(callback: () => Promise): Promise { } } +const quarantinedPoolPaths = new Set(); + +function emptyPool(): LoopbackPool { + return Array(HARPER_LOOPBACK_POOL_COUNT).fill(null); +} + /** * Reads the loopback pool from the pool file. The pool is a JSON array where each * index represents a loopback address (127.0.0.2, 127.0.0.3, etc.) and the value * is either null (available) or a process PID (in use). * - * A missing, unparseable, non-array, or empty file all reinitialize to an empty pool rather - * than throw. A non-empty array of any other length is trusted as-is, even if it doesn't - * match this process's configured count: two processes configured with different counts - * sharing this file would otherwise perpetually reinitialize each other's out of it. + * A missing file reads as an empty pool. An unusable one (unparseable, not an array, or an empty + * array) is quarantined instead: it is either still being written by a writer that doesn't publish + * by rename, or it has lost reservations whose holders may not have bound their address yet. This + * returns `null` until the file has gone UNUSABLE_POOL_QUARANTINE_MS unmodified, and an empty pool + * after that. A non-empty array of any other length is trusted as-is, even if it doesn't match + * this process's configured count: two processes configured with different counts sharing this + * file would otherwise perpetually reinitialize each other's out of it. * * @param poolPath The pool file path (overridable for tests; defaults to the shared pool file) - * @returns The loopback pool array + * @returns The loopback pool array, or `null` while the pool file is quarantined */ -export async function readPoolFile(poolPath: string = HARPER_LOOPBACK_POOL_PATH): Promise { +export async function readPoolFile(poolPath: string = HARPER_LOOPBACK_POOL_PATH): Promise { + let problem: string; try { - const content = await readFile(poolPath, 'utf-8'); - const parsed: unknown = JSON.parse(content); - if (!Array.isArray(parsed) || parsed.length === 0) { - throw new SyntaxError(`pool file did not contain a non-empty JSON array (got ${typeof parsed})`); + const parsed: unknown = JSON.parse(await readFile(poolPath, 'utf-8')); + if (Array.isArray(parsed) && parsed.length > 0) { + quarantinedPoolPaths.delete(poolPath); + return parsed as LoopbackPool; } - return parsed as LoopbackPool; + problem = Array.isArray(parsed) ? 'empty array' : `JSON ${parsed === null ? 'null' : typeof parsed} instead of an array`; } catch (error) { - if (error instanceof SyntaxError) { - console.warn(`[loopback-pool] ${poolPath} is corrupt; reinitializing. ${error.message}`); - } else if ((error as NodeJS.ErrnoException).code !== 'ENOENT') { - throw error; + if ((error as NodeJS.ErrnoException).code === 'ENOENT') return emptyPool(); + if (!(error instanceof SyntaxError)) throw error; + problem = error.message; + } + + let modifiedMs: number; + try { + modifiedMs = (await stat(poolPath)).mtimeMs; + } catch (error) { + if ((error as NodeJS.ErrnoException).code === 'ENOENT') return emptyPool(); + throw error; + } + const unmodifiedMs = Date.now() - modifiedMs; + if (unmodifiedMs < UNUSABLE_POOL_QUARANTINE_MS) { + if (!quarantinedPoolPaths.has(poolPath)) { + quarantinedPoolPaths.add(poolPath); + console.warn( + `[loopback-pool] ${poolPath} is unusable (${problem}); allocation waits until it has gone ${UNUSABLE_POOL_QUARANTINE_MS / 1000}s unmodified, then reinitializes it. Deleting it skips the wait, but only once every integration-test run on this machine has stopped.` + ); } - return Array(HARPER_LOOPBACK_POOL_COUNT).fill(null); + return null; } + quarantinedPoolPaths.delete(poolPath); + console.warn(`[loopback-pool] ${poolPath} has been unusable for ${Math.round(unmodifiedMs / 1000)}s (${problem}); reinitializing.`); + return emptyPool(); } let pendingWriteCounter = 0; @@ -420,7 +454,8 @@ export async function validateLoopbackAddressPool(): Promise<{ * 6. Checks that no other process accepts connections on the address's canary ports * 7. Returns the loopback address (e.g., "127.0.0.2") * - * If no addresses are available, waits and retries until one becomes available. + * If no addresses are available, or the pool file is quarantined (see readPoolFile), waits and + * retries until one becomes available. * * **Pool file location:** `${tmpdir()}/harper-integration-test-loopback-pool.json` * **Lock file location:** `${tmpdir()}/harper-integration-test-loopback-pool.lock` @@ -458,6 +493,7 @@ export async function getNextAvailableLoopbackAddress(): Promise { const assignedIndex = await withLock(async () => { // Read the pool file const loopbackPool = await readPoolFile(); + if (!loopbackPool) return null; // Find the first available index we haven't already found in-use this attempt let index: number | null = null; @@ -576,6 +612,11 @@ function removeDeadProcessesFromPool(loopbackPool: LoopbackPool) { /** * Releases a loopback address back to the pool, making it available for other processes. * + * Only a slot this process holds is cleared: once the pool has been reinitialized, the same slot + * may already be another runner's reservation. Nothing is written while the pool file is + * quarantined (see readPoolFile): the reservation is already gone from it, and rewriting it would + * end the quarantine early. + * * @param address The loopback address to release (e.g., "127.0.0.2") * @throws InvalidLoopbackAddressError if the address format is invalid */ @@ -586,6 +627,7 @@ export async function releaseLoopbackAddress(address: string): Promise { await withLock(async () => { // Read the pool file const loopbackPool = await readPoolFile(); + if (!loopbackPool || loopbackPool[index] !== process.pid) return; // Release the address by setting it to null loopbackPool[index] = null; @@ -597,12 +639,14 @@ export async function releaseLoopbackAddress(address: string): Promise { /** * Releases all loopback addresses assigned to the current process. - * Useful for cleanup during graceful shutdown. + * Useful for cleanup during graceful shutdown. Like releaseLoopbackAddress, writes nothing while + * the pool file is quarantined. */ export async function releaseAllLoopbackAddressesForCurrentProcess(): Promise { await withLock(async () => { // Read the pool file const loopbackPool = await readPoolFile(); + if (!loopbackPool) return; // Find and release all addresses assigned to this process for (let i = 0; i < loopbackPool.length; i++) { diff --git a/test/loopbackAddressPool.test.ts b/test/loopbackAddressPool.test.ts index 32f7d77..a4e6574 100644 --- a/test/loopbackAddressPool.test.ts +++ b/test/loopbackAddressPool.test.ts @@ -2,7 +2,7 @@ import { test, after, mock } from 'node:test'; import { deepStrictEqual, match, ok, rejects, strictEqual } from 'node:assert'; import { createServer, type AddressInfo, type Server } from 'node:net'; import { existsSync, mkdirSync, mkdtempSync, readdirSync, readFileSync, rmSync, writeFileSync } from 'node:fs'; -import { mkdtemp, readdir, readFile, rm, writeFile } from 'node:fs/promises'; +import { mkdtemp, readdir, readFile, rm, utimes, writeFile } from 'node:fs/promises'; import * as fsPromises from 'node:fs/promises'; import { tmpdir } from 'node:os'; import { join } from 'node:path'; @@ -97,6 +97,41 @@ async function withTempDir(body: (dir: string) => Promise): Promise } } +const POOL_FILE_NAME = 'harper-integration-test-loopback-pool.json'; + +/** Makes a file look like it was last written an hour ago, well past the quarantine. */ +async function age(path: string): Promise { + const anHourAgo = new Date(Date.now() - 3600000); + await utimes(path, anHourAgo, anHourAgo); +} + +/** + * A fresh copy of the pool module whose pool and lock live in `dir` and which hands out only + * 127.0.0.1. Both canary ports are 0 (an ephemeral bind, always free) so a real Harper on this + * machine can't make the result depend on the host. + */ +async function importIsolatedPool(dir: string): Promise { + const overrides: Record = { + TMPDIR: dir, + TMP: dir, + TEMP: dir, + HARPER_INTEGRATION_TEST_LOOPBACK_POOL_START: '1', + HARPER_INTEGRATION_TEST_LOOPBACK_POOL_COUNT: '1', + HARPER_INTEGRATION_TEST_CONFLICT_PROBE_PORT: '0', + HARPER_INTEGRATION_TEST_HTTP_CONFLICT_PROBE_PORT: '0', + }; + const saved = Object.fromEntries(Object.keys(overrides).map((name) => [name, process.env[name]])); + Object.assign(process.env, overrides); + try { + return await import(freshModuleUrl()); + } finally { + for (const [name, value] of Object.entries(saved)) { + if (value === undefined) delete process.env[name]; + else process.env[name] = value; + } + } +} + test('hands out an address when no other process listens on its ports', async () => { const address = await getNextAvailableLoopbackAddress(); strictEqual(address, HOST); @@ -226,40 +261,83 @@ test('startHarper and setupHarperWithFixture remove the install directory they c } }); -test('readPoolFile reinitializes instead of throwing on a truncated/corrupt pool file', async () => { +test('readPoolFile quarantines an unusable pool file, then reinitializes it once it has gone unmodified long enough', async (t) => { + t.mock.method(console, 'warn', () => {}); await withTempDir(async (dir) => { const poolPath = join(dir, 'pool.json'); - await writeFile(poolPath, '[1,2,nul'); + for (const content of ['[1,2,nul', '', 'null', '{}', '[]']) { + await writeFile(poolPath, content); + strictEqual(await readPoolFile(poolPath), null, `fresh ${JSON.stringify(content)} should be quarantined`); - const pool = await readPoolFile(poolPath); + await age(poolPath); + const pool = await readPoolFile(poolPath); + ok(pool && pool.length > 0 && pool.every((slot) => slot === null), `aged ${JSON.stringify(content)} should reinitialize`); + } + }); +}); - ok(Array.isArray(pool)); - ok(pool.length > 0); - ok(pool.every((slot) => slot === null)); +test('readPoolFile reads a pool file deleted between its read and its stat as missing', async () => { + const m = mock.module('node:fs/promises', { + namedExports: { + ...fsPromises, + readFile: async () => '[1,2,nul', + stat: async () => { + const error = new Error('simulated ENOENT') as NodeJS.ErrnoException; + error.code = 'ENOENT'; + throw error; + }, + }, }); + try { + const { readPoolFile: freshReadPoolFile } = await import(freshModuleUrl()); + const pool = await freshReadPoolFile('unused'); + ok(pool.length > 0 && pool.every((slot: number | null) => slot === null)); + } finally { + m.restore(); + } }); -test('readPoolFile reinitializes on valid JSON that is not a pool array', async () => { +test('a reservation lost to an unusable pool file is not reissued until the quarantine ends', async (t) => { + const warn = t.mock.method(console, 'warn', () => {}); await withTempDir(async (dir) => { - const poolPath = join(dir, 'pool.json'); - await writeFile(poolPath, 'null'); + const pool = await importIsolatedPool(dir); + const poolPath = join(dir, POOL_FILE_NAME); + strictEqual(await pool.getNextAvailableLoopbackAddress(), '127.0.0.1'); - const pool = await readPoolFile(poolPath); + // A writer that doesn't publish by rename dies mid-write, losing that reservation. + await writeFile(poolPath, '['); + const reissued = pool.getNextAvailableLoopbackAddress(); + try { + strictEqual(await Promise.race([reissued.then(() => 'reissued'), sleep(2500).then(() => 'waiting')]), 'waiting'); + + await pool.releaseLoopbackAddress('127.0.0.1'); + await pool.releaseAllLoopbackAddressesForCurrentProcess(); + strictEqual(await readFile(poolPath, 'utf-8'), '[', 'releasing must not end the quarantine'); - ok(Array.isArray(pool)); - ok(pool.every((slot) => slot === null)); + await age(poolPath); + strictEqual(await Promise.race([reissued, sleep(5000).then(() => 'still waiting')]), '127.0.0.1'); + } finally { + // Unblocks the allocation if an assertion above failed, so the test can't hang. + await rm(poolPath, { force: true }); + await reissued; + } + strictEqual(warn.mock.calls.filter((call) => String(call.arguments[0]).includes('is unusable')).length, 1); }); }); -test('readPoolFile reinitializes on an empty pool array instead of leaving no free slots', async () => { +test('releaseLoopbackAddress clears only a slot this process holds', async () => { await withTempDir(async (dir) => { - const poolPath = join(dir, 'pool.json'); - await writeFile(poolPath, '[]'); + const pool = await importIsolatedPool(dir); + const poolPath = join(dir, POOL_FILE_NAME); - const pool = await readPoolFile(poolPath); + strictEqual(await pool.getNextAvailableLoopbackAddress(), '127.0.0.1'); + await pool.releaseLoopbackAddress('127.0.0.1'); + deepStrictEqual(JSON.parse(await readFile(poolPath, 'utf-8')), [null]); - ok(pool.length > 0); - ok(pool.every((slot) => slot === null)); + const otherRunnerPid = process.pid + 1; + await writeFile(poolPath, JSON.stringify([otherRunnerPid])); + await pool.releaseLoopbackAddress('127.0.0.1'); + deepStrictEqual(JSON.parse(await readFile(poolPath, 'utf-8')), [otherRunnerPid]); }); }); From 7f3b99a25ed1a769b3165fa9ea820339ae045ef7 Mon Sep 17 00:00:00 2001 From: Kris Zyp Date: Thu, 1 Oct 2026 10:54:31 -0600 Subject: [PATCH 6/8] fix: publish the reinitialized pool when a quarantine ends readPoolFile now writes the empty pool it returns once an unusable file's quarantine has expired, so the file is repaired even when the caller is a release that has nothing of its own to clear, instead of every later release logging "reinitializing" over a file that stays unusable. releaseAllLoopbackAddressesForCurrentProcess skips its write when it cleared nothing. Also corrects the pending-file name rationale and an unclear readPoolFile doc clause. Co-Authored-By: Claude Opus 5.5 Dispatch-Task: fix-kriszyp_integration-testing_32-b8360e96 --- src/loopbackAddressPool.ts | 27 +++++++++++++++------------ test/loopbackAddressPool.test.ts | 1 + 2 files changed, 16 insertions(+), 12 deletions(-) diff --git a/src/loopbackAddressPool.ts b/src/loopbackAddressPool.ts index a0dca2a..bcafd81 100644 --- a/src/loopbackAddressPool.ts +++ b/src/loopbackAddressPool.ts @@ -30,10 +30,9 @@ const HARPER_LOOPBACK_POOL_LOCK_PATH = join(tmpdir(), 'harper-integration-test-l const LOCK_STALE_TIMEOUT_MS = 10000; const RETRY_DELAY_MS = 1000; -// How long an unusable pool file must go unmodified before it is reinitialized. The reservations -// it lost may belong to runners that haven't bound their address yet, which the conflict canary -// cannot see, so this has to outlast startHarper's reserve-to-bind time: DEFAULT_STARTUP_MAX_MS -// (300s under CI) in harperLifecycle.ts, plus a minute for the work before that watchdog starts. +// 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. const UNUSABLE_POOL_QUARANTINE_MS = 360000; // Port used as a conflict canary when allocating an address. This MUST be a port that @@ -223,10 +222,10 @@ function emptyPool(): LoopbackPool { * A missing file reads as an empty pool. An unusable one (unparseable, not an array, or an empty * array) is quarantined instead: it is either still being written by a writer that doesn't publish * by rename, or it has lost reservations whose holders may not have bound their address yet. This - * returns `null` until the file has gone UNUSABLE_POOL_QUARANTINE_MS unmodified, and an empty pool - * after that. A non-empty array of any other length is trusted as-is, even if it doesn't match - * this process's configured count: two processes configured with different counts sharing this - * file would otherwise perpetually reinitialize each other's out of it. + * returns `null` until the file has gone UNUSABLE_POOL_QUARANTINE_MS unmodified; after that it + * publishes and returns an empty pool. A non-empty array of any other length is trusted as-is, even + * if it doesn't match this process's configured count: two processes configured with different + * counts would otherwise keep wiping each other's reservations. * * @param poolPath The pool file path (overridable for tests; defaults to the shared pool file) * @returns The loopback pool array, or `null` while the pool file is quarantined @@ -265,7 +264,9 @@ export async function readPoolFile(poolPath: string = HARPER_LOOPBACK_POOL_PATH) } quarantinedPoolPaths.delete(poolPath); console.warn(`[loopback-pool] ${poolPath} has been unusable for ${Math.round(unmodifiedMs / 1000)}s (${problem}); reinitializing.`); - return emptyPool(); + const pool = emptyPool(); + await writePoolFile(pool, poolPath); + return pool; } let pendingWriteCounter = 0; @@ -274,8 +275,8 @@ let pendingWriteCounter = 0; * Writes the loopback pool to the pool file as JSON. * * Publishes by write-to-temp-then-rename, so readers only ever see the old complete file or - * the new complete file. The pending name must be unpredictable, not just unique — a fixed - * or guessable one could be pre-planted as a symlink. + * the new complete file. The pending name must be unpredictable, not just unique: `wx` refuses + * a file already at that name, so a guessable one could be pre-planted to make every write fail. * * @param pool The loopback pool array to persist * @param poolPath The pool file path (overridable for tests; defaults to the shared pool file) @@ -649,13 +650,15 @@ export async function releaseAllLoopbackAddressesForCurrentProcess(): Promise 0 && pool.every((slot) => slot === null), `aged ${JSON.stringify(content)} should reinitialize`); + deepStrictEqual(JSON.parse(await readFile(poolPath, 'utf-8')), pool, 'the reinitialized pool should be published'); } }); }); From 87d8b7a3be93edfbfb0835b2626b41e79f7f65ad Mon Sep 17 00:00:00 2001 From: Kris Zyp Date: Thu, 1 Oct 2026 11:13:04 -0600 Subject: [PATCH 7/8] chore: trim pool comments that restate the code Co-Authored-By: Claude Opus 5.5 Dispatch-Task: fix-kriszyp_integration-testing_32-b8360e96 --- src/loopbackAddressPool.ts | 16 +++++++--------- test/loopbackAddressPool.test.ts | 7 +++---- 2 files changed, 10 insertions(+), 13 deletions(-) diff --git a/src/loopbackAddressPool.ts b/src/loopbackAddressPool.ts index bcafd81..b7c107e 100644 --- a/src/loopbackAddressPool.ts +++ b/src/loopbackAddressPool.ts @@ -219,13 +219,12 @@ function emptyPool(): LoopbackPool { * index represents a loopback address (127.0.0.2, 127.0.0.3, etc.) and the value * is either null (available) or a process PID (in use). * - * A missing file reads as an empty pool. An unusable one (unparseable, not an array, or an empty - * array) is quarantined instead: it is either still being written by a writer that doesn't publish - * by rename, or it has lost reservations whose holders may not have bound their address yet. This - * returns `null` until the file has gone UNUSABLE_POOL_QUARANTINE_MS unmodified; after that it - * publishes and returns an empty pool. A non-empty array of any other length is trusted as-is, even - * if it doesn't match this process's configured count: two processes configured with different - * counts would otherwise keep wiping each other's reservations. + * An unusable file (unparseable, not an array, or an empty array) may still be mid-write by a + * writer that doesn't publish by rename, or may have lost reservations whose holders haven't bound + * their address yet, so it reads as `null` until it has gone UNUSABLE_POOL_QUARANTINE_MS + * unmodified, and only then is replaced with an empty pool. A non-empty array of another length + * is trusted as-is: two processes configured with different counts would otherwise keep wiping + * each other's reservations. * * @param poolPath The pool file path (overridable for tests; defaults to the shared pool file) * @returns The loopback pool array, or `null` while the pool file is quarantined @@ -640,8 +639,7 @@ export async function releaseLoopbackAddress(address: string): Promise { /** * Releases all loopback addresses assigned to the current process. - * Useful for cleanup during graceful shutdown. Like releaseLoopbackAddress, writes nothing while - * the pool file is quarantined. + * Useful for cleanup during graceful shutdown. */ export async function releaseAllLoopbackAddressesForCurrentProcess(): Promise { await withLock(async () => { diff --git a/test/loopbackAddressPool.test.ts b/test/loopbackAddressPool.test.ts index 1aaeebd..b781156 100644 --- a/test/loopbackAddressPool.test.ts +++ b/test/loopbackAddressPool.test.ts @@ -99,8 +99,7 @@ async function withTempDir(body: (dir: string) => Promise): Promise const POOL_FILE_NAME = 'harper-integration-test-loopback-pool.json'; -/** Makes a file look like it was last written an hour ago, well past the quarantine. */ -async function age(path: string): Promise { +async function ageBeyondQuarantine(path: string): Promise { const anHourAgo = new Date(Date.now() - 3600000); await utimes(path, anHourAgo, anHourAgo); } @@ -269,7 +268,7 @@ test('readPoolFile quarantines an unusable pool file, then reinitializes it once await writeFile(poolPath, content); strictEqual(await readPoolFile(poolPath), null, `fresh ${JSON.stringify(content)} should be quarantined`); - await age(poolPath); + await ageBeyondQuarantine(poolPath); const pool = await readPoolFile(poolPath); ok(pool && pool.length > 0 && pool.every((slot) => slot === null), `aged ${JSON.stringify(content)} should reinitialize`); deepStrictEqual(JSON.parse(await readFile(poolPath, 'utf-8')), pool, 'the reinitialized pool should be published'); @@ -315,7 +314,7 @@ test('a reservation lost to an unusable pool file is not reissued until the quar await pool.releaseAllLoopbackAddressesForCurrentProcess(); strictEqual(await readFile(poolPath, 'utf-8'), '[', 'releasing must not end the quarantine'); - await age(poolPath); + await ageBeyondQuarantine(poolPath); strictEqual(await Promise.race([reissued, sleep(5000).then(() => 'still waiting')]), '127.0.0.1'); } finally { // Unblocks the allocation if an assertion above failed, so the test can't hang. From 66408acfd20543bda660ce2083c9e8239058887a Mon Sep 17 00:00:00 2001 From: Kris Zyp Date: Tue, 6 Oct 2026 13:05:45 -0600 Subject: [PATCH 8/8] test: use nonzero loopback canary ports A port-zero TCP probe returns EADDRNOTAVAIL on macOS and Windows. Select two free ephemeral ports for isolated pool tests. Co-Authored-By: GPT-5 Codex Dispatch-Task: pr-maint-e87fb81eefc34798cd5b627901e19247 --- test/loopbackAddressPool.test.ts | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/test/loopbackAddressPool.test.ts b/test/loopbackAddressPool.test.ts index b781156..0c3ada6 100644 --- a/test/loopbackAddressPool.test.ts +++ b/test/loopbackAddressPool.test.ts @@ -106,18 +106,19 @@ async function ageBeyondQuarantine(path: string): Promise { /** * A fresh copy of the pool module whose pool and lock live in `dir` and which hands out only - * 127.0.0.1. Both canary ports are 0 (an ephemeral bind, always free) so a real Harper on this - * machine can't make the result depend on the host. + * 127.0.0.1. Both canary ports are chosen ephemerally, so a real Harper on this machine can't + * make the result depend on the host. */ async function importIsolatedPool(dir: string): Promise { + const [operationsPort, httpPort] = await twoFreePorts(); const overrides: Record = { TMPDIR: dir, TMP: dir, TEMP: dir, HARPER_INTEGRATION_TEST_LOOPBACK_POOL_START: '1', HARPER_INTEGRATION_TEST_LOOPBACK_POOL_COUNT: '1', - HARPER_INTEGRATION_TEST_CONFLICT_PROBE_PORT: '0', - HARPER_INTEGRATION_TEST_HTTP_CONFLICT_PROBE_PORT: '0', + HARPER_INTEGRATION_TEST_CONFLICT_PROBE_PORT: String(operationsPort), + HARPER_INTEGRATION_TEST_HTTP_CONFLICT_PROBE_PORT: String(httpPort), }; const saved = Object.fromEntries(Object.keys(overrides).map((name) => [name, process.env[name]])); Object.assign(process.env, overrides);