Extract shared dor/lib code into dor-lib-common

docs/specs/dor-cli.md

@@ -72,10 +72,14 @@ verbatim path — it fails with "The system cannot find the path specified."
 ## Spawning External Binaries
 
 Any time Dormouse spawns an external/user-installed binary — `dor ab` driving
-`agent-browser`, the agent-browser host running tab/eval/screenshot commands, dev
-harnesses launching `pnpm`/`agent-browser` — it goes through **`cross-spawn`**,
-never raw `node:child_process` `spawn`. This is mandatory for correctness on
-Windows, where two distinct failures bite a naive spawn:
+`agent-browser`, the agent-browser host running tab/eval/screenshot commands — it
+goes through **`spawnAndCapture` from the `dor-lib-common` package**, never raw
+`node:child_process` `spawn`. That helper is the single home for the hard-won
+Windows recipe, shared by `dor` and the `lib` host (which otherwise have no common
+code); both packages depend on `dor-lib-common`. It owns three concerns:
+
+**1. cross-spawn, not raw spawn.** Two distinct failures bite a naive spawn on
+Windows:
 
 - **ENOENT on a bare name.** Node's `spawn` does not consult `PATHEXT`, so a bare
   `agent-browser` never resolves the `agent-browser.cmd` PATH shim that npm/vfox
@@ -87,29 +91,30 @@ Windows, where two distinct failures bite a naive spawn:
 
 `cross-spawn` resolves the command via `PATH`/`PATHEXT` and routes `.cmd`/`.bat`
 through `cmd.exe` with correct argument escaping, and is a transparent passthrough
