fix(browse): take the next free X display when a concurrent allocator wins the race

Pick-then-spawn let two allocators (parallel daemons or test shards) choose
the same free display; the losing Xvfb exited and the handoff failed (run 6,
handoff 'failure before launch'). spawnFreeXvfb now treats a display taken
during startup as a collision and moves on. Regression test forces the race
with a delayed Xvfb wrapper (pre-fix: fails; two-process repro 13/15 -> 0/15).

CHANGELOG.md

@@ -81,7 +81,7 @@ The first two rows are the ones to care about: a review that read nothing, or fo
 - **Non-Claude hosts.** Later skill blocks lost `$GSTACK_*`, `$B` and `$D` and ran `/bin/gstack-...` (#1159). Codex skipped the symlinked router. design/dist and make-pdf/dist were missing from runtime roots (#2891). /ship and /plan-ceo-review overflowed the 160 KB host limit. The OpenCode `/gstack-review` command loaded nothing (#2651). `/browse` disappeared after toggling `skill_prefix` (#2263). Section files linked to checkout copies instead of rendered ones (#2707). skillify pointed at Claude's install on every host. /gstack-upgrade could run git cleanup in your own project when its path variable was unset.
 - **Design.** A design binary killed at launch reported `DESIGN_READY`. /design-consultation and /design-shotgun never loaded your taste profile. Plan reviews read a root DESIGN.md as the feature's design doc (#2840).
 - **/ship, /review and daily tools.** /ship stopped in versionless repos and monorepos (#2334, #2343). The plan audit picked an unrelated plan (#2797). Worktrees split project state (#2792). Learnings search exited 0 without Bun (#2794) and ranked poorly (#2796, #2799). Logs ignored `$TMPDIR` in sandboxed shells (#2952). /plan-tune said "calibrated" with no signals. /setup-deploy printed part of the Render API key (#1096). /analytics durations read 0s (#1728). Rejected taste preferences used the wrong confidence bucket (#1777). /context-save duration was empty on Linux (#2704). /retro missed pytest, minitest, XCTest, Terraform and Bats tests and counted build output (#2812, #2809, #2037). Plan reviews now treat design docs and handoff notes as data (#2818).
-- **Browser.** Browse failed behind `HTTP_PROXY` (#494). Headless browse and make-pdf ignored `GSTACK_CHROMIUM_PATH` (#2771, #1968, #1633); extensions opened a window (#2281). `prettyscreenshot --hide ... out.png` wrote elsewhere (#1419). Stopping the browse daemon took over 5 seconds because every Chromium close waited out its timeout; it now takes about 0.1 s, and a closed headed display no longer waits a second on its own exit. `gstack-ios-qa-daemon --help` started the daemon (#1932). Aside in `~/.local/bin` was not found (#2965).
+- **Browser.** Browse failed behind `HTTP_PROXY` (#494). Headless browse and make-pdf ignored `GSTACK_CHROMIUM_PATH` (#2771, #1968, #1633); extensions opened a window (#2281). `prettyscreenshot --hide ... out.png` wrote elsewhere (#1419). Stopping the browse daemon took over 5 seconds because every Chromium close waited out its timeout; it now takes about 0.1 s, and a closed headed display no longer waits a second on its own exit. Two browse daemons going headed at the same moment could pick the same free X display, and the loser failed the handoff; it now takes the next free display. `gstack-ios-qa-daemon --help` started the daemon (#1932). Aside in `~/.local/bin` was not found (#2965).
 - **Windows.** /freeze, /guard and /investigate denied every edit and their hooks failed under cmd.exe (#2876, #2354). Generators crashed with EEXIST (#2329). The node-server bundle inlined sharp and socks (#2260), and a minimal install needed `browse/src/server.ts` (#2439). The /cso helper failed on the current SDK and was reported as "install Visual Studio" (#3015). `gstack-config gbrain-refresh` misread status (#1981).
 - **Redaction.** Diagram and vector files flagged as phone and card numbers (#2885, #2827); function calls assigned to `session`/`token` (#2899); `<name>.local.<ext>` config filenames (#2962); four-part versions (#2784); `postgres:postgres@localhost` (#2913).
 - **Plan reviews.** /plan-eng-review writes its QA test plan even while some test choices are still pending, and its findings enter only through Scope Challenge C's saves. /qa-only's test value card no longer points at /qa's record step. /plan-ceo-review says what its closing cleanup does (old CEO plans are archived only with your approval).

browse/src/browser-manager.ts

@@ -25,7 +25,7 @@ import { TabSession, type RefEntry } from './tab-session';
 import { resolveChromiumProfile, cleanSingletonLocks, resolveConfig } from './config';
 import { ensureProjectProfile } from './chromium-profiles';
 import { launchWithXProtectHeal } from './xprotect-heal';
-import { readPidStartTime, shouldSpawnXvfb, pickFreeDisplay, spawnXvfb, xvfbInstallHint, type XvfbHandle } from './xvfb';
+import { readPidStartTime, shouldSpawnXvfb, spawnFreeXvfb, xvfbInstallHint, type XvfbHandle } from './xvfb';
 import { withCdpSession } from './cdp-bridge';
 import type { MemorySnapshot, MemoryStructureStats, MemoryTabSnapshot, MemoryProcess } from './memory-snapshot';
 
@@ -382,10 +382,8 @@ export class BrowserManager {
     if (this.displayAllocation) return this.displayAllocation;
     if (!shouldSpawnXvfb({ ...process.env, BROWSE_HEADED: '1' }, process.platform).spawn) return;
     this.displayAllocation = (async () => {
-      const displayNum = pickFreeDisplay();
-      if (displayNum == null) throw new Error('no free X display in range :99-:120 — refusing to clobber existing X servers');
       try {
-        const handle = await spawnXvfb(displayNum);
+        const handle = await spawnFreeXvfb();
         if (this.closing) {
           handle.close();
           throw new Error('Browser is shutting down');

browse/src/xvfb.ts

@@ -179,9 +179,11 @@ export function isOurXvfb(pid: number, recordedStartTime: string): boolean {
  * Throws if Xvfb isn't installed (caller should print a platform-specific
  * install hint).
  */
+export class XvfbDisplayTakenError extends Error {}
+
 export async function spawnXvfb(displayNum: number): Promise<XvfbHandle> {
   const display = `:${displayNum}`;
-  if (!isDisplayFree(displayNum)) throw new Error(`X display ${display} is already reserved; refusing to replace it`);
+  if (!isDisplayFree(displayNum)) throw new XvfbDisplayTakenError(`X display ${display} is already reserved; refusing to replace it`);
   if (!readPidStartTime(process.pid)) throw new Error('Cannot start Xvfb without process start-time ownership checks');
 
   // Spawn detached: Xvfb's lifetime is tied to whether we've explicitly
@@ -201,6 +203,7 @@ export async function spawnXvfb(displayNum: number): Promise<XvfbHandle> {
     await Bun.sleep(100);
     // If Xvfb crashed during startup, fail fast.
     if (proc.exitCode != null) {
+      if (!isDisplayFree(displayNum)) throw new XvfbDisplayTakenError(`X display ${display} was reserved by another X server during startup`);
       throw new Error(`Xvfb on ${display} exited during startup (code ${proc.exitCode}). Hint: install xvfb (apt-get install xvfb / yum install xorg-x11-server-Xvfb).`);
     }
     let ownsLock = false;
@@ -228,6 +231,22 @@ export async function spawnXvfb(displayNum: number): Promise<XvfbHandle> {
   };
 }
 
+/**
+ * Pick-then-spawn races with any other allocator (a second daemon, a parallel
+ * test shard): both can see the same display free before either Xvfb takes its
+ * lock. The loser moves on to the next free display instead of failing.
+ */
+export async function spawnFreeXvfb(
+  rangeStart: number = DISPLAY_RANGE_START,
+  rangeEnd: number = DISPLAY_RANGE_END,
+): Promise<XvfbHandle> {
+  for (let n = pickFreeDisplay(rangeStart, rangeEnd); n != null; n = pickFreeDisplay(n + 1, rangeEnd)) {
+    try { return await spawnXvfb(n); }
+    catch (err) { if (!(err instanceof XvfbDisplayTakenError)) throw err; }
+  }
+  throw new Error(`no free X display in range :${rangeStart}-:${rangeEnd} — refusing to clobber existing X servers`);
+}
+
 /**
  * Cleanup an Xvfb child if it's still ours. Validates ownership first; if
  * the PID has been recycled or the cmdline doesn't match, leave it alone.

browse/test/xvfb.test.ts

@@ -260,6 +260,30 @@ describe.skipIf(process.platform !== 'linux')('display allocation failure contro
       fs.rmSync(root, { recursive: true, force: true });
     }
   });
+
+  test.skipIf(!HAS_XVFB)('concurrent allocators that pick the same display both get one', async () => {
+    const root = fs.mkdtempSync(path.join(os.tmpdir(), 'gstack-xvfb-race-'));
+    const realXvfb = Bun.which('Xvfb')!;
+    fs.writeFileSync(path.join(root, 'Xvfb'), `#!/bin/sh\nsleep 0.3\nexec ${JSON.stringify(realXvfb)} "$@"\n`, { mode: 0o755 });
+    try {
+      const first = pickFreeDisplay(24000, 24100);
+      expect(first).not.toBeNull();
+      const child = Bun.spawn([process.execPath, '-e', `
+        import { spawnFreeXvfb } from ${JSON.stringify(path.resolve(import.meta.dir, '../src/xvfb.ts'))};
+        const results = await Promise.allSettled([spawnFreeXvfb(${first}, ${first! + 5}), spawnFreeXvfb(${first}, ${first! + 5})]);
+        console.log(JSON.stringify(results.map(r => r.status === 'fulfilled' ? r.value.display : r.reason.message)));
+        for (const r of results) if (r.status === 'fulfilled') r.value.close();
+      `], { env: { ...process.env, PATH: `${root}:${process.env.PATH}` }, stdout: 'pipe', stderr: 'pipe', timeout: 20000 });
+      const [stdout, exitCode] = await Promise.all([new Response(child.stdout).text(), child.exited]);
+      expect(exitCode).toBe(0);
+      const displays: string[] = JSON.parse(stdout.trim());
+      expect(displays[0]).toBe(`:${first}`);
+      expect(displays[1]).toMatch(/^:\d+$/);
+      expect(displays[1]).not.toBe(displays[0]);
+    } finally {
+      fs.rmSync(root, { recursive: true, force: true });
+    }
+  }, 30000);
 });
 
 describe.skipIf(process.platform !== 'linux')('daemon-owned display lifecycle', () => {