diff --git a/bin/gstack-review-dispatch b/bin/gstack-review-dispatch index a72e5cfd4..5c465eb49 100755 --- a/bin/gstack-review-dispatch +++ b/bin/gstack-review-dispatch @@ -34,7 +34,7 @@ import { TARGETED_RULES, parseClassList, targetedBriefSha, targetedRead } from ' import { resolveBaseBranch } from '../lib/ship-policy'; import { auditVerdict, eventsDigest, loadReaders, readersLine, scanEvents, type ToolEvent } from '../lib/review-fetch'; import { effectiveReaders, READERS_POLICY_DEFAULT, type EffectiveReaders } from '../lib/ship-policy-readers'; -import { defaultRoots, foundLines, inventory, quarantineStage, rerunBlocked, sweepResult, type Inventory } from '../lib/review-sweep'; +import { defaultRoots, foundLines, ignoredLines, inventory, quarantineStage, rerunBlocked, sweepResult, type Inventory } from '../lib/review-sweep'; import { REVIEW_DIFF_MAX_BYTES, ceilingContext, disclosureDigest, disclosureLine, parseTail, readSelection, tooLargeLine, unreadableLine, type PlanExclude, type ReaderDiff } from '../lib/review-exclude'; import { logShipCall } from '../lib/ship-call-log'; @@ -113,7 +113,9 @@ const HELP = `gstack-review-dispatch — verbatim reviewer briefs and loss-free sweep --manifest --stage --phase start|end the fetch rule's enforcement, per stage. start records an inventory of git repositories and files of 100 MB or more under the roots (the checkout's parent, home and the temp dir, or GSTACK_SWEEP_ROOTS; bounded in depth, entries and time) before the stage's readers - start. end, once the stage's readers are collected, diffs against it: a new repository with a network remote or a new big file + start (a directory below a root carrying .gstack-test-fixture, a test fixture root, is not entered). end, once the stage's + readers are collected, diffs against it: a new fixture root, or a repository whose network remote resolves to a local path + through url..insteadOf, prints FETCH_IGNORED: and is not a fetch; a new repository with a network remote or a new big file prints FETCH_VIOLATION: per path with a KEEP: line (never a delete command: no transcript attributes the path), quarantines every reader of the stage (reply to /quarantine/, findings out of merged.jsonl), plans one rerun per slot as #2 (RERUN:, not a review round) and exits 0; the same on a rerun prints FETCH_RERUN_SPENT (exit 3). A sweep that hit @@ -264,10 +266,10 @@ function sweep(): number { if (s.end) { process.stdout.write(`FETCH_SWEEP: ${s.end.verdict} stage=${stage} attempt=${s.attempt} found=${s.end.found.length} (recorded)\n`); return EXIT.ok; } let start: Inventory | null = null; try { start = s.start ? JSON.parse(fs.readFileSync(s.start.file, 'utf8')) as Inventory : null; } catch { start = null; } - const { found, verdict, partial } = sweepResult(start, start ? inventory(start.roots, checkout) : null); - s.end = { at: new Date().toISOString(), verdict, found, ...(partial ? { partial } : {}) }; - appendEvent(m.ledger, { event: 'fetch-sweep', pass: m.pass, stage, attempt: s.attempt, verdict, found, readers, ...(partial ? { partial } : {}) }); - const lines = [`FETCH_SWEEP: ${verdict} stage=${stage} attempt=${s.attempt} found=${found.length}${partial ? ` partial=${JSON.stringify(partial)} (a note: the sweep could not cover every root)` : ''}`]; + const { found, ignored = [], verdict, partial } = sweepResult(start, start ? inventory(start.roots, checkout) : null); + s.end = { at: new Date().toISOString(), verdict, found, ...(ignored.length ? { ignored } : {}), ...(partial ? { partial } : {}) }; + appendEvent(m.ledger, { event: 'fetch-sweep', pass: m.pass, stage, attempt: s.attempt, verdict, found, ...(ignored.length ? { ignored } : {}), readers, ...(partial ? { partial } : {}) }); + const lines = [`FETCH_SWEEP: ${verdict} stage=${stage} attempt=${s.attempt} found=${found.length}${partial ? ` partial=${JSON.stringify(partial)} (a note: the sweep could not cover every root)` : ''}`, ...ignoredLines(stage, s.attempt, ignored)]; if (verdict !== 'violation') { writeManifest(file, m); process.stdout.write(lines.join('\n') + '\n'); return EXIT.ok; } lines.push(...foundLines(stage, s.attempt, found, readers)); const q = quarantineStage(m, stage, s.attempt); diff --git a/changelog.d/capy-gst-1695-fixture-sweep.md b/changelog.d/capy-gst-1695-fixture-sweep.md new file mode 100644 index 000000000..f6ebc73a9 --- /dev/null +++ b/changelog.d/capy-gst-1695-fixture-sweep.md @@ -0,0 +1 @@ +- **A test run's fixture repositories no longer quarantine a /ship review pass.** /ship's fetch sweep counted any new git repository with a network remote under the temp dir as a reader's fetch, so a lane's background `bun test`, or a reader's own run that hit Bun's 5 s timeout before its cleanup, left fixtures such as `/tmp/ship-bin-*` (origin `https://github.com/fx/fx.git`) that quarantined the stage and then stopped its rerun with `FETCH_RERUN_BLOCKED`; three /ship passes were lost that way on 11-06 and 11-07. Every `bun test` process now puts its temp paths under one root carrying a `.gstack-test-fixture` marker (`TMPDIR` points there, so children spawned with the test's environment follow; a `Bun.spawn` child given no `env` keeps the original temp dir) and removes it after the last file, even after a timeout. Under the free and paid shard runners the shard's own sandbox carries the marker instead, so a shard's `TMPDIR` stays exactly as the runner set it. The sweep skips a marked root below a sweep root and a repository whose remotes resolve to a local path through `url..insteadOf`, and prints a `FETCH_IGNORED:` line for each; an unmarked clone with a real network remote still quarantines the stage and blocks its rerun. (GST-1695) diff --git a/docs/review-policy.md b/docs/review-policy.md index 2950ab21f..026a58dab 100644 --- a/docs/review-policy.md +++ b/docs/review-policy.md @@ -654,6 +654,17 @@ keep a network remote and files of 100 MB or more, outside `node_modules` and outside other repositories. An extracted archive, a smaller raw download, a fetch with no remote, a clone nested in another repository and a clone deleted before the stage ends all go unseen until the platform exposes reader transcripts. +A test run's fixtures are not fetches, and each new one prints a +`FETCH_IGNORED:` line with its reason instead: a directory below a sweep root +that carries `.gstack-test-fixture` (test-setup.ts puts every test's temp +paths under one such root per process and removes it after the last file), and a +repository whose every network-looking remote resolves to a local path through +`url..insteadOf` (the ship fixtures' `https://github.com/fx/fx.git`). A +marker on a sweep root itself is not honored, insteadOf that sends a local path +to the network still counts, and a clone made inside a live fixture root goes +unseen, as does one whose reader writes the marker or an insteadOf line itself. +A `Bun.spawn` child given no `env` keeps the temp dir its test process started +with, so its fixtures land outside the marked root and still count. What this protects: the readers' findings, the merged review and PR text never carry a sealed file a reader fetched or opened. What it cannot protect: data that diff --git a/docs/troubleshooting.md b/docs/troubleshooting.md index f1de2e14b..97b16cefe 100644 --- a/docs/troubleshooting.md +++ b/docs/troubleshooting.md @@ -3701,7 +3701,9 @@ exposes no reader id, so the stage is the unit: every reader of that stage is quarantined. Its reply moves to `/quarantine/` (mode 0600), its findings leave `merged.jsonl`, and it never reaches the Red Team, the judge, the returned output or PR text. `verify` prints `FETCH_VIOLATION:` and refuses -until the stage has rerun and swept clean. +until the stage has rerun and swept clean. A test run's fixtures (a root +carrying `.gstack-test-fixture`, or a repository whose remote resolves to a local +path through `url..insteadOf`) print `FETCH_IGNORED:` and never count. **Fix.** Nothing to rerun by hand: the pass plans one fresh attempt per slot (`RERUN:`, readers named `#2`; not a review round) and the pass program diff --git a/lib/fixture-marker.ts b/lib/fixture-marker.ts new file mode 100644 index 000000000..c5d10626f --- /dev/null +++ b/lib/fixture-marker.ts @@ -0,0 +1,6 @@ +/** + * The file that marks a test fixture root (GST-1695). test-setup.ts writes it into the one per-process directory every + * test's temp paths land under, and the fetch sweep (lib/review-sweep.ts) treats a directory below a sweep root that + * carries it as a test fixture, never a reader's fetch, so a concurrent or leftover `bun test` cannot quarantine a stage. + */ +export const FIXTURE_MARKER = '.gstack-test-fixture'; diff --git a/lib/review-sweep.ts b/lib/review-sweep.ts index 35f371680..705b3287d 100644 --- a/lib/review-sweep.ts +++ b/lib/review-sweep.ts @@ -10,11 +10,16 @@ * * Nothing here deletes anything, and nothing prints a delete command: no transcript can attribute a path to a reader today, * so every found path is reported as KEEP for the lane to inspect and leave alone. + * + * Two kinds of new directory are not fetches and print FETCH_IGNORED instead (GST-1695): a test fixture root below a sweep + * root that carries FIXTURE_MARKER (test-setup.ts puts every test's temp paths under one), and a repository whose every + * network remote resolves to a local path through url..insteadOf (the ship fixtures' fake GitHub origin). */ import { spawnSync } from 'node:child_process'; import * as fs from 'node:fs'; import * as os from 'node:os'; import * as path from 'node:path'; +import { FIXTURE_MARKER } from './fixture-marker'; import { networkTarget } from './review-fetch'; import { appendEvent, readLedger, readerKey } from './review-ledger'; import type { Manifest, PassReader, Stage } from './review-pass'; @@ -26,13 +31,15 @@ export const BIG_BYTES = 100 * 1024 * 1024; const SKIP = new Set(['node_modules', '.git']); const SCP = /^(?:[\w.-]+@)?[\w.-]+:(?!\/\/)[\w./-]+$/; -export interface Inventory { at: number; roots: string[]; repos: Record; big: Record; partial?: string } +export interface Inventory { at: number; roots: string[]; repos: Record; big: Record; ignored?: Record>; partial?: string } export interface Found { path: string; kind: 'repo' | 'download'; remote?: string; bytes: number | null } +/** A new directory the sweep saw and did not count: a marked test fixture root, or a repository whose remote resolves locally. */ +export interface Ignored { path: string; reason: 'fixture-marker' | 'local-remote'; remote?: string } export type SweepVerdict = 'clean' | 'violation' | 'partial'; export interface StageSweep { attempt: number; start?: { at: string; file: string; partial?: string }; - end?: { at: string; verdict: SweepVerdict; found: Found[]; partial?: string }; + end?: { at: string; verdict: SweepVerdict; found: Found[]; ignored?: Ignored[]; partial?: string }; /** Earlier attempts of this stage, newest last. */ history?: Array>; } @@ -66,13 +73,24 @@ export function networkRemote(v: string): boolean { } /** - * A repository's network remotes as git itself reads its config (quoted values, escapes and `#` included), classified on - * the raw value and recorded only through redactUrl. + * A repository's remotes as git itself reads its config (quoted values, escapes and `#` included), each rewritten by the + * longest matching url..insteadOf as git fetch does and classified on the rewritten raw value: `network` holds the ones git + * would fetch over the network, `local` the network-looking ones insteadOf sends to a local path. Recorded only through redactUrl. */ -function remotes(gitDir: string): string[] { - const r = spawnSync('git', ['config', '--file', path.join(gitDir, 'config'), '--null', '--get-regexp', '\\.url$'], { encoding: 'utf8', timeout: 5_000 }); - if (r.status !== 0 || !r.stdout) return []; - return r.stdout.split('\0').flatMap(e => { const i = e.indexOf('\n'); const v = e.slice(i + 1); return i >= 0 && networkRemote(v) ? [redactUrl(v)] : []; }); +function remotes(gitDir: string): { network: string[]; local: string[] } { + const r = spawnSync('git', ['config', '--file', path.join(gitDir, 'config'), '--null', '--get-regexp', '\\.url$|^url\\..*\\.insteadof$'], { encoding: 'utf8', timeout: 5_000 }); + const out = { network: [] as string[], local: [] as string[] }; + if (r.status !== 0 || !r.stdout) return out; + const entries = r.stdout.split('\0').flatMap(e => { const i = e.indexOf('\n'); return i >= 0 ? [{ key: e.slice(0, i), value: e.slice(i + 1) }] : []; }); + const rules = entries.filter(e => /^url\..*\.insteadof$/.test(e.key) && e.value).map(e => ({ from: e.value, to: e.key.slice(4, -'.insteadof'.length) })); + for (const { key, value } of entries) { + if (!/\.url$/.test(key)) continue; + const rule = rules.filter(x => value.startsWith(x.from)).sort((a, b) => b.from.length - a.from.length)[0]; + const url = rule ? rule.to + value.slice(rule.from.length) : value; + if (networkRemote(url)) out.network.push(redactUrl(url)); + else if (networkRemote(value)) out.local.push(redactUrl(value)); + } + return out; } /** The git directory a directory holds: its `.git` (a directory, or a file naming one), or itself when it is a bare repository. */ @@ -90,11 +108,12 @@ function gitDirOf(dir: string, names: Set): string | null { /** * Git repositories (with their redacted network remotes) and files of BIG_BYTES or more under the roots, to SWEEP_DEPTH, without following - * symlinks or entering node_modules, .git or a repository other than the checkout. Past SWEEP_MAX_ENTRIES entries or - * SWEEP_BUDGET_MS the inventory stops and says so in `partial`. + * symlinks or entering node_modules, .git, a repository other than the checkout or a directory below a root that carries + * FIXTURE_MARKER (recorded in `ignored`, like a repository whose only network-looking remotes resolve locally). Past + * SWEEP_MAX_ENTRIES entries or SWEEP_BUDGET_MS the inventory stops and says so in `partial`. */ export function inventory(roots: readonly string[], checkout: string, skip: readonly string[] = defaultSkip(), now = Date.now()): Inventory { - const inv: Inventory = { at: now, roots: roots.map(r => realOr(r)), repos: {}, big: {} }; + const inv: Inventory = { at: now, roots: roots.map(r => realOr(r)), repos: {}, big: {}, ignored: {} }; const deadline = Date.now() + SWEEP_BUDGET_MS; const home = realOr(checkout); const seen = new Set(); @@ -107,8 +126,14 @@ export function inventory(roots: readonly string[], checkout: string, skip: read entries += list.length; if (entries > SWEEP_MAX_ENTRIES) { inv.partial = `more than ${SWEEP_MAX_ENTRIES} entries`; return; } if (Date.now() > deadline) { inv.partial = `past ${SWEEP_BUDGET_MS / 1000}s`; return; } + if (depth > 0 && list.some(d => d.name === FIXTURE_MARKER && d.isFile())) { inv.ignored![dir] = { reason: 'fixture-marker' }; return; } const gitDir = gitDirOf(dir, new Set(list.map(d => d.name))); - if (gitDir) { inv.repos[dir] = remotes(gitDir); if (dir !== home) return; } + if (gitDir) { + const { network, local } = remotes(gitDir); + inv.repos[dir] = network; + if (!network.length && local.length) inv.ignored![dir] = { reason: 'local-remote', remote: local[0] }; + if (dir !== home) return; + } for (const d of list) { const p = path.join(dir, d.name); if (d.isFile()) { try { const s = fs.statSync(p); if (s.size >= BIG_BYTES) inv.big[p] = s.size; } catch { /* gone */ } continue; } @@ -145,11 +170,12 @@ export function sweepDiff(start: Inventory, end: Inventory): Found[] { * The end of a stage. With no start inventory, or one that stopped at its bounds, the sweep cannot tell a path the start * never reached from a new one, so what it finds is a `partial` note, never a violation. */ -export function sweepResult(start: Inventory | null, end: Inventory | null): { found: Found[]; verdict: SweepVerdict; partial?: string } { +export function sweepResult(start: Inventory | null, end: Inventory | null): { found: Found[]; ignored?: Ignored[]; verdict: SweepVerdict; partial?: string } { const found = start && end ? sweepDiff(start, end) : []; + const ignored = start && end ? Object.entries(end.ignored ?? {}).filter(([p]) => !(p in (start.ignored ?? {})) && !(p in start.repos)).map(([p, i]) => ({ path: p, ...i })) : []; const partial = !start ? 'no start inventory' : start.partial ?? end?.partial; const verdict = found.length && !start?.partial ? 'violation' : partial ? 'partial' : 'clean'; - return { found, verdict, ...(partial ? { partial } : {}) }; + return { found, ...(ignored.length ? { ignored } : {}), verdict, ...(partial ? { partial } : {}) }; } const q = (s: string) => `'${s.replace(/'/g, "'\\''")}'`; @@ -173,6 +199,15 @@ export function foundLines(stage: Stage, attempt: number, found: readonly Found[ }); } +/** One line per new directory the sweep did not count as a fetch, with the reason. */ +export function ignoredLines(stage: Stage, attempt: number, ignored: readonly Ignored[]): string[] { + return ignored.map(i => { + const remote = i.remote ? ` remote=${control(i.remote) ? JSON.stringify(i.remote) : i.remote}` : ''; + const why = i.reason === 'fixture-marker' ? `a test fixture root carrying ${FIXTURE_MARKER}` : 'its remote resolves to a local path through url..insteadOf'; + return `FETCH_IGNORED: stage=${stage} attempt=${attempt} path=${shownPath(i.path)} reason=${i.reason}${remote} (${why}, so not a fetch; docs/review-policy.md#what-readers-may-fetch)`; + }); +} + /** * A stage's pending rerun: while a path an earlier attempt of the stage found still exists, a reader could read it and * sweep clean, so no stage's start dispatches a reader (and the violation stays unresolved). Nothing is moved or deleted. diff --git a/scripts/harness-pins/scripts/lib/shard-engine.ts.json b/scripts/harness-pins/scripts/lib/shard-engine.ts.json index 84495fc10..7aafbeeb6 100644 --- a/scripts/harness-pins/scripts/lib/shard-engine.ts.json +++ b/scripts/harness-pins/scripts/lib/shard-engine.ts.json @@ -1 +1 @@ -{"blob":"dc714d7d897aed185c0ec3d2c92ad22d954039e5","since":6,"nonBehavioral":"createShardSandbox drops the Capy launch markers (GSTACK_SESSION_KIND, GSTACK_CAPY, GSTACK_EPHEMERAL, GSTACK_INSTALL_VERDICT) from shard environments; CI never sets them, so no case's input or verdict changes there, and on a Capy machine fixtures now see the session kind they set themselves instead of the installer's"} +{"blob":"8ee2872d038520d3d3d958d25fd18cf6609ffa69","since":6,"nonBehavioral":"createShardSandbox writes the .gstack-test-fixture marker into the shard state dir beside tmp/ (GST-1695); with it test-setup.ts keeps the shard TMPDIR exactly as main hands it, so no case sees a different temp layout, input or verdict, and tmp/ stays empty for the bootstrap retention scope"} diff --git a/scripts/lib/shard-engine.ts b/scripts/lib/shard-engine.ts index dc714d7d8..8ee2872d0 100644 --- a/scripts/lib/shard-engine.ts +++ b/scripts/lib/shard-engine.ts @@ -15,6 +15,7 @@ import { StringDecoder } from 'node:string_decoder'; import * as fs from 'node:fs'; import * as os from 'node:os'; import * as path from 'node:path'; +import { FIXTURE_MARKER } from '../../lib/fixture-marker'; const ROOT = path.resolve(import.meta.dir, '..', '..'); // Strict Bun-test output classification works around a Bun test runner bug @@ -568,6 +569,9 @@ export async function runShardChild(options: RunShardChildOptions): Promise /private/var) for lanes that compare paths. + * `stateDir` carries the test fixture marker (GST-1695): /ship's fetch sweep + * skips it, and test-setup.ts keeps the shard's TMPDIR instead of nesting its + * own root, so a case that compares its temp paths with `tmp` still matches. */ export function createShardSandbox( prefix: string, @@ -576,6 +580,7 @@ export function createShardSandbox( ): { stateDir: string; tmp: string; env: NodeJS.ProcessEnv } { const created = fs.mkdtempSync(path.join(os.tmpdir(), prefix)); const stateDir = options.realpath ? fs.realpathSync(created) : created; + fs.writeFileSync(path.join(stateDir, FIXTURE_MARKER), `gstack test shard sandbox (${prefix})\n`); const tmp = path.join(stateDir, 'tmp'); fs.mkdirSync(tmp); const env: NodeJS.ProcessEnv = { diff --git a/test-setup.ts b/test-setup.ts index 66e3d47f6..c170db1b9 100644 --- a/test-setup.ts +++ b/test-setup.ts @@ -15,9 +15,11 @@ * means new tests don't have to remember the dance, and the bug class * stays dead. */ -import { afterEach, beforeAll } from 'bun:test'; +import { afterAll, afterEach, beforeAll } from 'bun:test'; +import * as fs from 'node:fs'; import * as os from 'node:os'; import * as path from 'node:path'; +import { FIXTURE_MARKER } from './lib/fixture-marker'; import { CAPY_LAUNCH_MARKERS } from './scripts/lib/shard-engine'; // State-root hermeticity (docs/state-root.md). Tests isolate state with @@ -48,6 +50,35 @@ for (const name of ['GBRAIN_DATABASE_URL', 'GBRAIN_DIRECT_DATABASE_URL', 'DATABA delete process.env[name]; process.stderr.write(`test-setup: stripped inherited ${name}; tests must not reach a live gbrain\n`); } +// One fixture root per test process (GST-1695). TMPDIR (TMP and TEMP on Windows) +// points at it, so os.tmpdir() here, and $TMPDIR in a child spawned through +// node:child_process or with this process's env, put each fixture inside it; a +// Bun.spawn child given no env keeps the TMPDIR this process started with. It carries the fixture marker, so /ship's fetch sweep +// (lib/review-sweep.ts) never reads a concurrent or leftover run's fixture +// repositories as a reader's fetch, and this afterAll, which Bun runs once after +// the last file even when a test timed out before its own cleanup, removes it. +// Bun's test runner fires no process exit event; a killed run keeps the marker. +// A `bun test` whose TMPDIR already sits under a marked directory (an outer +// run's root, or a shard sandbox from scripts/lib/shard-engine.ts) keeps the +// TMPDIR it was handed: that marker covers it and its owner removes it. +const markedAncestor = (dir: string): string | null => { + for (let d = dir; ; d = path.dirname(d)) { + if (fs.existsSync(path.join(d, FIXTURE_MARKER))) return d; + if (path.dirname(d) === d) return null; + } +}; +const outerRoot = markedAncestor(os.tmpdir()); +if (outerRoot) process.env.GSTACK_TEST_TMP = outerRoot; +else { + const fixtureRoot = fs.mkdtempSync(path.join(os.tmpdir(), 'gst-')); + fs.writeFileSync(path.join(fixtureRoot, FIXTURE_MARKER), `gstack test fixture root of pid ${process.pid}\n`); + for (const name of ['GSTACK_TEST_TMP', 'TMPDIR', 'TMP', 'TEMP']) process.env[name] = fixtureRoot; + afterAll(() => { + try { fs.rmSync(fixtureRoot, { recursive: true, force: true, maxRetries: 3 }); } catch (err) { + process.stderr.write(`test-setup: could not remove fixture root ${fixtureRoot}: ${(err as Error).message}\n`); + } + }); +} process.env.GSTACK_TEST_LEGACY_ROOT = path.join(os.tmpdir(), `gstack-test-legacy-root-${process.pid}-${Date.now()}-unused`); // Workflow leases (bin/gstack-lease) default to the installation's git dir; // for the dev checkout under test that is this repo's .git, and a live test diff --git a/test/plan-count-file-permission.test.ts b/test/plan-count-file-permission.test.ts index 99b4065d2..41320c782 100644 --- a/test/plan-count-file-permission.test.ts +++ b/test/plan-count-file-permission.test.ts @@ -106,6 +106,7 @@ for (const variant of ['basic', 'intervening', 'cropped', 'same-basename', 'path : variant==='cropped' ? cropped.screen.replaceAll(path.dirname(cropped.hook.expected),path.dirname(expected)).replaceAll(path.basename(cropped.hook.expected),'report.md') : captured.screen.replaceAll(captured.expectedPath,expected).replace('../gstack-e2e-plan-ceo-paired-2Rv5Bi/gstack-test-plan-ceo-paired.md',expected).replaceAll('gstack-test-plan-ceo-paired.md','report.md'); if(variant==='same-basename')screen=screen.replaceAll(expected,'__ACTIVE_PLAN_PATH__').replaceAll('report.md','PLAN.md'); + screen=screen.split('\n').map(row=>{const cut=row.length>120?row.lastIndexOf(' ',120):-1;return cut>0?row.slice(0,cut)+'\n'+row.match(/^ */)![0]+row.slice(cut+1):row;}).join('\n'); fs.writeFileSync(fake,`#!${process.execPath}\n`+String.raw` import * as fs from 'node:fs';import * as path from 'node:path'; const item=JSON.parse(process.env.FILE_EPOCH_CASE);const log=e=>fs.appendFileSync(item.events,JSON.stringify(e)+'\n'); diff --git a/test/review-pass.test.ts b/test/review-pass.test.ts index f6e4503cc..f7f52a3e8 100644 --- a/test/review-pass.test.ts +++ b/test/review-pass.test.ts @@ -10,6 +10,7 @@ import * as path from 'node:path'; import { promptHash, sectionTexts, withWorkspace, workspaceParagraph, workspaceStep } from '../lib/review-brief'; import { CHUNK_CHARS, PARENT_ONLY_QA, buildManifest, chunkCount, chunkOf, markReader, renderStage, specialistFindings, type Manifest } from '../lib/review-pass'; import type { Dispatch } from '../lib/review-depth'; +import { FIXTURE_MARKER } from '../lib/fixture-marker'; const ROOT = path.resolve(import.meta.dir, '..'); const BIN = path.join(ROOT, 'bin'); @@ -1488,5 +1489,37 @@ describe('the fetch sweep quarantines a stage that fetched (GST-1322 PR B)', () const v = bin(dir, env, ['verify', '--ledger', m.ledger]); for (const text of [res, v.stdout, ...inventories, fs.readFileSync(m.ledger, 'utf8'), fs.readFileSync(path.join(out, 'pass-manifest.json'), 'utf8')]) expect(text).not.toMatch(/FAKETOKEN|ci-bot|bot:/); }, 240_000); + + // Value: protects=a test run's fixture repositories left beside a stage (a root carrying the fixture marker, ship-bin's clone whose https://github.com/fx/fx.git origin resolves locally) neither quarantine the stage nor block its rerun, while an unmarked network clone in the same stage still does both; + // fails_when=a reader's own `bun test` that timed out before cleanup, or a lane's background suite, quarantines the pass and FETCH_RERUN_BLOCKED stops it on the leftover fixture; why_new=GST-1695 (three /ship passes lost on 11-06/07 to /tmp/ship-bin-YUgvjd and a background suite's fixtures); + // seam=gstack-review-dispatch sweep --phase end and --phase start; no_claim=that a clone made inside a live fixture root is seen + test('leftover test fixtures are printed as ignored and never block the rerun; an unmarked network clone beside them still does', async () => { + const { dir, env } = repo(); + made.push(dir); + const out = path.join(dir, '.state/run/p1'); + const beam = path.join(dir, '.state/BEAM'); + const fixture = path.join(dir, '.state/gst-left'); + const shipBin = path.join(dir, '.state/ship-bin-left/work'); + const leftovers = () => { + clone(path.join(fixture, 'sweep-x', 'BEAM')); + fs.writeFileSync(path.join(fixture, FIXTURE_MARKER), ''); + clone(shipBin); + expect(spawnSync('git', ['config', `url.${path.join(dir, '.state/ship-bin-left/origin.git')}.insteadOf`, 'https://github.com/sealed-org/BEAM.git'], { cwd: shipBin, encoding: 'utf8', timeout: 30_000 }).status).toBe(0); + }; + const h = host(dir, env, { output: t => { if (t === 'ship f7p1 specialist-batched') { clone(beam); leftovers(); } return 'NO FINDINGS'; } }); + const input = { base: 'main', out, pass: 'f7p1', planFlags: ['--depth-floor', 'full'] }; + const blocked = await runPass(h, input); + const real = (p: string) => `'${fs.realpathSync(p)}'`; + expect(blocked).toContain(`FETCH_VIOLATION: stage=specialists attempt=1 kind=repo path=${real(beam)}`); + expect(blocked).toContain(`FETCH_IGNORED: stage=specialists attempt=1 path=${real(fixture)} reason=fixture-marker`); + expect(blocked).toContain(`FETCH_IGNORED: stage=specialists attempt=1 path=${real(shipBin)} reason=local-remote remote=https://github.com/sealed-org/BEAM.git`); + expect(blocked).toContain(`FETCH_RERUN_BLOCKED: stage=specialists attempt=2 path=${real(beam)}`); + expect(blocked).not.toMatch(/^FETCH_(VIOLATION|RERUN_BLOCKED): .*(gst-left|ship-bin-left)/m); + fs.rmSync(beam, { recursive: true, force: true }); + const res = await runPass(h, input); + expect(res).toContain('NEXT: judge (Phase 4)'); + expect(res).not.toContain('FETCH_RERUN_BLOCKED'); + expect([fs.existsSync(fixture), fs.existsSync(shipBin)]).toEqual([true, true]); + }, 240_000); }); const briefDirOf = (h: ReturnType, title: string) => briefDir(h.creates.find(c => c.title === title)?.prompt ?? ''); diff --git a/test/review-sweep.test.ts b/test/review-sweep.test.ts index eb9ab53f8..609179185 100644 --- a/test/review-sweep.test.ts +++ b/test/review-sweep.test.ts @@ -6,7 +6,8 @@ import { spawnSync } from 'node:child_process'; import * as fs from 'node:fs'; import * as os from 'node:os'; import * as path from 'node:path'; -import { BIG_BYTES, foundLines, inventory, networkRemote, quarantineStage, redactUrl, sweepDiff, sweepResult } from '../lib/review-sweep'; +import { BIG_BYTES, foundLines, ignoredLines, inventory, networkRemote, quarantineStage, redactUrl, sweepDiff, sweepResult } from '../lib/review-sweep'; +import { FIXTURE_MARKER } from '../lib/fixture-marker'; import { collect, ledgerBlockers } from '../lib/review-ledger'; import { ledgerPass } from '../lib/review-scope'; import type { Manifest } from '../lib/review-pass'; @@ -68,6 +69,44 @@ describe('review-sweep inventory diff', () => { expect(lines[0]).toMatch(/^FETCH_VIOLATION: stage=specialists attempt=1 kind=repo .* readers=specialist\/security \(attributed to the stage/); }); + // Value: protects=a test fixture root carrying the marker, or a repository whose network remote resolves to a local path through url..insteadOf, is never a fetch and is printed as ignored, while an unmarked network clone beside them, and a local remote that insteadOf sends to the network, still are; + // fails_when=a concurrent or leftover `bun test` (ship-bin's https://github.com/fx/fx.git, a sweep fixture's sealed-org remote) quarantines a stage and blocks its rerun, a marker on a sweep root hides the whole root, or insteadOf hides a network remote; + // why_new=GST-1695 (FETCH_RERUN_BLOCKED on /tmp/ship-bin-YUgvjd cost three /ship passes on 11-06/07); seam=inventory, sweepResult, ignoredLines; no_claim=that a clone made inside a live fixture root is seen + test('a marked fixture root and an insteadOf-local repository are ignored and printed; unmarked and insteadOf-network clones are still found', async () => { + const root = tmp(); + const checkout = path.join(root, 'checkout'); + repoAt(checkout); + fs.writeFileSync(path.join(root, FIXTURE_MARKER), ''); + const start = inventory([root], checkout, []); + await Bun.sleep(30); + const fixture = path.join(root, 'gst-left'); + fs.mkdirSync(fixture); + fs.writeFileSync(path.join(fixture, FIXTURE_MARKER), ''); + repoAt(path.join(fixture, 'sweep-x', 'BEAM'), 'https://github.com/sealed-org/BEAM.git'); + const config = (dir: string, key: string, value: string) => expect(spawnSync('git', ['config', key, value], { cwd: dir, encoding: 'utf8', timeout: 30_000 }).status).toBe(0); + const shipBin = path.join(root, 'ship-bin-left', 'work'); + repoAt(shipBin, 'https://github.com/fx/fx.git'); + config(shipBin, `url.${path.join(root, 'ship-bin-left', 'origin.git')}.insteadOf`, 'https://github.com/fx/fx.git'); + repoAt(path.join(root, 'mirror'), 'https://github.com/fx/fx.git'); + config(path.join(root, 'mirror'), 'url.https://example.com/m/.insteadOf', 'https://github.com/'); + repoAt(path.join(root, 'fromlocal'), '/srv/app.git'); + config(path.join(root, 'fromlocal'), 'url.https://github.com/x/.insteadOf', '/srv/'); + repoAt(path.join(root, 'BEAM'), 'https://github.com/sealed-org/BEAM.git'); + const r = sweepResult(start, inventory([root], checkout, [])); + expect(r.verdict).toBe('violation'); + expect(r.found.map(f => [path.relative(root, f.path), f.remote]).sort()).toEqual([ + ['BEAM', 'https://github.com/sealed-org/BEAM.git'], ['fromlocal', 'https://github.com/x/app.git'], ['mirror', 'https://example.com/m/fx/fx.git'], + ]); + expect((r.ignored ?? []).map(i => [path.relative(root, i.path), i.reason, i.remote]).sort()).toEqual([ + ['gst-left', 'fixture-marker', undefined], [path.join('ship-bin-left', 'work'), 'local-remote', 'https://github.com/fx/fx.git'], + ]); + const lines = ignoredLines('specialists', 1, r.ignored ?? []); + expect(lines.sort()).toEqual([ + expect.stringMatching(new RegExp(`^FETCH_IGNORED: stage=specialists attempt=1 path='${fixture}' reason=fixture-marker \\(`)), + expect.stringMatching(new RegExp(`^FETCH_IGNORED: stage=specialists attempt=1 path='${shipBin}' reason=local-remote remote=https://github.com/fx/fx.git \\(`)), + ]); + }); + // Value: protects=a start walk that stopped at its bounds cannot tell a repository it never reached from a new one, so what the end walk finds past it is a note, never a quarantine; // fails_when=a partial start turns pre-existing repositories into FETCH_VIOLATION and FETCH_RERUN_SPENT on runs where nobody fetched; why_new=ship-c-p1 specialist-batched#1 (CRITICAL); seam=sweepResult; no_claim=that a partial sweep sees every fetch test('a partial start inventory makes what the end walk finds a partial note, not a violation', () => { diff --git a/test/shard-engine.test.ts b/test/shard-engine.test.ts index 60c99de61..d501fb24d 100644 --- a/test/shard-engine.test.ts +++ b/test/shard-engine.test.ts @@ -6,10 +6,11 @@ * run-shard-child and strict-output-* tests pin the moved primitives. */ import { afterEach, describe, expect, spyOn, test } from 'bun:test'; -import type { ChildProcess } from 'node:child_process'; +import { spawnSync, type ChildProcess } from 'node:child_process'; import * as fs from 'node:fs'; import * as os from 'node:os'; import * as path from 'node:path'; +import { FIXTURE_MARKER } from '../lib/fixture-marker'; import { cliUsage, BunTestOutputClassifier, @@ -273,6 +274,34 @@ describe('shard engine: capture and isolation', () => { } }); + // Value: protects=a shard's state dir carries the test fixture marker beside an empty tmp/, and a bun test run in that sandbox keeps the shard's TMPDIR as handed instead of nesting its own root; + // fails_when=the preload nests gst-XXXXXX under the shard's TMPDIR, so a case that compares its mkdtemp parent with the runner's temp dir fails (qa-bootstrap's 'wrong bootstrap root or run' on screen run 42333856900), + // or the marker lands inside tmp/ and breaks the retention scope's empty-dir check; why_new=GST-1695 screen red; seam=createShardSandbox, test-setup.ts; no_claim=that a test which sets its own TMPDIR is marked + test('a shard sandbox is marked beside its tmp, and bun test inside it keeps the shard TMPDIR', () => { + const outside = path.dirname(process.env.GSTACK_TEST_TMP ?? os.tmpdir()); + const inherited = process.env.TMPDIR; + process.env.TMPDIR = outside; + let box: ReturnType; + try { box = createShardSandbox('gstack-unit-shard-', { ...process.env }, { realpath: true }); } finally { process.env.TMPDIR = inherited; } + try { + expect(fs.statSync(path.join(box.stateDir, FIXTURE_MARKER)).isFile()).toBe(true); + expect(fs.readdirSync(box.tmp)).toEqual([]); + const work = fs.mkdtempSync(path.join(box.stateDir, 'probe-')); + const report = path.join(work, 'report.json'); + const file = path.join(work, 'probe.test.ts'); + fs.writeFileSync(file, [ + "import { test } from 'bun:test';", + "import * as fs from 'node:fs';", + "import * as os from 'node:os';", + `test('reports', () => { fs.writeFileSync(${JSON.stringify(report)}, JSON.stringify({ tmp: os.tmpdir(), root: process.env.GSTACK_TEST_TMP })); });`, + ].join('\n')); + const r = spawnSync(process.execPath, ['test', file], { cwd: path.resolve(import.meta.dir, '..'), encoding: 'utf8', timeout: 60_000, env: box.env }); + expect(r.status, r.stderr).toBe(0); + expect(JSON.parse(fs.readFileSync(report, 'utf8'))).toEqual({ tmp: box.tmp, root: box.stateDir }); + expect(fs.readdirSync(box.tmp)).toEqual([]); + } finally { fs.rmSync(box.stateDir, { recursive: true, force: true }); } + }); + test('flag loop: declared handlers consume values; anything else is Unknown argument', () => { const seen: Array<[string, string | undefined]> = []; parseCliFlags(['--on', '--value', 'x', '--value'], { diff --git a/test/test-fixture-root.test.ts b/test/test-fixture-root.test.ts new file mode 100644 index 000000000..9cdc36f6d --- /dev/null +++ b/test/test-fixture-root.test.ts @@ -0,0 +1,78 @@ +/** + * GST-1695: every temp path a test makes lands under one per-process root that carries the fixture marker + * (lib/fixture-marker.ts), and test-setup.ts removes that root after the last file, even when a test timed out + * before its own cleanup ran. + */ +import { describe, expect, test } from 'bun:test'; +import { spawnSync } from 'node:child_process'; +import * as fs from 'node:fs'; +import * as os from 'node:os'; +import * as path from 'node:path'; +import { FIXTURE_MARKER } from '../lib/fixture-marker'; +import { shipFixture } from './helpers/ship-bin-fixture'; + +const ROOT = path.resolve(import.meta.dir, '..'); + +describe('the per-process test fixture root (GST-1695)', () => { + // Value: protects=os.tmpdir() in a test process, and so every fixture helper that makes a git repository there (ship-bin's fake https://github.com/fx/fx.git origin included), resolves inside one root that carries the fixture marker; + // fails_when=fixtures land straight in the OS temp dir with nothing marking them, so /ship's fetch sweep reads a concurrent test run's repositories as a reader's fetch; why_new=GST-1695 (FETCH_RERUN_BLOCKED on /tmp/ship-bin-YUgvjd); seam=test-setup.ts; no_claim=that a test which sets its own TMPDIR stays inside the root + test('os.tmpdir() and a ship-bin fixture live under the marked root (this process\'s, or the shard sandbox\'s)', () => { + const root = process.env.GSTACK_TEST_TMP ?? ''; + expect(root).not.toBe(''); + expect(path.relative(root, os.tmpdir()).startsWith('..')).toBe(false); + expect(fs.statSync(path.join(root, FIXTURE_MARKER)).isFile()).toBe(true); + const fx = shipFixture(); + try { + expect(path.relative(fs.realpathSync(root), fx.root).startsWith('..')).toBe(false); + } finally { fx.cleanup(); } + }); + + // Value: protects=a fixture whose test hit its timeout before its cleanup line still leaves nothing behind once the run ends; + // fails_when=Bun's timeout skips the fixture's cleanup and its git repository stays in the temp dir for the next /ship sweep; why_new=GST-1695 (a reader's own `bun test` hit Bun's 5 s timeout and left /tmp/ship-bin-YUgvjd); + // seam=test-setup.ts afterAll; no_claim=that a SIGKILLed run cleans up (its root keeps the marker instead) + test('a child bun test whose fixture test timed out before cleanup leaves no fixture behind', () => { + const work = fs.mkdtempSync(path.join(path.dirname(process.env.GSTACK_TEST_TMP ?? os.tmpdir()), 'fixture-root-')); + try { + const report = path.join(work, 'report.json'); + const file = path.join(work, 'leftover.test.ts'); + fs.writeFileSync(file, [ + "import { test } from 'bun:test';", + "import { spawnSync } from 'node:child_process';", + "import * as fs from 'node:fs';", + "import * as os from 'node:os';", + "import * as path from 'node:path';", + "test('times out before its cleanup', async () => {", + " const repo = fs.mkdtempSync(path.join(os.tmpdir(), 'ship-bin-'));", + " spawnSync('git', ['init', '-q', repo], { timeout: 30_000 });", + " const root = process.env.GSTACK_TEST_TMP ?? '';", + ` fs.writeFileSync(${JSON.stringify(report)}, JSON.stringify({ root, repo, git: fs.existsSync(path.join(repo, '.git')), marked: root !== '' && fs.existsSync(path.join(root, ${JSON.stringify(FIXTURE_MARKER)})) }));`, + ' await Bun.sleep(5_000);', + ' fs.rmSync(repo, { recursive: true, force: true });', + '}, 200);', + ].join('\n')); + const r = spawnSync(process.execPath, ['test', file], { cwd: ROOT, encoding: 'utf8', timeout: 60_000, env: { ...process.env, TMPDIR: work, TMP: work, TEMP: work } }); + expect(r.stderr).toContain('timed out after 200ms'); + const seen = JSON.parse(fs.readFileSync(report, 'utf8')) as { root: string; repo: string; git: boolean; marked: boolean }; + expect([seen.git, seen.marked, seen.root !== '' && seen.repo.startsWith(seen.root + path.sep)]).toEqual([true, true, true]); + expect([fs.existsSync(seen.repo), fs.existsSync(seen.root)]).toEqual([false, false]); + } finally { fs.rmSync(work, { recursive: true, force: true }); } + }); + + // Value: protects=a `bun test` a test spawns with a TMPDIR inside this run's root sees exactly that TMPDIR, so fixtures that compare a child's os.tmpdir() with the dir they handed it keep working; + // fails_when=the child's preload nests a second root and moves its TMPDIR (the plan-design UI fixtures' registration tests failed this way); why_new=GST-1695 test:quick on the first draft; seam=test-setup.ts; no_claim=none + test('a child bun test handed a TMPDIR inside this run\'s root keeps it', () => { + const work = fs.mkdtempSync(path.join(os.tmpdir(), 'nested-')); + const report = path.join(work, 'report.json'); + const file = path.join(work, 'nested.test.ts'); + fs.writeFileSync(file, [ + "import { test } from 'bun:test';", + "import * as fs from 'node:fs';", + "import * as os from 'node:os';", + `test('reports', () => { fs.writeFileSync(${JSON.stringify(report)}, JSON.stringify({ tmp: os.tmpdir(), root: process.env.GSTACK_TEST_TMP })); });`, + ].join('\n')); + const r = spawnSync(process.execPath, ['test', file], { cwd: ROOT, encoding: 'utf8', timeout: 60_000, env: { ...process.env, TMPDIR: work, TMP: work, TEMP: work } }); + expect(r.status, r.stderr).toBe(0); + expect(JSON.parse(fs.readFileSync(report, 'utf8'))).toEqual({ tmp: work, root: process.env.GSTACK_TEST_TMP }); + expect(fs.readdirSync(work).sort()).toEqual(['nested.test.ts', 'report.json']); + }); +}); diff --git a/test/test-free-shards.test.ts b/test/test-free-shards.test.ts index 265f0b75f..2ef262197 100644 --- a/test/test-free-shards.test.ts +++ b/test/test-free-shards.test.ts @@ -199,7 +199,7 @@ describe('test-free-shards: exclusive host-state phase', () => { let child: ReturnType | undefined; let watchdog: ReturnType | undefined; try { - for (const file of ['scripts/test-free-shards.ts', 'scripts/test-strict-output.ts', 'scripts/lib/shard-engine.ts', 'scripts/lib/free-home-guard.ts', 'scripts/lib/free-ci-health.ts', 'scripts/lib/windows-curation.ts', 'scripts/lib/shard-jobs.ts', 'lib/state-root.ts', + for (const file of ['scripts/test-free-shards.ts', 'scripts/test-strict-output.ts', 'scripts/lib/shard-engine.ts', 'scripts/lib/free-home-guard.ts', 'scripts/lib/free-ci-health.ts', 'scripts/lib/windows-curation.ts', 'scripts/lib/shard-jobs.ts', 'lib/state-root.ts', 'lib/fixture-marker.ts', 'test/helpers/paid-test-set.ts', 'test/helpers/touchfiles.ts', 'test/helpers/touchfiles-data.ts', 'test/helpers/test-selection.ts']) { const target = path.join(directory, file); fs.mkdirSync(path.dirname(target), { recursive: true }); diff --git a/test/ubicloud-pregate-remote.test.ts b/test/ubicloud-pregate-remote.test.ts index 82709c95c..be47dcdc2 100644 --- a/test/ubicloud-pregate-remote.test.ts +++ b/test/ubicloud-pregate-remote.test.ts @@ -20,7 +20,7 @@ afterAll(() => fs.rmSync(SCRATCH, { recursive: true, force: true })); const COPIED = ['scripts/ubicloud/pregate-remote.sh', 'bin/gstack-wtree', 'bin/gstack-path-spelling.sh', 'scripts/test-free-shards.ts', 'scripts/test-strict-output.ts', 'scripts/lib/shard-engine.ts', 'scripts/lib/free-home-guard.ts', - 'scripts/lib/free-ci-health.ts', 'scripts/lib/windows-curation.ts', 'scripts/lib/shard-jobs.ts', 'lib/state-root.ts', 'test/helpers/paid-test-set.ts', + 'scripts/lib/free-ci-health.ts', 'scripts/lib/windows-curation.ts', 'scripts/lib/shard-jobs.ts', 'lib/state-root.ts', 'lib/fixture-marker.ts', 'test/helpers/paid-test-set.ts', 'test/helpers/touchfiles.ts', 'test/helpers/touchfiles-data.ts', 'test/helpers/test-selection.ts']; const TESTS: Record = { 'test/alpha.test.ts': "import { expect, test } from 'bun:test';\ntest('alpha', () => { expect(1).toBe(1); });\n",