From ec36e9089d0e59751bcbce0cb05c622b5b671791 Mon Sep 17 00:00:00 2001 From: garrytan Date: Mon, 26 Oct 2026 14:14:25 +0000 Subject: [PATCH] fix(tests): remove three free-tier fixture races: atomic result files in dotenv-isolation and claude-code-runner, per-fixture Git repo in cso-git-hardening (GSTA-107) Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> --- CHANGELOG.md | 9 +++++++++ browse/test/dotenv-isolation.test.ts | 26 ++++++++++++++++++++++++-- test/claude-code-runner.test.ts | 18 ++++++++++++++---- test/cso-git-hardening.test.ts | 23 ++++++++++++++--------- 4 files changed, 61 insertions(+), 15 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index e9d148247..eaf47d24b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,14 @@ # Changelog +## [Unreleased] + +**Three free-tier fixtures stop racing themselves (GSTA-107 daily flaky pass).** Two tests polled for a result file's existence while its writer created it with a plain `writeFileSync`, so a poll between create and write read an empty file; a third copied a shared template repository with `fs.cpSync` and threw once inside the directory walk. + +### Fixed +- **`browse/test/dotenv-isolation.test.ts`** (Free Tests 39902683183 and 40322888041, shard 2, "JSON Parse error: Unexpected EOF" at the `agent-env.json` read): the probe writes its result to a temp file and renames it into place, and `waitForFile` waits for content, not only for the file. A new test feeds the old race (an empty file filled 150 ms later) and runs the probe source itself. +- **`test/claude-code-runner.test.ts`** (Free Tests 39694715116, shard 13, `running(pid)` false at the timeout case): the descendant publishes `descendant.pid` by rename, and the test polls `publishedPid()`, a positive integer, instead of the file's existence, so an empty read can no longer become pid 0. +- **`test/cso-git-hardening.test.ts`** (Free Tests 40288142343, shard 17, `fs.cpSync` threw in `fixture()` with an empty message and passed on the serial retry): every fixture builds its own repository with Git instead of copying a shared template; the `beforeAll` still pays the cold macOS Git spawns once, on a throwaway repository. No timeout changed. + ## [1.91.246.0] - 2026-10-26 **The README is a first-ten-minutes page again: what gstack is, how to install it, the first three skills to try, the skills tables, platform notes, and links to docs/ for everything else.** It was 862 lines and 90 KB, with CSO producer sandboxing, Codex model-selection order and the uninstall script in the path of someone deciding whether to try it. It is now about 300 lines and 33 KB. Nothing was deleted: every removed section moved to a docs/ page, verbatim except the "Proactive skill suggestions" paragraph in docs/why-gstack.md, which now describes the core-set default and `gstack-config set proactive core|all|off` landing in #3432, and every anchor other files link to (`#install--about-30-seconds`, `#gbrain--persistent-knowledge-for-your-coding-agent`) still resolves. diff --git a/browse/test/dotenv-isolation.test.ts b/browse/test/dotenv-isolation.test.ts index c1bb710c9..63bb82967 100644 --- a/browse/test/dotenv-isolation.test.ts +++ b/browse/test/dotenv-isolation.test.ts @@ -22,13 +22,22 @@ function seedHostileProject(dir: string): void { fs.writeFileSync(path.join(dir, 'bunfig.toml'), 'preload = ["./preload.ts"]\n'); } +// The probe publishes its result by rename, so a poller never sees the file +// half-written (Free Tests 39902683183 and 40322888041: a 50 ms poll read the +// empty file writeFileSync had just created, "JSON Parse error: Unexpected EOF"). function probeSource(out: string): string { - return `require('node:fs').writeFileSync(${JSON.stringify(out)}, JSON.stringify({ sandbox: process.env.GSTACK_CHROMIUM_NO_SANDBOX ?? null }));\n`; + return `const fs = require('node:fs'); const tmp = ${JSON.stringify(out)} + '.tmp'; fs.writeFileSync(tmp, JSON.stringify({ sandbox: process.env.GSTACK_CHROMIUM_NO_SANDBOX ?? null })); fs.renameSync(tmp, ${JSON.stringify(out)});\n`; } +/** The probe's complete result: waits for the file to exist and to hold content, not only to exist. */ async function waitForFile(file: string): Promise { const deadline = Date.now() + 20_000; - while (!fs.existsSync(file) && Date.now() < deadline) await Bun.sleep(50); + let content = ''; + while (Date.now() < deadline) { + try { content = fs.readFileSync(file, 'utf-8'); } catch {} + if (content) return content; + await Bun.sleep(50); + } return fs.readFileSync(file, 'utf-8'); } @@ -44,6 +53,19 @@ afterEach(() => { }); describe('D0: project .env and bunfig.toml are ignored', () => { + test('waitForFile returns a result that is still being written only once it holds content', async () => { + const out = path.join(path.dirname(project), 'late.json'); + fs.writeFileSync(out, ''); + setTimeout(() => fs.writeFileSync(out, '{"sandbox":null}'), 150); + expect(JSON.parse(await waitForFile(out))).toEqual({ sandbox: null }); + const atomic = path.join(path.dirname(project), 'atomic.json'); + fs.writeFileSync(path.join(path.dirname(project), 'atomic-probe.ts'), probeSource(atomic)); + const run = Bun.spawnSync([process.execPath, '--no-env-file', '--config=/dev/null', path.join(path.dirname(project), 'atomic-probe.ts')], { cwd: project, env: { PATH: process.env.PATH ?? '' }, stdout: 'pipe', stderr: 'pipe', timeout: 30_000 }); + expect(run.exitCode, run.stderr.toString()).toBe(0); + expect(JSON.parse(fs.readFileSync(atomic, 'utf-8'))).toEqual({ sandbox: null }); + expect(fs.existsSync(atomic + '.tmp')).toBe(false); + }); + test('every compiled binary in scripts/build.sh turns dotenv and bunfig autoload off, and the flags work', () => { const lines = fs.readFileSync(path.join(ROOT, 'scripts', 'build.sh'), 'utf-8').split('\n').filter(l => /build --compile/.test(l)); expect(lines.length).toBeGreaterThanOrEqual(5); diff --git a/test/claude-code-runner.test.ts b/test/claude-code-runner.test.ts index 8b4254afc..d4f22ac7e 100644 --- a/test/claude-code-runner.test.ts +++ b/test/claude-code-runner.test.ts @@ -18,8 +18,10 @@ const RESOLVED_DIR = realpathSync(DIR); // Publish readiness only after the grandchild has initialized and flushed both // inherited pipes; a PID returned by spawn alone does not establish that state. +// The pid lands by rename, so a poller that sees the file sees the whole pid +// (Free Tests 39694715116: an existence poll read the empty file as pid 0). writeFileSync(DESCENDANT, ` -import { appendFileSync, writeFileSync } from 'node:fs'; +import { appendFileSync, renameSync, writeFileSync } from 'node:fs'; const record = stage => { if (process.env.FAKE_MODE === 'timeout') { try { appendFileSync(${JSON.stringify(DESCENDANT_STAGES)}, JSON.stringify({ stage, atUnixMs: Date.now(), processNs: process.hrtime.bigint().toString(), pid: process.pid }) + '\\n'); } catch {} @@ -32,7 +34,8 @@ await new Promise(resolve => process.stdout.write(' ', resolve)); record('stdout_flushed'); await new Promise(resolve => process.stderr.write(' ', resolve)); record('stderr_flushed'); -writeFileSync(process.env.PID_FILE!, String(process.pid)); +writeFileSync(process.env.PID_FILE! + '.tmp', String(process.pid)); +renameSync(process.env.PID_FILE! + '.tmp', process.env.PID_FILE!); record('pid_published'); `); @@ -136,6 +139,13 @@ function running(pid: number): boolean { } catch { return false; } } +/** The descendant's published pid: a positive integer in the pid file, or undefined while unpublished. */ +function publishedPid(): number | undefined { + let text: string; + try { text = readFileSync(PID, 'utf8'); } catch { return undefined; } + return /^[1-9]\d*$/.test(text) ? Number(text) : undefined; +} + async function expectDescendantDead() { const pid = Number(readFileSync(PID, 'utf8')); try { @@ -295,8 +305,8 @@ describe('Claude Code restricted execution', () => { try { expect(fireTimeout).toBeDefined(); const readyBy = Date.now() + 2000; - while (!existsSync(PID) && Date.now() < readyBy) await Bun.sleep(5); - expect(running(Number(readFileSync(PID, 'utf8')))).toBe(true); + while (publishedPid() === undefined && Date.now() < readyBy) await Bun.sleep(5); + expect(running(publishedPid()!)).toBe(true); const start = Date.now(); const expire = fireTimeout!; fireTimeout = undefined; diff --git a/test/cso-git-hardening.test.ts b/test/cso-git-hardening.test.ts index cbea74b7a..8c1aeedd0 100644 --- a/test/cso-git-hardening.test.ts +++ b/test/cso-git-hardening.test.ts @@ -1,4 +1,4 @@ -import { afterAll, afterEach, beforeAll, describe, expect, spyOn, test } from 'bun:test'; +import { afterEach, beforeAll, describe, expect, spyOn, test } from 'bun:test'; import * as fs from 'node:fs'; import * as os from 'node:os'; import * as path from 'node:path'; @@ -10,20 +10,25 @@ const roots:string[]=[]; afterEach(()=>{for(const root of roots.splice(0))fs.rmSync(root,{recursive:true,force:true});}); const fixtureGit=(repo:string,home:string)=>(...args:string[])=>{const result=spawnSync('/usr/bin/git',['-C',repo,...args],{encoding:'utf8',env:{HOME:home,PATH:'/usr/bin:/bin'},timeout:30_000});if(result.status)throw new Error(result.stderr);return result.stdout;}; -let template=''; -// The first /usr/bin/git spawns on a cold macOS runner can eat a test's whole 5 s budget before the code under test runs; pay them once here. -beforeAll(()=>{ - template=fs.realpathSync(fs.mkdtempSync(path.join(os.tmpdir(),'cso-git-hardening-template-'))); - const repo=path.join(template,'repo'),git=fixtureGit(repo,template);fs.mkdirSync(repo); +// Each fixture builds its own repository from Git; nothing is copied from a shared +// template (Free Tests 40288142343 shard 17: fs.cpSync of the template threw inside +// its directory walk once and passed on the serial retry). +function seedRepo(repo:string,home:string){ + const git=fixtureGit(repo,home);fs.mkdirSync(repo); git('init','-q');git('config','user.email','fixture@example.test');git('config','user.name','Fixture'); fs.writeFileSync(path.join(repo,'tracked.ts'),'export const tracked = true\n');git('add','tracked.ts');git('commit','-qm','base'); + return git; +} +// The first /usr/bin/git spawns on a cold macOS runner can eat a test's whole 5 s budget before the code under test runs; pay them once here. +beforeAll(()=>{ + const warm=fs.realpathSync(fs.mkdtempSync(path.join(os.tmpdir(),'cso-git-hardening-warm-'))); + try{seedRepo(path.join(warm,'repo'),warm);}finally{fs.rmSync(warm,{recursive:true,force:true});} },{timeout:30_000}); -afterAll(()=>fs.rmSync(template,{recursive:true,force:true})); function fixture(){ const root=fs.realpathSync(fs.mkdtempSync(path.join(os.tmpdir(),'cso-git-hardening-'))),repo=path.join(root,'repo'),runDir=path.join(root,'state','run'); - roots.push(root);fs.cpSync(path.join(template,'repo'),repo,{recursive:true});fs.mkdirSync(runDir,{recursive:true,mode:0o700}); - return{root,repo,runDir,git:fixtureGit(repo,root)}; + roots.push(root);const git=seedRepo(repo,root);fs.mkdirSync(runDir,{recursive:true,mode:0o700}); + return{root,repo,runDir,git}; } function replaceWithSymlinkAfterLstat(target:string,replacement:string,occurrence=1){