Merge pull request #48 from itsjling/codex/issue-026-headless-presenter

Make the presenter headless and automation-friendly

Author: itsjling

scripts/cli-args.mjs

@@ -18,6 +18,7 @@ const valueOptions = new Set([
   '--batch-size',
   '--jobs',
   '--port',
+  '--host',
 ]);
 const flagOptions = new Set([
   '--help',
@@ -26,6 +27,7 @@ const flagOptions = new Set([
   '--no-agent',
   '--force',
   '--worktree',
+  '--no-browser',
 ]);
 const pathOptions = new Set([
   '--summaries',
@@ -61,6 +63,8 @@ Options:
   --force             Regenerate all agent notes
   --remote NAME|URL   Git remote (default: origin)
   --port NUMBER       Local page port (default: 2299)
+  --host ADDRESS      Page bind address (default: localhost)
+  --no-browser        Do not open the page in a browser
   -h, --help          Show this help
   -v, --version       Show the installed version
 
@@ -344,6 +348,8 @@ export function parseCliArgs(
     agentArgs,
     port: Number(portValue),
     portWasPassed: options.has('--port'),
+    host: options.get('--host') || 'localhost',
+    browserEnabled: !options.has('--no-browser'),
     forceSummaryRegeneration: options.has('--force'),
   };
 }

scripts/present.mjs

@@ -48,7 +48,7 @@ if (cli.doctor) {
   process.exit(report.ready ? 0 : 1);
 }
 
-const { agentEnabled, port } = cli;
+const { agentEnabled, browserEnabled, host, port } = cli;
 const feedArgs = [...cli.feedArgs];
 const agentArgs = [...cli.agentArgs];
 if (agentEnabled) {
@@ -168,6 +168,8 @@ function startSite() {
       outputPath,
       '--port',
       String(port),
+      '--host',
+      host,
       '--project',
       projectKey,
       ...(!cli.portWasPassed ? ['--increment-port'] : []),
@@ -190,7 +192,7 @@ function startSite() {
       }
       console.log(line);
       const match = line.match(/^Diffsplain: (http:\/\/\S+)$/);
-      if (!browserOpened && !browserOpenTimer && match) {
+      if (browserEnabled && !browserOpened && !browserOpenTimer && match) {
         browserOpenTimer = setTimeout(() => {
           browserOpenTimer = undefined;
           browserOpened = true;

scripts/serve-built.mjs

@@ -24,6 +24,7 @@ function option(name, fallback) {
 const output = resolve(option('--output', resolve(root, '.cache/diff-data.json')));
 const project = option('--project', '');
 const portValue = option('--port', '2299');
+const host = option('--host', 'localhost');
 if (!/^\d+$/.test(portValue) || Number(portValue) > 65_535) {
   throw new Error('--port must be a number from 0 to 65535');
 }
@@ -86,6 +87,15 @@ async function fetchAsset(request) {
   return fileResponse(file);
 }
 
+function jsonResponse(value) {
+  return new Response(JSON.stringify(value), {
+    headers: {
+      'cache-control': 'no-store',
+      'content-type': 'application/json; charset=utf-8',
+    },
+  });
+}
+
 function nodeRequest(request) {
   const host = request.headers.host || 'localhost';
   const init = {
@@ -111,15 +121,20 @@ async function send(nodeResponse, response) {
 const server = createServer(async (request, response) => {
   try {
     const webRequest = nodeRequest(request);
-    if (new URL(webRequest.url).pathname === '/events') {
+    const url = new URL(webRequest.url);
+    if (url.pathname === '/health') {
+      await send(response, jsonResponse(readyState));
+      return;
+    }
+    if (url.pathname === '/events') {
       response.writeHead(200, {
         'content-type': 'text/event-stream',
         'cache-control': 'no-store',
         connection: 'keep-alive',
       });
       response.write('retry: 250\nevent: ready\ndata: {}\n\n');
       eventClients.add(response);
-      const requestProject = new URL(webRequest.url).searchParams.get('project');
+      const requestProject = url.searchParams.get('project');
       if (project && requestProject === project) {
         console.log('Diffsplain tab: connected');
       }
@@ -148,19 +163,53 @@ watchFile(output, { interval: 100 }, (current, previous) => {
 });
 
 let selectedPort = Number(portValue);
+let readyState;
+let closing = false;
+
+function urlFor(address, port) {
+  const formattedAddress = address.includes(':') ? `[${address}]` : address;
+  return `http://${formattedAddress}:${port}`;
+}
+
+function isLoopback(address) {
+  return (
+    address === 'localhost' ||
+    address === '::1' ||
+    address === '::ffff:127.0.0.1' ||
+    /^127(?:\.\d{1,3}){3}$/.test(address)
+  );
+}
 
 function listen() {
-  server.listen(selectedPort, 'localhost');
+  server.listen(selectedPort, host);
 }
 
 server.on('listening', () => {
+  if (closing) {
+    server.close();
+    return;
+  }
   const address = server.address();
+  const readyAddress =
+    address && typeof address === 'object' ? address.address : host;
   const readyPort =
     address && typeof address === 'object' ? address.port : selectedPort;
+  const url = urlFor(host, readyPort);
   const projectHash = project
     ? `#project=${encodeURIComponent(project)}`
     : '';
-  console.log(`Diffsplain: http://localhost:${readyPort}${projectHash}`);
+  readyState = {
+    status: 'ok',
+    address: readyAddress,
+    port: readyPort,
+  };
+  if (!isLoopback(readyAddress)) {
+    console.warn(
+      `Warning: Diffsplain is listening on ${readyAddress}. Anyone who can reach this address can view this review.`,
+    );
+  }
+  console.log(`Diffsplain: ${url}${projectHash}`);
+  console.log(JSON.stringify({ event: 'ready', ...readyState, url }));
 });
 
 server.on('error', (error) => {
@@ -175,22 +224,20 @@ server.on('error', (error) => {
     return;
   }
   console.error(`Could not start Diffsplain: ${error.message}`);
-  process.exitCode = 1;
+  close(1);
 });
 
 listen();
 
-let closing = false;
-function close() {
+function close(exitCode = 0) {
   if (closing) return;
   closing = true;
+  process.exitCode = exitCode;
   unwatchFile(output);
   for (const client of eventClients) client.end();
   eventClients.clear();
-  server.close(() => {
-    process.exitCode = 0;
-  });
+  if (server.listening) server.close();
 }
 
-process.on('SIGINT', close);
-process.on('SIGTERM', close);
+process.on('SIGINT', () => close());
+process.on('SIGTERM', () => close());

tests/cli-args.test.mjs

@@ -17,10 +17,22 @@ test('leaves agent selection open when no agent is passed', () => {
   assert.equal(parsed.agent, undefined);
   assert.equal(parsed.port, 2299);
   assert.equal(parsed.portWasPassed, false);
+  assert.equal(parsed.host, 'localhost');
+  assert.equal(parsed.browserEnabled, true);
   assert.deepEqual(parsed.feedArgs, ['--repo', cwd, '--checkout']);
   assert.deepEqual(parsed.agentArgs, ['--repo', cwd, '--checkout']);
 });
 
+test('accepts headless browser and explicit bind options', () => {
+  const parsed = parseCliArgs(['--no-browser', '--host', '0.0.0.0'], {
+    callerDirectory: cwd,
+    pathExists: missing,
+  });
+
+  assert.equal(parsed.browserEnabled, false);
+  assert.equal(parsed.host, '0.0.0.0');
+});
+
 test('accepts a GitHub owner/name repo and a branch', () => {
   const parsed = parseCliArgs(['acme/widgets', '--branch', 'feature/search'], {
     callerDirectory: cwd,

tests/present-instances.test.mjs

@@ -60,6 +60,22 @@ function waitForUrl(child) {
   });
 }
 
+function waitForText(stream, pattern) {
+  return new Promise((resolve, reject) => {
+    let output = '';
+    const timer = setTimeout(() => {
+      reject(new Error(`Did not find ${pattern}: ${output}`));
+    }, 12_000);
+    stream.on('data', (chunk) => {
+      output += chunk;
+      if (pattern.test(output)) {
+        clearTimeout(timer);
+        resolve(output);
+      }
+    });
+  });
+}
+
 async function waitFor(read, timeout = 8_000) {
   const deadline = Date.now() + timeout;
   let lastError;
@@ -308,3 +324,73 @@ test('reuses a matching project tab when it reconnects', async () => {
     await rm(root, { recursive: true, force: true });
   }
 });
+
+test('stays available when browser launch fails and skips it when asked', async () => {
+  const root = await mkdtemp(join(tmpdir(), 'diffsplain-headless-'));
+  const browserLog = join(root, 'browser.log');
+  const browser = join(root, 'browser');
+  let failingPresenter;
+  let headlessPresenter;
+
+  try {
+    const repo = await makeRepo(root, 'repo', 'file.txt');
+    failingPresenter = spawn(
+      process.execPath,
+      [
+        script,
+        '--repo',
+        repo,
+        '--worktree',
+        '--no-agent',
+        '--port',
+        '0',
+      ],
+      {
+        cwd: root,
+        env: { ...process.env, BROWSER: join(root, 'missing-browser') },
+        stdio: ['ignore', 'pipe', 'pipe'],
+      },
+    );
+    const browserFailure = waitForText(
+      failingPresenter.stderr,
+      /Could not open the browser:/,
+    );
+    const failingUrl = await waitForUrl(failingPresenter);
+    await browserFailure;
+    assert.equal((await fetch(new URL('health', failingUrl))).status, 200);
+    assert.equal(failingPresenter.exitCode, null);
+
+    await writeFile(browser, '#!/bin/sh\nprintf opened > "$BROWSER_LOG"\n');
+    await chmod(browser, 0o755);
+    headlessPresenter = spawn(
+      process.execPath,
+      [
+        script,
+        '--repo',
+        repo,
+        '--worktree',
+        '--no-agent',
+        '--no-browser',
+        '--port',
+        '0',
+      ],
+      {
+        cwd: root,
+        env: { ...process.env, BROWSER: browser, BROWSER_LOG: browserLog },
+        stdio: ['ignore', 'pipe', 'pipe'],
+      },
+    );
+    const headlessUrl = await waitForUrl(headlessPresenter);
+    assert.equal((await fetch(new URL('health', headlessUrl))).status, 200);
+    await new Promise((resolve) => setTimeout(resolve, 900));
+    await assert.rejects(readFile(browserLog, 'utf8'));
+  } finally {
+    if (failingPresenter && failingPresenter.exitCode === null) {
+      await stop(failingPresenter);
+    }
+    if (headlessPresenter && headlessPresenter.exitCode === null) {
+      await stop(headlessPresenter);
+    }
+    await rm(root, { recursive: true, force: true });
+  }
+});