diff --git a/packages/lib/src/services/sandbox/preview/__tests__/dev-preview-core.test.ts b/packages/lib/src/services/sandbox/preview/__tests__/dev-preview-core.test.ts index 3e997799b8..b9c6cfb410 100644 --- a/packages/lib/src/services/sandbox/preview/__tests__/dev-preview-core.test.ts +++ b/packages/lib/src/services/sandbox/preview/__tests__/dev-preview-core.test.ts @@ -4,6 +4,7 @@ import type { SandboxServiceInfo } from '../../sandbox-host'; import { classifyDetectedDevServer, isHttpPortSlotFree, + describeHttpPortSlot, planDevServerService, describeServiceState, resolveDevPreviewHolder, @@ -183,12 +184,30 @@ describe('isHttpPortSlotFree — "is 8080 free: no relay, no user process"', () expected: false, }); assert({ - given: 'a listener on 8080 whose pid differs from the running relay', - should: 'not be free', - actual: isHttpPortSlotFree({ listeners: [{ port: 8080, pid: 9 }], relay: relayService(5173, { pid: 42 }) }), + given: 'a PROBED listener on 8080 whose pid differs from the running relay', + should: 'not be free — a fresh pid that is not the relay is a user process', + actual: isHttpPortSlotFree({ listeners: [{ port: 8080, pid: 9 }], relay: relayService(5173, { pid: 42 }), listenerSource: 'probe' }), expected: false, }); }); + + it('a running relay owns 8080 whatever a WATCH pid says — that pid can be stale', () => { + // Production, first re-pick: the relay restarted as pid 4698, the watch + // snapshot still said pid 11264, and the pane read "held by another + // process (pid 11264)". The channel never reports a re-bind it missed. + assert({ + given: 'relay running as pid 4698, watch snapshot says pid 11264 on 8080', + should: 'attribute the slot to the relay', + actual: describeHttpPortSlot({ listeners: [{ port: 8080, pid: 11264 }], relay: relayService(3000, { pid: 4698 }) }), + expected: 'relay', + }); + assert({ + given: 'the same disagreement from a PROBE', + should: 'attribute it to a user process', + actual: describeHttpPortSlot({ listeners: [{ port: 8080, pid: 11264 }], relay: relayService(3000, { pid: 4698 }), listenerSource: 'probe' }), + expected: 'user-process', + }); + }); }); describe('planDevServerService', () => { @@ -235,13 +254,19 @@ describe('planDevServerService', () => { }); }); - it('refuses when a foreign pid holds 8080 even though the relay claims to be running', () => { + it('refuses when a PROBED foreign pid holds 8080 even though the relay claims to be running — but not on a watch pid', () => { assert({ - given: 'relay running as pid 42, but pid 9 listens on 8080', + given: 'relay running as pid 42, but a probe says pid 9 listens on 8080', should: 'refuse with http-port-busy — the relay is not the one being served', - actual: planDevServerService(planInput({ detected: detected(5173), row: relayRow(5173), relay: relayService(5173, { pid: 42 }), listeners: [{ port: 8080, pid: 9 }] })), + actual: planDevServerService(planInput({ detected: detected(5173), row: relayRow(5173), relay: relayService(5173, { pid: 42 }), listeners: [{ port: 8080, pid: 9 }], listenerSource: 'probe' })), expected: { action: 'refuse', reason: 'http-port-busy', targetPort: 5173 }, }); + assert({ + given: 'the same disagreement from the watch snapshot', + should: 'NOT refuse — the pid may be the relay\'s previous incarnation', + actual: planDevServerService(planInput({ detected: detected(5173), row: relayRow(5173), relay: relayService(5173, { pid: 42 }), listeners: [{ port: 8080, pid: 9 }] })).action, + expected: 'none', + }); }); it('does nothing when the identical relay is already live and recorded', () => { @@ -526,11 +551,17 @@ describe('describeServiceState', () => { expected: { status: 'blocked', targetPort: 5173, message: HTTP_PORT_BUSY_MESSAGE }, }); assert({ - given: 'a relay row, relay RUNNING as pid 42, pid 9 on 8080', - should: 'still be blocked — a running relay that lost the bind is not serving', - actual: describeServiceState({ liveInstanceId: INSTANCE, row: relayRow(5173), relay: relayService(5173, { pid: 42 }), listeners: [{ port: 8080, pid: 9 }] }).status, + given: 'a relay row, relay RUNNING as pid 42, a PROBE says pid 9 on 8080', + should: 'still be blocked — a fresh pid that is not the relay is a stranger on the slot', + actual: describeServiceState({ liveInstanceId: INSTANCE, row: relayRow(5173), relay: relayService(5173, { pid: 42 }), listeners: [{ port: 8080, pid: 9 }], listenerSource: 'probe' }).status, expected: 'blocked', }); + assert({ + given: 'the same, but the pid comes from the WATCH snapshot', + should: 'NOT be blocked — a relay that lost its bind exits and stops being running; a stale watch pid is the relay\'s previous incarnation (production, first re-pick)', + actual: describeServiceState({ liveInstanceId: INSTANCE, row: relayRow(5173), relay: relayService(5173, { pid: 42 }), listeners: [{ port: 8080, pid: 9 }] }).status, + expected: 'live', + }); }); it('describes a direct 8080 preview from the listener snapshot alone', () => { diff --git a/packages/lib/src/services/sandbox/preview/dev-preview-core.ts b/packages/lib/src/services/sandbox/preview/dev-preview-core.ts index db4420c319..f871000ac5 100644 --- a/packages/lib/src/services/sandbox/preview/dev-preview-core.ts +++ b/packages/lib/src/services/sandbox/preview/dev-preview-core.ts @@ -304,6 +304,15 @@ export interface HttpPortSlotInput { listeners: readonly ListeningPort[]; /** The relay service as reported now, or null when none is defined. */ relay: SandboxServiceInfo | null; + /** + * Where `listeners` came from (see {@link ListenerSource}). Decides whether + * a listener's PID may contradict a running relay: a `watch` pid can be + * STALE — the relay restarted under a new pid and the channel never said + * so (seen in production: the first re-pick rendered a healthy relay as + * "held by another process (pid )") — so only a fresh `probe` + * pid may. Defaults to `'watch'`. + */ + listenerSource?: ListenerSource; } /** @@ -350,18 +359,23 @@ export type HttpPortSlotHolder = 'none' | 'relay' | 'user-process'; * `=== 'none'`; the UI asks this fuller form so it can say which of the two * holders is in the way and how to release it, instead of surfacing a 409. * - * A listener whose pid matches a live relay — or whose pid is unknown while a - * relay is live — is the relay; any other listener is a user process. A live - * relay with no listener in the snapshot still holds the slot (the snapshot - * may predate its bind). Misattributing a user process to the relay is - * self-correcting: the planned relay fails to bind, lands in `failed`, and - * the next call sees a non-live relay beside a listener. + * A live relay holds the slot: a `running` relay bound 8080 or it would have + * failed, so a listener beside it is the relay itself — even when the + * snapshot's pid disagrees, because a `watch` pid may be stale (the relay + * restarted; the channel never reported it). Only a fresh `probe` pid that + * differs from the relay's names a user process. No live relay and a + * listener is a user process; a live relay with no listener in the snapshot + * still holds the slot (the snapshot may predate its bind). Misattributing + * a user process to the relay is self-correcting: the planned relay fails + * to bind, lands in `failed`, and the next call sees a non-live relay beside + * a listener. */ -export function describeHttpPortSlot({ listeners, relay }: HttpPortSlotInput): HttpPortSlotHolder { +export function describeHttpPortSlot({ listeners, relay, listenerSource = 'watch' }: HttpPortSlotInput): HttpPortSlotHolder { const listener = listeners.find((entry) => entry.port === SPRITE_HTTP_PORT); const relayAlive = isRelayAlive(relay); if (!listener) return relayAlive ? 'relay' : 'none'; if (!relayAlive) return 'user-process'; + if (listenerSource !== 'probe') return 'relay'; const pidsAgree = listener.pid === undefined || relay.pid === undefined || listener.pid === relay.pid; return pidsAgree ? 'relay' : 'user-process'; } @@ -632,7 +646,7 @@ export function planDevServerService(input: PlanDevServerServiceInput): DevServe }; } - if (describeHttpPortSlot({ listeners, relay }) === 'user-process') { + if (describeHttpPortSlot({ listeners, relay, listenerSource: input.listenerSource }) === 'user-process') { return { action: 'refuse', reason: 'http-port-busy', targetPort }; } @@ -788,7 +802,7 @@ export function describeServiceState({ liveInstanceId, row, relay, listeners, li const targetListening = isPortListening(listeners, row.targetPort, listenerSource); - const holder = describeHttpPortSlot({ listeners: listeners ?? [], relay }); + const holder = describeHttpPortSlot({ listeners: listeners ?? [], relay, listenerSource }); // Detected, recorded, and serving NOTHING — no relay for a non-8080 target // means the sprite URL (which routes to 8080 alone) reaches nothing. Three diff --git a/packages/lib/src/services/sandbox/preview/dev-preview-status.ts b/packages/lib/src/services/sandbox/preview/dev-preview-status.ts index 59d3366209..e16d65a1d8 100644 --- a/packages/lib/src/services/sandbox/preview/dev-preview-status.ts +++ b/packages/lib/src/services/sandbox/preview/dev-preview-status.ts @@ -223,7 +223,7 @@ export function buildDevPreviewStatus({ holder, sandbox, liveInstanceId, row, re let slot: DevPreviewSlotReport = { known: false }; if (sandbox === 'attached' && listeners !== null) { - const slotHolder = describeHttpPortSlot({ listeners, relay }); + const slotHolder = describeHttpPortSlot({ listeners, relay, listenerSource }); const listener = listeners.find((entry) => entry.port === SPRITE_HTTP_PORT); const pid = slotHolder === 'user-process' && listener?.pid !== undefined ? listener.pid : null; slot = {