Skip to content

Commit fb9e4c6

Browse files
spencermarxruvnet
spencermarx
and
ruvnet
committed
fix(dashboard): resolve dev proxy port race condition
Replace concurrently + sleep 2 with scripts/dev.ts that starts the server, polls for the port file (with mtime freshness check), then starts Vite with PORT env var — eliminating the race entirely. Also: server deletes stale port file on startup before binding, and vite.config.ts prioritizes PORT env var for proxy target. Co-Authored-By: claude-flow <ruv@ruv.net>
1 parent 804b6f3 commit fb9e4c6

6 files changed

Lines changed: 268 additions & 11 deletions

File tree

‎packages/dashboard-e2e/src/dashboard-smoke.test.ts‎

Lines changed: 62 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,9 +5,12 @@
55
* respond correctly. No mocks — all HTTP requests hit the running server.
66
*/
77

8+
import { readFileSync, existsSync } from "node:fs";
9+
import { resolve } from "node:path";
810
import { describe, it, expect, beforeAll, afterAll } from "vitest";
911
import {
1012
startTestServer,
13+
startServerEarly,
1114
type ServerInstance,
1215
} from "./helpers/server-harness.js";
1316

@@ -68,6 +71,65 @@ describe("Dashboard smoke tests", () => {
6871
});
6972
});
7073

