diff --git a/packages/cli/__tests__/commands/start.test.ts b/packages/cli/__tests__/commands/start.test.ts index af7a7ef57..60eea2924 100644 --- a/packages/cli/__tests__/commands/start.test.ts +++ b/packages/cli/__tests__/commands/start.test.ts @@ -1022,11 +1022,24 @@ describe("start command — orchestrator session strategy display", () => { // --------------------------------------------------------------------------- describe("stop command", () => { + /** Helper: mock exec to simulate a dashboard process on a given port. */ + function mockDashboardOnPort(dashboardPort: number, pid = "12345"): void { + mockExec.mockImplementation(async (cmd: string, args: string[] = []) => { + if (cmd === "kill") return { stdout: "", stderr: "" }; + if (cmd === "ps") return { stdout: "node /fake/web/dist-server/start-all.js", stderr: "" }; + if (cmd === "lsof") { + const portArg = args.find((a) => a.startsWith(":")); + if (portArg === `:${dashboardPort}`) return { stdout: pid, stderr: "" }; + } + throw new Error("no process"); + }); + } + it("stops orchestrator session and dashboard", async () => { mockConfigRef.current = makeConfig({ "my-app": makeProject() }); mockSessionManager.get.mockResolvedValue({ id: "app-orchestrator", status: "running" }); mockSessionManager.kill.mockResolvedValue(undefined); - mockExec.mockResolvedValue({ stdout: "12345", stderr: "" }); + mockDashboardOnPort(3000); await program.parseAsync(["node", "test", "stop"]); @@ -1059,7 +1072,7 @@ describe("stop command", () => { mockConfigRef.current = makeConfig({ "my-app": makeProject() }); mockSessionManager.get.mockResolvedValue({ id: "app-orchestrator", status: "running" }); mockSessionManager.kill.mockResolvedValue(undefined); - mockExec.mockResolvedValue({ stdout: "12345", stderr: "" }); + mockDashboardOnPort(3000); await program.parseAsync(["node", "test", "stop", "--purge-session"]); @@ -1073,13 +1086,7 @@ describe("stop command", () => { mockSessionManager.get.mockResolvedValue({ id: "app-orchestrator", status: "running" }); mockSessionManager.kill.mockResolvedValue(undefined); // Port 3000 has nothing, but port 3001 has the orphaned dashboard - mockExec.mockImplementation(async (cmd: string, args: string[] = []) => { - if (cmd === "kill") return { stdout: "", stderr: "" }; - if (cmd === "ps") return { stdout: "node /fake/web/dist-server/start-all.js", stderr: "" }; - const portArg = args.find((a) => a.startsWith(":")); - if (portArg === ":3001") return { stdout: "99999", stderr: "" }; - throw new Error("no process"); - }); + mockDashboardOnPort(3001, "99999"); await program.parseAsync(["node", "test", "stop"]); @@ -1104,9 +1111,11 @@ describe("stop command", () => { if (pid === "22222") return { stdout: "node /fake/web/dist-server/start-all.js", stderr: "" }; return { stdout: "", stderr: "" }; } - const portArg = args.find((a) => a.startsWith(":")); - if (portArg === ":3001") return { stdout: "11111", stderr: "" }; - if (portArg === ":3002") return { stdout: "22222", stderr: "" }; + if (cmd === "lsof") { + const portArg = args.find((a) => a.startsWith(":")); + if (portArg === ":3001") return { stdout: "11111", stderr: "" }; + if (portArg === ":3002") return { stdout: "22222", stderr: "" }; + } throw new Error("no process"); }); @@ -1119,6 +1128,39 @@ describe("stop command", () => { // Should skip port 3001 (python) and find the dashboard on 3002 expect(output).toContain("was on port 3002"); }); + + it("only kills dashboard PIDs when port has mixed processes", async () => { + mockConfigRef.current = makeConfig({ "my-app": makeProject() }); + mockSessionManager.get.mockResolvedValue({ id: "app-orchestrator", status: "running" }); + mockSessionManager.kill.mockResolvedValue(undefined); + // Port 3000 has two processes: a dashboard and an unrelated sidecar + mockExec.mockImplementation(async (cmd: string, args: string[] = []) => { + if (cmd === "kill") { + // Only the dashboard PID should be killed, not the sidecar + expect(args).toEqual(["11111"]); + return { stdout: "", stderr: "" }; + } + if (cmd === "ps") { + const pid = args[1]; + if (pid === "11111") return { stdout: "node /fake/web/dist-server/start-all.js", stderr: "" }; + if (pid === "22222") return { stdout: "nginx: worker process", stderr: "" }; + return { stdout: "", stderr: "" }; + } + if (cmd === "lsof") { + const portArg = args.find((a) => a.startsWith(":")); + if (portArg === ":3000") return { stdout: "11111\n22222", stderr: "" }; + } + throw new Error("no process"); + }); + + await program.parseAsync(["node", "test", "stop"]); + + const output = vi + .mocked(console.log) + .mock.calls.map((c) => c.join(" ")) + .join("\n"); + expect(output).toContain("Dashboard stopped"); + }); }); // --------------------------------------------------------------------------- diff --git a/packages/cli/src/commands/start.ts b/packages/cli/src/commands/start.ts index b04a2498a..5e6a20791 100644 --- a/packages/cli/src/commands/start.ts +++ b/packages/cli/src/commands/start.ts @@ -1145,20 +1145,14 @@ async function runStartup( // Keep dashboard process alive if it was started if (dashboardProcess) { - // Ensure the dashboard child is killed when the parent exits (e.g. Ctrl+C). - // Node.js does not guarantee signal propagation to child processes. - // Registering a SIGINT handler suppresses Node's default exit, so we - // must call process.exit() ourselves after cleaning up the child. - /* c8 ignore start -- signal handlers only fire on process termination */ - const killAndExit = (): void => { - try { - dashboardProcess?.kill("SIGTERM"); - } catch { - // already dead - } - process.exit(); - }; - const killDashboardOnly = (): void => { + // Kill the dashboard child when the parent exits for any reason + // (Ctrl+C, SIGTERM from `ao stop`, normal exit, etc.). + // We use the `exit` event instead of SIGINT/SIGTERM to avoid + // conflicting with the shutdown handler in registerStart that + // flushes lifecycle state and calls process.exit() with the + // correct exit code (130 for SIGINT, 0 for SIGTERM). + /* c8 ignore start -- exit handler only fires on process termination */ + const killDashboardChild = (): void => { try { dashboardProcess?.kill("SIGTERM"); } catch { @@ -1166,14 +1160,10 @@ async function runStartup( } }; /* c8 ignore stop */ - process.on("SIGINT", killAndExit); - process.on("SIGTERM", killAndExit); - process.on("exit", killDashboardOnly); + process.on("exit", killDashboardChild); dashboardProcess.on("exit", (code) => { - process.removeListener("SIGINT", killAndExit); - process.removeListener("SIGTERM", killAndExit); - process.removeListener("exit", killDashboardOnly); + process.removeListener("exit", killDashboardChild); if (openAbort) openAbort.abort(); if (code !== 0 && code !== null) { console.error(chalk.red(`Dashboard exited with code ${code}`)); @@ -1190,25 +1180,13 @@ async function runStartup( * Uses lsof to find the process listening on the port, then kills it. * Best effort — if it fails, just warn the user. */ -async function killOnPort(port: number): Promise { - try { - const { stdout } = await exec("lsof", ["-ti", `:${port}`]); - const pids = stdout - .trim() - .split("\n") - .filter((p) => p.length > 0); - if (pids.length === 0) return false; - await exec("kill", pids); - return true; - } catch { - return false; - } -} +/** Pattern matching AO dashboard processes (production and dev mode). */ +const DASHBOARD_CMD_PATTERN = /next-server|start-all\.js|next dev|ao-web/; /** * Check whether a process listening on the given port is an AO dashboard - * (next-server / node running start-all.js). Only kill if it matches, - * to avoid terminating unrelated services during the port-range scan. + * (next-server, start-all.js, or next dev). Only kills matching PIDs, + * leaving unrelated co-listeners (sidecars, SO_REUSEPORT) untouched. */ async function killDashboardOnPort(port: number): Promise { try { @@ -1219,20 +1197,21 @@ async function killDashboardOnPort(port: number): Promise { .filter((p) => p.length > 0); if (pids.length === 0) return false; - // Verify at least one PID is an AO dashboard process - const isDashboard = await Promise.all( - pids.map(async (pid) => { - try { - const { stdout: cmdline } = await exec("ps", ["-p", pid, "-o", "args="]); - return /next-server|start-all\.js/.test(cmdline); - } catch { - return false; + // Filter to only dashboard PIDs + const dashboardPids: string[] = []; + for (const pid of pids) { + try { + const { stdout: cmdline } = await exec("ps", ["-p", pid, "-o", "args="]); + if (DASHBOARD_CMD_PATTERN.test(cmdline)) { + dashboardPids.push(pid); } - }), - ); - if (!isDashboard.some(Boolean)) return false; + } catch { + // process vanished — skip + } + } + if (dashboardPids.length === 0) return false; - await exec("kill", pids); + await exec("kill", dashboardPids); return true; } catch { return false; @@ -1240,9 +1219,8 @@ async function killDashboardOnPort(port: number): Promise { } async function stopDashboard(port: number): Promise { - // 1. Try the expected port — no process-name check needed because - // the caller already knows a dashboard was started on this port - if (await killOnPort(port)) { + // 1. Try the expected port — verify it's a dashboard before killing + if (await killDashboardOnPort(port)) { console.log(chalk.green("Dashboard stopped")); return; }