diff --git a/src/cli.ts b/src/cli.ts index 9f06d115c..698199037 100644 --- a/src/cli.ts +++ b/src/cli.ts @@ -1040,7 +1040,6 @@ program const dir = noteQuery(queryRoot(dirArg)); const { existsSync } = await import("node:fs"); const { resolve, basename } = await import("node:path"); - const { spawn } = await import("node:child_process"); const { fileURLToPath } = await import("node:url"); const { contextDirFor } = await import("./context/node-file.js"); const { startVizServer } = await import("./viz/serve.js"); @@ -1078,10 +1077,7 @@ program repoName: basename(root), }); console.log(`${cmd("graft viz")} → ${srv.url} (ctrl-c to stop)`); - if (opts.open) { - const opener = process.platform === "darwin" ? "open" : process.platform === "win32" ? "start" : "xdg-open"; - spawn(opener, [srv.url], { stdio: "ignore", detached: true, shell: process.platform === "win32" }).unref(); - } + if (opts.open) openBrowser(srv.url); }); program diff --git a/src/util/browser.ts b/src/util/browser.ts index b715b9b24..f30d714a6 100644 --- a/src/util/browser.ts +++ b/src/util/browser.ts @@ -5,7 +5,13 @@ export function openBrowser(url: string): void { if (process.env.GRAFT_NO_BROWSER) return; const opener = process.platform === "darwin" ? "open" : process.platform === "win32" ? "start" : "xdg-open"; try { - spawn(opener, [url], { stdio: "ignore", detached: true, shell: process.platform === "win32" }).unref(); + const child = spawn(opener, [url], { stdio: "ignore", detached: true, shell: process.platform === "win32" }); + // A missing opener (no xdg-open on WSL or a minimal Linux) fails later, as an + // 'error' event: unheard, it would end this process, the viz server with it. + child.on("error", () => { + /* the printed URL is the fallback */ + }); + child.unref(); } catch { /* the printed URL is the fallback */ } diff --git a/test/browser-opener.test.ts b/test/browser-opener.test.ts new file mode 100644 index 000000000..52fa3f4ea --- /dev/null +++ b/test/browser-opener.test.ts @@ -0,0 +1,92 @@ +/** + * A missing browser opener must not take the process down (#819). On Linux + * without xdg-open (WSL, minimal images) spawn fails asynchronously with an + * 'error' event; unheard, it ends the process, and `viz` lost its server. + * Linux only: macOS always has `open`, and Windows goes through the shell. + */ +import { test } from "node:test"; +import assert from "node:assert/strict"; +import { spawn, spawnSync } from "node:child_process"; +import { mkdirSync, mkdtempSync, rmSync, writeFileSync } from "node:fs"; +import { createRequire } from "node:module"; +import { tmpdir } from "node:os"; +import { join, resolve } from "node:path"; +import { pathToFileURL } from "node:url"; + +const skip = process.platform !== "linux" ? "the opener is xdg-open on Linux only" : false; +const TSX = pathToFileURL(createRequire(join(process.cwd(), "x.js")).resolve("tsx")).href; +const CLI = resolve(process.cwd(), "src/cli.ts"); + +/** An environment whose PATH has no xdg-open (node runs by its full path). */ +function noOpenerEnv(dir: string): NodeJS.ProcessEnv { + const bin = join(dir, "bin"); + mkdirSync(bin, { recursive: true }); + const env = { ...process.env, PATH: bin }; + delete env.GRAFT_NO_BROWSER; + return env; +} + +test("openBrowser survives a missing opener", { skip }, () => { + const dir = mkdtempSync(join(tmpdir(), "graft-opener-")); + try { + const browser = pathToFileURL(resolve(process.cwd(), "src/util/browser.ts")).href; + const script = + `import { openBrowser } from ${JSON.stringify(browser)};\n` + + `openBrowser("http://127.0.0.1:1/");\n` + + `setTimeout(() => console.log("still alive"), 500);\n`; + const r = spawnSync(process.execPath, ["--import", TSX, "--input-type=module", "-e", script], { + cwd: dir, + env: noOpenerEnv(dir), + encoding: "utf8", + timeout: 30_000, + }); + assert.equal(r.status, 0, r.stderr); + assert.match(r.stdout, /still alive/); + assert.doesNotMatch(r.stderr, /ENOENT/); + } finally { + rmSync(dir, { recursive: true, force: true }); + } +}); + +test("viz keeps serving when the browser opener is missing", { skip }, async () => { + const dir = mkdtempSync(join(tmpdir(), "graft-viz-opener-")); + const contextDir = join(dir, "graft"); + mkdirSync(contextDir); + writeFileSync( + join(contextDir, "alpha.md"), + `---\nname: Alpha\nslug: alpha\ntype: system\nsources: []\nlinks: []\n---\nbody\n`, + ); + const child = spawn(process.execPath, ["--import", TSX, CLI, "--dir", contextDir, "viz", dir, "--port", "4851"], { + cwd: dir, + env: noOpenerEnv(dir), + stdio: ["ignore", "pipe", "pipe"], + }); + let out = ""; + let err = ""; + child.stdout.on("data", (d) => (out += d)); + child.stderr.on("data", (d) => (err += d)); + const exited = new Promise((r) => child.on("exit", (code) => r(code))); + try { + const url = await new Promise((res, rej) => { + const t = setTimeout(() => rej(new Error(`no URL printed\n${out}\n${err}`)), 30_000); + child.stdout.on("data", () => { + const m = /(http:\/\/127\.0\.0\.1:\d+)/.exec(out); + if (m) { + clearTimeout(t); + res(m[1]!); + } + }); + child.on("exit", (code) => rej(new Error(`viz exited (${code}) before serving\n${out}\n${err}`))); + }); + // The failed spawn reports within milliseconds; give it time to land. + const early = await Promise.race([exited, new Promise((r) => setTimeout(() => r("running"), 1000))]); + assert.equal(early, "running", `viz exited early\n${err}`); + const res = await fetch(`${url}/api/context-graph`); + assert.equal(res.status, 200); + assert.doesNotMatch(err, /ENOENT/); + } finally { + child.kill(); + await exited; + rmSync(dir, { recursive: true, force: true }); + } +});