From 47a6c10f3c71ecbab6e2058e3e39873faef6983e Mon Sep 17 00:00:00 2001 From: miga-heygen Date: Sun, 13 Sep 2026 00:56:16 +0000 Subject: [PATCH] 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