Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 7 additions & 2 deletions src/cli/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,12 @@ async function waitForProxy(timeoutMs = 8_000): Promise<LiveProxy | null> {
async function chooseListenPort(requestedPort?: number): Promise<number> {
const config = loadConfig();
const preferred = requestedPort ?? config.port ?? 10100;
const selected = await findAvailablePort(preferred, config.hostname ?? "127.0.0.1");
// Brief prefer-retry covers stop→start races (update restart, `ocx restart`) where the
// old process has exited but the listen socket is still draining.
const selected = await findAvailablePort(preferred, config.hostname ?? "127.0.0.1", {
preferRetryMs: 750,
preferRetryIntervalMs: 50,
});
if (selected !== preferred) {
console.log(`⚠️ Port ${preferred} is busy; starting opencodex on ${selected}.`);
}
Expand Down Expand Up @@ -587,7 +592,7 @@ switch (command) {
const jobId = args[1];
if (!jobId) process.exit(1);
const channel = normalizeUpdateChannel(args[2]);
runGuiUpdateWorker(jobId, channel, args[3] === "restart");
await runGuiUpdateWorker(jobId, channel, args[3] === "restart");
break;
}
case "restart": {
Expand Down
46 changes: 44 additions & 2 deletions src/config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -825,26 +825,68 @@ export function isOcxStartCommandLine(commandLine: string): boolean {
return hasOcxEntrypoint && /(?:^|[\s"'])start(?:$|[\s"'])/.test(normalized);
}

/** Per-process memo: waitForProxy/findLiveProxy used to spawn powershell on every 150ms poll. */
const ocxStartProcessCache = new Map<number, boolean>();

function isLikelyOcxStartProcess(pid: number): boolean {
const cached = ocxStartProcessCache.get(pid);
if (cached !== undefined) return cached;
const commandLine = readProcessCommandLine(pid);
if (commandLine === undefined) return false;
return isOcxStartCommandLine(commandLine);
const ok = isOcxStartCommandLine(commandLine);
ocxStartProcessCache.set(pid, ok);
return ok;
}

/**
* Alive pid from the pid file without the expensive Windows command-line probe.
* Safe for liveness polls: callers still identity-check /healthz before trusting the proxy.
* Destructive stop/kill paths should keep using {@link readPid}, which verifies the cmdline.
*/
export function readAlivePid(): number | null {
const pid = readPidFileValue();
if (pid === null) return null;
try {
process.kill(pid, 0);
return pid;
} catch (e: unknown) {
if ((e as NodeJS.ErrnoException).code === "EPERM") return pid;
return null;
}
}

function readProcessCommandLine(pid: number): string | undefined {
try {
if (process.platform === "win32") {
// Prefer WMIC over PowerShell: much faster cold start, and windowsHide avoids console flash.
// Fall back to PowerShell when WMIC is absent (newer Windows images).
const wmic = `${process.env.SystemRoot ?? "C:\\Windows"}\\System32\\wbem\\WMIC.exe`;
try {
const output = execFileSync(wmic, [
"process", "where", `ProcessId=${pid}`, "get", "CommandLine", "/VALUE",
], { encoding: "utf-8", stdio: ["ignore", "pipe", "ignore"], timeout: 3000, windowsHide: true });
const match = /^CommandLine=(.*)$/m.exec(output.replace(/\r/g, ""));
const value = match?.[1]?.trim();
if (value) return value;
} catch {
/* WMIC missing or failed — fall through */
}
const output = execFileSync("powershell.exe", [
"-NoProfile",
"-NoLogo",
"-NonInteractive",
"-WindowStyle",
"Hidden",
"-Command",
`(Get-CimInstance Win32_Process -Filter "ProcessId = ${pid}").CommandLine`,
], { encoding: "utf-8", stdio: ["ignore", "pipe", "ignore"], timeout: 3000 });
], { encoding: "utf-8", stdio: ["ignore", "pipe", "ignore"], timeout: 3000, windowsHide: true });
return output.trim() || undefined;
}
const output = execFileSync("ps", ["-p", String(pid), "-o", "command="], {
encoding: "utf-8",
stdio: ["ignore", "pipe", "ignore"],
timeout: 1000,
windowsHide: true,
});
return output.trim() || undefined;
} catch {
Expand Down
2 changes: 1 addition & 1 deletion src/lib/process-control.ts
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,7 @@ export function killProxy(pid: number): void {
if (process.platform === "win32") {
const taskkill = `${process.env.SystemRoot ?? "C:\\Windows"}\\System32\\taskkill.exe`;
try {
execFileSync(taskkill, ["/PID", String(pid), "/T", "/F"], { stdio: "pipe" });
execFileSync(taskkill, ["/PID", String(pid), "/T", "/F"], { stdio: "pipe", windowsHide: true });
} catch (err) {
if (isProcessAlive(pid)) throw err;
}
Expand Down
46 changes: 44 additions & 2 deletions src/server/ports.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,50 @@ export async function isPortAvailable(port: number, hostname = "127.0.0.1"): Pro
});
}

export async function findAvailablePort(preferredPort: number, hostname = "127.0.0.1"): Promise<number> {
if (await isPortAvailable(preferredPort, hostname)) return preferredPort;
export type WaitForPortOptions = {
timeoutMs?: number;
intervalMs?: number;
};

/** Poll until `port` accepts a bind, or until the timeout elapses. */
export async function waitForPortAvailable(
port: number,
hostname = "127.0.0.1",
opts: WaitForPortOptions = {},
): Promise<boolean> {
const timeoutMs = opts.timeoutMs ?? 5000;
const intervalMs = opts.intervalMs ?? 50;
const deadline = Date.now() + timeoutMs;
for (;;) {
if (await isPortAvailable(port, hostname)) return true;
if (Date.now() >= deadline) return false;
await new Promise(resolve => setTimeout(resolve, intervalMs));
}
}

export type FindAvailablePortOptions = {
/** How long to keep retrying the preferred port before falling back to an ephemeral port. */
preferRetryMs?: number;
preferRetryIntervalMs?: number;
};

export async function findAvailablePort(
preferredPort: number,
hostname = "127.0.0.1",
opts: FindAvailablePortOptions = {},
): Promise<number> {
const preferRetryMs = opts.preferRetryMs ?? 0;
if (preferRetryMs > 0) {
if (await waitForPortAvailable(preferredPort, hostname, {
timeoutMs: preferRetryMs,
intervalMs: opts.preferRetryIntervalMs ?? 50,
})) {
return preferredPort;
}
} else if (await isPortAvailable(preferredPort, hostname)) {
return preferredPort;
}

return await new Promise((resolve, reject) => {
const server = createServer();
server.once("error", reject);
Expand Down
6 changes: 4 additions & 2 deletions src/server/proxy-liveness.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
*
* Lives outside cli.ts (which dispatches argv at module top level) so tests can import it.
*/
import { loadConfig, readPid, readRuntimePort } from "../config";
import { loadConfig, readAlivePid, readRuntimePort } from "../config";

export interface HealthzIdentity {
service?: unknown;
Expand Down Expand Up @@ -86,7 +86,9 @@ export async function proxyIdentityAt(
* found and a foreign listener on the configured port is rejected.
*/
export async function findLiveProxy(io: LivenessIo = {}): Promise<LiveProxy | null> {
const readPidFn = io.readPidFn ?? readPid;
// Prefer the cheap alive-pid check: the Windows cmdline probe (WMIC/PowerShell) is too
// expensive for waitForProxy's 150ms poll loop, and /healthz identity is the real trust gate.
const readPidFn = io.readPidFn ?? readAlivePid;
const readRuntimeFn = io.readRuntimeFn ?? readRuntimePort;
const configFn = io.configFn ?? loadConfig;

Expand Down
35 changes: 25 additions & 10 deletions src/update/job.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,9 @@ import { spawn, spawnSync } from "node:child_process";
import { existsSync, mkdirSync, readFileSync } from "node:fs";
import { dirname, join } from "node:path";
import { fileURLToPath } from "node:url";
import { atomicWriteFile, getConfigDir, readPid } from "../config";
import { atomicWriteFile, getConfigDir, loadConfig, readPid } from "../config";
import { killProxy } from "../lib/process-control";
import { waitForPortAvailable } from "../server/ports";
import { isServiceInstalled } from "../service";
import {
type Channel,
Expand Down Expand Up @@ -158,18 +159,23 @@ export function restartCommand(
serviceInstalled: boolean,
installer: Installer,
launcher = packageLauncherPath(),
port?: number,
): { mode: "service" | "proxy"; bin: string; args: string[]; display: string } {
const mode = serviceInstalled ? "service" : "proxy";
const pinPort = !serviceInstalled && typeof port === "number" && Number.isFinite(port) && port > 0;
const startArgs = pinPort
? [launcher, "start", "--port", String(Math.trunc(port))]
: [launcher, "start"];
if (installer === "npm") {
const bin = nodeBin();
const args = serviceInstalled ? [launcher, "service", "install"] : [launcher, "start"];
const args = serviceInstalled ? [launcher, "service", "install"] : startArgs;
return { mode, bin, args, display: formatCommand(bin, args) };
}
// bun/source installs: restart via the current runtime executable + package launcher (both real
// .exe files), NOT the `ocx.cmd` shim. Spawning a `.cmd` shell-less throws EINVAL on Windows
// Node/Bun ≥18.20/20.12 (CVE-2024-27980 hardening) — the same class the npm path (nodeBin) avoids.
const bin = process.execPath;
const args = serviceInstalled ? [launcher, "service", "install"] : [launcher, "start"];
const args = serviceInstalled ? [launcher, "service", "install"] : startArgs;
return { mode, bin, args, display: formatCommand(bin, args) };
}

Expand Down Expand Up @@ -264,8 +270,8 @@ function runLoggedCommand(job: UpdateJobState, bin: string, args: string[], time
return { status: result.status, signal: result.signal };
}

function spawnDetachedStart(job: UpdateJobState, installer: Installer): void {
const cmd = restartCommand(false, installer);
function spawnDetachedStart(job: UpdateJobState, installer: Installer, port?: number): void {
const cmd = restartCommand(false, installer, packageLauncherPath(), port);
const env = { ...process.env };
delete env.OCX_SERVICE;
updateJob(job, {}, `$ ${cmd.display}`);
Expand All @@ -278,9 +284,12 @@ function spawnDetachedStart(job: UpdateJobState, installer: Installer): void {
child.unref();
}

function restartAfterUpdate(job: UpdateJobState): void {
async function restartAfterUpdate(job: UpdateJobState): Promise<void> {
const serviceInstalled = isServiceInstalled();
const cmd = restartCommand(serviceInstalled, job.installer);
const config = loadConfig();
const port = config.port ?? 10100;
const hostname = config.hostname ?? "127.0.0.1";
const cmd = restartCommand(serviceInstalled, job.installer, packageLauncherPath(), port);
if (serviceInstalled) {
const result = runLoggedCommand(job, cmd.bin, cmd.args, RESTART_TIMEOUT_MS);
if (result.status !== 0) {
Expand All @@ -293,11 +302,17 @@ function restartAfterUpdate(job: UpdateJobState): void {
if (pid) {
updateJob(job, {}, `Stopping current proxy PID ${pid}.`);
killProxy(pid);
// Windows taskkill can leave the listen socket busy briefly; wait only after a real
// kill so cold restarts are not charged a multi-second prefer-wait.
const freed = await waitForPortAvailable(port, hostname, { timeoutMs: 2000, intervalMs: 25 });
if (!freed) {
updateJob(job, {}, `Port ${port} still busy after stop; starting with --port ${port} anyway.`);
}
}
spawnDetachedStart(job, job.installer);
spawnDetachedStart(job, job.installer, port);
}

export function runGuiUpdateWorker(jobId: string, channel: Channel, restart: boolean): void {
export async function runGuiUpdateWorker(jobId: string, channel: Channel, restart: boolean): Promise<void> {
let job = readUpdateJob(jobId);
const check = checkForUpdate(channel);
const now = new Date().toISOString();
Expand Down Expand Up @@ -362,7 +377,7 @@ export function runGuiUpdateWorker(jobId: string, channel: Channel, restart: boo

if (restart) {
job = updateJob(job, { status: "restarting" }, "Update installed. Restarting proxy...");
restartAfterUpdate(job);
await restartAfterUpdate(job);
updateJob(job, { status: "succeeded", restarted: true }, "Restart requested.");
return;
}
Expand Down
37 changes: 36 additions & 1 deletion tests/ports.test.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { afterEach, describe, expect, test } from "bun:test";
import { createServer, type Server } from "node:net";
import { findAvailablePort, isAddrInUse, isPortAvailable, shouldPersistSelectedPort } from "../src/server/ports";
import { findAvailablePort, isAddrInUse, isPortAvailable, shouldPersistSelectedPort, waitForPortAvailable } from "../src/server/ports";

const servers: Server[] = [];

Expand Down Expand Up @@ -54,6 +54,41 @@ describe("port selection", () => {
expect(shouldPersistSelectedPort(10100, 10100, 10100)).toBe(false);
});

test("waitForPortAvailable resolves once a busy port is released", async () => {
const { server, port } = await listen();
expect(await isPortAvailable(port)).toBe(false);

const waiting = waitForPortAvailable(port, "127.0.0.1", { timeoutMs: 2000, intervalMs: 25 });
await close(server);
const idx = servers.indexOf(server);
if (idx >= 0) servers.splice(idx, 1);

await expect(waiting).resolves.toBe(true);
expect(await isPortAvailable(port)).toBe(true);
});

test("waitForPortAvailable returns false when the port stays busy past the timeout", async () => {
const { port } = await listen();
await expect(waitForPortAvailable(port, "127.0.0.1", { timeoutMs: 80, intervalMs: 20 })).resolves.toBe(false);
expect(await isPortAvailable(port)).toBe(false);
});

test("findAvailablePort retries the preferred port briefly before falling back", async () => {
const { server, port } = await listen();
expect(await isPortAvailable(port)).toBe(false);

const pending = findAvailablePort(port, "127.0.0.1", { preferRetryMs: 500, preferRetryIntervalMs: 25 });
// Free the preferred port during the retry window.
setTimeout(() => {
void close(server).then(() => {
const idx = servers.indexOf(server);
if (idx >= 0) servers.splice(idx, 1);
});
}, 60);

expect(await pending).toBe(port);
});

test("isAddrInUse recognizes bind conflicts by code or message and rejects everything else", () => {
expect(isAddrInUse(Object.assign(new Error("listen failed"), { code: "EADDRINUSE" }))).toBe(true);
expect(isAddrInUse(new Error("listen EADDRINUSE: address already in use ::1:8123"))).toBe(true);
Expand Down
11 changes: 11 additions & 0 deletions tests/update-job.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -98,6 +98,17 @@ describe("GUI update execution decisions", () => {
});
});

test("proxy restart pins --port so post-update start does not hop to an ephemeral port", () => {
const proxy = restartCommand(false, "npm", "/pkg/bin/ocx.mjs", 10100);
expect(proxy.mode).toBe("proxy");
expect(proxy.args).toEqual(["/pkg/bin/ocx.mjs", "start", "--port", "10100"]);
expect(proxy.display).toContain("start --port 10100");
// Service reinstall keeps its own port contract — do not append --port.
expect(restartCommand(true, "npm", "/pkg/bin/ocx.mjs", 10100).args).toEqual([
"/pkg/bin/ocx.mjs", "service", "install",
]);
});

test("a running job prevents a second update job", () => {
const now = new Date().toISOString();
const job: UpdateJobState = {
Expand Down
5 changes: 4 additions & 1 deletion tests/usage-debug.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -150,7 +150,10 @@ describe("appendUsageDebug", () => {
});

test("keeps file size bounded by MAX_LINES across long runs", () => {
for (let i = 0; i < USAGE_DEBUG_MAX_LINES * 3; i++) {
// Cross the rotate threshold twice — enough to prove the bound holds across
// multiple rewrites without MAX*3 appends (that path times out under full-suite load).
const total = USAGE_DEBUG_MAX_LINES + USAGE_DEBUG_KEEP_LINES + 25;
for (let i = 0; i < total; i++) {
appendUsageDebug(sample({ requestId: `ocx-${i}`, ts: i }));
}
const path = usageDebugPath();
Expand Down
5 changes: 4 additions & 1 deletion tests/windows-deploy-close-regressions.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,10 @@ describe("update-job restart avoids the shell-less .cmd EINVAL (Windows, bun/sou
});
test("bun/source restart uses the runtime executable + launcher (a real .exe, no shell)", () => {
// restartCommand's non-npm branch resolves to process.execPath + the package launcher.
expect(src).toMatch(/const bin = process\.execPath;\s*\n\s*const args = serviceInstalled \? \[launcher, "service", "install"\] : \[launcher, "start"\];/);
// Proxy mode may pin --port via startArgs; service mode stays install-only.
expect(src).toMatch(/const bin = process\.execPath;\s*\n\s*const args = serviceInstalled \? \[launcher, "service", "install"\] : startArgs;/);
expect(src).toContain('? [launcher, "start", "--port", String(Math.trunc(port))]');
expect(src).toContain(': [launcher, "start"]');
});
});

Expand Down
Loading