fix(cli): repair editor launch, process cleanup and port selection on Windows
Open-in-editor was broken outright, and took the daemon with it. All four editors ship as `.cmd` shims on Windows and libuv's PATH search only tries `.com` and `.exe` — so `where code` found the shim, onPath said yes, and the spawn failed with ENOENT. spawn reports that asynchronously, so the try/catch never saw it, and an `error` event with no listener throws: one click on a file it could not open exited the server. The URL-scheme fallback was no better, since it embedded a Windows path with backslashes, which are not legal in a URI path. Both are fixed, and every detached spawn here now has an error listener. `--exec` orphaned the dev server. The child is spawned with a shell, so on Windows it is cmd.exe and the real tree is cmd.exe -> pnpm -> node -> Vite; child.kill() maps to TerminateProcess on cmd.exe alone and left that tree holding the port, outliving airship and blocking the next launch. taskkill /T takes the descendants with it. Since that is already a forced kill, the SIGTERM-then-grace-then-SIGKILL escalation is skipped there. firstFreePort connect-probed, which answers "is anything accepting" rather than "can I bind" — and the two diverge on Windows, where Hyper-V, WSL2 and Docker Desktop reserve whole port ranges that accept nothing yet refuse to be bound. It now binds and lets go. The residual case is handled properly too: listen() had no error listener, so EADDRINUSE/EACCES became an uncaughtException that escaped the caller's try/catch, leaving the dev server we started unstopped. Also: opencode resolves through PATHEXT, because `npm i -g opencode-ai` — which our own error message recommends — writes opencode.cmd, never opencode.exe; its credential probe reads %LOCALAPPDATA%/%APPDATA% rather than XDG paths that are never set on Windows; SIGBREAK is registered, since Windows never delivers SIGTERM; and openInBrowser uses `cmd /c start ""` rather than a shell, where Node does no argument escaping and a URL carrying `&` would be split in two.
This commit is contained in:
@@ -204,6 +204,28 @@ async function assertFree(port: number): Promise<void> {
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Turn a failure to bind the overlay's port into something actionable.
|
||||
*
|
||||
* EACCES is the Windows-specific one: Hyper-V, WSL2 and Docker Desktop reserve
|
||||
* whole ranges of dynamic ports, and a port inside one accepts no connection
|
||||
* yet refuses to be bound — so it looks free right up until it isn't.
|
||||
*/
|
||||
function asBindError(err: unknown, port: number): unknown {
|
||||
const code = (err as NodeJS.ErrnoException | null)?.code;
|
||||
if (code === "EADDRINUSE") {
|
||||
return new CliError(`Port ${port} is already in use`, {
|
||||
hint: "Pass --port with a free one.",
|
||||
});
|
||||
}
|
||||
if (code === "EACCES") {
|
||||
return new CliError(`Not allowed to bind port ${port}`, {
|
||||
hint: "On Windows this port may sit in a reserved range (see `netsh interface ipv4 show excludedportrange tcp`). Pass --port with one outside it.",
|
||||
});
|
||||
}
|
||||
return err;
|
||||
}
|
||||
|
||||
export const serve = defineCommand({
|
||||
args: argsFor(SERVE_FLAGS),
|
||||
meta: {
|
||||
@@ -274,7 +296,7 @@ export const serve = defineCommand({
|
||||
// We started the dev server; if the proxy cannot come up it is ours to
|
||||
// clean up, or the user is left with a stray process holding the port.
|
||||
await dev?.stop();
|
||||
throw err;
|
||||
throw asBindError(err, port);
|
||||
}
|
||||
|
||||
if (opts.json) {
|
||||
@@ -332,5 +354,12 @@ export const serve = defineCommand({
|
||||
};
|
||||
process.on("SIGINT", onSignal);
|
||||
process.on("SIGTERM", onSignal);
|
||||
// Windows never delivers SIGTERM — registering it is legal, it just never
|
||||
// fires — so without SIGBREAK the only clean exit there is Ctrl-C. Ctrl-Break
|
||||
// and a console close would otherwise skip shutdown entirely and strand the
|
||||
// `opencode serve` child and any dev server we started.
|
||||
if (process.platform === "win32") {
|
||||
process.on("SIGBREAK", onSignal);
|
||||
}
|
||||
},
|
||||
});
|
||||
|
||||
@@ -172,6 +172,25 @@ export async function detectTarget(
|
||||
return first ? { ...first, listening: false } : undefined;
|
||||
}
|
||||
|
||||
/**
|
||||
* Whether we can actually take this port, by taking it and letting go.
|
||||
*
|
||||
* `isListening` answers a different question — "is something accepting here" —
|
||||
* and the two diverge on Windows, where Hyper-V, WSL2 and Docker Desktop
|
||||
* reserve whole ranges of dynamic ports (`netsh interface ipv4 show
|
||||
* excludedportrange tcp`). Nothing accepts on a reserved port, so a connect
|
||||
* probe calls it free, and the bind then fails with EACCES.
|
||||
*/
|
||||
function canBind(port: number, host: string): Promise<boolean> {
|
||||
return new Promise((resolvePromise) => {
|
||||
const probe = net.createServer();
|
||||
probe.once("error", () => resolvePromise(false));
|
||||
probe.listen({ host, port }, () => {
|
||||
probe.close(() => resolvePromise(true));
|
||||
});
|
||||
});
|
||||
}
|
||||
|
||||
/** A port nothing is bound to, so the proxy does not collide on startup. */
|
||||
export async function firstFreePort(
|
||||
start: number,
|
||||
@@ -182,7 +201,7 @@ export async function firstFreePort(
|
||||
// Also ordered: "the first free port at or after `start`" is the answer, so
|
||||
// the probes cannot be collapsed into one parallel batch.
|
||||
// biome-ignore lint/performance/noAwaitInLoops: ordered first-match search
|
||||
if (!(await isListening(port, host))) {
|
||||
if (await canBind(port, host)) {
|
||||
return port;
|
||||
}
|
||||
}
|
||||
|
||||
+38
-13
@@ -10,11 +10,13 @@
|
||||
* first, and our own shutdown would be racing a corpse.
|
||||
*/
|
||||
|
||||
import { type ChildProcess, spawn } from "node:child_process";
|
||||
import { type ChildProcess, spawn, spawnSync } from "node:child_process";
|
||||
import { isListening } from "./detect";
|
||||
import { CliError } from "./errors";
|
||||
import { style } from "./terminal";
|
||||
|
||||
const WIN32 = process.platform === "win32";
|
||||
|
||||
const READY_TIMEOUT_MS = 90_000;
|
||||
const POLL_INTERVAL_MS = 250;
|
||||
const STOP_GRACE_MS = 3000;
|
||||
@@ -33,9 +35,16 @@ function killTree(child: ChildProcess, signal: NodeJS.Signals): void {
|
||||
return;
|
||||
}
|
||||
try {
|
||||
if (process.platform === "win32") {
|
||||
// No process groups to negate; Node maps this to TerminateProcess.
|
||||
child.kill(signal);
|
||||
if (WIN32) {
|
||||
// `child` here is cmd.exe, not the dev server — we spawn with `shell`,
|
||||
// so the real tree is cmd.exe -> pnpm -> node -> Vite. `child.kill()`
|
||||
// maps to TerminateProcess on cmd.exe alone and leaves that tree running,
|
||||
// still holding the port, outliving airship and blocking the next launch.
|
||||
// taskkill /T is the only way to take the descendants with it.
|
||||
spawnSync("taskkill", ["/pid", String(child.pid), "/T", "/F"], {
|
||||
stdio: "ignore",
|
||||
windowsHide: true,
|
||||
});
|
||||
return;
|
||||
}
|
||||
// Negative pid = the whole group, which is what actually stops a dev server
|
||||
@@ -87,12 +96,25 @@ export async function startDevServer(opts: {
|
||||
child.once("exit", (code, signal) => {
|
||||
exited = { code, signal };
|
||||
});
|
||||
// A shell that will not start emits `error` and never `exit`. Without this
|
||||
// the readiness loop below would wait the full 90s for a process that was
|
||||
// never running — and an unheard `error` event throws.
|
||||
child.once("error", () => {
|
||||
exited ??= { code: null, signal: null };
|
||||
});
|
||||
|
||||
const stop = async (): Promise<void> => {
|
||||
if (exited) {
|
||||
return;
|
||||
}
|
||||
killTree(child, "SIGTERM");
|
||||
// Windows has no graceful signal — killTree already went straight to
|
||||
// `taskkill /F`, so there is nothing to escalate to and nothing to wait
|
||||
// for. Polling for three seconds before repeating the same forced kill
|
||||
// would only delay shutdown.
|
||||
if (WIN32) {
|
||||
return;
|
||||
}
|
||||
for (let waited = 0; waited < STOP_GRACE_MS; waited += POLL_INTERVAL_MS) {
|
||||
if (exited) {
|
||||
return;
|
||||
@@ -134,26 +156,29 @@ export async function startDevServer(opts: {
|
||||
}
|
||||
}
|
||||
|
||||
const BROWSER_OPENERS: Record<string, string> = {
|
||||
darwin: "open",
|
||||
win32: "start",
|
||||
};
|
||||
|
||||
/** Open a URL in the default browser, best-effort. */
|
||||
export function openInBrowser(url: string): void {
|
||||
const command = BROWSER_OPENERS[process.platform] ?? "xdg-open";
|
||||
// `cmd /c start "" <url>` on Windows rather than `shell: true` + `start`.
|
||||
// With a shell Node does no argument escaping, so a URL carrying `&` would be
|
||||
// split by cmd.exe into two commands; and `start` reads its first quoted
|
||||
// argument as a window title, which is what the empty string absorbs. Same
|
||||
// form as openUrl in @airship/server's open-editor.
|
||||
const [command, args] =
|
||||
process.platform === "win32"
|
||||
? (["cmd", ["/c", "start", "", url]] as const)
|
||||
: ([process.platform === "darwin" ? "open" : "xdg-open", [url]] as const);
|
||||
try {
|
||||
// Detached and fully ignored: the browser outliving airship is the point,
|
||||
// and its stdio would otherwise pollute the terminal.
|
||||
const child = spawn(command, [url], {
|
||||
const child = spawn(command, [...args], {
|
||||
detached: true,
|
||||
shell: process.platform === "win32",
|
||||
stdio: "ignore",
|
||||
windowsHide: true,
|
||||
});
|
||||
child.unref();
|
||||
child.on("error", () => {
|
||||
// No browser, no display, no `open` — the URL is printed either way.
|
||||
});
|
||||
child.unref();
|
||||
} catch {
|
||||
// Same: --open is a convenience, never a reason to fail a launch.
|
||||
}
|
||||
|
||||
@@ -96,7 +96,29 @@ export interface OpencodeHandle {
|
||||
url: string;
|
||||
}
|
||||
|
||||
const BINARY = process.platform === "win32" ? "opencode.exe" : "opencode";
|
||||
/**
|
||||
* The names `opencode` can go by on disk, most-specific first.
|
||||
*
|
||||
* Hardcoding `opencode.exe` on Windows found nothing for the install we
|
||||
* ourselves recommend: `npm i -g opencode-ai` writes `opencode.cmd` and
|
||||
* `opencode.ps1` shims, never a `.exe`. So a perfectly good opencode reported
|
||||
* "No `opencode` binary found on PATH" and every edit through that backend
|
||||
* failed. PATHEXT is the OS's own answer to which extensions are executable;
|
||||
* the fallback matches its default.
|
||||
*/
|
||||
const BINARY_NAMES: string[] =
|
||||
process.platform === "win32"
|
||||
? [
|
||||
...(process.env.PATHEXT ?? ".COM;.EXE;.BAT;.CMD")
|
||||
.split(";")
|
||||
.filter(Boolean)
|
||||
.map((ext) => `opencode${ext.toLowerCase()}`),
|
||||
// Extension-less last, so a real launcher always wins — but still
|
||||
// present, because dropping it would be its own regression for anyone
|
||||
// whose opencode is a bare binary on PATH.
|
||||
"opencode",
|
||||
]
|
||||
: ["opencode"];
|
||||
/** A cold first launch on a slow machine must not read as a broken install. */
|
||||
const START_TIMEOUT_MS = 30_000;
|
||||
|
||||
@@ -115,9 +137,11 @@ export function resolveOpencodeBinary(override?: string): string | null {
|
||||
if (!dir) {
|
||||
continue;
|
||||
}
|
||||
const candidate = join(dir, BINARY);
|
||||
if (existsSync(candidate)) {
|
||||
return candidate;
|
||||
for (const name of BINARY_NAMES) {
|
||||
const candidate = join(dir, name);
|
||||
if (existsSync(candidate)) {
|
||||
return candidate;
|
||||
}
|
||||
}
|
||||
}
|
||||
return null;
|
||||
@@ -255,9 +279,16 @@ function registerExitHooks(): void {
|
||||
return;
|
||||
}
|
||||
hooksRegistered = true;
|
||||
for (const signal of ["exit", "SIGINT", "SIGTERM"] as const) {
|
||||
// SIGBREAK on Windows, where SIGTERM is accepted by `process.once` but never
|
||||
// delivered — Ctrl-Break would otherwise leave this child holding its port.
|
||||
// "exit" remains the backstop that catches everything else.
|
||||
const signals = ["exit", "SIGINT", "SIGTERM"] as const;
|
||||
for (const signal of signals) {
|
||||
process.once(signal, () => shutdownServer());
|
||||
}
|
||||
if (process.platform === "win32") {
|
||||
process.once("SIGBREAK", () => shutdownServer());
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
|
||||
@@ -524,8 +524,17 @@ export async function startServer(opts: ServerOptions): Promise<RunningServer> {
|
||||
socket.on("close", () => tunnelled.delete(socket as Socket));
|
||||
});
|
||||
|
||||
await new Promise<void>((resolve) => {
|
||||
server.listen(opts.port, () => resolve());
|
||||
// The `error` listener is what makes this rejectable. Without it an
|
||||
// EADDRINUSE — or, on Windows, the EACCES you get from a port inside a
|
||||
// Hyper-V/WSL2 reserved range — is emitted with nobody listening, becomes an
|
||||
// uncaughtException, and escapes the caller's try/catch, so the dev server
|
||||
// airship started with `--exec` is never stopped either.
|
||||
await new Promise<void>((resolve, reject) => {
|
||||
server.once("error", reject);
|
||||
server.listen(opts.port, () => {
|
||||
server.removeListener("error", reject);
|
||||
resolve();
|
||||
});
|
||||
});
|
||||
|
||||
const addr = server.address() as AddressInfo | null;
|
||||
|
||||
@@ -8,12 +8,15 @@
|
||||
* a column and are awkward to detect. So: try the binary, fall back to the
|
||||
* scheme.
|
||||
*/
|
||||
import { spawn, spawnSync } from "node:child_process";
|
||||
import { type ChildProcess, spawn, spawnSync } from "node:child_process";
|
||||
import { existsSync } from "node:fs";
|
||||
import { platform } from "node:os";
|
||||
import { resolve, sep } from "node:path";
|
||||
import { resolve } from "node:path";
|
||||
import { isPathInside, toPosixPath } from "@airship/core";
|
||||
import type { Editor } from "@airship/protocol";
|
||||
|
||||
const WIN32 = platform() === "win32";
|
||||
|
||||
export interface OpenRequest {
|
||||
column?: number;
|
||||
editor?: Editor;
|
||||
@@ -48,8 +51,10 @@ const LAUNCHERS: Record<Editor, Launcher> = {
|
||||
const PREFERENCE: Editor[] = ["vscode", "cursor", "windsurf", "zed"];
|
||||
|
||||
/** Vite serves out-of-root files under `/@fs/<abs path>`. */
|
||||
const VITE_FS_PREFIX = /^\/@fs(\/.*)$/;
|
||||
const VITE_FS_PREFIX = /^\/@fs\/(.*)$/;
|
||||
const LEADING_SLASHES = /^\/+/;
|
||||
/** `C:` — a Windows path that is already rooted at a drive. */
|
||||
const WIN32_DRIVE = /^[a-zA-Z]:/;
|
||||
|
||||
export function openInEditor(cwd: string, req: OpenRequest): OpenResult {
|
||||
const root = resolve(cwd);
|
||||
@@ -57,7 +62,12 @@ export function openInEditor(cwd: string, req: OpenRequest): OpenResult {
|
||||
// This socket is unauthenticated and local, and this handler spawns
|
||||
// processes — without the containment check any page the browser loads could
|
||||
// ask the daemon to open arbitrary files on disk.
|
||||
if (abs !== root && !abs.startsWith(root + sep)) {
|
||||
//
|
||||
// isPathInside rather than a bare startsWith: the two sides reach here by
|
||||
// different routes and can disagree on symlinks (macOS `/tmp`, `/var`) or on
|
||||
// case (Windows, including the drive letter), and a false "outside the
|
||||
// project" on the user's own file is indistinguishable from a real refusal.
|
||||
if (!isPathInside(root, abs)) {
|
||||
return { error: "path is outside the project", ok: false };
|
||||
}
|
||||
if (!existsSync(abs)) {
|
||||
@@ -73,22 +83,13 @@ export function openInEditor(cwd: string, req: OpenRequest): OpenResult {
|
||||
if (!onPath(launcher.bin)) {
|
||||
continue;
|
||||
}
|
||||
try {
|
||||
// Detached and unref'd: the editor outlives the daemon, and inheriting
|
||||
// stdio would wire its output into ours. Never `shell: true` — argv
|
||||
// arrays mean a path with a space or a quote is just a path.
|
||||
spawn(launcher.bin, launcher.args(target), {
|
||||
detached: true,
|
||||
stdio: "ignore",
|
||||
}).unref();
|
||||
if (launch(launcher.bin, launcher.args(target))) {
|
||||
return { editor, ok: true };
|
||||
} catch {
|
||||
// Fall through to the next candidate, then to the URL scheme.
|
||||
}
|
||||
}
|
||||
|
||||
const wanted = req.editor ?? PREFERENCE[0];
|
||||
if (openUrl(`${LAUNCHERS[wanted].scheme}://file/${abs}:${line}:${column}`)) {
|
||||
if (openUrl(editorUrl(LAUNCHERS[wanted].scheme, abs, line, column))) {
|
||||
return { editor: wanted, ok: true };
|
||||
}
|
||||
return {
|
||||
@@ -111,16 +112,83 @@ export function openInEditor(cwd: string, req: OpenRequest): OpenResult {
|
||||
function projectPath(file: string): string {
|
||||
const fs = file.match(VITE_FS_PREFIX);
|
||||
if (fs?.[1]) {
|
||||
return fs[1];
|
||||
// The remainder is already absolute. On POSIX it needs the slash the
|
||||
// capture dropped; on Windows it opens with a drive letter and must NOT
|
||||
// get one, because `resolve` reads a leading slash as rooted-but-
|
||||
// deviceless and inherits the device from cwd — turning
|
||||
// `/@fs/C:/Users/me/x.tsx` into `C:\C:\Users\me\x.tsx`.
|
||||
return WIN32_DRIVE.test(fs[1]) ? fs[1] : `/${fs[1]}`;
|
||||
}
|
||||
// A real absolute path exists on disk; a URL path like `/src/App.tsx` does
|
||||
// not, and is meant to be read relative to the project root.
|
||||
if (file.startsWith("/") && !existsSync(file)) {
|
||||
// not, and is meant to be read relative to the project root. On Windows the
|
||||
// probe is skipped: a leading slash with no drive letter is never an absolute
|
||||
// path there, so `existsSync` would only ever be answering about a different
|
||||
// file — `<cwd's drive>:\src\App.tsx`, which routinely exists.
|
||||
if (file.startsWith("/") && (WIN32 || !existsSync(file))) {
|
||||
return file.replace(LEADING_SLASHES, "");
|
||||
}
|
||||
return file;
|
||||
}
|
||||
|
||||
/**
|
||||
* Start a detached child, or report that it could not start.
|
||||
*
|
||||
* `shell` on Windows because every one of these editors ships as a `.cmd` shim
|
||||
* (`code.cmd`, `cursor.cmd`, …) and libuv's PATH search only ever tries `.com`
|
||||
* and `.exe`. `where code` finds the shim, so `onPath` says yes, and the bare
|
||||
* spawn then fails with ENOENT — which is how this managed to be 100% broken on
|
||||
* Windows while looking fine. Node applies cmd-specific argument escaping when
|
||||
* `shell` is set, so a path with a space in it survives.
|
||||
*
|
||||
* The `error` listener is not optional. spawn reports a failure to start
|
||||
* asynchronously, so the `try` here never sees it; an EventEmitter that emits
|
||||
* `error` with nobody listening throws, and this runs in the daemon, so one
|
||||
* click on a file it cannot open took the whole server down.
|
||||
*
|
||||
* `pid` is undefined when the spawn failed outright, which is the only
|
||||
* synchronous signal available — and it is what lets the caller fall through to
|
||||
* the URL scheme instead of reporting a success that never happened.
|
||||
*/
|
||||
function launch(bin: string, args: string[]): boolean {
|
||||
try {
|
||||
const child = spawn(bin, args, {
|
||||
detached: true,
|
||||
shell: WIN32,
|
||||
stdio: "ignore",
|
||||
windowsHide: true,
|
||||
});
|
||||
detach(child);
|
||||
return child.pid !== undefined;
|
||||
} catch {
|
||||
return false;
|
||||
}
|
||||
}
|
||||
|
||||
/** Let the child outlive us, and never let its `error` reach the top level. */
|
||||
function detach(child: ChildProcess): void {
|
||||
child.on("error", () => {
|
||||
// No editor, no PATH entry, no display. The caller reports it; the daemon
|
||||
// must not die of it.
|
||||
});
|
||||
child.unref();
|
||||
}
|
||||
|
||||
/**
|
||||
* `vscode://file/...` for the fallback path.
|
||||
*
|
||||
* Backslashes are not legal in a URI path, so a Windows path has to be
|
||||
* rewritten or the scheme handler simply ignores it. POSIX paths already open
|
||||
* with `/`, which is why there is only one after `file` in the template.
|
||||
*/
|
||||
function editorUrl(
|
||||
scheme: string,
|
||||
abs: string,
|
||||
line: number,
|
||||
column: number
|
||||
): string {
|
||||
return `${scheme}://file/${toPosixPath(abs)}:${line}:${column}`;
|
||||
}
|
||||
|
||||
function candidates(explicit?: Editor): Editor[] {
|
||||
if (explicit) {
|
||||
return [explicit];
|
||||
@@ -140,10 +208,12 @@ function onPath(bin: string): boolean {
|
||||
if (hit !== undefined) {
|
||||
return hit;
|
||||
}
|
||||
const probe = platform() === "win32" ? "where" : "which";
|
||||
const probe = WIN32 ? "where" : "which";
|
||||
let found = false;
|
||||
try {
|
||||
found = spawnSync(probe, [bin], { stdio: "ignore" }).status === 0;
|
||||
found =
|
||||
spawnSync(probe, [bin], { stdio: "ignore", windowsHide: true }).status ===
|
||||
0;
|
||||
} catch {
|
||||
found = false;
|
||||
}
|
||||
@@ -158,12 +228,20 @@ function openUrl(url: string): boolean {
|
||||
if (os === "darwin") {
|
||||
bin = "open";
|
||||
} else if (os === "win32") {
|
||||
// `cmd /c start "" <url>` rather than `shell: true`: `start` takes a window
|
||||
// title as its first quoted argument, so the empty string is what stops it
|
||||
// swallowing the URL as one. cmd.exe is a real .exe, so no shell needed.
|
||||
bin = "cmd";
|
||||
args = ["/c", "start", "", url];
|
||||
}
|
||||
try {
|
||||
spawn(bin, args, { detached: true, stdio: "ignore" }).unref();
|
||||
return true;
|
||||
const child = spawn(bin, args, {
|
||||
detached: true,
|
||||
stdio: "ignore",
|
||||
windowsHide: true,
|
||||
});
|
||||
detach(child);
|
||||
return child.pid !== undefined;
|
||||
} catch {
|
||||
return false;
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user