chore(cli): extract shared detached-spawn helper (#2729)

* chore(cli): extract shared detached-spawn helper

Four CLI sites copy-pasted the detached LaunchServices spawn recipe
(detached, stdio ignore, unref, ELECTRON_RUN_AS_NODE scrubbed so the
launched GUI does not boot as a headless Node host): launchDesktop,
ok open, the single-file open path, and ok bug-report's Finder reveal.
Consolidate into spawnDetachedScrubbed in cli/src/utils/detached-spawn.ts
with the scrub encoded once. bug-report previously inherited the parent
env unscrubbed; it now goes through the same scrub (a no-op for the
Finder reveal target).

* chore(cli): make scrubElectronRunAsNode module-private

Post-consolidation the only remaining caller of scrubElectronRunAsNode is
spawnDetachedScrubbed in the same file, so the export no longer earns its
place. Drop the export and the two direct unit tests; the scrub and
non-mutation contract stays pinned behaviorally through the
spawnDetachedScrubbed env tests (explicit-env scrub + process.env default
non-mutation). No coverage lost, narrower module surface.

Addresses review feedback on #2729.

GitOrigin-RevId: 558ec9b093d1b23f1b0ab90de8a65f4e959ea10a

Author: amikofalvy

packages/cli/src/commands/bug-report.ts

@@ -1,9 +1,8 @@
-import { spawn as childSpawn } from 'node:child_process';
 import { platform } from 'node:os';
-import { withHiddenWindowsConsole } from '@inkeep/open-knowledge-server';
 import { Command } from 'commander';
 import { getCliLogger } from '../cli.ts';
 import { collectReportBundle } from '../report-bundle.ts';
+import { spawnDetachedScrubbed } from '../utils/detached-spawn.ts';
 import { defaultBugReportZipPath } from './bug-report-bundle.ts';
 
 export function bugReportCommand(): Command {
@@ -31,11 +30,7 @@ export function bugReportCommand(): Command {
 
         if (opts.reveal && platform() === 'darwin') {
           try {
-            childSpawn(
-              '/usr/bin/open',
-              ['-R', zipPath],
-              withHiddenWindowsConsole({ detached: true, stdio: 'ignore' as const }),
-            ).unref();
+            spawnDetachedScrubbed('/usr/bin/open', ['-R', zipPath]);
           } catch {}
         }
       } catch (err) {

packages/cli/src/commands/desktop-dispatch.ts

@@ -10,10 +10,11 @@
  * and the caller falls through to the existing `ok start` flow.
  */
 
-import type { spawn as NativeSpawn, SpawnOptions } from 'node:child_process';
+import type { spawn as NativeSpawn } from 'node:child_process';
 import { statSync } from 'node:fs';
 import { homedir } from 'node:os';
 import { join } from 'node:path';
+import { spawnDetachedScrubbed } from '../utils/detached-spawn.ts';
 
 /**
  * macOS bundle identifier for the desktop app. Reused for protocol
@@ -222,21 +223,7 @@ export function launchDesktop(deps: LaunchDeps): void {
   log(
     'Launching OpenKnowledge desktop (use `ok start` for the browser server, or `OK_FORCE_BROWSER=1` to always skip)',
   );
-  // Scrub `ELECTRON_RUN_AS_NODE` from the spawned `open`'s env. The CLI
-  // wrapper (`Contents/Resources/cli/bin/ok.sh`) sets it to 1 so the bundled
-  // Electron binary acts as a Node host; LaunchServices propagates the
-  // caller's env into the desktop process it spawns, and the desktop
-  // Electron main process sees `ELECTRON_RUN_AS_NODE=1`, runs as a headless
-  // Node host with no script, and exits immediately. Symptom: the
-  // "Launching OpenKnowledge desktop" line prints but no GUI appears.
-  const env = { ...process.env };
-  delete env.ELECTRON_RUN_AS_NODE;
-  const child = deps.spawn('open', ['-b', DESKTOP_BUNDLE_ID], {
-    detached: true,
-    stdio: 'ignore',
-    env,
-  } satisfies SpawnOptions);
-  child.unref();
+  spawnDetachedScrubbed('open', ['-b', DESKTOP_BUNDLE_ID], { spawn: deps.spawn });
 }
 
 /**

packages/cli/src/commands/open.test.ts

@@ -1,5 +1,5 @@
 import { describe, expect, test } from 'bun:test';
-import { createRealOpenDeps, type OpenDeps, runOpen, scrubElectronRunAsNode } from './open.ts';
+import { createRealOpenDeps, type OpenDeps, runOpen } from './open.ts';
 
 function makeDeps(overrides: Partial<OpenDeps> = {}): {
   deps: OpenDeps;
@@ -224,17 +224,3 @@ describe('createRealOpenDeps wiring', () => {
     expect(deps.detectBundlePath()).toBe('/Applications/OpenKnowledge.app');
   });
 });
-
-describe('scrubElectronRunAsNode', () => {
-  test('removes ELECTRON_RUN_AS_NODE so the spawned target does not inherit it', () => {
-    const scrubbed = scrubElectronRunAsNode({ ELECTRON_RUN_AS_NODE: '1', PATH: '/usr/bin' });
-    expect(scrubbed.ELECTRON_RUN_AS_NODE).toBeUndefined();
-    expect(scrubbed.PATH).toBe('/usr/bin');
-  });
-
-  test('does not mutate the input env', () => {
-    const input = { ELECTRON_RUN_AS_NODE: '1' };
-    scrubElectronRunAsNode(input);
-    expect(input.ELECTRON_RUN_AS_NODE).toBe('1');
-  });
-});

packages/cli/src/commands/open.ts

@@ -27,7 +27,6 @@
  * non-TTY/headless invocations (an agent shelling out). The verb spawns its own
  * `open "<url>"` (LaunchServices) rather than `launchDesktop`.
  */
-import { spawn as nodeSpawn } from 'node:child_process';
 import { statSync } from 'node:fs';
 import { join, resolve } from 'node:path';
 import { MANAGED_ARTIFACT_SCOPES, type SkillScope } from '@inkeep/open-knowledge-core';
@@ -37,9 +36,9 @@ import {
   encodeSkillRoute,
   resolveLockDir,
   resolveUiInfo,
-  withHiddenWindowsConsole,
 } from '@inkeep/open-knowledge-server';
 import { Command } from 'commander';
+import { spawnDetachedScrubbed } from '../utils/detached-spawn.ts';
 import { createRealDetectDeps, type DetectResult, detectDesktop } from './desktop-dispatch.ts';
 
 export interface OpenOptions {
@@ -72,19 +71,6 @@ export interface OpenDeps {
   error: (message: string) => void;
 }
 
-/**
- * Copy `process.env` minus `ELECTRON_RUN_AS_NODE`. The CLI wrapper sets that
- * var so the bundled Electron binary runs as a Node host; if the
- * LaunchServices-spawned target (the desktop app for `openknowledge://`, or the
- * browser for http) inherited it, it would start as a headless Node host with
- * no script and exit immediately. Mirrors `launchDesktop`.
- */
-export function scrubElectronRunAsNode(env: NodeJS.ProcessEnv): NodeJS.ProcessEnv {
-  const next = { ...env };
-  delete next.ELECTRON_RUN_AS_NODE;
-  return next;
-}
-
 /**
  * Build the real side-effect surface. `detect` is injectable so the
  * `bundlePath ?? null` collapse can be unit-tested without a real macOS /
@@ -116,16 +102,7 @@ export function createRealOpenDeps(
       }
     },
     openTarget: (target) => {
-      const child = nodeSpawn(
-        'open',
-        [target],
-        withHiddenWindowsConsole({
-          detached: true,
-          stdio: 'ignore' as const,
-          env: scrubElectronRunAsNode(process.env),
-        }),
-      );
-      child.unref();
+      spawnDetachedScrubbed('open', [target]);
     },
     log: (message) => process.stdout.write(`${message}\n`),
     error: (message) => process.stderr.write(`${message}\n`),

packages/cli/src/commands/single-file-open.ts

@@ -13,7 +13,6 @@
  *     a tab, and removes the temp projectDir on teardown.
  */
 
-import { spawn as nodeSpawn } from 'node:child_process';
 import { existsSync } from 'node:fs';
 import { rm } from 'node:fs/promises';
 import { resolve } from 'node:path';
@@ -24,8 +23,8 @@ import {
   SingleFileNotFoundError,
   SingleFileNotMarkdownError,
   type SingleFileOpenPlan,
-  withHiddenWindowsConsole,
 } from '@inkeep/open-knowledge-server';
+import { spawnDetachedScrubbed } from '../utils/detached-spawn.ts';
 import { createRealDetectDeps, type DetectResult, detectDesktop } from './desktop-dispatch.ts';
 import { createRealOpenDeps, runOpen } from './open.ts';
 
@@ -53,35 +52,14 @@ export interface SingleFileOpenDeps {
   error: (message: string) => void;
 }
 
-/**
- * Copy `process.env` minus `ELECTRON_RUN_AS_NODE` — mirrors `ok open`. The CLI
- * wrapper sets that var so the bundled Electron binary runs as a Node host;
- * leaking it to the LaunchServices-spawned target would start it headless and
- * exit immediately.
- */
-function scrubElectronRunAsNode(env: NodeJS.ProcessEnv): NodeJS.ProcessEnv {
-  const next = { ...env };
-  delete next.ELECTRON_RUN_AS_NODE;
-  return next;
-}
-
 export function createRealSingleFileOpenDeps(
   detect: () => DetectResult = () => detectDesktop(createRealDetectDeps()),
 ): SingleFileOpenDeps {
   return {
     prepare: prepareSingleFileOpen,
     detectBundlePath: () => detect().bundlePath ?? null,
     openTarget: (target) => {
-      const child = nodeSpawn(
-        'open',
-        [target],
-        withHiddenWindowsConsole({
-          detached: true,
-          stdio: 'ignore' as const,
-          env: scrubElectronRunAsNode(process.env),
-        }),
-      );
-      child.unref();
+      spawnDetachedScrubbed('open', [target]);
     },
     runProjectOpen: (docName, projectRoot) =>
       runOpen(docName, { project: projectRoot }, createRealOpenDeps()),

packages/cli/src/utils/detached-spawn.test.ts

@@ -0,0 +1,67 @@
+import { describe, expect, test } from 'bun:test';
+import type { SpawnOptions } from 'node:child_process';
+import { spawnDetachedScrubbed } from './detached-spawn.ts';
+
+describe('spawnDetachedScrubbed', () => {
+  interface Captured {
+    command?: string;
+    args?: readonly string[];
+    opts?: SpawnOptions;
+  }
+
+  function makeFakeSpawn(captured: Captured, onUnref: () => void) {
+    return ((command: string, args: readonly string[], opts: SpawnOptions) => {
+      captured.command = command;
+      captured.args = args;
+      captured.opts = opts;
+      return { unref: onUnref };
+    }) as unknown as NonNullable<Parameters<typeof spawnDetachedScrubbed>[2]>['spawn'];
+  }
+
+  test('spawns detached, stdio:ignore, windowsHide, and unref()s the child', () => {
+    const captured: Captured = {};
+    let unrefCalled = false;
+    spawnDetachedScrubbed('open', ['-b', 'com.example.app'], {
+      spawn: makeFakeSpawn(captured, () => {
+        unrefCalled = true;
+      }),
+    });
+
+    expect(captured.command).toBe('open');
+    expect(captured.args).toEqual(['-b', 'com.example.app']);
+    expect(captured.opts?.detached).toBe(true);
+    expect(captured.opts?.stdio).toBe('ignore');
+    expect(captured.opts?.windowsHide).toBe(true);
+    expect(unrefCalled).toBe(true);
+  });
+
+  test('scrubs ELECTRON_RUN_AS_NODE from the child env', () => {
+    const captured: Captured = {};
+    spawnDetachedScrubbed('open', ['target'], {
+      spawn: makeFakeSpawn(captured, () => {}),
+      env: { ELECTRON_RUN_AS_NODE: '1', PATH: '/usr/bin' },
+    });
+
+    expect(captured.opts?.env).toBeDefined();
+    expect(captured.opts?.env).not.toHaveProperty('ELECTRON_RUN_AS_NODE');
+    expect(captured.opts?.env?.PATH).toBe('/usr/bin');
+  });
+
+  test('defaults env to a scrubbed copy of process.env', () => {
+    const prevValue = process.env.ELECTRON_RUN_AS_NODE;
+    process.env.ELECTRON_RUN_AS_NODE = '1';
+    try {
+      const captured: Captured = {};
+      spawnDetachedScrubbed('open', ['target'], {
+        spawn: makeFakeSpawn(captured, () => {}),
+      });
+
+      expect(captured.opts?.env).not.toHaveProperty('ELECTRON_RUN_AS_NODE');
+      // The scrub copies — this process keeps its own env untouched.
+      expect(process.env.ELECTRON_RUN_AS_NODE).toBe('1');
+    } finally {
+      if (prevValue === undefined) delete process.env.ELECTRON_RUN_AS_NODE;
+      else process.env.ELECTRON_RUN_AS_NODE = prevValue;
+    }
+  });
+});

packages/cli/src/utils/detached-spawn.ts

@@ -0,0 +1,63 @@
+/**
+ * Shared recipe for handing a target to the OS (LaunchServices `open`, Finder
+ * reveal) as an independent child that survives this CLI process exiting.
+ *
+ * Every site that launches the desktop app (or any GUI target) from the CLI
+ * must use this instead of a bare `spawn`: the packaged CLI wrapper
+ * (`Contents/Resources/cli/bin/ok.sh`) sets `ELECTRON_RUN_AS_NODE=1` so the
+ * bundled Electron binary acts as a Node host, and LaunchServices propagates
+ * the caller's env into the process it spawns — an Electron GUI target that
+ * inherits the var boots as a headless Node host with no script and exits
+ * immediately. Symptom: the launch line prints but no window appears. The
+ * scrub here is what keeps each call site from having to get that right
+ * independently.
+ */
+import {
+  type ChildProcess,
+  type spawn as NativeSpawn,
+  spawn as nativeSpawn,
+} from 'node:child_process';
+import { withHiddenWindowsConsole } from '@inkeep/open-knowledge-server';
+
+/**
+ * Copy `env` minus `ELECTRON_RUN_AS_NODE`. Non-mutating — the input env
+ * (typically `process.env`) is left intact for this process. Module-private:
+ * the scrub + non-mutation contract is pinned through `spawnDetachedScrubbed`.
+ */
+function scrubElectronRunAsNode(env: NodeJS.ProcessEnv): NodeJS.ProcessEnv {
+  const next = { ...env };
+  delete next.ELECTRON_RUN_AS_NODE;
+  return next;
+}
+
+export interface SpawnDetachedScrubbedOptions {
+  /** Override for tests — defaults to `node:child_process#spawn`. */
+  spawn?: typeof NativeSpawn;
+  /** Env to copy + scrub — defaults to `process.env`. */
+  env?: NodeJS.ProcessEnv;
+}
+
+/**
+ * Spawn `command` detached (own process group, no stdio ties, `unref()`ed so
+ * the CLI's event loop can drain) with `ELECTRON_RUN_AS_NODE` scrubbed from
+ * the child env. Returns the child for callers that want a handle; most
+ * fire-and-forget.
+ */
+export function spawnDetachedScrubbed(
+  command: string,
+  args: readonly string[],
+  opts: SpawnDetachedScrubbedOptions = {},
+): ChildProcess {
+  const spawnFn = opts.spawn ?? nativeSpawn;
+  const child = spawnFn(
+    command,
+    [...args],
+    withHiddenWindowsConsole({
+      detached: true,
+      stdio: 'ignore' as const,
+      env: scrubElectronRunAsNode(opts.env ?? process.env),
+    }),
+  );
+  child.unref();
+  return child;
+}