diff --git a/packages/cli/src/commands/preview.ts b/packages/cli/src/commands/preview.ts index 5203d7eead..e0999e1706 100644 --- a/packages/cli/src/commands/preview.ts +++ b/packages/cli/src/commands/preview.ts @@ -66,7 +66,9 @@ 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 { + PreviewPortUnavailableError, listBackgroundPreviewStatuses, readBackgroundPreviewStatus, startBackgroundPreview, @@ -126,6 +128,16 @@ 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_CODE; + if (error instanceof PreviewPortUnavailableError) return "preview-port-unavailable"; + return "preview-start-failed"; +} + export default defineCommand({ meta: { name: "preview", @@ -296,7 +308,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 +397,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 +511,14 @@ 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)); + writeLifecycleJson( + lifecycleFailurePayload("start", backgroundStartFailureCode(error), message), + ); } else { clack.log.error(message); } @@ -904,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; @@ -1026,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 c9635677f5..283c555b09 100644 --- a/packages/cli/src/commands/previewLifecycle.test.ts +++ b/packages/cli/src/commands/previewLifecycle.test.ts @@ -3,7 +3,9 @@ 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 { + PreviewPortUnavailableError, buildBackgroundPreviewArgs, listBackgroundPreviewStatuses, previewSessionPath, @@ -29,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 () => [], @@ -99,6 +123,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"); @@ -379,6 +433,170 @@ 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("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 b892cf9c9d..17709aa31f 100644 --- a/packages/cli/src/commands/previewLifecycle.ts +++ b/packages/cli/src/commands/previewLifecycle.ts @@ -16,6 +16,25 @@ 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"; + +/** + * 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; @@ -49,6 +68,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 { @@ -159,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, ); } @@ -179,13 +205,13 @@ function matchingServerAtPort( ); } -function sameProjectPorts(servers: ActiveServer[], projectDir: string): Set { +function sameProjectServers(servers: ActiveServer[], projectDir: string): ActiveServer[] { const project = normalized(projectDir); - return new Set( - servers - .filter((server) => normalized(server.projectDir) === project) - .map((server) => server.port), - ); + return servers.filter((server) => normalized(server.projectDir) === project); +} + +function sameProjectPorts(servers: ActiveServer[], projectDir: string): Set { + return new Set(sameProjectServers(servers, projectDir).map((server) => server.port)); } function stopProcess(pid: number): void { @@ -426,14 +452,52 @@ 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; +} + +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; `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, +): 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, candidates); + return { + type: "reused", + port: reusableExisting.port, + pid: reusableExisting.pid ? Number(reusableExisting.pid) : null, + logPath: null, + }; +} + 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); @@ -442,23 +506,25 @@ 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 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); - if (reusableExisting && !dependencies.forceNew) { - return { - type: "reused", - port: reusableExisting.port, - pid: reusableExisting.pid ? Number(reusableExisting.pid) : null, - logPath: null, - }; - } + 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 // after the owned listener is gone. Readiness must identify a newly appeared @@ -471,6 +537,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( @@ -479,27 +591,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( @@ -530,13 +625,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; }