feat: the inbox page — one click from a prepared review to a live diffity session

packages/api/package.json

@@ -1,6 +1,6 @@
 {
   "name": "@diffity/api",
-  "version": "0.10.9",
+  "version": "0.10.10",
   "private": true,
   "type": "module",
   "main": "./dist/index.js",

packages/cli/package.json

@@ -1,6 +1,6 @@
 {
   "name": "@naturalcycles/diffity",
-  "version": "0.10.9",
+  "version": "0.10.10",
   "description": "Agent-agnostic, GitHub-style diff viewer and code review tool with a live agent loop",
   "type": "module",
   "bin": {

packages/cli/src/inbox/daemon.ts

@@ -1,4 +1,4 @@
-import { createServer, type Server } from 'node:http';
+import { createServer, type Server, type ServerResponse } from 'node:http';
 import { existsSync, readdirSync, readFileSync, rmSync } from 'node:fs';
 import { basename, join } from 'node:path';
 import { getViewerLogin, searchReviewRequested, viewPr } from '@diffity/github';
@@ -10,6 +10,9 @@ import { removeWorktree, cloneDir } from './worktree.js';
 import { InboxStore } from './store.js';
 import { runTick, type Forge } from './tick.js';
 import { buildView } from './view.js';
+import { resolveOpen } from './open.js';
+import { openPreparedSession, realOpenSessionDeps, type OpenSessionDeps } from './open-session.js';
+import { inboxPage } from './page.js';
 
 const realForge: Forge = {
   viewerLogin: getViewerLogin,
@@ -32,6 +35,8 @@ export interface DaemonOptions {
   once?: boolean;
   /** The forge to poll; defaults to the real GitHub one. Overridden only by tests. */
   forge?: Forge;
+  /** How a prepared review is brought up as a session; defaults to the real one. Tests override it. */
+  openDeps?: OpenSessionDeps;
 }
 
 /**
@@ -85,7 +90,8 @@ export async function runDaemon(
 
   // Bind the port first: it is the daemon's singleton lock, so a second daemon exits here (via the
   // server's error handler) before it can reclaim and kill the first one's in-flight servers.
-  const server = await bindInboxServer(store, config, log);
+  const openDeps = options.openDeps ?? realOpenSessionDeps(nodePath, entry);
+  const server = await bindInboxServer(store, config, log, openDeps);
   reclaimLeftoverServers(log);
   const timer = setInterval(() => void tick(), config.pollMinutes * 60_000);
   void tick();
@@ -135,17 +141,64 @@ function reclaimLeftoverServers(log: (message: string) => void): void {
   }
 }
 
-export function startInboxServer(store: InboxStore, config: InboxConfig, log: (message: string) => void): Server {
+export function startInboxServer(store: InboxStore, config: InboxConfig, log: (message: string) => void, openDeps: OpenSessionDeps): Server {
   const server = createServer((req, res) => {
-    const openBase = `http://localhost:${config.port}`;
-    if (req.method === 'GET' && (req.url === '/api/inbox' || req.url === '/api/inbox/')) {
-      const view = buildView(store, openBase, new Date().toISOString());
-      res.writeHead(200, { 'Content-Type': 'application/json' });
-      res.end(JSON.stringify(view));
-      return;
+    // The whole handler is guarded: an unhandled throw here (a malformed percent-escape, say) would
+    // otherwise have no catch and take the long-running daemon down with it.
+    try {
+      // Loopback binding is not enough on its own: a page on another site can rebind its own
+      // hostname to 127.0.0.1, so a stranger's Host header must not reach the reviewer's PR list.
+      // Judged against the connection's own port, which is the port actually bound.
+      if (!isLocalHost(req.headers.host, req.socket.localPort)) {
+        res.writeHead(403, { 'Content-Type': 'text/plain; charset=utf-8' });
+        res.end('forbidden');
+        return;
+      }
+
+      // The host the reader actually used — localhost or 127.0.0.1, already checked — so the links
+      // on the page point back at the same origin and a click on them is not cross-site.
+      const openBase = `http://${req.headers.host}`;
+      const url = req.url ?? '/';
+
+      if (req.method === 'GET' && (url === '/' || url === '/index.html')) {
+        res.writeHead(200, { 'Content-Type': 'text/html; charset=utf-8' });
+        res.end(inboxPage());
+        return;
+      }
+      if (req.method === 'GET' && (url === '/api/inbox' || url === '/api/inbox/')) {
+        const view = buildView(store, openBase, new Date().toISOString());
+        res.writeHead(200, { 'Content-Type': 'application/json' });
+        res.end(JSON.stringify(view));
+        return;
+      }
+      if (req.method === 'GET' && url.startsWith('/open/')) {
+        // A state-changing GET, so a cross-site fetch — a drive-by trying to spawn a session — is
+        // refused; a click from the inbox page itself is same-origin, and a direct navigation none.
+        if (req.headers['sec-fetch-site'] === 'cross-site') {
+          res.writeHead(403, { 'Content-Type': 'text/plain; charset=utf-8' });
+          res.end('forbidden');
+          return;
+        }
+        let id: string;
+        try {
+          id = decodeURIComponent(url.slice('/open/'.length));
+        } catch {
+          res.writeHead(400, { 'Content-Type': 'text/plain; charset=utf-8' });
+          res.end('bad request');
+          return;
+        }
+        void handleOpen(store, id, openDeps, log, res).catch(err => log(`open failed: ${err instanceof Error ? err.message : err}`));
+        return;
+      }
+      res.writeHead(404, { 'Content-Type': 'application/json' });
+      res.end(JSON.stringify({ error: 'not found' }));
+    } catch (err) {
+      log(`request handler error: ${err instanceof Error ? err.message : err}`);
+      if (!res.headersSent) {
+        res.writeHead(500, { 'Content-Type': 'text/plain; charset=utf-8' });
+      }
+      res.end('internal error');
     }
-    res.writeHead(404, { 'Content-Type': 'application/json' });
-    res.end(JSON.stringify({ error: 'not found' }));
   });
   server.on('error', err => {
     const code = (err as NodeJS.ErrnoException).code;
@@ -159,8 +212,38 @@ export function startInboxServer(store: InboxStore, config: InboxConfig, log: (m
   return server;
 }
 
+/** Brings a prepared review up as a live session and redirects the browser to it. */
+async function handleOpen(store: InboxStore, id: string, openDeps: OpenSessionDeps, log: (message: string) => void, res: ServerResponse): Promise<void> {
+  try {
+    const resolution = resolveOpen(store, id);
+    if (!resolution.ok) {
+      res.writeHead(resolution.status, { 'Content-Type': 'text/plain; charset=utf-8' });
+      res.end(resolution.message);
+      return;
+    }
+    log(`opening ${id}`);
+    const { url, imported, importError } = await openPreparedSession(resolution.pr.worktreePath!, resolution.pr.bundlePath!, openDeps);
+    if (!imported) {
+      log(`opened ${id} but its findings did not import: ${importError}`);
+    }
+    res.writeHead(302, { Location: url });
+    res.end();
+  } catch (err) {
+    log(`could not open ${id}: ${err instanceof Error ? err.message : err}`);
+    if (!res.headersSent) {
+      res.writeHead(500, { 'Content-Type': 'text/plain; charset=utf-8' });
+      res.end(`Could not open ${id}: ${err instanceof Error ? err.message : err}`);
+    }
+  }
+}
+
+/** A request whose Host is this loopback server's own address (localhost or 127.0.0.1, right port). */
+function isLocalHost(host: string | undefined, port: number | undefined): boolean {
+  return port != null && (host === `localhost:${port}` || host === `127.0.0.1:${port}`);
+}
+
 /** Resolves once the port is held; a clash exits through the server's own error handler first. */
-function bindInboxServer(store: InboxStore, config: InboxConfig, log: (message: string) => void): Promise<Server> {
-  const server = startInboxServer(store, config, log);
+function bindInboxServer(store: InboxStore, config: InboxConfig, log: (message: string) => void, openDeps: OpenSessionDeps): Promise<Server> {
+  const server = startInboxServer(store, config, log, openDeps);
   return new Promise(resolve => server.once('listening', () => resolve(server)));
 }

packages/cli/src/inbox/open-session.ts

@@ -0,0 +1,96 @@
+import { spawn, execFileSync } from 'node:child_process';
+import { readFileSync, realpathSync } from 'node:fs';
+import { createHash } from 'node:crypto';
+import { checkInstanceHealth, findInstanceForRepo } from '../registry.js';
+
+/**
+ * Brings a prepared review up as a live diffity session the reviewer can open: a server over the
+ * worktree, diffing against the pull request's base, with the prepared findings imported. Returns
+ * the URL to send the browser to. Import failure is not fatal — the diff is still worth opening —
+ * but it is surfaced to the caller.
+ */
+export async function openPreparedSession(worktree: string, bundlePath: string, deps: OpenSessionDeps): Promise<OpenedSession> {
+  const ref = deps.baseRefOf(bundlePath);
+  const port = await deps.ensureServer(worktree, ref);
+  try {
+    deps.importBundle(worktree, bundlePath);
+  } catch (err) {
+    return { url: `http://localhost:${port}/`, imported: false, importError: err instanceof Error ? err.message : String(err) };
+  }
+  return { url: `http://localhost:${port}/`, imported: true };
+}
+
+/** The base commit a bundle was built against; the session diffs the worktree against it. */
+export function baseRefOf(bundlePath: string): string {
+  const bundle = JSON.parse(readFileSync(bundlePath, 'utf-8')) as { baseSha?: string | null };
+  if (!bundle.baseSha) {
+    throw new Error(`The bundle at ${bundlePath} records no base commit.`);
+  }
+  return bundle.baseSha;
+}
+
+/**
+ * The real `ensureServer`: a healthy diffity already serving this worktree is reused, otherwise one
+ * is started in the reviewer's own diffity (no data-dir override), so it shows up in `diffity list`
+ * and behaves like any session they opened themselves.
+ */
+export function realOpenSessionDeps(nodePath: string, entry: string): OpenSessionDeps {
+  return {
+    baseRefOf,
+    ensureServer: (worktree, ref) => ensureServer(nodePath, entry, worktree, ref),
+    importBundle: (worktree, bundlePath) => {
+      execFileSync(nodePath, [entry, '--repo', worktree, 'agent', 'import-bundle', bundlePath], { stdio: 'pipe' });
+    },
+  };
+}
+
+export async function ensureServer(nodePath: string, entry: string, worktree: string, ref: string, waitMs = 30_000): Promise<number> {
+  const hash = repoHash(worktree);
+  // A healthy server already on this worktree is reused as-is. The worktree lives under the inbox's
+  // own directory and is only ever served at the pull request's base, so its ref is the one wanted.
+  const existing = findInstanceForRepo(hash);
+  if (existing && await checkInstanceHealth(existing.port)) {
+    return existing.port;
+  }
+
+  const child = spawn(nodePath, [entry, '--repo', worktree, '--no-open', '--quiet', ref], { detached: true, stdio: 'ignore' });
+  child.unref();
+
+  const deadline = Date.now() + waitMs;
+  while (Date.now() < deadline) {
+    await sleep(400);
+    const entryRow = findInstanceForRepo(hash);
+    if (entryRow && await checkInstanceHealth(entryRow.port)) {
+      return entryRow.port;
+    }
+  }
+  try { if (child.pid) process.kill(child.pid, 'SIGTERM'); } catch { /* already gone */ }
+  throw new Error(`diffity did not start for ${worktree} within ${waitMs / 1000}s`);
+}
+
+/** The server registers under the hash of its resolved repo root, so resolve symlinks before hashing. */
+export function repoHash(worktree: string): string {
+  let root = worktree;
+  try { root = realpathSync(worktree); } catch { /* not yet on disk; hash the path as given */ }
+  return createHash('sha256').update(root).digest('hex').slice(0, 12);
+}
+
+function sleep(ms: number): Promise<void> {
+  return new Promise(resolve => setTimeout(resolve, ms));
+}
+
+export interface OpenSessionDeps {
+  /** Reads the base ref recorded in the bundle, so the session diffs the same change. */
+  baseRefOf(bundlePath: string): string;
+  /** Ensures a diffity server for the worktree at that ref and returns its port. */
+  ensureServer(worktree: string, ref: string): Promise<number>;
+  /** Adds the prepared review's threads and tours to the running session. */
+  importBundle(worktree: string, bundlePath: string): void;
+}
+
+export interface OpenedSession {
+  url: string;
+  imported: boolean;
+  /** Why the import did not happen, when it didn't; the diff is still opened. */
+  importError?: string;
+}

packages/cli/src/inbox/open.ts

@@ -0,0 +1,24 @@
+import type { InboxPr, InboxStore } from './store.js';
+
+export type OpenResolution =
+  | { ok: true; pr: InboxPr }
+  | { ok: false; status: number; message: string };
+
+/**
+ * Whether a pull request can be opened, and why not when it can't. Only a prepared (or stale but
+ * still prepared) review has a worktree and a bundle to open; a queued, skipped or failed one has
+ * nothing to show yet.
+ */
+export function resolveOpen(store: InboxStore, id: string): OpenResolution {
+  const pr = store.get(id);
+  if (!pr) {
+    return { ok: false, status: 404, message: `No pull request ${id} in the inbox.` };
+  }
+  if (pr.status !== 'prepared' && pr.status !== 'stale') {
+    return { ok: false, status: 409, message: `${id} is ${pr.status}, not ready to open.` };
+  }
+  if (!pr.worktreePath || !pr.bundlePath) {
+    return { ok: false, status: 409, message: `${id} has no prepared worktree to open.` };
+  }
+  return { ok: true, pr };
+}