fix: a registered server is trusted only once it has said which repository it serves

packages/api/package.json

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

packages/cli/package.json

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

packages/cli/src/agent-session.ts

@@ -1,7 +1,7 @@
 import { createHash } from 'node:crypto';
 import { AGENT_TRAFFIC_HEADER, type ReviewSession } from '@diffity/api';
 import { getRepoRoot } from '@diffity/git';
-import { findInstanceForRepo, type RegistryEntry } from './registry.js';
+import { findServingInstance, type RegistryEntry } from './registry.js';
 import { getCurrentSession, getSessionById } from './session.js';
 
 /** How the server tells the agent's own traffic from a page somebody is looking at. */
@@ -10,12 +10,13 @@ export const AGENT_HEADER: Record<string, string> = { [AGENT_TRAFFIC_HEADER]: '1
 /** The registry's own health probe allows the same; a wedged server must not hang every command. */
 export const SERVER_TIMEOUT_MS = 2000;
 
-export function findRunningInstance(): RegistryEntry | null {
+/** The server registered for this repository, once it has confirmed it is serving this repository. */
+export function findServingInstanceForCwd(): Promise<RegistryEntry | null> {
   const repoRoot = getRepoRoot();
   if (!repoRoot) {
-    return null;
+    return Promise.resolve(null);
   }
-  return findInstanceForRepo(createHash('sha256').update(repoRoot).digest('hex').slice(0, 12));
+  return findServingInstance(createHash('sha256').update(repoRoot).digest('hex').slice(0, 12), repoRoot);
 }
 
 /**
@@ -84,7 +85,7 @@ export async function resolveAgentSession(explicitId?: string): Promise<ReviewSe
     return getSessionById(explicitId);
   }
 
-  const instance = findRunningInstance();
+  const instance = await findServingInstanceForCwd();
   if (instance) {
     const fromServer = (await fetchServerSession(instance.port)) ?? (await fetchLegacySession(instance.port));
     if (fromServer) {

packages/cli/src/agent.ts

@@ -27,7 +27,7 @@ import { baseShaOf, buildBundle, exportMismatch, importBundle, importMismatch, s
 import { answerLiveRequest } from './live.js';
 import { clampClientWait, CLIENT_WAIT_CAP_SECONDS } from './live-wait.js';
 import { directiveFor } from './live-intent.js';
-import { AGENT_HEADER, SERVER_TIMEOUT_MS, findRunningInstance, resolveAgentSession } from './agent-session.js';
+import { AGENT_HEADER, SERVER_TIMEOUT_MS, findServingInstanceForCwd, resolveAgentSession } from './agent-session.js';
 import type { Session } from './session.js';
 import { createTour, addTourStep, updateTourStatus, deleteTour, deleteToursForSession, getTour } from './tours.js';
 import { unansweredRequest } from './live-unanswered.js';
@@ -399,7 +399,7 @@ Examples:
     .option('--timeout <seconds>', `How long to wait before giving up (each poll caps at ${CLIENT_WAIT_CAP_SECONDS}s and returns; call again to keep waiting)`, '900')
     .action(async (opts: { timeout: string }) => {
       const session = await requireSession(agent.opts().session);
-      const instance = findRunningInstance();
+      const instance = await findServingInstanceForCwd();
       if (!instance) {
         console.error(pc.red('No diffity is running for this repository — start one first'));
         process.exitCode = 1;
@@ -481,7 +481,7 @@ Examples:
     .option('--json', 'Output as JSON')
     .action(async (opts: { json?: boolean }) => {
       await requireSession(agent.opts().session);
-      const instance = findRunningInstance();
+      const instance = await findServingInstanceForCwd();
       const status = instance ? await fetchLiveStatus(instance.port) : null;
 
       if (opts.json) {

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

@@ -2,7 +2,7 @@ import { spawn, execFile } from 'node:child_process';
 import { promisify } from 'node:util';
 import { readFileSync, realpathSync } from 'node:fs';
 import { createHash } from 'node:crypto';
-import { checkInstanceHealth, findInstanceForRepo } from '../registry.js';
+import { checkInstanceHealth, findServingInstance, readRegistry } from '../registry.js';
 
 /**
  * Brings a prepared review up as a live diffity session the reviewer can open: a server over the
@@ -55,23 +55,30 @@ export function realOpenSessionDeps(nodePath: string, entry: string): OpenSessio
 }
 
 export async function ensureServer(nodePath: string, entry: string, worktree: string, ref: string, prNumber: number | undefined, 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)) {
+  // A server already registered for this worktree is reused only once it has confirmed it is the
+  // one registered: alive, and serving this worktree. The worktree lives under the inbox's own
+  // directory and is only ever served at the pull request's base, so a genuine one is the one wanted.
+  const existing = await findServingInstance(repoHash(worktree), resolvedRoot(worktree));
+  if (existing) {
     return existing.port;
   }
 
   const child = spawn(nodePath, serverArgs(entry, worktree, ref, prNumber), { detached: true, stdio: 'ignore' });
   child.unref();
+  let exited: string | null = null;
+  child.once('exit', (code, signal) => { exited = signal ? `signal ${signal}` : `exit code ${code}`; });
 
   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;
+    if (exited) {
+      throw new Error(`diffity ended with ${exited} before registering for ${worktree}`);
+    }
+    // The child's own registration, by pid: a row for the same worktree left by an earlier server
+    // must not stand in for it.
+    const own = readRegistry().find(row => row.pid === child.pid);
+    if (own && await checkInstanceHealth(own.port)) {
+      return own.port;
     }
   }
   try { if (child.pid) process.kill(child.pid, 'SIGTERM'); } catch { /* already gone */ }
@@ -89,9 +96,16 @@ export function serverArgs(entry: string, worktree: string, ref: string, prNumbe
 
 /** 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);
+  return createHash('sha256').update(resolvedRoot(worktree)).digest('hex').slice(0, 12);
+}
+
+/** The path as the server will report it; a worktree not yet on disk is taken as given. */
+function resolvedRoot(worktree: string): string {
+  try {
+    return realpathSync(worktree);
+  } catch {
+    return worktree;
+  }
 }
 
 function sleep(ms: number): Promise<void> {

packages/cli/src/registry.ts

@@ -252,6 +252,57 @@ export function checkInstanceHealth(port: number): Promise<boolean> {
   });
 }
 
+/**
+ * What the server on a port says about itself: `serves` when /api/info names that repository root,
+ * `other` when it names a different one, `silent` when nothing usable came back in time. Ports are
+ * reused, and a registry that knows only its own data directory can hand a freed port to another
+ * server — so an entry is trusted only once the server on its port has said whose it is.
+ */
+export function instanceServes(port: number, repoRoot: string): Promise<'serves' | 'other' | 'silent'> {
+  return new Promise((resolve) => {
+    const req = get(`http://localhost:${port}/api/info`, { headers: { [AGENT_TRAFFIC_HEADER]: '1' } }, (res) => {
+      let body = '';
+      res.setEncoding('utf-8');
+      res.on('data', (chunk: string) => { body += chunk; });
+      res.on('end', () => {
+        try {
+          const root = (JSON.parse(body) as { root?: unknown }).root;
+          resolve(res.statusCode !== 200 || typeof root !== 'string' ? 'silent' : root === repoRoot ? 'serves' : 'other');
+        } catch {
+          resolve('silent');
+        }
+      });
+    });
+    req.on('error', () => resolve('silent'));
+    req.setTimeout(2000, () => {
+      req.destroy();
+      resolve('silent');
+    });
+  });
+}
+
+/**
+ * The registered server for a repository, if it is still that server: alive, and serving that root.
+ * An entry that is certainly stale — its process gone, or its port answered for another repository —
+ * is dropped from the registry so nothing else trusts it. One that is alive but did not answer is a
+ * server that is busy, not gone: it is left registered and simply not reused this time.
+ */
+export async function findServingInstance(repoHash: string, repoRoot: string): Promise<RegistryEntry | null> {
+  const entry = findInstanceForRepo(repoHash);
+  if (!entry) {
+    return null;
+  }
+  if (!entryIsAlive(entry)) {
+    deregisterInstance(entry.pid);
+    return null;
+  }
+  const answer = await instanceServes(entry.port, repoRoot);
+  if (answer === 'other') {
+    deregisterInstance(entry.pid);
+  }
+  return answer === 'serves' ? entry : null;
+}
+
 export function killInstance(entry: RegistryEntry): void {
   // A reused pid is somebody else's process; deregistering is all there is left to do.
   if (entryIsAlive(entry)) {

packages/cli/tests/agent-session-precedence.test.ts

@@ -105,6 +105,12 @@ describe('resolveAgentSession', () => {
     expect(getCurrentSession()?.id).toBe(decoy.id);
 
     stub = createServer((req, res) => {
+      // A server is trusted only once its info route names the repository it serves.
+      if (req.url === '/api/info' && req.method === 'GET') {
+        res.writeHead(200, { 'Content-Type': 'application/json' });
+        res.end(JSON.stringify({ name: 'repo', root: repoDir }));
+        return;
+      }
       if (req.url === '/api/sessions/ensure' && req.method === 'POST') {
         res.writeHead(200, { 'Content-Type': 'application/json' });
         res.end(JSON.stringify(other));
@@ -133,7 +139,7 @@ describe('resolveAgentSession', () => {
     stub = createServer((req, res) => {
       if (req.url === '/api/info' && req.method === 'GET') {
         res.writeHead(200, { 'Content-Type': 'application/json' });
-        res.end(JSON.stringify({ name: 'repo', sessionId: named.id }));
+        res.end(JSON.stringify({ name: 'repo', root: repoDir, sessionId: named.id }));
         return;
       }
       // An old server answers an unknown route with the page and a 200.

packages/cli/tests/agent-session.test.ts

@@ -79,4 +79,29 @@ describe('getSessionById', () => {
     expect(after.id).not.toBe(before.id);
     expect(getSessionById(before.id)?.id).toBe(after.id);
   });
+
+  it('does not take a session from a server that turns out to serve another repository', async () => {
+    const { createHash } = await import('node:crypto');
+    const { createServer } = await import('node:http');
+    const { registerInstance, readRegistry } = await import('../src/registry.js');
+    const { resolveAgentSession } = await import('../src/agent-session.js');
+    const { getRepoRoot } = await import('@diffity/git');
+
+    const impostor = createServer((_req, res) => {
+      res.writeHead(200, { 'Content-Type': 'application/json' });
+      res.end(JSON.stringify({ id: 'not-yours', ref: 'work', headHash: 'x', root: '/somewhere/else' }));
+    });
+    await new Promise<void>(resolve => impostor.listen(0, '127.0.0.1', () => resolve()));
+    const port = (impostor.address() as { port: number }).port;
+    const repoRoot = getRepoRoot()!;
+    const repoHash = createHash('sha256').update(repoRoot).digest('hex').slice(0, 12);
+    registerInstance({ pid: process.pid, port, repoRoot, repoHash, repoName: 'repo', ref: 'work', description: '', startedAt: new Date().toISOString() });
+    try {
+      const session = await resolveAgentSession();
+      expect(session?.id).not.toBe('not-yours');
+      expect(readRegistry().find(e => e.port === port)).toBeUndefined();
+    } finally {
+      impostor.close();
+    }
+  });
 });

packages/cli/tests/inbox-open.test.ts

@@ -1,7 +1,8 @@
 import { describe, it, expect, beforeEach, afterEach } from 'vitest';
 import { request } from 'node:http';
 import { execFileSync, spawn, type ChildProcess } from 'node:child_process';
-import { mkdtempSync, rmSync, writeFileSync } from 'node:fs';
+import { mkdtempSync, realpathSync, rmSync, writeFileSync } from 'node:fs';
+import { createServer } from 'node:http';
 import { dirname, join } from 'node:path';
 import { fileURLToPath } from 'node:url';
 import { tmpdir } from 'node:os';
@@ -149,20 +150,51 @@ describe('the real ensureServer', () => {
     execFileSync('git', ['add', '.'], { cwd: repo, stdio: 'pipe' });
     execFileSync('git', ['commit', '-m', 'init'], { cwd: repo, stdio: 'pipe' });
 
+    // Another server — alive, answering — has taken the port a registered entry for this repo still
+    // names, which is how one pull request's page came to show another's code.
+    const impostor = createServer((_req, res) => {
+      res.writeHead(200, { 'Content-Type': 'application/json' });
+      res.end(JSON.stringify({ name: 'other', branch: 'main', root: join(root, 'somebody-else') }));
+    });
+    await new Promise<void>(resolve => impostor.listen(0, '127.0.0.1', () => resolve()));
+    const impostorPort = (impostor.address() as { port: number }).port;
+    registerInstance({
+      pid: process.pid, port: impostorPort, repoRoot: realpathSync(repo), repoHash: repoHash(repo), repoName: 'repo',
+      ref: 'work', description: '', startedAt: new Date().toISOString(),
+    });
+
     let port = 0;
     try {
       port = await ensureServer(process.execPath, ENTRY, repo, 'work', undefined, 20_000);
-      // The entry the server registered must carry the hash open-session looks it up by.
+      expect(port).not.toBe(impostorPort);
+      // The entry the server registered must carry the hash open-session looks it up by, and the
+      // impostor's row is gone.
       const entry = readRegistry().find(e => e.port === port);
       expect(entry).toBeDefined();
       expect(entry!.repoHash).toBe(repoHash(repo));
+      expect(readRegistry().find(e => e.port === impostorPort)).toBeUndefined();
     } finally {
+      impostor.close();
       const entry = readRegistry().find(e => e.port === port);
       if (entry) { try { process.kill(entry.pid, 'SIGKILL'); } catch { /* gone */ } }
       if (prev === undefined) delete process.env.DIFFITY_DATA_DIR; else process.env.DIFFITY_DATA_DIR = prev;
     }
   }, 30_000);
 
+  it('fails as soon as the server it started has exited', async () => {
+    const prev = process.env.DIFFITY_DATA_DIR;
+    process.env.DIFFITY_DATA_DIR = join(root, 'dead-data');
+    const dying = join(root, 'dying.mjs');
+    writeFileSync(dying, 'process.exit(2);\n');
+    const started = Date.now();
+    try {
+      await expect(ensureServer(process.execPath, dying, join(root, 'wt'), 'work', undefined, 20_000)).rejects.toThrow(/exit code 2/);
+      expect(Date.now() - started).toBeLessThan(10_000);
+    } finally {
+      if (prev === undefined) delete process.env.DIFFITY_DATA_DIR; else process.env.DIFFITY_DATA_DIR = prev;
+    }
+  }, 15_000);
+
   it('throws when nothing registers before the deadline', async () => {
     const prev = process.env.DIFFITY_DATA_DIR;
     process.env.DIFFITY_DATA_DIR = join(root, 'empty-data');