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 <noreply@paperclip.ing>
This commit is contained in:
Nicky Leach
GitHub
parent
236588c753
commit
b872cd3d1b
|
|
@ -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<Set<number>> {
|
||||
const leased = new Set<number>();
|
||||
const instancesDir = path.join(REAL_PAPERCLIP_HOME, "instances");
|
||||
let instanceEntries: Awaited<ReturnType<typeof fs.readdir>>;
|
||||
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<ReturnType<typeof fs.readdir>>;
|
||||
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<number>): Promise<boolean> {
|
||||
if (locallyLeasedPorts.has(port)) return false;
|
||||
|
||||
const facts = await readListenerBindFacts(port);
|
||||
if (facts?.present) return false;
|
||||
|
||||
return await new Promise<boolean>((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<number>): Promise<number> {
|
||||
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
|
||||
|
|
|
|||
Loading…
Reference in New Issue