diff --git a/packages/stats/src/port-conflict.ts b/packages/stats/src/port-conflict.ts index db290a8cc..28ad5cebf 100644 --- a/packages/stats/src/port-conflict.ts +++ b/packages/stats/src/port-conflict.ts @@ -14,14 +14,31 @@ interface PortHolder { image: string; } +/** Header stamped on every dashboard response so reuse probes can identify us. */ +export const STATS_DASHBOARD_HEADER = "x-omp-stats-dashboard"; + async function probeStatsDashboard(port: number): Promise { try { const response = await fetch(`http://localhost:${port}/api/stats/models`, { signal: AbortSignal.timeout(STATS_PROBE_TIMEOUT_MS), }); - const isDashboard = response.status === 200; - await response.body?.cancel(); - return isDashboard; + if (response.status !== 200) { + await response.body?.cancel(); + return false; + } + // A live omp-stats dashboard stamps this header on every response. + if (response.headers.get(STATS_DASHBOARD_HEADER)) { + await response.body?.cancel(); + return true; + } + // Older dashboards predate the header; fall back to the response shape + // (`/api/stats/models` returns a JSON array) so we never reuse — or later + // kill — a foreign 200 responder such as an SPA dev server catch-all. + if (!(response.headers.get("content-type") ?? "").includes("application/json")) { + await response.body?.cancel(); + return false; + } + return Array.isArray(await response.json()); } catch { return false; } diff --git a/packages/stats/src/server.ts b/packages/stats/src/server.ts index 6c77364cc..31bacbfe9 100644 --- a/packages/stats/src/server.ts +++ b/packages/stats/src/server.ts @@ -20,7 +20,7 @@ import { import { decodeEmbeddedClientArchive } from "./embedded-client"; import embeddedClientArchiveTxt from "./embedded-client.generated.txt"; import { getGainDashboardStats } from "./gain-aggregator"; -import { recoverStatsPort } from "./port-conflict"; +import { recoverStatsPort, STATS_DASHBOARD_HEADER } from "./port-conflict"; const EMBEDDED_CLIENT_ARCHIVE = decodeEmbeddedClientArchive(embeddedClientArchiveTxt); @@ -301,11 +301,13 @@ function createDashboardServer(port: number) { const url = new URL(req.url); const path = url.pathname; - // CORS headers for local development + // CORS headers for local development; the identity header lets another + // omp session's reuse probe positively recognize this dashboard. const corsHeaders: Record = { "Access-Control-Allow-Origin": "*", "Access-Control-Allow-Methods": "GET, POST, OPTIONS", "Access-Control-Allow-Headers": "Content-Type", + [STATS_DASHBOARD_HEADER]: "1", }; if (req.method === "OPTIONS") { diff --git a/packages/stats/test/server-port-conflict.test.ts b/packages/stats/test/server-port-conflict.test.ts index 4abbd8408..21ef852e9 100644 --- a/packages/stats/test/server-port-conflict.test.ts +++ b/packages/stats/test/server-port-conflict.test.ts @@ -1,10 +1,11 @@ import { afterEach, describe, expect, it } from "bun:test"; import type { Subprocess } from "bun"; +import { STATS_DASHBOARD_HEADER } from "../src/port-conflict"; import { startServer } from "../src/server"; const holderProcesses: Array> = []; -async function startBunHolder(status: number) { +async function startBunHolder(responseExpr: string) { const reservation = Bun.serve({ hostname: "127.0.0.1", port: 0, @@ -13,7 +14,7 @@ async function startBunHolder(status: number) { const port = reservation.port; reservation.stop(true); - const source = `Bun.serve({ hostname: "127.0.0.1", port: ${port}, fetch: () => new Response("holder", { status: ${status} }) }); process.stdout.write("ready"); await Promise.withResolvers().promise;`; + const source = `Bun.serve({ hostname: "127.0.0.1", port: ${port}, fetch: () => ${responseExpr} }); process.stdout.write("ready"); await Promise.withResolvers().promise;`; const child = Bun.spawn([process.execPath, "-e", source], { stdin: "ignore", stdout: "pipe", @@ -42,12 +43,14 @@ afterEach(async () => { }); describe("startServer port conflicts", () => { - it("reuses a live stats dashboard without stopping it", async () => { + it("reuses a live stats dashboard identified by its header", async () => { const existing = Bun.serve({ hostname: "127.0.0.1", port: 0, fetch: request => - new URL(request.url).pathname === "/api/stats/models" ? Response.json([]) : new Response("dashboard"), + new URL(request.url).pathname === "/api/stats/models" + ? Response.json([], { headers: { [STATS_DASHBOARD_HEADER]: "1" } }) + : new Response("dashboard"), }); try { @@ -55,16 +58,35 @@ describe("startServer port conflicts", () => { expect(server.port).toBe(existing.port); server.stop(); + // The foreign server is untouched: it still answers on the port. const response = await fetch(`http://127.0.0.1:${existing.port}/api/stats/models`); expect(response.status).toBe(200); + expect(response.headers.get(STATS_DASHBOARD_HEADER)).toBe("1"); await response.body?.cancel(); } finally { existing.stop(true); } }); + it("does not reuse a foreign 200 responder and reclaims the port instead", async () => { + // An SPA dev server catch-all: 200 JSON, but no dashboard header and not + // the models array shape. Must not be treated as a reusable dashboard. + const holder = await startBunHolder('Response.json({ app: "spa" })'); + const server = await startServer(holder.port); + + try { + expect(server.port).toBe(holder.port); + expect(await holder.child.exited).not.toBe(0); + const response = await fetch(`http://127.0.0.1:${holder.port}/api/stats/models`); + expect(response.headers.get(STATS_DASHBOARD_HEADER)).toBe("1"); + await response.body?.cancel(); + } finally { + server.stop(); + } + }); + it("reclaims an unresponsive Bun listener and starts the dashboard", async () => { - const holder = await startBunHolder(404); + const holder = await startBunHolder('new Response("holder", { status: 404 })'); const server = await startServer(holder.port); try {