From b872cd3d1b404bdaff70af493a2973ceb7e5d6ec Mon Sep 17 00:00:00 2001 From: Nicky Leach Date: Thu, 3 Sep 2026 13:18:37 -0700 Subject: [PATCH] test(server): select exposure reservation host ports at run time (#12783) ## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - The server manages runtime exposure and host port leases for workspace services > - This test suite used fixed host port pairs inside the Linux ephemeral port range > - An unrelated short-lived socket could take one pair and cause a false test failure > - This pull request selects free host port pairs at run time and starts above the low lease lane > - The benefit is a more stable test suite with the same deterministic allocator checks ## Linked Issues or Issue Description **What happened?** The runtime exposure reservation test suite used two fixed app and HMR port pairs. These ports sit inside the Linux ephemeral port range. An unrelated socket could use a pair during the test, and the guest bind could fail with `EADDRINUSE`. **Expected behavior** The suite must select two free app and HMR port pairs before each test. It must avoid the low lease lane that a live instance can own without a listener. **Steps to reproduce** 1. Run `npx vitest run server/src/__tests__/workspace-runtime-exposure-reservation.test.ts`. 2. Start another process that briefly uses one fixed test port. 3. Observe that the guest bind can fail even when the allocator works correctly. **Paperclip version or commit** `c982003e00f4e8a325bafec3af4ddb113c0c1f8a` **Deployment mode** Local dev test run. **Installation method** Built from source with pnpm. **Agent adapter(s) involved** Not adapter-specific. This change tests the runtime exposure allocator. **Database mode** Not database-related. ## What Changed - Select two free app and HMR port pairs in `beforeEach`. - Start the scan 500 ports above the runtime exposure range minimum. - Keep the synthetic host stub limited to the selected pairs. - Keep all seven test cases and the existing lifecycle coverage. ## Verification - Run `npx vitest run server/src/__tests__/workspace-runtime-exposure-reservation.test.ts`. - Run `npx vitest run server/src/services/workspace-runtime-exposure.test.ts`. - Run `pnpm --filter @paperclipai/server exec tsc --noEmit`. - Confirm the full CI suite reaches a terminal green state. ## Risks Low risk. This change updates one test file and does not change production code. A port can still become busy after discovery and before the guest bind; the test documents this remaining race. ## Model Used OpenAI GPT-5 (`gpt-5`), tool use and code execution. ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [x] All Paperclip CI gates are green - [x] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge --------- Co-authored-by: Paperclip --- ...space-runtime-exposure-reservation.test.ts | 158 ++++++++++++++++-- 1 file changed, 146 insertions(+), 12 deletions(-) diff --git a/server/src/__tests__/workspace-runtime-exposure-reservation.test.ts b/server/src/__tests__/workspace-runtime-exposure-reservation.test.ts index a01d12c8d3..09fcf23ed7 100644 --- a/server/src/__tests__/workspace-runtime-exposure-reservation.test.ts +++ b/server/src/__tests__/workspace-runtime-exposure-reservation.test.ts @@ -18,15 +18,29 @@ * * The broker is a fake, so no Tailscale Serve state is ever read or mutated. * Allocation is driven by an injected `isPortAvailable` that models a synthetic - * host, and the only ports it will ever return are the `425xx`/`525xx` pairs - * named below — deliberately far from a canary lane on `42000`/`42001`, which - * this suite must not disturb. Guests do bind those two pairs on loopback, so - * the readiness and exposure lifecycle is exercised for real; they are reaped in - * `afterEach`. `PAPERCLIP_HOME` is redirected to a temp dir so the local-service - * registry never touches the real instance on this host. + * host, and the only ports it will ever return are two app/HMR pairs the suite + * selects at run time: `beforeEach` scans upward from + * `RUNTIME_EXPOSURE_SUITE_APP_PORT_START` and keeps the two lowest pairs that + * read free on the real loopback host at that moment. The suite never pins a + * fixed host port as a constant, so a short-lived socket that another process + * holds on the runner cannot make this suite fail. The scan never takes the + * low lane at the bottom of the dedicated range: a real instance can hold a + * pair there under an open lease with no listener bound, which is the exact + * incident this file reproduces, so a scan that starts there could seize a + * pair a live instance still owns. The scan also rejects a port that any + * local Paperclip instance's on-disk service registry still names, even with + * no listener bound — the same open-lease shape, for a real instance instead + * of this suite's own fixture, which a listener-only probe cannot see. Guests + * do bind the two selected pairs on loopback, so the readiness and exposure + * lifecycle is exercised for real; they are reaped in `afterEach`. A pair + * that reads free at discovery can still be taken by another process before + * the guest binds it; this suite does not close that window. `PAPERCLIP_HOME` + * is redirected to a temp dir so the local-service registry never touches the + * real instance on this host. */ import { randomUUID } from "node:crypto"; import fs from "node:fs/promises"; +import net from "node:net"; import os from "node:os"; import path from "node:path"; import { afterAll, afterEach, beforeAll, beforeEach, describe, expect, it } from "vitest"; @@ -41,8 +55,15 @@ import { type Db, } from "@paperclipai/db"; import { eq } from "drizzle-orm"; +import { + deriveViteHmrPort, + RUNTIME_EXPOSURE_APP_PORT_MAX, + RUNTIME_EXPOSURE_APP_PORT_MIN, + RUNTIME_EXPOSURE_HMR_PORT_OFFSET, +} from "@paperclipai/shared"; import type { BrokerClient, BrokerListenerRequest } from "../services/runtime-exposure/broker-client.js"; +import { readListenerBindFacts } from "../services/runtime-exposure/loopback-listener.js"; import { reconcilePersistedRuntimeServicesOnStartup, resetRuntimeServicesForTests, @@ -54,12 +75,116 @@ import { startEmbeddedPostgresTestDatabase, } from "./helpers/embedded-postgres.js"; -/** The pair lane B holds under an open lease. */ -const LEASED_APP_PORT = 42_501; -const LEASED_HMR_PORT = 52_501; -/** The next pair a correct allocator must relocate to. */ -const NEXT_APP_PORT = 42_502; -const NEXT_HMR_PORT = 52_502; +/** + * The first app port this suite may select. A real instance can hold a pair + * in the low lane, at `RUNTIME_EXPOSURE_APP_PORT_MIN`, under an open lease + * with no listener bound — the exact incident this file reproduces. The scan + * must start clear of that lane, so this suite never seizes a pair a live + * instance still owns. + */ +const RUNTIME_EXPOSURE_SUITE_APP_PORT_START = RUNTIME_EXPOSURE_APP_PORT_MIN + 500; + +/** + * The pair lane B holds under an open lease, and the next pair a correct + * allocator must relocate to. `beforeEach` discovers both on the real host, so + * a fixed constant here can never lose a race with a short-lived socket + * elsewhere on the runner. + */ +let LEASED_APP_PORT: number; +let LEASED_HMR_PORT: number; +let NEXT_APP_PORT: number; +let NEXT_HMR_PORT: number; + +/** + * The real, unredirected Paperclip home directory. `beforeEach` later points + * `PAPERCLIP_HOME` at a throwaway temp dir for the suite under test, so this + * must be read before that happens. Mirrors the default in + * `resolvePaperclipHomeDir`. + */ +const REAL_PAPERCLIP_HOME = process.env.PAPERCLIP_HOME?.trim() || path.join(os.homedir(), ".paperclip"); + +/** + * Every port a local Paperclip instance's on-disk service registry currently + * records, read once per case from the real, unredirected Paperclip home. A + * registry record survives a stopped process with no listener, so it can + * still name the port even after the exact "listener-free lease" condition + * this suite reproduces. A listener-only scan cannot see that: it would treat + * the port as free and let the suite's own guest bind it, which can then + * block the owning instance when it resumes. + */ +async function readLocallyLeasedPorts(): Promise> { + const leased = new Set(); + const instancesDir = path.join(REAL_PAPERCLIP_HOME, "instances"); + let instanceEntries: Awaited>; + try { + instanceEntries = await fs.readdir(instancesDir, { withFileTypes: true }); + } catch { + return leased; + } + for (const instanceEntry of instanceEntries) { + if (!instanceEntry.isDirectory()) continue; + const registryDir = path.join(instancesDir, instanceEntry.name, "runtime-services"); + let recordEntries: Awaited>; + try { + recordEntries = await fs.readdir(registryDir, { withFileTypes: true }); + } catch { + continue; + } + for (const recordEntry of recordEntries) { + if (!recordEntry.isFile() || !recordEntry.name.endsWith(".json")) continue; + try { + const raw = JSON.parse( + await fs.readFile(path.join(registryDir, recordEntry.name), "utf8"), + ) as { port?: unknown }; + if (typeof raw.port !== "number") continue; + leased.add(raw.port); + // The registry records only the app port a managed process bound. Its + // Vite HMR companion is the same fixed offset away and is leased too. + leased.add(raw.port + RUNTIME_EXPOSURE_HMR_PORT_OFFSET); + } catch { + continue; + } + } + } + return leased; +} + +/** + * A real loopback bind probe, matching production's `isLoopbackPortAvailable` + * and the identical helper in `workspace-runtime-exposure.test.ts`, plus the + * `locallyLeasedPorts` check above that a listener probe alone cannot make. + */ +async function isLoopbackPortFree(port: number, locallyLeasedPorts: Set): Promise { + if (locallyLeasedPorts.has(port)) return false; + + const facts = await readListenerBindFacts(port); + if (facts?.present) return false; + + return await new Promise((resolve) => { + const probe = net.createServer(); + probe.unref(); + probe.once("error", () => resolve(false)); + probe.listen(port, "127.0.0.1", () => { + probe.close(() => resolve(true)); + }); + }); +} + +/** + * The first app port at or above `startAt` whose HMR companion is also free on + * the real host right now, mirroring the allocator's own scan. + */ +async function findFreeExposureAppPort(startAt: number, locallyLeasedPorts: Set): Promise { + for (let appPort = startAt; appPort <= RUNTIME_EXPOSURE_APP_PORT_MAX; appPort += 1) { + if ( + await isLoopbackPortFree(appPort, locallyLeasedPorts) + && await isLoopbackPortFree(deriveViteHmrPort(appPort), locallyLeasedPorts) + ) { + return appPort; + } + } + throw new Error("no free app/HMR port pair available in the dedicated runtime exposure range"); +} const embeddedPostgresSupport = await getEmbeddedPostgresTestSupport(); @@ -175,6 +300,15 @@ const GUEST_COMMAND = }, 60_000); beforeEach(async () => { + // Discover two verified-free pairs on the real host right before each + // case runs, so the window between the probe and the guest bind stays + // as short as possible. + const locallyLeasedPorts = await readLocallyLeasedPorts(); + LEASED_APP_PORT = await findFreeExposureAppPort(RUNTIME_EXPOSURE_SUITE_APP_PORT_START, locallyLeasedPorts); + LEASED_HMR_PORT = deriveViteHmrPort(LEASED_APP_PORT); + NEXT_APP_PORT = await findFreeExposureAppPort(LEASED_APP_PORT + 1, locallyLeasedPorts); + NEXT_HMR_PORT = deriveViteHmrPort(NEXT_APP_PORT); + workspaceRoot = await fs.mkdtemp(path.join(os.tmpdir(), "pap17419-workspace-")); paperclipHome = await fs.mkdtemp(path.join(os.tmpdir(), "pap17419-home-")); // Redirect the local-service registry into a throwaway instance. Without