From 47a6c10f3c71ecbab6e2058e3e39873faef6983e Mon Sep 17 00:00:00 2001 From: miga-heygen Date: Sun, 13 Sep 2026 00:56:16 +0000 Subject: [PATCH 1/3] fix: honor explicit --port when reusing a background preview server startBackgroundPreview never compared a reuse candidate's actual bound port against the caller's explicitly requested --port before returning it, so a second `preview --port ` call could silently return an existing server on a different port instead of honoring the request. Reuses the existing PreviewServerPortMismatchError (already used by findPreviewServerForProject for the same discipline) instead of adding a new error type: an explicit --port that doesn't match the reuse candidate now throws a clear conflict rather than substituting the wrong port silently. A bare launch with no --port keeps reusing any project-matching server, unchanged. Deliberately out of scope: the interactive/embedded launch path (runEmbeddedMode -> findPortAndServe) has the same silent-substitution shape when a same-project server is found mid-scan at a port other than the one requested. That path's output isn't the JSON lifecycle schema this bug was reported against, and fixing it would mean touching the interactive dev-server bind/scan loop rather than a simple pre-return check, so it's left as a follow-up candidate. Co-Authored-By: Miguel Angel --- packages/cli/src/commands/preview.ts | 19 +++++---- .../cli/src/commands/previewLifecycle.test.ts | 31 ++++++++++++++ packages/cli/src/commands/previewLifecycle.ts | 41 +++++++++++++++---- 3 files changed, 74 insertions(+), 17 deletions(-) diff --git a/packages/cli/src/commands/preview.ts b/packages/cli/src/commands/preview.ts index 5203d7eead..3fc2b65f20 100644 --- a/packages/cli/src/commands/preview.ts +++ b/packages/cli/src/commands/preview.ts @@ -66,6 +66,7 @@ import { killOrphanedProcesses, killProcessTree } from "../utils/orphanCleanup.j import { resolveProject, resolveProjectOrThrow } from "../utils/project.js"; import { resolveAutoProxy } from "../utils/projectConfig.js"; import { studioProxyEnv } from "../utils/studioProxyEnv.js"; +import { PreviewServerPortMismatchError } from "../utils/studioSelectionClient.js"; import { listBackgroundPreviewStatuses, readBackgroundPreviewStatus, @@ -296,7 +297,7 @@ export default defineCommand({ if (args["browser-gpu"] === true) process.env.PRODUCER_BROWSER_GPU_MODE = "hardware"; if (args["browser-gpu"] === false) process.env.PRODUCER_BROWSER_GPU_MODE = "software"; const startPort = parseInt(args.port ?? "3002", 10); - const preferredContextPort = hasExplicitPreviewPort(process.argv) ? startPort : undefined; + const explicitPort = hasExplicitPreviewPort(process.argv) ? startPort : undefined; if (args.status || args.stop) { try { @@ -385,18 +386,13 @@ export default defineCommand({ json: Boolean(args.json), fields: args["context-fields"] as string | undefined, detail: args["context-detail"] as string | undefined, - ...(preferredContextPort === undefined ? {} : { preferredPort: preferredContextPort }), + preferredPort: explicitPort, }); } if (args.selection) { const project = resolveProject(args.dir); - return printCurrentSelection( - project.dir, - startPort, - Boolean(args.json), - preferredContextPort, - ); + return printCurrentSelection(project.dir, startPort, Boolean(args.json), explicitPort); } const rawArg = args.dir; @@ -504,11 +500,16 @@ export default defineCommand({ // the existing managed server resolved earlier. Only an explicit // --browser-gpu/--no-browser-gpu request authorizes replacement. browserGpuMode: args["browser-gpu"] === undefined ? undefined : browserGpuMode, + preferredPort: explicitPort, }); } catch (error) { const message = errorMessage(error); if (args.json) { - writeLifecycleJson(lifecycleFailurePayload("start", "preview-start-failed", message)); + const code = + error instanceof PreviewServerPortMismatchError + ? "preview-port-mismatch" + : "preview-start-failed"; + writeLifecycleJson(lifecycleFailurePayload("start", code, message)); } else { clack.log.error(message); } diff --git a/packages/cli/src/commands/previewLifecycle.test.ts b/packages/cli/src/commands/previewLifecycle.test.ts index c9635677f5..a24a68a9db 100644 --- a/packages/cli/src/commands/previewLifecycle.test.ts +++ b/packages/cli/src/commands/previewLifecycle.test.ts @@ -3,6 +3,7 @@ import { tmpdir } from "node:os"; import { join, resolve } from "node:path"; import { describe, expect, it, vi } from "vitest"; import type { ActiveServer } from "../server/portUtils.js"; +import { PreviewServerPortMismatchError } from "../utils/studioSelectionClient.js"; import { buildBackgroundPreviewArgs, listBackgroundPreviewStatuses, @@ -99,6 +100,36 @@ describe("background preview lifecycle", () => { expect(spawn).not.toHaveBeenCalled(); }); + it("throws a port-mismatch error when the caller explicitly requests a port the reused server isn't on", async () => { + const spawn = vi.fn(); + const scan = vi.fn(async () => [server]); + + await expect( + startBackgroundPreview(projectDir, 3002, { + scan, + spawn, + stateHome: mkdtempSync(join(tmpdir(), "hf-preview-state-")), + preferredPort: server.port + 1, + }), + ).rejects.toThrow(PreviewServerPortMismatchError); + expect(spawn).not.toHaveBeenCalled(); + }); + + it("reuses normally when the caller's explicit port matches the reused server", async () => { + const spawn = vi.fn(); + const scan = vi.fn(async () => [server]); + + const result = await startBackgroundPreview(projectDir, 3002, { + scan, + spawn, + stateHome: mkdtempSync(join(tmpdir(), "hf-preview-state-")), + preferredPort: server.port, + }); + + expect(result).toMatchObject({ type: "reused", port: server.port }); + expect(spawn).not.toHaveBeenCalled(); + }); + it("discovers managed previews outside the default port scan and removes stale records", async () => { const stateHome = mkdtempSync(join(tmpdir(), "hf-preview-state-")); const otherProjectDir = resolve("/tmp/hyperframes-preview-managed-custom-port"); diff --git a/packages/cli/src/commands/previewLifecycle.ts b/packages/cli/src/commands/previewLifecycle.ts index b892cf9c9d..5c8e68940b 100644 --- a/packages/cli/src/commands/previewLifecycle.ts +++ b/packages/cli/src/commands/previewLifecycle.ts @@ -16,6 +16,7 @@ import { dirname, join, resolve } from "node:path"; import { scanActiveServers, type ActiveServer } from "../server/portUtils.js"; import type { BrowserGpuMode } from "../browser/gpuPolicy.js"; import { isProcessDescendant, killProcessTree, processIdentity } from "../utils/orphanCleanup.js"; +import { PreviewServerPortMismatchError } from "../utils/studioSelectionClient.js"; export interface PreviewSession { pid: number; @@ -49,6 +50,8 @@ interface LifecycleDependencies { stateHome?: string; forceNew?: boolean; browserGpuMode?: BrowserGpuMode; + /** Set only when the caller explicitly passed --port, not the CLI default. */ + preferredPort?: number; } function defaultStateHome(): string { @@ -426,6 +429,34 @@ function savedOwnedPreview( return matchingServer(savedPortServers, projectDir); } +/** + * Returns a reuse result when `reusableExisting` is a valid reuse candidate, + * or `null` when the caller must fall through to a fresh launch. Throws when + * the candidate's port conflicts with an explicit --port request rather than + * silently substituting the wrong port. + */ +function reuseExistingPreview( + reusableExisting: ActiveServer | null, + dependencies: LifecycleDependencies, +): { type: "reused"; port: number; pid: number | null; logPath: string | null } | null { + if (!reusableExisting || dependencies.forceNew) return null; + // An explicit --port that doesn't match the reuse candidate is a conflict + // the caller must resolve, not a silent substitution. A bare launch has no + // preferred port, so reusing any project-matching server stays correct. + if ( + dependencies.preferredPort !== undefined && + reusableExisting.port !== dependencies.preferredPort + ) { + throw new PreviewServerPortMismatchError(dependencies.preferredPort, [reusableExisting]); + } + return { + type: "reused", + port: reusableExisting.port, + pid: reusableExisting.pid ? Number(reusableExisting.pid) : null, + logPath: null, + }; +} + export async function startBackgroundPreview( projectDir: string, startPort: number, @@ -451,14 +482,8 @@ export async function startBackgroundPreview( ? matchingServer([ownedExisting], projectDir, dependencies.browserGpuMode) : null; const reusableExisting = reusableOwned ?? (ownedExisting ? null : requestedExisting); - if (reusableExisting && !dependencies.forceNew) { - return { - type: "reused", - port: reusableExisting.port, - pid: reusableExisting.pid ? Number(reusableExisting.pid) : null, - logPath: null, - }; - } + const reused = reuseExistingPreview(reusableExisting, dependencies); + if (reused) return reused; await stopOwnedPreviewBeforeReplacement(ownedExisting, projectDir, dependencies); // Snapshot every same-project listener in the prospective launch range only // after the owned listener is gone. Readiness must identify a newly appeared From c86e5f964129b006710ef8c317408027d5d54a11 Mon Sep 17 00:00:00 2001 From: miga-heygen Date: Mon, 14 Sep 2026 18:17:56 +0000 Subject: [PATCH 2/3] fix: honor explicit --port on every background preview exit path MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `startBackgroundPreview` already refused to reuse a same-project server on the wrong port when --port was explicit, but the fresh-launch path still trusted whatever port the detached child bound. The child scans upward from --port and takes the first free port, so `--background --port N` with N busy reported `{ type: "started", port: N+1 }` and recorded ownership of it. - After the child comes up, compare its port with the explicit --port. On a mismatch, reap the wrapper, wait until the substitute stops answering, and throw `PreviewPortUnavailableError` (JSON code `preview-port-unavailable`). If the substitute never stops, fail loudly instead of reporting it. - Share one `unmetPreferredPort` predicate between the reuse and launch paths, and one `awaitServerGone` wait between the reap path and `stopBackgroundPreview`. - When several same-project servers are running, prefer the one on the explicit --port as the reuse candidate instead of the lowest port. - Map launch failures to JSON codes in one place (`backgroundStartFailureCode`). Tests: preferred port bound → reported as-is; preferred port taken → reaped and rejected with both ports, no session record; substitute that keeps serving → distinct error; bare launch keeps the next free port; explicit port picks the matching sibling. Deleting the launch-path check, the candidate ordering, or the post-kill wait each turns its test red. Co-Authored-By: Miguel Ángel --- packages/cli/src/commands/preview.ts | 16 ++- .../cli/src/commands/previewLifecycle.test.ts | 95 ++++++++++++ packages/cli/src/commands/previewLifecycle.ts | 135 ++++++++++++++---- 3 files changed, 210 insertions(+), 36 deletions(-) diff --git a/packages/cli/src/commands/preview.ts b/packages/cli/src/commands/preview.ts index 3fc2b65f20..15fc7615d6 100644 --- a/packages/cli/src/commands/preview.ts +++ b/packages/cli/src/commands/preview.ts @@ -68,6 +68,7 @@ import { resolveAutoProxy } from "../utils/projectConfig.js"; import { studioProxyEnv } from "../utils/studioProxyEnv.js"; import { PreviewServerPortMismatchError } from "../utils/studioSelectionClient.js"; import { + PreviewPortUnavailableError, listBackgroundPreviewStatuses, readBackgroundPreviewStatus, startBackgroundPreview, @@ -127,6 +128,13 @@ type CompactSelectionPayload = Pick< const DEFAULT_CONTEXT_FIELDS: ContextField[] = ["server", "selection", "lint", "capabilities"]; +/** Distinguishes an unhonoured explicit --port from a generic launch failure. */ +function backgroundStartFailureCode(error: unknown): string { + if (error instanceof PreviewServerPortMismatchError) return "preview-port-mismatch"; + if (error instanceof PreviewPortUnavailableError) return "preview-port-unavailable"; + return "preview-start-failed"; +} + export default defineCommand({ meta: { name: "preview", @@ -505,11 +513,9 @@ export default defineCommand({ } catch (error) { const message = errorMessage(error); if (args.json) { - const code = - error instanceof PreviewServerPortMismatchError - ? "preview-port-mismatch" - : "preview-start-failed"; - writeLifecycleJson(lifecycleFailurePayload("start", code, message)); + writeLifecycleJson( + lifecycleFailurePayload("start", backgroundStartFailureCode(error), message), + ); } else { clack.log.error(message); } diff --git a/packages/cli/src/commands/previewLifecycle.test.ts b/packages/cli/src/commands/previewLifecycle.test.ts index a24a68a9db..afba92cb81 100644 --- a/packages/cli/src/commands/previewLifecycle.test.ts +++ b/packages/cli/src/commands/previewLifecycle.test.ts @@ -5,6 +5,7 @@ import { describe, expect, it, vi } from "vitest"; import type { ActiveServer } from "../server/portUtils.js"; import { PreviewServerPortMismatchError } from "../utils/studioSelectionClient.js"; import { + PreviewPortUnavailableError, buildBackgroundPreviewArgs, listBackgroundPreviewStatuses, previewSessionPath, @@ -30,6 +31,28 @@ function savePreviewSession(stateHome: string): void { ); } +/** + * Launch dependencies whose detached wrapper (PID 4321) brings up `server` + * (its own PID 9876) once spawned, and takes it down again when killed. + */ +function reachableChildDependencies(stateHome: string, { immortal = false } = {}) { + let spawned = false; + let killed = false; + const liveServer = { ...server, pid: "9876" }; + return { + scan: async () => (spawned && (immortal || !killed) ? [liveServer] : []), + spawn: () => { + spawned = true; + return { pid: 4321, unref: vi.fn() }; + }, + sleep: async () => {}, + kill: vi.fn(() => { + killed = true; + }), + stateHome, + }; +} + async function expectStaleSessionRemoved(stateHome: string): Promise { const status = await readBackgroundPreviewStatus(projectDir, 3002, { scan: async () => [], @@ -410,6 +433,78 @@ describe("background preview lifecycle", () => { ).toMatchObject({ pid: 4321 }); }); + it("reports the explicitly requested port when the detached child binds it", async () => { + const stateHome = mkdtempSync(join(tmpdir(), "hf-preview-state-")); + const dependencies = reachableChildDependencies(stateHome); + + const result = await startBackgroundPreview(projectDir, server.port, { + ...dependencies, + preferredPort: server.port, + }); + + expect(result).toMatchObject({ type: "started", port: server.port, pid: 9876 }); + expect(dependencies.kill).not.toHaveBeenCalled(); + expect(existsSync(previewSessionPath(projectDir, stateHome))).toBe(true); + }); + + it("reaps a detached child that could not bind the explicitly requested port", async () => { + // The child scans upward from --port and lands on the next free port. + const requestedPort = server.port - 1; + const stateHome = mkdtempSync(join(tmpdir(), "hf-preview-state-")); + const dependencies = reachableChildDependencies(stateHome); + + const launch = startBackgroundPreview(projectDir, requestedPort, { + ...dependencies, + preferredPort: requestedPort, + }); + + await expect(launch).rejects.toThrow(PreviewPortUnavailableError); + await expect(launch).rejects.toMatchObject({ requestedPort, boundPort: server.port }); + // The wrapper PID is reaped, not the server's self-reported PID. + expect(dependencies.kill).toHaveBeenCalledExactlyOnceWith(4321); + expect(existsSync(previewSessionPath(projectDir, stateHome))).toBe(false); + }); + + it("fails loudly when the reaped child keeps serving the substitute port", async () => { + const requestedPort = server.port - 1; + const stateHome = mkdtempSync(join(tmpdir(), "hf-preview-state-")); + const dependencies = reachableChildDependencies(stateHome, { immortal: true }); + + await expect( + startBackgroundPreview(projectDir, requestedPort, { + ...dependencies, + preferredPort: requestedPort, + }), + ).rejects.toThrow(/did not stop after failing to bind port/); + expect(dependencies.kill).toHaveBeenCalledExactlyOnceWith(4321); + expect(existsSync(previewSessionPath(projectDir, stateHome))).toBe(false); + }); + + it("keeps the next free port when no explicit port was requested", async () => { + const stateHome = mkdtempSync(join(tmpdir(), "hf-preview-state-")); + const dependencies = reachableChildDependencies(stateHome); + + const result = await startBackgroundPreview(projectDir, server.port - 1, dependencies); + + expect(result).toMatchObject({ type: "started", port: server.port }); + expect(dependencies.kill).not.toHaveBeenCalled(); + }); + + it("reuses the same-project server on the explicit port when several are running", async () => { + const sibling = { ...server, port: server.port + 1, pid: "5555" }; + const spawn = vi.fn(); + + const result = await startBackgroundPreview(projectDir, 3002, { + scan: async () => [server, sibling], + spawn, + stateHome: mkdtempSync(join(tmpdir(), "hf-preview-state-")), + preferredPort: sibling.port, + }); + + expect(result).toMatchObject({ type: "reused", port: sibling.port, pid: 5555 }); + expect(spawn).not.toHaveBeenCalled(); + }); + it("reaps a detached child that never becomes reachable without recording ownership", async () => { const stateHome = mkdtempSync(join(tmpdir(), "hf-preview-state-")); const kill = vi.fn(); diff --git a/packages/cli/src/commands/previewLifecycle.ts b/packages/cli/src/commands/previewLifecycle.ts index 5c8e68940b..281015565c 100644 --- a/packages/cli/src/commands/previewLifecycle.ts +++ b/packages/cli/src/commands/previewLifecycle.ts @@ -18,6 +18,24 @@ import type { BrowserGpuMode } from "../browser/gpuPolicy.js"; import { isProcessDescendant, killProcessTree, processIdentity } from "../utils/orphanCleanup.js"; import { PreviewServerPortMismatchError } from "../utils/studioSelectionClient.js"; +/** + * The detached child could not bind the explicitly requested --port and came + * up elsewhere. The launcher reaps it rather than reporting the substitute. + */ +export class PreviewPortUnavailableError extends Error { + readonly requestedPort: number; + readonly boundPort: number; + + constructor(requestedPort: number, boundPort: number) { + super( + `Port ${requestedPort} is already in use; the background preview would have started on port ${boundPort} instead. Free port ${requestedPort}, pick another --port, or omit --port to accept the next free port.`, + ); + this.name = "PreviewPortUnavailableError"; + this.requestedPort = requestedPort; + this.boundPort = boundPort; + } +} + export interface PreviewSession { pid: number; wrapperIdentity?: string; @@ -182,6 +200,12 @@ function matchingServerAtPort( ); } +/** Lists servers on `port` first so an explicit --port wins candidate selection. */ +function preferPort(servers: ActiveServer[], port: number | undefined): ActiveServer[] { + if (port === undefined) return servers; + return [...servers].sort((a, b) => Number(b.port === port) - Number(a.port === port)); +} + function sameProjectPorts(servers: ActiveServer[], projectDir: string): Set { const project = normalized(projectDir); return new Set( @@ -429,6 +453,14 @@ function savedOwnedPreview( return matchingServer(savedPortServers, projectDir); } +/** + * The explicit --port that `port` fails to satisfy, or undefined when `port` + * is acceptable. A bare launch has no preferred port, so any port satisfies it. + */ +function unmetPreferredPort(port: number, preferredPort: number | undefined): number | undefined { + return preferredPort !== undefined && port !== preferredPort ? preferredPort : undefined; +} + /** * Returns a reuse result when `reusableExisting` is a valid reuse candidate, * or `null` when the caller must fall through to a fresh launch. Throws when @@ -443,12 +475,8 @@ function reuseExistingPreview( // An explicit --port that doesn't match the reuse candidate is a conflict // the caller must resolve, not a silent substitution. A bare launch has no // preferred port, so reusing any project-matching server stays correct. - if ( - dependencies.preferredPort !== undefined && - reusableExisting.port !== dependencies.preferredPort - ) { - throw new PreviewServerPortMismatchError(dependencies.preferredPort, [reusableExisting]); - } + const unmet = unmetPreferredPort(reusableExisting.port, dependencies.preferredPort); + if (unmet !== undefined) throw new PreviewServerPortMismatchError(unmet, [reusableExisting]); return { type: "reused", port: reusableExisting.port, @@ -473,7 +501,11 @@ export async function startBackgroundPreview( // single per-project ownership record would orphan the old listener. const scanStart = saved?.port ?? startPort; const scanned = await scan(scanStart); - const requestedExisting = matchingServer(scanned, projectDir, dependencies.browserGpuMode); + const requestedExisting = matchingServer( + preferPort(scanned, dependencies.preferredPort), + projectDir, + dependencies.browserGpuMode, + ); const ownedExisting = savedOwnedPreview(scanned, saved, projectDir); // A saved managed preview is the authoritative same-project instance. An // explicit GPU-policy change replaces it; it must not silently adopt an @@ -496,6 +528,52 @@ export async function startBackgroundPreview( dependencies, ); + const kill = dependencies.kill ?? stopProcess; + const server = await awaitStartedServer(projectDir, startPort, preLaunchPorts, dependencies); + if (!server) { + kill(pid); + throw new Error(`background preview did not become ready; see ${logPath}`); + } + // The child scans upward from --port and binds the first free port. An + // explicit --port is a promise to the caller, so a child that landed + // elsewhere is reaped rather than reported (or recorded) as a success. + const unmet = unmetPreferredPort(server.port, dependencies.preferredPort); + if (unmet !== undefined) { + // Wait for the substitute to stop answering before reporting failure, so + // a concurrent bare launch or --status cannot adopt a server that is + // already shutting down. + kill(pid); + if (!(await awaitServerGone(projectDir, startPort, server.port, dependencies))) { + throw new Error( + `background preview on port ${server.port} did not stop after failing to bind port ${unmet}; see ${logPath}`, + ); + } + throw new PreviewPortUnavailableError(unmet, server.port); + } + const ready = readyPreviewSession( + server, + pid, + wrapperIdentity, + projectDir, + logPath, + dependencies, + ); + writePreviewSession(ready.session, stateHome); + return { + type: "started", + ...ready.session, + pid: ready.publicPid, + }; +} + +/** Polls for a same-project server that appeared after launch; null on timeout. */ +async function awaitStartedServer( + projectDir: string, + startPort: number, + preLaunchPorts: Set, + dependencies: LifecycleDependencies, +): Promise { + const scan = dependencies.scan ?? scanActiveServers; const sleep = dependencies.sleep ?? delay; for (let attempt = 0; attempt < 50; attempt++) { const server = startedServer( @@ -504,27 +582,10 @@ export async function startBackgroundPreview( preLaunchPorts, dependencies.browserGpuMode, ); - if (server) { - const ready = readyPreviewSession( - server, - pid, - wrapperIdentity, - projectDir, - logPath, - dependencies, - ); - writePreviewSession(ready.session, stateHome); - return { - type: "started", - ...ready.session, - pid: ready.publicPid, - }; - } + if (server) return server; await sleep(200); } - - (dependencies.kill ?? stopProcess)(pid); - throw new Error(`background preview did not become ready; see ${logPath}`); + return null; } export async function stopBackgroundPreview( @@ -555,13 +616,25 @@ export async function stopBackgroundPreview( const kill = dependencies.kill ?? stopProcess; kill(ownedStopTargetPid(saved, pid, dependencies)); + if (!(await awaitServerGone(projectDir, scanStart, server.port, dependencies))) { + throw new Error(`background preview did not stop for ${resolve(projectDir)}`); + } + removePreviewSession(projectDir, stateHome); + return true; +} + +/** Polls until no same-project server answers on `port`; false if it never leaves. */ +async function awaitServerGone( + projectDir: string, + scanStart: number, + port: number, + dependencies: LifecycleDependencies, +): Promise { + const scan = dependencies.scan ?? scanActiveServers; const sleep = dependencies.sleep ?? delay; for (let attempt = 0; attempt < 25; attempt++) { - if (!matchingServerAtPort(await scan(scanStart), projectDir, server.port)) { - removePreviewSession(projectDir, stateHome); - return true; - } + if (!matchingServerAtPort(await scan(scanStart), projectDir, port)) return true; await sleep(100); } - throw new Error(`background preview did not stop for ${resolve(projectDir)}`); + return false; } From dad0541d1f5befa458dbe18462201c2f0101abe2 Mon Sep 17 00:00:00 2001 From: miga-heygen Date: Tue, 15 Sep 2026 00:42:57 +0000 Subject: [PATCH 3/3] fix(cli): reuse the same-project server on the explicit --port MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `startBackgroundPreview` picked its reuse candidate before honouring `--port`, so with an owned server on 3002 and an unmanaged same-project server on 3003, `--port 3003` rejected with "No Studio preview server … on port 3003. Matching server port: 3002" even though 3003 was running. - A policy-matching same-project server already on the explicit port is now the reuse candidate, owned or not; the owned server stays authoritative otherwise, and an explicit GPU-policy change on the requested port still replaces it. - `PreviewServerPortMismatchError` receives every policy-matching same-project server, so the ports it reports are the ones running. - `policyMatchingServers` is the single owner of the same-project + GPU-policy predicate; `matchingServer` derives from it. - `BackgroundPreviewResult` names the launch result union once; `PREVIEW_PORT_MISMATCH_CODE` replaces three literal error codes. Tests: unmanaged sibling on the explicit port is reused without touching the owned server; mismatch error lists every same-project port and no foreign-project port; owned server on the explicit port is replaced on a GPU-policy change. Each new check fails when its guard is removed. Co-Authored-By: Miguel Ángel --- packages/cli/src/commands/preview.ts | 9 +- .../cli/src/commands/previewLifecycle.test.ts | 92 +++++++++++++++++++ packages/cli/src/commands/previewLifecycle.ts | 75 ++++++++------- 3 files changed, 140 insertions(+), 36 deletions(-) diff --git a/packages/cli/src/commands/preview.ts b/packages/cli/src/commands/preview.ts index 15fc7615d6..e0999e1706 100644 --- a/packages/cli/src/commands/preview.ts +++ b/packages/cli/src/commands/preview.ts @@ -128,9 +128,12 @@ type CompactSelectionPayload = Pick< const DEFAULT_CONTEXT_FIELDS: ContextField[] = ["server", "selection", "lint", "capabilities"]; +/** JSON error code for an explicit --port that no same-project server is on. */ +const PREVIEW_PORT_MISMATCH_CODE = "preview-port-mismatch"; + /** Distinguishes an unhonoured explicit --port from a generic launch failure. */ function backgroundStartFailureCode(error: unknown): string { - if (error instanceof PreviewServerPortMismatchError) return "preview-port-mismatch"; + if (error instanceof PreviewServerPortMismatchError) return PREVIEW_PORT_MISMATCH_CODE; if (error instanceof PreviewPortUnavailableError) return "preview-port-unavailable"; return "preview-start-failed"; } @@ -911,7 +914,7 @@ async function printCurrentSelection( return; } if (err instanceof PreviewServerPortMismatchError) { - printSelectionFailure("preview-port-mismatch", err.message, json); + printSelectionFailure(PREVIEW_PORT_MISMATCH_CODE, err.message, json); return; } throw err; @@ -1033,7 +1036,7 @@ async function printCurrentContext( return; } if (err instanceof PreviewServerPortMismatchError) { - printSelectionFailure("preview-port-mismatch", err.message, options.json); + printSelectionFailure(PREVIEW_PORT_MISMATCH_CODE, err.message, options.json); return; } throw err; diff --git a/packages/cli/src/commands/previewLifecycle.test.ts b/packages/cli/src/commands/previewLifecycle.test.ts index afba92cb81..283c555b09 100644 --- a/packages/cli/src/commands/previewLifecycle.test.ts +++ b/packages/cli/src/commands/previewLifecycle.test.ts @@ -505,6 +505,98 @@ describe("background preview lifecycle", () => { expect(spawn).not.toHaveBeenCalled(); }); + it("reuses the unmanaged sibling on the explicit port instead of rejecting against the owned server", async () => { + const stateHome = mkdtempSync(join(tmpdir(), "hf-preview-state-")); + const owned = { ...server, port: 3002 }; + const sibling = { ...server, port: 3003, pid: "5555" }; + writePreviewSession( + { pid: 4321, port: owned.port, projectDir, logPath: "/tmp/preview.log" }, + stateHome, + ); + const spawn = vi.fn(); + const kill = vi.fn(); + + const result = await startBackgroundPreview(projectDir, 3002, { + kill, + scan: async () => [owned, sibling], + spawn, + stateHome, + preferredPort: sibling.port, + }); + + expect(result).toMatchObject({ type: "reused", port: sibling.port, pid: 5555 }); + expect(spawn).not.toHaveBeenCalled(); + expect(kill).not.toHaveBeenCalled(); + }); + + it("lists every same-project server in the port-mismatch error, not just the reuse candidate", async () => { + const stateHome = mkdtempSync(join(tmpdir(), "hf-preview-state-")); + const owned = { ...server, port: 3002 }; + const sibling = { ...server, port: 3003, pid: "5555" }; + const foreign = { + ...server, + port: 3004, + projectDir: resolve("/tmp/hyperframes-preview-lifecycle-other"), + pid: "7777", + }; + writePreviewSession( + { pid: 4321, port: owned.port, projectDir, logPath: "/tmp/preview.log" }, + stateHome, + ); + const spawn = vi.fn(); + + const failure = await startBackgroundPreview(projectDir, 3002, { + scan: async () => [owned, sibling, foreign], + spawn, + stateHome, + preferredPort: 3004, + }).then( + () => null, + (error: unknown) => error, + ); + + expect(failure).toBeInstanceOf(PreviewServerPortMismatchError); + expect(failure).toMatchObject({ requestedPort: 3004, ports: [3002, 3003] }); + expect(spawn).not.toHaveBeenCalled(); + }); + + it("replaces the owned server on the explicit port when the GPU policy changes", async () => { + const stateHome = mkdtempSync(join(tmpdir(), "hf-preview-state-")); + const owned = { ...server, port: 3002, browserGpuMode: "hardware" as const }; + const replacement = { ...server, port: 3002, pid: "5432", browserGpuMode: "software" as const }; + writePreviewSession( + { pid: 4321, port: owned.port, projectDir, logPath: "/tmp/preview.log" }, + stateHome, + ); + let ownedRunning = true; + let replacementRunning = false; + const scan = vi.fn(async () => [ + ...(ownedRunning ? [owned] : []), + ...(replacementRunning ? [replacement] : []), + ]); + const kill = vi.fn((pid: number) => { + if (pid === 4321) ownedRunning = false; + }); + const spawn = vi.fn(() => { + replacementRunning = true; + return { pid: 5432, unref: vi.fn() }; + }); + + const result = await startBackgroundPreview(projectDir, 3002, { + browserGpuMode: "software", + kill, + scan, + sleep: async () => {}, + spawn, + stateHome, + preferredPort: 3002, + }); + + expect(kill).toHaveBeenCalledWith(4321); + expect(spawn).toHaveBeenCalledOnce(); + expect(result).toMatchObject({ type: "started", port: 3002, pid: 5432 }); + }); + it("reaps a detached child that never becomes reachable without recording ownership", async () => { const stateHome = mkdtempSync(join(tmpdir(), "hf-preview-state-")); const kill = vi.fn(); diff --git a/packages/cli/src/commands/previewLifecycle.ts b/packages/cli/src/commands/previewLifecycle.ts index 281015565c..17709aa31f 100644 --- a/packages/cli/src/commands/previewLifecycle.ts +++ b/packages/cli/src/commands/previewLifecycle.ts @@ -180,12 +180,17 @@ function matchingServer( projectDir: string, browserGpuMode?: BrowserGpuMode, ): ActiveServer | null { - return ( - servers.find( - (server) => - normalized(server.projectDir) === normalized(projectDir) && - (browserGpuMode === undefined || server.browserGpuMode === browserGpuMode), - ) ?? null + return policyMatchingServers(servers, projectDir, browserGpuMode)[0] ?? null; +} + +/** Same-project servers that also satisfy the requested GPU policy, if any. */ +function policyMatchingServers( + servers: ActiveServer[], + projectDir: string, + browserGpuMode?: BrowserGpuMode, +): ActiveServer[] { + return sameProjectServers(servers, projectDir).filter( + (server) => browserGpuMode === undefined || server.browserGpuMode === browserGpuMode, ); } @@ -200,19 +205,13 @@ function matchingServerAtPort( ); } -/** Lists servers on `port` first so an explicit --port wins candidate selection. */ -function preferPort(servers: ActiveServer[], port: number | undefined): ActiveServer[] { - if (port === undefined) return servers; - return [...servers].sort((a, b) => Number(b.port === port) - Number(a.port === port)); +function sameProjectServers(servers: ActiveServer[], projectDir: string): ActiveServer[] { + const project = normalized(projectDir); + return servers.filter((server) => normalized(server.projectDir) === project); } function sameProjectPorts(servers: ActiveServer[], projectDir: string): Set { - const project = normalized(projectDir); - return new Set( - servers - .filter((server) => normalized(server.projectDir) === project) - .map((server) => server.port), - ); + return new Set(sameProjectServers(servers, projectDir).map((server) => server.port)); } function stopProcess(pid: number): void { @@ -461,22 +460,31 @@ function unmetPreferredPort(port: number, preferredPort: number | undefined): nu return preferredPort !== undefined && port !== preferredPort ? preferredPort : undefined; } +type BackgroundPreviewResult = + | { type: "reused"; port: number; pid: number | null; logPath: string | null } + | { type: "started"; port: number; pid: number; logPath: string }; + /** * Returns a reuse result when `reusableExisting` is a valid reuse candidate, * or `null` when the caller must fall through to a fresh launch. Throws when * the candidate's port conflicts with an explicit --port request rather than - * silently substituting the wrong port. + * silently substituting the wrong port; `candidates` is every policy-matching + * same-project server the scan found, reported so the error names real + * alternatives. */ function reuseExistingPreview( reusableExisting: ActiveServer | null, + candidates: ActiveServer[], dependencies: LifecycleDependencies, -): { type: "reused"; port: number; pid: number | null; logPath: string | null } | null { +): Extract | null { if (!reusableExisting || dependencies.forceNew) return null; // An explicit --port that doesn't match the reuse candidate is a conflict // the caller must resolve, not a silent substitution. A bare launch has no // preferred port, so reusing any project-matching server stays correct. + // The error lists every candidate, not just the chosen one, so the + // "matching ports" it reports are the ones actually running. const unmet = unmetPreferredPort(reusableExisting.port, dependencies.preferredPort); - if (unmet !== undefined) throw new PreviewServerPortMismatchError(unmet, [reusableExisting]); + if (unmet !== undefined) throw new PreviewServerPortMismatchError(unmet, candidates); return { type: "reused", port: reusableExisting.port, @@ -489,10 +497,7 @@ export async function startBackgroundPreview( projectDir: string, startPort: number, dependencies: LifecycleDependencies = {}, -): Promise< - | { type: "reused"; port: number; pid: number | null; logPath: string | null } - | { type: "started"; port: number; pid: number; logPath: string } -> { +): Promise { const scan = dependencies.scan ?? scanActiveServers; const stateHome = dependencies.stateHome ?? defaultStateHome(); const saved = readPreviewSession(projectDir, stateHome); @@ -501,20 +506,24 @@ export async function startBackgroundPreview( // single per-project ownership record would orphan the old listener. const scanStart = saved?.port ?? startPort; const scanned = await scan(scanStart); - const requestedExisting = matchingServer( - preferPort(scanned, dependencies.preferredPort), - projectDir, - dependencies.browserGpuMode, - ); + const candidates = policyMatchingServers(scanned, projectDir, dependencies.browserGpuMode); + // An explicit --port names the server to reuse: a policy-matching server + // already on that port satisfies the request whether or not it is the owned + // one, so `--port 3003` reuses the 3003 sibling instead of reporting a + // mismatch against the owned 3002. + const onPreferredPort = + candidates.find((server) => server.port === dependencies.preferredPort) ?? null; + const requestedExisting = candidates[0] ?? null; const ownedExisting = savedOwnedPreview(scanned, saved, projectDir); - // A saved managed preview is the authoritative same-project instance. An - // explicit GPU-policy change replaces it; it must not silently adopt an - // unmanaged sibling that happens to match the new policy. + // Otherwise a saved managed preview is the authoritative same-project + // instance. An explicit GPU-policy change replaces it; it must not silently + // adopt an unmanaged sibling that happens to match the new policy. const reusableOwned = ownedExisting ? matchingServer([ownedExisting], projectDir, dependencies.browserGpuMode) : null; - const reusableExisting = reusableOwned ?? (ownedExisting ? null : requestedExisting); - const reused = reuseExistingPreview(reusableExisting, dependencies); + const reusableExisting = + onPreferredPort ?? reusableOwned ?? (ownedExisting ? null : requestedExisting); + const reused = reuseExistingPreview(reusableExisting, candidates, dependencies); if (reused) return reused; await stopOwnedPreviewBeforeReplacement(ownedExisting, projectDir, dependencies); // Snapshot every same-project listener in the prospective launch range only