diff --git a/server/src/__tests__/workspace-runtime.test.ts b/server/src/__tests__/workspace-runtime.test.ts index 7f902c85de4..7da3f5e2447 100644 --- a/server/src/__tests__/workspace-runtime.test.ts +++ b/server/src/__tests__/workspace-runtime.test.ts @@ -61,7 +61,13 @@ import { buildWorkspaceRealizationRequest, readWorkspaceRealizationRequest, } from "../services/workspace-realization.ts"; -import { deriveViteHmrPort, type Environment, type EnvironmentLease } from "@paperclipai/shared"; +import { + deriveViteHmrPort, + RUNTIME_EXPOSURE_APP_PORT_MAX, + RUNTIME_EXPOSURE_APP_PORT_MIN, + type Environment, + type EnvironmentLease, +} from "@paperclipai/shared"; import { resolvePaperclipConfigPath } from "../paths.ts"; import type { WorkspaceOperation } from "@paperclipai/shared"; import type { WorkspaceOperationRecorder } from "../services/workspace-operations.ts"; @@ -7454,6 +7460,42 @@ describeEmbeddedPostgres("workspace runtime startup reconciliation", () => { await expect(fetch(service!.url!)).rejects.toThrow(); }); + /** + * A free loopback port inside the dedicated runtime exposure app range, + * falling back to an OS-assigned ephemeral port when the whole range is + * busy on this host. + * + * The adoption test below used to reserve with `listen(0)`, and on Linux CI + * the kernel's ephemeral span contains the dedicated range, so roughly one + * run in thirty landed in it by accident — the exact shape that made the + * startup drift sweep quarantine the row instead of adopting it. Reserving + * in-range on purpose turns that once-probabilistic collision into the case + * the test always exercises. + */ + async function reserveAdoptionTestPort(): Promise { + for (let candidate = RUNTIME_EXPOSURE_APP_PORT_MIN; candidate <= RUNTIME_EXPOSURE_APP_PORT_MAX; candidate += 1) { + const probe = net.createServer(); + const bound = await new Promise((resolve) => { + probe.once("error", () => resolve(false)); + probe.listen(candidate, "127.0.0.1", () => resolve(true)); + }); + if (!bound) continue; + await new Promise((resolve, reject) => { + probe.close((error) => error ? reject(error) : resolve()); + }); + return candidate; + } + const fallback = net.createServer(); + await new Promise((resolve) => fallback.listen(0, "127.0.0.1", resolve)); + const address = fallback.address(); + const port = typeof address === "object" && address ? address.port : null; + await new Promise((resolve, reject) => { + fallback.close((error) => error ? reject(error) : resolve()); + }); + if (!port) throw new Error("Failed to reserve pnpm reconciliation test port"); + return port; + } + it("re-adopts a live service whose shell command differs from the surviving process argv", async () => { const workspaceRoot = await fs.mkdtemp(path.join(os.tmpdir(), "paperclip-runtime-pnpm-reconcile-")); const paperclipHome = await fs.mkdtemp(path.join(os.tmpdir(), "paperclip-runtime-home-")); @@ -7462,14 +7504,7 @@ describeEmbeddedPostgres("workspace runtime startup reconciliation", () => { process.env.PAPERCLIP_HOME = paperclipHome; process.env.PAPERCLIP_INSTANCE_ID = `runtime-pnpm-reconcile-${randomUUID()}`; - const portProbe = net.createServer(); - await new Promise((resolve) => portProbe.listen(0, "127.0.0.1", resolve)); - const address = portProbe.address(); - const port = typeof address === "object" && address ? address.port : null; - await new Promise((resolve, reject) => { - portProbe.close((error) => error ? reject(error) : resolve()); - }); - if (!port) throw new Error("Failed to reserve pnpm reconciliation test port"); + const port = await reserveAdoptionTestPort(); const companyId = randomUUID(); const projectId = randomUUID(); diff --git a/server/src/services/runtime-exposure/port-reservation.test.ts b/server/src/services/runtime-exposure/port-reservation.test.ts index 1ee65a21165..805a43bc6f6 100644 --- a/server/src/services/runtime-exposure/port-reservation.test.ts +++ b/server/src/services/runtime-exposure/port-reservation.test.ts @@ -320,6 +320,29 @@ describe("regression 3: Serve mapping ownership mismatch is visible and fails cl expect(drift).toEqual([]); }); + it("does not report a running pre-exposure service's own in-range port as drift", () => { + // The row is RUNNING with `exposure: null` — a pre-exposure service whose + // configured port happens to sit inside the dedicated range. At startup its + // surviving listener is unattributable (the in-memory runtime registry is + // empty), which used to read as drift and quarantine the row before the + // reconciler could adopt the service. + const drift = findExposureReservationDrift({ + persistedRows: [stoppedTornDownRow({ status: "running", exposure: null })], + livePorts: new Set([42001, 52001]), + }); + + expect(drift).toEqual([]); + }); + + it("still sweeps a stopped pre-exposure row's reserved pair", () => { + const drift = findExposureReservationDrift({ + persistedRows: [stoppedTornDownRow({ exposure: null })], + livePorts: new Set([42001]), + }); + + expect(drift).toMatchObject([{ port: 42001, reason: "live_listener", conflictingOwner: null }]); + }); + it("does not report a healthy running lane as drift", () => { const drift = findExposureReservationDrift({ persistedRows: [{ diff --git a/server/src/services/runtime-exposure/port-reservation.ts b/server/src/services/runtime-exposure/port-reservation.ts index 5fbb5659c2a..efb4da30877 100644 --- a/server/src/services/runtime-exposure/port-reservation.ts +++ b/server/src/services/runtime-exposure/port-reservation.ts @@ -185,6 +185,24 @@ function rowClaimsLiveExposure(row: PersistedExposureRowSnapshot): boolean { return Boolean(row.exposure) && row.exposure!.state !== "removed"; } +/** + * True when the row itself says nothing of its own should be listening: it is + * recorded stopped/failed, its exposure was explicitly torn down, or it never + * reached `running` and left only a reserved pair behind. + * + * A RUNNING row with no exposure state at all is the one shape that must not + * count as dormant. Pre-exposure services carry a plain `port` column with + * `exposure: null`, and when that port happens to sit inside the dedicated + * range, a dormant classification turns the row's own live listener into a + * "drift" finding — which the startup sweep then quarantines instead of + * adopting, orphaning a healthy surviving service. + */ +export function isDormantExposureRow(row: PersistedExposureRowSnapshot): boolean { + if (row.status === "stopped" || row.status === "failed") return true; + if (row.exposure) return row.exposure.state === "removed"; + return row.status !== "running"; +} + function rowIdentity(row: PersistedExposureRowSnapshot): ExposureOwnerIdentity { return { runtimeServiceId: row.id, @@ -459,8 +477,7 @@ export function findExposureReservationDrift( const drift: ExposureReservationDrift[] = []; for (const row of rows) { - const dormant = row.status === "stopped" || row.status === "failed" || !rowClaimsLiveExposure(row); - if (!dormant) continue; + if (!isDormantExposureRow(row)) continue; const owner = rowIdentity(row); for (const port of collectRowExposurePorts(row)) { const mappingOwner = mappingOwnerByPort.get(port) ?? null; diff --git a/server/src/services/workspace-runtime.ts b/server/src/services/workspace-runtime.ts index ade07864892..0e9983f902c 100644 --- a/server/src/services/workspace-runtime.ts +++ b/server/src/services/workspace-runtime.ts @@ -89,6 +89,7 @@ import { ExposurePortPairClaims, findExposurePairConflict, findExposureReservationDrift, + isDormantExposureRow, isExposureAdoptionPermitted, type BrokerMappingSnapshot, type ExposureOwnerIdentity, @@ -4066,13 +4067,12 @@ async function detectPersistedExposureReservationDrift(input: { issueId: row.issueId, })); - // Probe only the ports dormant rows actually reserve; a startup sweep must not - // walk the whole dedicated range. + // Probe only the ports dormant rows actually reserve; a startup sweep must + // not walk the whole dedicated range, and a running pre-exposure service's + // own port is not a reservation to police at all. const candidatePorts = new Set(); for (const row of snapshots) { - if (row.status !== "stopped" && row.status !== "failed" && row.exposure && row.exposure.state !== "removed") { - continue; - } + if (!isDormantExposureRow(row)) continue; for (const port of collectRowExposurePorts(row)) candidatePorts.add(port); }