From b83e14ad2cf0c1328c0bd11d24dd2f2462641aa9 Mon Sep 17 00:00:00 2001 From: Nicky Leach Date: Thu, 20 Aug 2026 12:05:35 -0700 Subject: [PATCH] fix(runtime): stop the readiness probe from stealing the guest exposure port (#11788) ## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work. > - The workspace runtime starts guest processes and exposes their ports. > - The readiness wait bound the guest port to test whether it was ready. > - That bind could take the port before the guest process used it. > - This pull request reads listener state without a competing bind and recovers from a real port collision. > - The benefit is stable runtime exposure and a clear recovery path for a genuine collision. ## Linked Issues or Issue Description **What happened?** The managed HTTPS exposure test failed intermittently with `listen EADDRINUSE` on `127.0.0.1:42000`. The readiness wait bound the guest port before the guest process could bind it. **Expected behavior** The readiness wait must not hold the guest port. The runtime must recover when an external process owns the assigned port. **Steps to reproduce** 1. Run `npx vitest run server/src/services/workspace-runtime-exposure.test.ts` from the repository root. 2. Inject a delayed guest bind and a widened readiness-probe hold. 3. Observe the port collision before this fix and the successful retry after this fix. **Paperclip version or commit** `b375bbd913cb2edc8e077f4339ce0745e53bd462` **Deployment mode** Built from source with the server test suite. **Installation method** Built from source with pnpm. **Agent adapter(s) involved** Not adapter-specific. This is a core runtime test. **Database mode** Not database-related. **Relevant logs or output** Before this fix, the test reported `listen EADDRINUSE: address already in use 127.0.0.1:42000`. ## What Changed - Read listener presence from `/proc` on Linux instead of binding the guest port. - Keep the bind probe as the fallback on non-Linux hosts. - Capture the current port owner when an exposed guest exits with `EADDRINUSE`. - Quarantine the app and HMR pair, then allocate the next free port pair within the existing range. - Add a deterministic regression test for quarantine, re-allocation, and self-diagnosis logging. ## Verification - Run `npx vitest run server/src/services/workspace-runtime-exposure.test.ts` from the repository root. - The target suite passes 19 tests locally. - The related runtime suites pass 105, 128, and 21 tests locally. - Run `tsc -p server/tsconfig.json` to check the changed server files. - CI must pass the general server shard and all required checks. - Greptile must report 5/5 with no open P2 comments, recommendations, or follow-ups. ## Risks The Linux readiness path now depends on `/proc` listener data. Non-Linux hosts retain the existing bind-probe fallback. The port range and allocation limit do not change. ## Model Used OpenAI GPT-5. This agent used tool calls for repository checks and GitHub PR management. Priya Raman authored the code change. ## 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 --- .../workspace-runtime-exposure.test.ts | 295 ++++++++++++++++++ server/src/services/workspace-runtime.ts | 220 ++++++++++++- 2 files changed, 508 insertions(+), 7 deletions(-) diff --git a/server/src/services/workspace-runtime-exposure.test.ts b/server/src/services/workspace-runtime-exposure.test.ts index 9032eae8ff..d82382c2d4 100644 --- a/server/src/services/workspace-runtime-exposure.test.ts +++ b/server/src/services/workspace-runtime-exposure.test.ts @@ -17,6 +17,8 @@ import { readListenerBindFacts, } from "./runtime-exposure/loopback-listener.js"; import { + classifyExposureHostCollisions, + type ExposurePortHostState, resetRuntimeServicesForTests, setWorkspaceRuntimeExposureDepsForTests, startRuntimeServicesForWorkspaceControl, @@ -121,6 +123,58 @@ for (const q of [p, p + 10000]) { setInterval(() => {}, 1000); `; +/** + * A guest that prints a synthetic `EADDRINUSE` line for its assigned port with no + * host listener behind it, then exits. It models guest-controlled output that + * claims a collision the host cannot confirm. The runtime must not quarantine the + * pair on the printed line alone. Otherwise repeated starts drain the shared + * exposure-port pool. The start must surface the failure terminally after a single + * allocation. + */ +const SYNTHETIC_EADDRINUSE_ON_BASE_PORT_GUEST = ` +import http from "node:http"; +const p = Number(process.env.PORT); +if (p === 42000) { + process.stderr.write("node:events:497\\nError: listen EADDRINUSE: address already in use 127.0.0.1:" + p + "\\n"); + process.exit(1); +} +const health = (rq, r) => { if (rq.url === "/api/health") { r.setHeader("content-type", "application/json"); r.end(JSON.stringify({ status: "ok" })); return true; } return false; }; +for (const q of [p, p + 10000]) { + http.createServer((rq, r) => { if (health(rq, r)) return; r.statusCode = 200; r.end("ok"); }).listen(q, "127.0.0.1"); +} +setInterval(() => {}, 1000); +`; + +/** + * A guest that exits with `EADDRINUSE` on an unrelated auxiliary port, never on + * its assigned app or HMR port. It models a fixed helper listener (for example an + * inspector or metrics port) that an external process already holds. The managed + * start must not quarantine the valid exposure pair; it must surface the failure + * terminally after a single allocation. + */ +const EADDRINUSE_ON_AUXILIARY_PORT_GUEST = ` +import http from "node:http"; +process.stderr.write("node:events:497\\nError: listen EADDRINUSE: address already in use 127.0.0.1:39999\\n"); +process.exit(1); +`; + +/** + * A guest that fails with `EADDRINUSE` on an unrelated auxiliary port, and also + * prints the assigned app port on a separate, benign line. It models mixed + * startup output: one line names the assigned port for an informational reason, + * a different line reports the auxiliary-port conflict. The parser must match the + * error and the port on the same line, so the benign mention of the assigned port + * must not trigger a wrong quarantine. The managed start must surface the failure + * terminally after a single allocation. + */ +const EADDRINUSE_ON_AUXILIARY_PORT_WITH_ASSIGNED_MENTION_GUEST = ` +import http from "node:http"; +const p = Number(process.env.PORT); +process.stderr.write("[dev] server ready on http://127.0.0.1:" + p + "/\\n"); +process.stderr.write("node:events:497\\nError: listen EADDRINUSE: address already in use 127.0.0.1:39999\\n"); +process.exit(1); +`; + beforeAll(async () => { guestDir = await fs.mkdtemp(path.join(os.tmpdir(), "pap-17256-guest-")); await fs.writeFile(path.join(guestDir, "dev-runner.mjs"), PRE_MANAGED_EXPOSURE_GUEST); @@ -130,6 +184,28 @@ beforeAll(async () => { path.join(guestDir, "dev-runner-bind-conflict.mjs"), 'process.stderr.write("local_trusted requires server.bind=loopback\\n"); process.exit(1);\n', ); + // A guest that prints a synthetic EADDRINUSE line for its assigned port with no + // host listener behind it. It models guest-controlled output that claims a + // collision the host cannot confirm. The start must not quarantine the pair. + await fs.writeFile( + path.join(guestDir, "dev-runner-eaddrinuse-synthetic.mjs"), + SYNTHETIC_EADDRINUSE_ON_BASE_PORT_GUEST, + ); + // A guest that fails on a fixed auxiliary port, not on its assigned app or HMR + // port. It models an unrelated helper listener that an external process holds. + // The assigned exposure pair stays valid, so the start must not quarantine it. + await fs.writeFile( + path.join(guestDir, "dev-runner-eaddrinuse-auxiliary.mjs"), + EADDRINUSE_ON_AUXILIARY_PORT_GUEST, + ); + // A guest that fails on an auxiliary port and also prints the assigned app port + // on a separate, benign line. It models mixed output where the assigned port + // appears for an unrelated reason. The parser must match the error and the port + // on the same line, so the start must not quarantine the valid exposure pair. + await fs.writeFile( + path.join(guestDir, "dev-runner-eaddrinuse-auxiliary-mixed.mjs"), + EADDRINUSE_ON_AUXILIARY_PORT_WITH_ASSIGNED_MENTION_GUEST, + ); }); afterAll(async () => { @@ -671,3 +747,222 @@ describe("the deployed failure shape: loopback app port, wildcard HMR (PAP-17256 expect(calls).toEqual(["reserve", "remove"]); }, 20_000); }); + +describe("recovers when a guest loses its assigned exposure port during startup (PAP-17256)", () => { + // The quarantine decision itself is unit-tested through + // `classifyExposureHostCollisions` below. A deterministic end-to-end quarantine + // test is not reachable here: a real host listener that holds the assigned port + // is intercepted earlier, either by the pre-spawn ownership guard or by the + // allocated-port bind wait, before the EADDRINUSE-text quarantine branch runs. + it("does not quarantine the pair when the guest fabricates an assigned-port EADDRINUSE with no host listener", async () => { + const { broker } = createBroker(); + const reservedAppPorts: number[] = []; + const recordingBroker: BrokerClient = { + ...broker, + async reserve(runtimeId, requested) { + reservedAppPorts.push(requested[0]!.port); + return broker.reserve(runtimeId, requested); + }, + }; + installDeps({ broker: recordingBroker }); + + const logs: string[] = []; + const error = await startRuntimeServicesForWorkspaceControl({ + ...startInput({ + serviceName: "paperclip-dev", + command: `${guestCommand("dev-runner-eaddrinuse-synthetic.mjs")} --bind lan`, + expose: LEGACY_HTTP_EXPOSE, + port: { type: "auto", envKey: "PORT" }, + }), + onLog: async (_stream, chunk) => { + logs.push(chunk); + }, + }).then(() => null, (err: unknown) => err as Error); + + // No host listener owns 42000, so the printed EADDRINUSE line is unverified. + // The start fails terminally after ONE allocation and never burns the pool. + expect(error).not.toBeNull(); + expect(error!.message).toContain("42000"); + expect(error!.message).toContain("not verified"); + expect(reservedAppPorts).toEqual([42_000]); + + // No quarantine and no re-allocation happened for the unverified claim. + const diagnosis = logs.join(""); + expect(diagnosis).not.toContain("Quarantined pair"); + expect(diagnosis).not.toContain("collided during startup"); + }, 25_000); + + it("does not quarantine the pair for an EADDRINUSE on an unrelated auxiliary port", async () => { + const { broker } = createBroker(); + const reservedAppPorts: number[] = []; + const recordingBroker: BrokerClient = { + ...broker, + async reserve(runtimeId, requested) { + reservedAppPorts.push(requested[0]!.port); + return broker.reserve(runtimeId, requested); + }, + }; + installDeps({ broker: recordingBroker }); + + const logs: string[] = []; + const error = await startRuntimeServicesForWorkspaceControl({ + ...startInput({ + serviceName: "paperclip-dev", + command: `${guestCommand("dev-runner-eaddrinuse-auxiliary.mjs")} --bind lan`, + expose: LEGACY_HTTP_EXPOSE, + port: { type: "auto", envKey: "PORT" }, + }), + onLog: async (_stream, chunk) => { + logs.push(chunk); + }, + }).then(() => null, (err: unknown) => err as Error); + + // The failure names an unrelated port, so the assigned pair is not a + // collision. The start fails terminally after ONE allocation, and never + // burns the bounded retries on a valid pair. + expect(error).not.toBeNull(); + expect(error!.message).toContain("39999"); + expect(reservedAppPorts).toEqual([42_000]); + + // No quarantine and no re-allocation happened for the auxiliary conflict. + const diagnosis = logs.join(""); + expect(diagnosis).not.toContain("Quarantined pair"); + expect(diagnosis).not.toContain("collided during startup"); + }, 25_000); + + it("does not quarantine when an auxiliary EADDRINUSE mixes with a benign assigned-port line", async () => { + const { broker } = createBroker(); + const reservedAppPorts: number[] = []; + const recordingBroker: BrokerClient = { + ...broker, + async reserve(runtimeId, requested) { + reservedAppPorts.push(requested[0]!.port); + return broker.reserve(runtimeId, requested); + }, + }; + installDeps({ broker: recordingBroker }); + + const logs: string[] = []; + const error = await startRuntimeServicesForWorkspaceControl({ + ...startInput({ + serviceName: "paperclip-dev", + command: `${guestCommand("dev-runner-eaddrinuse-auxiliary-mixed.mjs")} --bind lan`, + expose: LEGACY_HTTP_EXPOSE, + port: { type: "auto", envKey: "PORT" }, + }), + onLog: async (_stream, chunk) => { + logs.push(chunk); + }, + }).then(() => null, (err: unknown) => err as Error); + + // The assigned port 42000 appears on a benign line, but EADDRINUSE names only + // the auxiliary port 39999. The parser matches the error and the port on the + // same line, so the assigned pair is not a collision. The start fails + // terminally after ONE allocation and never quarantines the valid pair. + expect(error).not.toBeNull(); + expect(error!.message).toContain("39999"); + expect(reservedAppPorts).toEqual([42_000]); + + const diagnosis = logs.join(""); + expect(diagnosis).not.toContain("Quarantined pair"); + expect(diagnosis).not.toContain("collided during startup"); + }, 25_000); +}); + +describe("classifyExposureHostCollisions gates quarantine on verified host state", () => { + const state = (over: Partial & { port: number }): ExposurePortHostState => ({ + named: false, + listenerPresent: false, + ownerPid: null, + ownerProcessGroupId: null, + ...over, + }); + + it("reports a host collision when a named port has a present listener owned by another process", () => { + const result = classifyExposureHostCollisions({ + childPid: 4242, + ports: [ + state({ port: 42_000, named: true, listenerPresent: true, ownerPid: 9999 }), + state({ port: 52_000, named: false, listenerPresent: false, ownerPid: null }), + ], + }); + expect(result).toEqual({ hostCollisionPorts: [42_000], hostCollision: true }); + }); + + it("treats a present listener with an unknown owner as a host collision", () => { + // /proc proves a listener, and a guest that lost its assigned port does not + // hold it, so an unknown owner still counts as the host. + const result = classifyExposureHostCollisions({ + childPid: 4242, + ports: [state({ port: 42_000, named: true, listenerPresent: true, ownerPid: null })], + }); + expect(result).toEqual({ hostCollisionPorts: [42_000], hostCollision: true }); + }); + + it("does not report a collision when the named port has no listener", () => { + // A synthetic EADDRINUSE line with no host listener behind it. Quarantine here + // would drain the shared exposure-port pool across repeated starts. + const result = classifyExposureHostCollisions({ + childPid: 4242, + ports: [state({ port: 42_000, named: true, listenerPresent: false, ownerPid: null })], + }); + expect(result).toEqual({ hostCollisionPorts: [], hostCollision: false }); + }); + + it("does not report a collision when the guest itself owns the present listener", () => { + // The guest bound and holds its own port, so this is not a host collision. + const result = classifyExposureHostCollisions({ + childPid: 4242, + ports: [state({ port: 42_000, named: true, listenerPresent: true, ownerPid: 4242 })], + }); + expect(result).toEqual({ hostCollisionPorts: [], hostCollision: false }); + }); + + it("does not report a collision when a guest descendant owns the present listener", () => { + // The runtime launches the guest as a shell process group leader (pid 4242). + // The real dev server binds the port from a descendant (pid 9999) that shares + // the shell process group. The owner pid differs from the shell pid, but the + // owner process group id matches it, so this is the guest, not the host. A raw + // pid equality would quarantine this valid pair until the shared pool drains. + const result = classifyExposureHostCollisions({ + childPid: 4242, + ports: [ + state({ + port: 42_000, + named: true, + listenerPresent: true, + ownerPid: 9999, + ownerProcessGroupId: 4242, + }), + ], + }); + expect(result).toEqual({ hostCollisionPorts: [], hostCollision: false }); + }); + + it("reports a host collision when the owner and its process group are both external", () => { + // A different process group holds the port, so neither the shell pid nor the + // process group matches. This is a real external owner and quarantine is right. + const result = classifyExposureHostCollisions({ + childPid: 4242, + ports: [ + state({ + port: 42_000, + named: true, + listenerPresent: true, + ownerPid: 9999, + ownerProcessGroupId: 8888, + }), + ], + }); + expect(result).toEqual({ hostCollisionPorts: [42_000], hostCollision: true }); + }); + + it("ignores a present listener on a port the failure text did not name", () => { + // An auxiliary-port conflict leaves the assigned pair valid. + const result = classifyExposureHostCollisions({ + childPid: 4242, + ports: [state({ port: 42_000, named: false, listenerPresent: true, ownerPid: 9999 })], + }); + expect(result).toEqual({ hostCollisionPorts: [], hostCollision: false }); + }); +}); diff --git a/server/src/services/workspace-runtime.ts b/server/src/services/workspace-runtime.ts index ade0786489..f305aec894 100644 --- a/server/src/services/workspace-runtime.ts +++ b/server/src/services/workspace-runtime.ts @@ -54,6 +54,7 @@ import { isLocalServiceProcessInWorkspace, openLocalServiceLogFile, readLocalServiceProcessCwd, + readLocalServiceProcessGroupId, readLocalServicePortOwner, removeLocalServiceRegistryRecord, terminateLocalService, @@ -79,7 +80,7 @@ import { reserveExposure, type ExposureManagerDeps, } from "./runtime-exposure/exposure-manager.js"; -import { diagnoseRuntimeListenerBinds } from "./runtime-exposure/loopback-listener.js"; +import { diagnoseRuntimeListenerBinds, readListenerBindFacts } from "./runtime-exposure/loopback-listener.js"; import { allocateExposurePortPair } from "./runtime-exposure/port-pair.js"; import { buildExposureReservationLedger, @@ -266,11 +267,28 @@ const DEFAULT_TAILSCALE_BROKER_SOCKET = "/run/paperclip-tailscale-broker/broker. class RuntimeServicePortBindCollision extends Error { readonly port: number; + /** + * Who held the port when the collision was seen, captured at failure time. + * Null when no owner remained (a transient racer that already released it). + */ + readonly diagnosis: string | null; - constructor(port: number) { - super(`Runtime service could not bind allocated port ${port}`); + /** + * True when the port is only a preference and the caller may re-allocate a + * different one. Exposed runtimes always draw from the dedicated broker range, + * so a collision on the assigned port is recoverable by taking the next pair. + */ + readonly exposureReallocatable: boolean; + + constructor(port: number, diagnosis: string | null = null, exposureReallocatable = false) { + super( + `Runtime service could not bind allocated port ${port}` + + (diagnosis ? ` (${diagnosis})` : ""), + ); this.name = "RuntimeServicePortBindCollision"; this.port = port; + this.diagnosis = diagnosis; + this.exposureReallocatable = exposureReallocatable; } } @@ -4241,6 +4259,102 @@ async function canBindRuntimePort(port: number): Promise { }); } +/** + * True when a loopback listener is present on the port, read WITHOUT binding it. + * + * The readiness wait must never bind the exact port the guest is about to bind. + * A probe `listen()` holds the port for the length of one bind/close, and if the + * guest's own `listen()` lands in that window the guest fails with EADDRINUSE on + * its assigned port. That self-inflicted race is the runtime exposure port flake + * (a slow guest under load loses the race to the parent probe). A `/proc` read + * carries the same "a listener appeared" signal with no bind. Where `/proc` is + * absent (non-Linux dev hosts), fall back to the bind probe; those hosts do not + * run the concurrent managed lanes that expose the race. + */ +async function hasLoopbackPortListener(port: number): Promise { + const facts = await readListenerBindFacts(port).catch(() => null); + if (facts) return facts.present; + return !(await canBindRuntimePort(port)); +} + +/** + * True when one line of the failure text reports an EADDRINUSE bind conflict on + * the given port. + * + * Node prints the failing bind on a single line, for example + * `Error: listen EADDRINUSE: address already in use 127.0.0.1:42000`. The match + * requires the EADDRINUSE marker and the port on the SAME line. So an + * auxiliary-port conflict on one line cannot combine with an unrelated + * assigned-port mention on a different, benign line and trigger a wrong + * quarantine. + * + * Node formats a bind address as `host:port`, so the failing port always + * follows a colon (for example `127.0.0.1:42000` or `:::42000`). The match + * requires that colon and a full-number boundary. So a different port in the + * same line cannot look like the assigned app or HMR port, and port 4200 never + * matches `:42000`. + */ +function eaddrinuseTextNamesPort(text: string, port: number): boolean { + const eaddrinusePattern = /EADDRINUSE|address already in use/i; + const portPattern = new RegExp(`:${port}(?![0-9])`); + return text + .split(/\r?\n/) + .some((line) => eaddrinusePattern.test(line) && portPattern.test(line)); +} + +/** Live host listener state for one assigned exposure port, read after a failure. */ +export interface ExposurePortHostState { + port: number; + /** True when the failure text reports EADDRINUSE for this port on one line. */ + named: boolean; + /** True when a real listener holds the port now (from /proc or lsof). */ + listenerPresent: boolean; + /** The listener pid, or null when unknown. */ + ownerPid: number | null; + /** The process group id of the listener owner, or null when unknown. */ + ownerProcessGroupId: number | null; +} + +/** + * Decide which assigned exposure ports a real host listener owns after a guest + * start failure. + * + * The guest owns its own output, so an assigned-port EADDRINUSE line is a claim, + * not proof. A managed guest can print a synthetic EADDRINUSE line for its + * assigned port with no host listener behind it. A quarantine on that text alone + * would drain the shared exposure-port pool across repeated starts. So a port + * counts as a host collision only when all of the following hold: + * - the failure text names the port with EADDRINUSE, and + * - a real listener is present on the port now, and + * - the listener owner is not the guest process the runtime just launched, nor a + * descendant of it. + * + * The runtime launches the guest as a shell process group leader, so the real dev + * server usually binds the port from a descendant, not the shell pid itself. The + * ownership test therefore matches the shell pid against both the owner pid and + * the owner process group id. This is the same process-group attribution that + * `isLocalServiceProcessOwnedBy` applies on the host platform. A raw pid equality + * would treat a guest descendant as an external owner and quarantine a valid pair. + * + * An unknown owner with a present listener counts as a host owner: the /proc read + * proves a listener, and a guest that lost its assigned port does not hold it. + */ +export function classifyExposureHostCollisions(input: { + childPid: number | null; + ports: ExposurePortHostState[]; +}): { hostCollisionPorts: number[]; hostCollision: boolean } { + const hostCollisionPorts: number[] = []; + for (const state of input.ports) { + const guestOwnsPort = + input.childPid != null && + (state.ownerPid === input.childPid || state.ownerProcessGroupId === input.childPid); + if (state.named && state.listenerPresent && !guestOwnsPort) { + hostCollisionPorts.push(state.port); + } + } + return { hostCollisionPorts, hostCollision: hostCollisionPorts.length > 0 }; +} + async function readReservedRuntimePorts(input: { db?: Db; ports: number[]; @@ -4717,9 +4831,10 @@ async function waitForAllocatedPortBind(input: { return; } - // A failed bind probe proves only that some listener appeared. If listener ownership cannot + // A present listener proves only that some listener appeared. If listener ownership cannot // be attributed to this child after a stability delay, retry instead of accepting a sibling. - if (!(await canBindRuntimePort(input.port))) { + // The presence read never binds the port, so it cannot steal the port from the child. + if (await hasLoopbackPortListener(input.port)) { await delay(250); if (input.child.exitCode !== null || input.child.signalCode !== null) { throw new Error("service process exited after losing its allocated port"); @@ -5950,6 +6065,67 @@ async function spawnLocalRuntimeService(input: StartLocalRuntimeServiceInput): P && /(?:EADDRINUSE|address already in use)/i.test(`${failureMessage}\n${serviceOutputExcerpt}`), ) ); + // An exposed guest that exits with EADDRINUSE on its ASSIGNED port lost the + // port after allocation. Only the assigned app or HMR port is re-allocatable + // from the dedicated broker range, so quarantine and re-allocation apply only + // when the failure names one of those ports. An unrelated auxiliary-port + // conflict (a different port the guest also bound) leaves the valid pair + // intact and surfaces as a terminal error, not a quarantine that burns the + // bounded retries. + const exposureAssignedPorts: number[] = + exposureConfig && port + ? [port, ...(exposureConfig.includePaperclipViteHmr ? [deriveViteHmrPort(port)] : [])] + : []; + const collisionText = `${failureMessage}\n${serviceOutputExcerpt}`; + const exposureNamedPorts = exposureAssignedPorts.filter((candidate) => + eaddrinuseTextNamesPort(collisionText, candidate), + ); + // The guest owns its own output, so an assigned-port EADDRINUSE line is a + // claim, not proof. A managed guest can print a synthetic EADDRINUSE line for + // its assigned port with no host listener behind it. A quarantine on that text + // alone would burn the shared exposure-port pool across repeated starts. So the + // runtime reads live host listener state and quarantines only after it confirms + // that a real host listener owns the port. + const exposureTextNamesAssignedPort = Boolean( + exposureConfig && port && exposureNamedPorts.length > 0, + ); + let exposureCollisionDiagnosis: string | null = null; + let exposureHostCollision = false; + if (exposureTextNamesAssignedPort) { + const facts: string[] = []; + const hostStates: ExposurePortHostState[] = []; + for (const collisionPort of exposureAssignedPorts) { + // `readLocalServicePortOwner` reads lsof and returns the listener pid, so a + // non-null pid also proves a present listener. `readListenerBindFacts` reads + // /proc for the same presence signal and the bound addresses. + const ownerPid = await readLocalServicePortOwner(collisionPort).catch(() => null); + const bind = await readListenerBindFacts(collisionPort).catch(() => null); + // Read the owner process group id too. The guest runs as a shell process + // group leader, so the real dev server usually binds the port from a + // descendant. The classify step matches the shell pid against the owner pgid + // to keep a guest descendant from looking like an external owner. + const ownerProcessGroupId = + ownerPid != null ? await readLocalServiceProcessGroupId(ownerPid).catch(() => null) : null; + hostStates.push({ + port: collisionPort, + named: exposureNamedPorts.includes(collisionPort), + listenerPresent: Boolean(bind?.present) || ownerPid != null, + ownerPid, + ownerProcessGroupId, + }); + const boundTo = bind?.present ? bind.addresses.join(", ") : "no listener"; + facts.push(`port ${collisionPort} bound to ${boundTo}, owner pid ${ownerPid ?? "none"}`); + } + exposureHostCollision = classifyExposureHostCollisions({ + childPid: child.pid ?? null, + ports: hostStates, + }).hostCollision; + exposureCollisionDiagnosis = facts.join("; "); + } + // Quarantine the whole assigned pair, not only the named port. The broker + // allocates the app and HMR ports as one unit, so a re-allocation must skip + // both to land on the next free pair. + const exposureCollisionPorts: number[] = exposureHostCollision ? exposureAssignedPorts : []; if (child.pid) { await terminateLocalService({ pid: child.pid, @@ -5967,12 +6143,37 @@ async function spawnLocalRuntimeService(input: StartLocalRuntimeServiceInput): P await persistRuntimeServiceRecord(record.db, record).catch(() => undefined); } if (bindCollision && port) throw new RuntimeServicePortBindCollision(port); + if (exposureHostCollision && port) { + // A verified host listener holds the assigned exposure port. Quarantine the + // pair so the bounded re-allocation never re-offers it, then throw a retryable + // collision. `startLocalRuntimeService` re-runs allocation, which skips the + // quarantined pair and takes the next free pair inside the dedicated range. + // This hardens a real host that races an external process for a range port. + for (const collisionPort of exposureCollisionPorts) { + quarantinedRuntimeExposurePorts.add(collisionPort); + } + if (input.onLog) { + await input.onLog( + "stderr", + `[service:${serviceName}] exposure port ${port} collided during startup (EADDRINUSE); ` + + `${exposureCollisionDiagnosis ?? "owner unavailable"}. ` + + `Quarantined pair ${exposureCollisionPorts.join("/")} and reallocating.\n`, + ).catch(() => undefined); + } + throw new RuntimeServicePortBindCollision(port, exposureCollisionDiagnosis, true); + } const deploymentBindConflict = /local_trusted requires server\.bind=loopback/i.test( `${failureMessage}\n${serviceOutputExcerpt}`, ); + // The guest reported an assigned-port EADDRINUSE, but no host listener owned + // the port. Explain that the runtime did not quarantine the pair, so a future + // occurrence needs no diagnostic cycle and the pool stays intact. + const unverifiedExposureCollision = exposureTextNamesAssignedPort && !exposureHostCollision; const actionableFailure = deploymentBindConflict ? `${failureMessage} | deployment/bind conflict: local_trusted requires server.bind=loopback; the managed runtime requested an incompatible bind mode` - : failureMessage; + : unverifiedExposureCollision + ? `${failureMessage} | exposure port collision not verified: the guest reported EADDRINUSE on assigned port ${exposureNamedPorts.join("/")}, but no host listener owns it (${exposureCollisionDiagnosis ?? "owner unavailable"}); the runtime did not quarantine the pair` + : failureMessage; throw new Error( `Failed to start runtime service "${serviceName}": ${actionableFailure}${serviceOutputExcerpt ? ` | output: ${serviceOutputExcerpt.trim()}` : ""}`, ); @@ -6075,7 +6276,12 @@ async function startLocalRuntimeService( } return started; } catch (error) { - if (!(error instanceof RuntimeServicePortBindCollision) || !retryBindCollisions) throw error; + if ( + !(error instanceof RuntimeServicePortBindCollision) + || !(retryBindCollisions || error.exposureReallocatable) + ) { + throw error; + } excludedPorts.add(error.port); started = null; }