74+
describe("port discovery", () => {
75+
it("health endpoint is reachable at the port advertised in server-port file", () => {
76+
// The Vite dev proxy reads .ocr/data/server-port to find the server.
77+
// If this file points to the wrong port, the dashboard is unreachable.
78+
const portFile = resolve(server.ocrDir, "data", "server-port");
79+
const advertisedPort = parseInt(
80+
readFileSync(portFile, "utf-8").trim(),
81+
10,
82+
);
83+
84+
// The advertised port must match where the server actually listens
85+
expect(advertisedPort).toBe(server.port);
86+
});
87+
88+
it("stale port file is cleared before server binds its port", async () => {
89+
// Regression test for the dev proxy port race condition.
90+
//
91+
// In the real dev flow, Vite reads .ocr/data/server-port during
92+
// its ~2s startup delay to configure the proxy target. If a stale
93+
// file from a previous run still has the old port, Vite targets
94+
// a dead address → blank page, "Unexpected token '<'".
95+
//
96+
// We reproduce the exact race: write a stale port file, fork the
97+
// server, and read the file during the startup window (before the
98+
// server is healthy). This is what Vite does. The stale value must
99+
// already be gone — the server should delete it synchronously at
100+
// the start of initialization, before any async work.
101+
const early = startServerEarly({ stalePort: 9999 });
102+
103+
try {
104+
// Wait just long enough for the server's synchronous init to run.
105+
// The port file deletion is synchronous and happens before DB open.
106+
await new Promise((r) => setTimeout(r, 500));
107+
108+
// Read the file the way Vite would — during the startup window.
109+
// The stale value (9999) must NOT be present.
110+
if (existsSync(early.portFilePath)) {
111+
const value = parseInt(
112+
readFileSync(early.portFilePath, "utf-8").trim(),
113+
10,
114+
);
115+
expect(value).not.toBe(9999);
116+
}
117+
// If the file doesn't exist, that's also correct — stale deleted,
118+
// server hasn't written the new value yet. Vite falls back to 4173.
119+
120+
// After full startup, verify the file has the correct port
121+
await early.waitForHealth();
122+
const finalPort = parseInt(
123+
readFileSync(early.portFilePath, "utf-8").trim(),
124+
10,
125+
);
126+
expect(finalPort).toBe(early.port);
127+
} finally {
128+
await early.cleanup();
129+
}
130+
});
131+
});
132+
71133
describe("API endpoints", () => {
72134
it("GET /api/sessions returns an array", async () => {
73135
const res = await apiFetch("/api/sessions");

‎packages/dashboard-e2e/src/helpers/server-harness.ts‎

Lines changed: 79 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ import {
1212
mkdirSync,
1313
rmSync,
1414
realpathSync,
15+
writeFileSync,
1516
} from "node:fs";
1617
import { tmpdir } from "node:os";
1718
import { execFileSync } from "node:child_process";
@@ -25,13 +26,19 @@ export interface ServerInstance {
2526
port: number;
2627
baseUrl: string;
2728
token: string;
29+
ocrDir: string;
2830
process: ChildProcess;
2931
cleanup: () => Promise<void>;
3032
}
3133

3234
let portCounter = 14_000 + Math.floor(Math.random() * 1000);
3335

34-
export async function startTestServer(): Promise<ServerInstance> {
36+
export interface StartOptions {
37+
/** Write a stale port file before starting the server (for testing port file cleanup). */
38+
stalePort?: number;
39+
}
40+
41+
export async function startTestServer(opts?: StartOptions): Promise<ServerInstance> {
3542
const port = portCounter++;
3643
const tmpDir = realpathSync(
3744
mkdtempSync(resolve(tmpdir(), "ocr-dash-e2e-")),
@@ -47,6 +54,14 @@ export async function startTestServer(): Promise<ServerInstance> {
4754
mkdirSync(resolve(tmpDir, ".ocr", "sessions"), { recursive: true });
4855
mkdirSync(resolve(tmpDir, ".ocr", "data"), { recursive: true });
4956

57+
// Optionally write a stale port file to test cleanup behavior
58+
if (opts?.stalePort) {
59+
writeFileSync(
60+
resolve(tmpDir, ".ocr", "data", "server-port"),
61+
String(opts.stalePort),
62+
);
63+
}
64+
5065
const child = fork(SERVER_ENTRY, [], {
5166
cwd: tmpDir,
5267
env: {
@@ -71,6 +86,7 @@ export async function startTestServer(): Promise<ServerInstance> {
7186
port,
7287
baseUrl,
7388
token,
89+
ocrDir: resolve(tmpDir, ".ocr"),
7490
process: child,
7591
cleanup: async () => {
7692
child.kill();
@@ -84,6 +100,68 @@ export async function startTestServer(): Promise<ServerInstance> {
84100
};
85101
}
86102

103+
/**
104+
* Start a server but return immediately — before waiting for health.
105+
* Used to inspect the port file during the startup window.
106+
*/
107+
export function startServerEarly(opts?: StartOptions): {
108+
portFilePath: string;
109+
child: ChildProcess;
110+
port: number;
111+
tmpDir: string;
112+
waitForHealth: () => Promise<void>;
113+
cleanup: () => Promise<void>;
114+
} {
115+
const port = portCounter++;
116+
const tmpDir = realpathSync(
117+
mkdtempSync(resolve(tmpdir(), "ocr-dash-e2e-")),
118+
);
119+
120+
execFileSync("git", ["init"], { cwd: tmpDir, stdio: "ignore" });
121+
execFileSync("git", ["commit", "--allow-empty", "-m", "init"], {
122+
cwd: tmpDir,
123+
stdio: "ignore",
124+
});
125+
mkdirSync(resolve(tmpDir, ".ocr", "skills"), { recursive: true });
126+
mkdirSync(resolve(tmpDir, ".ocr", "sessions"), { recursive: true });
127+
mkdirSync(resolve(tmpDir, ".ocr", "data"), { recursive: true });
128+
129+
const portFilePath = resolve(tmpDir, ".ocr", "data", "server-port");
130+
131+
if (opts?.stalePort) {
132+
writeFileSync(portFilePath, String(opts.stalePort));
133+
}
134+
135+
const child = fork(SERVER_ENTRY, [], {
136+
cwd: tmpDir,
137+
env: {
138+
...process.env,
139+
PORT: String(port),
140+
NODE_ENV: "test",
141+
NO_COLOR: "1",
142+
},
143+
stdio: "pipe",
144+
});
145+
146+
const baseUrl = `http://127.0.0.1:${port}`;
147+
148+
return {
149+
portFilePath,
150+
child,
151+
port,
152+
tmpDir,
153+
waitForHealth: () => waitForHealth(baseUrl, 15_000),
154+
cleanup: async () => {
155+
child.kill();
156+
await new Promise<void>((r) => {
157+
child.on("exit", () => r());
158+
setTimeout(() => r(), 5_000);
159+
});
160+
rmSync(tmpDir, { recursive: true, force: true });
161+
},
162+
};
163+
}
164+
87165
async function waitForHealth(
88166
baseUrl: string,
89167
timeoutMs: number,

‎packages/dashboard/package.json‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@
44
"private": true,
55
"type": "module",
66
"scripts": {
7-
"dev": "concurrently -n server,client -c blue,green \"pnpm dev:server\" \"sleep 2 && pnpm dev:client\"",
7+
"dev": "tsx scripts/dev.ts",
88
"dev:client": "vite",
99
"dev:server": "tsx watch src/server/index.ts",
1010
"build": "pnpm build:client && pnpm build:server",

‎packages/dashboard/scripts/dev.ts‎

Lines changed: 112 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,112 @@
1+
/**
2+
* Dev startup script — starts server and Vite client in sequence.
3+
*
4+
* Replaces `concurrently "server" "sleep 2 && client"` to eliminate
5+
* the port race condition. The server may auto-increment its port if
6+
* the default (4173) is in use. This script waits for the server to
7+
* write .ocr/data/server-port, then starts Vite with the correct
8+
* PORT env var so the proxy always targets the right address.
9+
*/
10+
11+
import { spawn } from "node:child_process";
12+
import { existsSync, readFileSync, statSync } from "node:fs";
13+
import { join, dirname } from "node:path";
14+
15+
function findPortFile(): string | null {
16+
let dir = process.cwd();
17+
while (true) {
18+
const portFile = join(dir, ".ocr", "data", "server-port");
19+
if (existsSync(join(dir, ".ocr"))) return portFile;
20+
const parent = dirname(dir);
21+
if (parent === dir) return null;
22+
dir = parent;
23+
}
24+
}
25+
26+
async function waitForPortFile(
27+
portFile: string,
28+
startTime: number,
29+
timeoutMs: number,
30+
): Promise<number> {
31+
const deadline = startTime + timeoutMs;
32+
while (Date.now() < deadline) {
33+
if (existsSync(portFile)) {
34+
try {
35+
const stat = statSync(portFile);
36+
// Only accept the file if it was written after we started
37+
if (stat.mtimeMs >= startTime) {
38+
const port = parseInt(readFileSync(portFile, "utf-8").trim(), 10);
39+
if (!isNaN(port)) return port;
40+
}
41+
} catch {
42+
// File may be in the process of being written
43+
}
44+
}
45+
await new Promise((r) => setTimeout(r, 100));
46+
}
47+
48+
throw new Error(
49+
`Server did not write port file within ${timeoutMs}ms.\n` +
50+
` Expected at: ${portFile}\n` +
51+
` Check the server logs above for errors.`,
52+
);
53+
}
54+
55+
async function main(): Promise<void> {
56+
const portFile = findPortFile();
57+
if (!portFile) {
58+
console.error(
59+
"Error: Could not find .ocr/ directory. Run `ocr init` first.",
60+
);
61+
process.exit(1);
62+
}
63+
64+
// Capture timestamp BEFORE spawning so a fast server write isn't missed
65+
const startTime = Date.now();
66+
67+
// Start the server (tsx watch for hot reload)
68+
const server = spawn("pnpm", ["dev:server"], {
69+
stdio: "inherit",
70+
shell: true,
71+
});
72+
73+
// Wait for the server to bind and write its port
74+
let port: number;
75+
try {
76+
port = await waitForPortFile(portFile, startTime, 30_000);
77+
} catch (err) {
78+
console.error(String(err));
79+
server.kill();
80+
process.exit(1);
81+
}
82+
83+
console.log(`\n Vite proxy → http://127.0.0.1:${port}\n`);
84+
85+
// Start Vite with the confirmed port
86+
const client = spawn("pnpm", ["dev:client"], {
87+
stdio: "inherit",
88+
shell: true,
89+
env: { ...process.env, PORT: String(port) },
90+
});
91+
92+
// Forward exit signals
93+
const cleanup = (): void => {
94+
server.kill();
95+
client.kill();
96+
};
97+
98+
process.on("SIGINT", cleanup);
99+
process.on("SIGTERM", cleanup);
100+
101+
// Exit when either process exits
102+
const onExit = (name: string) => (code: number | null) => {
103+
console.log(`\n ${name} exited (code ${code})`);
104+
cleanup();
105+
process.exit(code ?? 1);
106+
};
107+
108+
server.on("exit", onExit("server"));
109+
client.on("exit", onExit("client"));
110+
}
111+
112+
main();

‎packages/dashboard/src/server/index.ts‎

Lines changed: 11 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -154,13 +154,21 @@ export async function startServer(options: StartServerOptions = {}): Promise<voi
154154
const aiCliService = new AiCliService(ocrDir)
155155
const db = await openDb(ocrDir)
156156

157-
// ── PID tracking file ──
158-
// Write process PID so other tooling can detect an already-running server
159-
// and clean up orphaned processes.
157+
// ── Tracking files ──
160158
const dataDir = join(ocrDir, 'data')
161159
const pidFilePath = join(dataDir, 'dashboard.pid')
160+
const portFilePath = join(dataDir, 'server-port')
162161
mkdirSync(dataDir, { recursive: true })
163162

163+
// Remove stale port file immediately so the Vite dev proxy does not
164+
// read a leftover port from a previous server instance. The correct
165+
// port is written after the server binds successfully.
166+
try { unlinkSync(portFilePath) } catch { /* may not exist */ }
167+
168+
// ── PID tracking file ──
169+
// Write process PID so other tooling can detect an already-running server
170+
// and clean up orphaned processes.
171+
164172
if (existsSync(pidFilePath)) {
165173
try {
166174
const oldPid = parseInt(readFileSync(pidFilePath, 'utf-8').trim(), 10)
@@ -413,7 +421,6 @@ export async function startServer(options: StartServerOptions = {}): Promise<voi
413421

414422
// Write actual port so the Vite dev proxy can discover it.
415423
// In dev mode, Vite starts after the server (sleep 2) and reads this file.
416-
const portFilePath = join(dataDir, 'server-port')
417424
writeFileSync(portFilePath, String(actualPort), { mode: 0o600 })
418425

419426
console.log(` Server: http://localhost:${actualPort}`)

‎packages/dashboard/vite.config.ts‎

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -6,12 +6,10 @@ import tailwindcss from '@tailwindcss/vite'
66

77
/**
88
* Discover the server port. Priority:
9-
* 1. PORT env var (explicit override)
10-
* 2. `.ocr/data/server-port` file (written by the server on startup)
9+
* 1. PORT env var — set by scripts/dev.ts after the server binds,
10+
* guaranteeing the correct port with zero race condition
11+
* 2. `.ocr/data/server-port` file — fallback for manual Vite starts
1112
* 3. Default 4173
12-
*
13-
* Walks up from CWD to find `.ocr/` — Vite runs from packages/dashboard/
14-
* but `.ocr/` lives at the monorepo root.
1513
*/
1614
function resolveServerPort(): number {
1715
if (process.env.PORT) return parseInt(process.env.PORT, 10)

0 commit comments

Comments
 (0)