diff --git a/CHANGELOG.md b/CHANGELOG.md index 7c7504bcd..38f335970 100644 --- a/CHANGELOG.md +++ b/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. 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). +- **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, and an Xvfb that fails to start reports its own error. `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); `.local.` 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). diff --git a/browse/src/xvfb.ts b/browse/src/xvfb.ts index b58bea94f..48cc5d6f8 100644 --- a/browse/src/xvfb.ts +++ b/browse/src/xvfb.ts @@ -188,12 +188,34 @@ export async function spawnXvfb(displayNum: number): Promise { // Spawn detached: Xvfb's lifetime is tied to whether we've explicitly // killed it via the handle's close() method, not to the parent process. - const proc = Bun.spawn(['Xvfb', display, '-screen', '0', '1920x1080x24', '-ac'], { - windowsHide: true, - stdio: ['ignore', 'ignore', 'ignore'], - }); + // Startup stderr goes to a private file: a holder that takes the display and + // exits before we look leaves no lock behind, only Xvfb's own message. + const stderrDir = fs.mkdtempSync(path.join(os.tmpdir(), 'gstack-xvfb-')); + const stderrPath = path.join(stderrDir, 'stderr.log'); + const stderrFd = fs.openSync(stderrPath, 'w', 0o600); + let proc: ReturnType; + try { + proc = Bun.spawn(['Xvfb', display, '-screen', '0', '1920x1080x24', '-ac'], { + windowsHide: true, + stdio: ['ignore', 'ignore', stderrFd], + }); + } catch (err) { + fs.rmSync(stderrDir, { recursive: true, force: true }); + throw err; + } finally { + fs.closeSync(stderrFd); + } proc.unref(); const startTime = readPidStartTime(proc.pid); + try { + return await awaitXvfbReady(proc, displayNum, startTime, stderrPath); + } finally { + fs.rmSync(stderrDir, { recursive: true, force: true }); + } +} + +async function awaitXvfbReady(proc: ReturnType, displayNum: number, startTime: string, stderrPath: string): Promise { + const display = `:${displayNum}`; // Wait for the X server to become reachable — Xvfb takes a few hundred ms // to bind. Probe via xdpyinfo with retries. @@ -203,8 +225,13 @@ export async function spawnXvfb(displayNum: number): Promise { 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 stderr = ''; + try { stderr = fs.readFileSync(stderrPath, 'utf8'); } catch {} + if (!isDisplayFree(displayNum) || /Server is already active for display/.test(stderr)) { + throw new XvfbDisplayTakenError(`X display ${display} was reserved by another X server during startup`); + } + const fatal = stderr.split('\n').map(line => line.replace(/^\(EE\)\s*/, '').trim()).filter(Boolean).slice(-3).join(' '); + throw new Error(`Xvfb on ${display} exited during startup (code ${proc.exitCode})${fatal ? `: ${fatal}` : ''}. Hint: install xvfb (apt-get install xvfb / yum install xorg-x11-server-Xvfb).`); } let ownsLock = false; try { ownsLock = Number(fs.readFileSync(`/tmp/.X${displayNum}-lock`, 'utf8').trim()) === proc.pid; } catch {} diff --git a/browse/test/xvfb.test.ts b/browse/test/xvfb.test.ts index efccf2d8e..047468673 100644 --- a/browse/test/xvfb.test.ts +++ b/browse/test/xvfb.test.ts @@ -261,6 +261,33 @@ describe.skipIf(process.platform !== 'linux')('display allocation failure contro } }); + test.skipIf(!HAS_XVFB)('a display whose short-lived holder is gone by the exit check still counts as taken', async () => { + const root = fs.mkdtempSync(path.join(os.tmpdir(), 'gstack-xvfb-gone-')); + const realXvfb = Bun.which('Xvfb')!; + const first = pickFreeDisplay(26000, 26100); + expect(first).not.toBeNull(); + // Run 10 (1ffd38e): another test's Xvfb held the display just long enough + // to win the lock, then closed before the exit check looked for it. + fs.writeFileSync(path.join(root, 'Xvfb'), [ + '#!/bin/sh', + `if [ "$1" = ":${first}" ]; then printf '(EE) Server is already active for display ${first}\\n' >&2; exit 1; fi`, + `exec ${JSON.stringify(realXvfb)} "$@"`, '', + ].join('\n'), { mode: 0o755 }); + try { + const child = Bun.spawn([process.execPath, '-e', ` + import { spawnFreeXvfb } from ${JSON.stringify(path.resolve(import.meta.dir, '../src/xvfb.ts'))}; + try { const handle = await spawnFreeXvfb(${first}, ${first! + 3}); console.log(handle.display); handle.close(); } + catch (err) { console.log(err.message); } + `], { 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); + expect(stdout.trim()).toBe(`:${first! + 1}`); + expect(isDisplayFree(first!)).toBe(true); + } finally { + fs.rmSync(root, { recursive: true, force: true }); + } + }, 30000); + 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')!;