-on POSIX. Use it with the same `(command, args, options)` signature as
-`child_process.spawn`; it is bundled into `dist/dor.js` and the sidecar `.cjs`
-by esbuild.
-
-Caveat: a literal `%VAR%` inside an argument can still be expanded by `cmd.exe`
-when it passes through a `.cmd` shim — an unavoidable Windows batch limitation, not
-something `cross-spawn` (or any wrapper) can fully prevent. Our forwarded
-arguments (URLs, selectors, and the host's hardcoded `eval` scripts) contain no
-`%VAR%` patterns, so this does not arise in practice.
-
-### Resolve on `exit`, not `close`
-
-When buffering a spawned command's output, resolve on the child's **`exit`**
-event, not `close`. `agent-browser open` launches a long-lived per-session daemon
-that on Windows inherits the parent's stdout/stderr pipes; those pipes never reach
-EOF while the daemon lives, so `close` (which waits for stdio to drain) never
-fires and the spawn hangs forever. `exit` fires when the foreground process ends
-regardless of the lingering pipe. The two spawn helpers
-(`dor/src/commands/agent-browser.ts`, `lib/src/host/agent-browser-host.ts`) wait
-for `close` but fall back to `exit` after a short grace (`CLOSE_GRACE_MS`), so a
-normal command's full output still flushes while the daemon case can't hang.
-(POSIX dodges this because the daemon double-forks and detaches from the inherited
-fds, closing the pipe — which is why this never surfaced on macOS.)
+on POSIX. Caveat: a literal `%VAR%` inside an argument can still be expanded by
+`cmd.exe` when it passes through a `.cmd` shim — an unavoidable Windows batch
+limitation. Our forwarded arguments (URLs, selectors, the host's hardcoded `eval`
+scripts) contain no `%VAR%` patterns, so this does not arise in practice.
+
+**2. `windowsHide`.** cross-spawn runs `.cmd` shims through `cmd.exe`; without
+`windowsHide` each spawn flashes a console window that steals focus — and the
+panel's screenshot loop spawns one per stream-frame pulse, so a live page would
+flicker windows several times a second.
+
+**3. Resolve on `exit`, not `close`, with an exit-time snapshot.** `agent-browser
+open` launches a long-lived per-session daemon that on Windows inherits the
+parent's stdout/stderr pipes; those pipes never reach EOF while the daemon lives,
+so `close` (which waits for stdio to drain) never fires and a `close`-only wait
+hangs forever. `spawnAndCapture` waits for `close` but falls back to `exit` after
+a short grace, and resolves the grace path with the output snapshotted at `exit`
+so the daemon's post-command scribbles don't leak into the result. (POSIX dodges
+the whole thing because the daemon double-forks and detaches from the inherited
+fds, closing the pipe — which is why none of this surfaced on macOS.)
+
+Resolution: `dor-lib-common`'s package `exports` point at its built `dist` (clean,
+Node-type-free `.d.ts` for `dor`'s `tsc`, which deliberately avoids `@types/node`);
+every esbuild/Vite consumer (`dist/dor.js`, the sidecar `.cjs`, vscode-ext) inlines
+it. `dor`'s `prebuild` builds `dor-lib-common` first so its `.d.ts` exists.
 
 ## Host Plumbing
 

dor-lib-common/package.json

@@ -0,0 +1,24 @@
+{
+  "name": "dor-lib-common",
+  "version": "0.0.0",
+  "license": "FSL-1.1-MIT",
+  "private": true,
+  "type": "module",
+  "exports": {
+    ".": {
+      "types": "./dist/index.d.ts",
+      "default": "./dist/index.js"
+    }
+  },
+  "scripts": {
+    "build": "tsc -p tsconfig.json",
+    "test": "pnpm run build && node --test test/*.test.mjs"
+  },
+  "dependencies": {
+    "cross-spawn": "^7.0.6"
+  },
+  "devDependencies": {
+    "@types/node": "^22.12.0",
+    "typescript": "^6.0.3"
+  }
+}

dor-lib-common/src/agent-browser.ts

@@ -0,0 +1,43 @@
+// Workspace id baked into managed agent-browser session names. Hardcoded until
+// Dormouse exposes real workspaces; encoded now to avoid a later rename. Private:
+// callers build session names through sessionForKey, never by hand.
+const WORKSPACE_ID = '1';
+
+/** Env var that overrides which agent-browser binary to run; shared so `dor ab`
+ * and the host key off the same name. */
+export const AGENT_BROWSER_BIN_ENV = 'DORMOUSE_AGENT_BROWSER_BIN';
+
+/** Default binary name, resolved on PATH when no override/explicit path is given. */
+export const DEFAULT_AGENT_BROWSER_BIN = 'agent-browser';
+
+/** argv for `agent-browser stream status --json` against a session — the command
+ * whose output {@link parseStreamPort} reads. */
+export function streamStatusArgs(session: string): string[] {
+  return ['--session', session, 'stream', 'status', '--json'];
+}
+
+/**
+ * Managed, workspace-scoped agent-browser session name: `dormouse.<workspaceId>.<key>`.
+ * agent-browser session names become filesystem paths (the socket dir), so `/`
+ * can't separate the namespace — the daemon fails to start; dots keep it
+ * readable. Shared by `dor ab` (--key resolution) and the lib host (GUI sessions).
+ */
+export function sessionForKey(key: string): string {
+  return `dormouse.${WORKSPACE_ID}.${key}`;
+}
+
+/**
+ * Parse the stream WebSocket port from `agent-browser stream status --json`.
+ * The CLI wraps payloads as either `{ port }` or `{ data: { port } }`; tolerate
+ * both, and return undefined for anything malformed or non-finite. Shared by
+ * `dor ab` (surface binding) and the lib host (panel stream recovery).
+ */
+export function parseStreamPort(stdout: string): number | undefined {
+  try {
+    const parsed = JSON.parse(stdout) as { port?: unknown; data?: { port?: unknown } };
+    const port = parsed.data?.port ?? parsed.port;
+    return typeof port === 'number' && Number.isFinite(port) ? port : undefined;
+  } catch {
+    return undefined;
+  }
+}

dor-lib-common/src/cross-spawn.d.ts

@@ -0,0 +1,11 @@
+// cross-spawn ships no types. Declare the single call shape we use; with
+// @types/node present, ChildProcess/SpawnOptions are the real Node types, so
+// this stays internal to dor-lib-common and never leaks into the public surface.
+declare module 'cross-spawn' {
+  import type { ChildProcess, SpawnOptions } from 'node:child_process';
+  export default function spawn(
+    command: string,
+    args: readonly string[],
+    options?: SpawnOptions,
+  ): ChildProcess;
+}

dor-lib-common/src/index.ts

@@ -0,0 +1,9 @@
+export { spawnAndCapture } from './spawn.js';
+export type { SpawnCaptureResult } from './spawn.js';
+export {
+  parseStreamPort,
+  sessionForKey,
+  streamStatusArgs,
+  AGENT_BROWSER_BIN_ENV,
+  DEFAULT_AGENT_BROWSER_BIN,
+} from './agent-browser.js';

dor-lib-common/src/spawn.ts

@@ -0,0 +1,71 @@
+import spawn from 'cross-spawn';
+
+export interface SpawnCaptureSuccess {
+  readonly ok: true;
+  readonly exitCode: number;
+  readonly stdout: string;
+  readonly stderr: string;
+}
+
+export interface SpawnCaptureFailure {
+  readonly ok: false;
+  /** Spawn-level failure — the process never ran (e.g. ENOENT). */
+  readonly error: { readonly code?: string; readonly message: string };
+}
+
+export type SpawnCaptureResult = SpawnCaptureSuccess | SpawnCaptureFailure;
+
+// Grace window for 'close' to win after 'exit' before we resolve anyway. Long
+// enough that a normal command's stdio drains (its output is written before the
+// process exits), short enough that the daemon-holds-the-pipe case (see below)
+// doesn't feel like a hang.
+const CLOSE_GRACE_MS = 250;
+
+/**
+ * Spawn an external binary and capture its stdout/stderr — the single home for
+ * the hard-won Windows recipe `dor` and the agent-browser host both need. See
+ * docs/specs/dor-cli.md → "Spawning External Binaries".
+ *
+ *  - cross-spawn resolves PATHEXT and routes `.cmd`/`.bat` through cmd.exe; Node's
+ *    own spawn ENOENTs on a bare name and (>=22) EINVALs on a `.cmd` by full path.
+ *  - windowsHide stops a console window flashing and stealing focus per spawn.
+ *  - resolve on 'exit' (not 'close') with a grace + an exit-time output snapshot:
+ *    `agent-browser open` leaves a daemon that on Windows inherits our stdio
+ *    pipes, so 'close' never fires (waiting on it alone hangs forever) and the
+ *    daemon's post-exit scribbles would otherwise leak into the captured output.
+ *
+ * Never throws: a spawn-level failure resolves as `{ ok: false, error }`.
+ */
+export function spawnAndCapture(binary: string, args: readonly string[]): Promise<SpawnCaptureResult> {
+  return new Promise((resolve) => {
+    const child = spawn(binary, args, { stdio: ['ignore', 'pipe', 'pipe'], windowsHide: true });
+    let stdout = '';
+    let stderr = '';
+    // Latch on the first terminal event so the error-vs-exit/close race can't
+    // double-resolve; clearTimeout drops the grace timer once we've settled.
+    let settled = false;
+    let graceTimer: ReturnType<typeof setTimeout> | undefined;
+    const settle = (apply: () => void): void => {
+      if (settled) return;
+      settled = true;
+      if (graceTimer !== undefined) clearTimeout(graceTimer);
+      apply();
+    };
+    child.stdout?.on('data', (chunk: unknown) => { stdout += String(chunk); });
+    child.stderr?.on('data', (chunk: unknown) => { stderr += String(chunk); });
+    child.on('error', (error: NodeJS.ErrnoException) =>
+      settle(() => resolve({ ok: false, error: { code: error.code, message: error.message } })));
+    const finish = (code: number | null, out: string, err: string): void =>
+      settle(() => resolve({ ok: true, exitCode: code ?? 1, stdout: out, stderr: err }));
+    // 'close' is the clean path (process exited and stdio drained). Fall back to
+    // 'exit' for the daemon-holds-the-pipe case where 'close' never fires; the
+    // grace lets 'close' win first so a normal command's full output flushes, and
+    // the exit-time snapshot keeps post-exit daemon noise out of the result.
+    child.on('close', (code: number | null) => finish(code, stdout, stderr));
+    child.on('exit', (code: number | null) => {
+      const out = stdout;
+      const err = stderr;
+      graceTimer = setTimeout(() => finish(code, out, err), CLOSE_GRACE_MS);
+    });
+  });
+}

dor-lib-common/test/agent-browser.test.mjs

@@ -0,0 +1,22 @@
+import test from 'node:test';
+import assert from 'node:assert/strict';
+import { parseStreamPort, sessionForKey } from '../dist/index.js';
+
+test('sessionForKey namespaces a key under the workspace', () => {
+  assert.equal(sessionForKey('default'), 'dormouse.1.default');
+  assert.equal(sessionForKey('gui-abc'), 'dormouse.1.gui-abc');
+});
+
+test('parseStreamPort reads a top-level port', () => {
+  assert.equal(parseStreamPort(JSON.stringify({ port: 61218 })), 61218);
+});
+
+test('parseStreamPort reads a nested data.port', () => {
+  assert.equal(parseStreamPort(JSON.stringify({ data: { port: 5173 } })), 5173);
+});
+
+test('parseStreamPort returns undefined for malformed or portless output', () => {
+  assert.equal(parseStreamPort('not json'), undefined);
+  assert.equal(parseStreamPort(JSON.stringify({ data: {} })), undefined);
+  assert.equal(parseStreamPort(JSON.stringify({ port: 'nope' })), undefined);
+});

dor-lib-common/test/spawn.test.mjs

@@ -0,0 +1,25 @@
+import test from 'node:test';
+import assert from 'node:assert/strict';
+import { spawnAndCapture } from '../dist/index.js';
+
+const node = process.execPath;
+
+test('captures stdout and a zero exit code', async () => {
+  const result = await spawnAndCapture(node, ['-e', 'process.stdout.write("hello")']);
+  assert.equal(result.ok, true);
+  assert.equal(result.exitCode, 0);
+  assert.equal(result.stdout, 'hello');
+});
+
+test('captures stderr and a non-zero exit code', async () => {
+  const result = await spawnAndCapture(node, ['-e', 'process.stderr.write("boom"); process.exit(3)']);
+  assert.equal(result.ok, true);
+  assert.equal(result.exitCode, 3);
+  assert.equal(result.stderr, 'boom');
+});
+
+test('reports a missing binary as a spawn failure, never throwing', async () => {
+  const result = await spawnAndCapture('dormouse-no-such-binary-xyz', []);
+  assert.equal(result.ok, false);
+  assert.equal(result.error.code, 'ENOENT');
+});

dor-lib-common/tsconfig.json

@@ -0,0 +1,20 @@
+{
+  "compilerOptions": {
+    "target": "ES2022",
+    "lib": ["ES2022"],
+    "module": "ES2022",
+    "moduleResolution": "bundler",
+    "outDir": "dist",
+    "rootDir": "src",
+    "declaration": true,
+    "incremental": true,
+    "tsBuildInfoFile": "dist/.tsbuildinfo",
+    "strict": true,
+    "noUnusedLocals": true,
+    "noUnusedParameters": true,
+    "skipLibCheck": true,
+    "types": ["node"]
+  },
+  "include": ["src/**/*"],
+  "exclude": ["node_modules", "dist"]
+}

dor/package.json

@@ -12,7 +12,7 @@
     "dist"
   ],
   "scripts": {
-    "prebuild": "node ../scripts/generate-dor-version.mjs",
+    "prebuild": "pnpm --filter dor-lib-common build && node ../scripts/generate-dor-version.mjs",
     "build": "tsc -p tsconfig.json && esbuild src/dor.ts --bundle --format=esm --platform=node --outfile=dist/dor.js --banner:js=\"import { createRequire } from 'module'; const require = createRequire(import.meta.url);\"",
     "test": "pnpm run build && node --test test/*.test.mjs"
   },
@@ -22,6 +22,6 @@
   },
   "dependencies": {
     "@stricli/core": "^1.2.7",
-    "cross-spawn": "^7.0.6"
+    "dor-lib-common": "workspace:*"
   }
 }