From 068087f33c91ca0c9aecded34da849cfe13f2d7c Mon Sep 17 00:00:00 2001 From: Garry Tan Date: Wed, 4 Nov 2026 00:14:07 -0800 Subject: [PATCH] fix(ci-carry): a per_file remainder file that skips here too is not owed (GST-1314) (#3985) * fix(ci-carry): a per_file remainder file that skips here too is not owed (GST-1314); anchor per_file paths, --no-env-file, declare gstack's per_file runner Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> * chore: name the changelog fragment after the branch Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> * fix(evidence): a truncated per_file log never counts a remainder file not owed (GST-1314) Review-Finding: p1b/outside-voice#1 reader=outside-voice Review-Finding: p1b/red-team#3 reader=red-team Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> * docs(changelog): GST-1314 fragment names the truncated-log refusal Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> * docs(pregate): a per_file log cut at 2 MB still refuses (GST-1314) Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> --------- Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> --- .gstack/pregate.json | 2 +- bin/gstack-evidence | 39 ++++++++++++++----- changelog.d/gsta258-f-carry-skip-here.md | 10 +++++ docs/pregate.md | 30 +++++++++------ lib/ci-carry.ts | 8 ++-- test/ship-ci-carry.test.ts | 49 ++++++++++++++++++++++-- 6 files changed, 108 insertions(+), 30 deletions(-) create mode 100644 changelog.d/gsta258-f-carry-skip-here.md diff --git a/.gstack/pregate.json b/.gstack/pregate.json index 6f86797c9..14f5f74c6 100644 --- a/.gstack/pregate.json +++ b/.gstack/pregate.json @@ -29,7 +29,7 @@ "skipped": "bun --no-env-file --config=/dev/null -e 'import { isPaidTestFile } from \"./test/helpers/paid-test-set.ts\"; import * as fs from \"node:fs\"; for (const f of fs.readFileSync(process.env.PREGATE_FILES_FILE, \"utf8\").split(/\\r?\\n/)) if (f && isPaidTestFile(f)) console.log(f);'" }, "ship_lanes_from": { - "bun run test": { "profiles": ["ci-reuse/free-tests"], "inventory": "bun --no-env-file --config=/dev/null run scripts/test-free-shards.ts --list | sed -n 's/^ //p'" } + "bun run test": { "profiles": ["ci-reuse/free-tests"], "inventory": "bun --no-env-file --config=/dev/null run scripts/test-free-shards.ts --list | sed -n 's/^ //p'", "per_file": "bun --no-env-file test --timeout=30000 {files}" } }, "warn": [], "guards": [ diff --git a/bin/gstack-evidence b/bin/gstack-evidence index 54d0acd73..084f6de83 100755 --- a/bin/gstack-evidence +++ b/bin/gstack-evidence @@ -138,7 +138,8 @@ interface EvidenceRecord { tests_unit?: "files"; /** Why a carried lane was not owed at all: every job it names was path-skipped by an unchanged gate (record-ci, GST-1190). */ not_owed?: string; - skipped_here?: { label: string; files: string[]; tests_ran: number | "unknown" }; + /** The per_file run of the inventory files CI skipped entirely; not_owed: the ones that skipped every test here too (GST-1314). */ + skipped_here?: { label: string; files: string[]; tests_ran: number | "unknown"; not_owed?: string[] }; } /** Bytes of streamed output kept for the tests_ran parse (the 2MB log would truncate the summary). */ @@ -184,6 +185,15 @@ const ANSI = /\u001B\[[0-?]*[ -/]*[@-~]/g; const strip = (l: string) => l.replace(ANSI, "").replace(/\r$/, ""); const count = (text: string, word: string): number => Number(new RegExp(`(\\d+) ${word}\\b`).exec(text)?.[1] ?? 0); +/** The tests bun's last terminal summary in a run's log registered, skips included ("Ran N tests across M files"); 0 when the log or the summary is missing, or the log is incomplete (capped at LOG_MAX_BYTES or cut by a write failure), since its last summary need not be the run's. */ +function registeredBunTests(logPath: string | undefined): number { + let text = ""; + try { text = logPath ? readFileSync(logPath, "utf8") : ""; } catch { return 0; } + if (/^\[gstack-evidence: log (truncated at 2MB|ended early)/m.test(text)) return 0; + const last = text.split("\n").map(strip).findLast((l) => /^Ran \d+ tests? across \d+ files?\. \[/.test(l)); + return Number(/^Ran (\d+)/.exec(last ?? "")?.[1] ?? 0); +} + /** * Bun: the last "Ran N tests across M files" block; executed = its " N pass" + * " N fail" lines. When that block ends a `bun test --shard=i/N` process (bun's @@ -733,14 +743,23 @@ async function cmdRecordCi(argv: string[]): Promise { let here: EvidenceRecord["skipped_here"]; if (v.remainder?.length) { const hereLabel = `${label}.skipped`; - console.error(`gstack-evidence record-ci: ${v.remainder.length} inventory file(s) skipped every test in CI; running them here (label=${hereLabel})`); - const hereCommand = perFileCommand(m.per_file!, v.remainder); - const exit = await cmdRun(["--label", hereLabel, "--", hereCommand]); - if (exit !== 0) return none("skipped_failed", `the per_file run of ${v.remainder.length} skipped file(s) exited ${exit} (label=${hereLabel})`); - let rows: EvidenceRecord[] = []; - try { rows = readJsonl(ledgerPath().file); } catch { rows = []; } - here = { label: hereLabel, files: v.remainder, tests_ran: rows.findLast((r) => r.label === hereLabel && r.source === undefined && r.cmd_sha256 === sha256(hereCommand) && r.wtree === tree)?.tests_ran ?? "unknown" }; - if (typeof here.tests_ran !== "number" || here.tests_ran <= 0) return none("skipped_zero_run", `the per_file run of ${v.remainder.length} skipped file(s) ran ${here.tests_ran} tests here (label=${hereLabel}); a lane passes on its runner count, never exit 0 alone`); + console.error(`gstack-evidence record-ci: ${v.remainder.length} inventory file(s) skipped every test in CI; running them here one at a time (label=${hereLabel})`); + let ran = 0; + const notOwed: string[] = []; + for (const file of v.remainder) { + const hereCommand = perFileCommand(m.per_file!, [file]); + const exit = await cmdRun(["--label", hereLabel, "--", hereCommand]); + if (exit !== 0) return none("skipped_failed", `the per_file run of ${file} exited ${exit} (label=${hereLabel})`); + let rows: EvidenceRecord[] = []; + try { rows = readJsonl(ledgerPath().file); } catch { rows = []; } + const row = rows.findLast((r) => r.label === hereLabel && r.source === undefined && r.cmd_sha256 === sha256(hereCommand) && r.wtree === tree); + const n = row?.tests_ran ?? "unknown"; + if (typeof n === "number" && n > 0) { ran += n; continue; } + if (n === 0 && registeredBunTests(row?.log_path) > 0) { notOwed.push(file); continue; } + return none("skipped_zero_run", `the per_file run of ${file} ran ${n} tests here and registered none it skipped (label=${hereLabel}); a lane passes on its runner count, never exit 0 alone`); + } + here = { label: hereLabel, files: v.remainder, tests_ran: ran, ...(notOwed.length ? { not_owed: notOwed } : {}) }; + if (notOwed.length) console.error(`gstack-evidence record-ci: not owed here: ${notOwed.length} remainder file(s) skipped every test on CI and here too (env- or tool-gated by design): ${notOwed.join(", ")}`); for (const e of v.runs) { const changed = runChanged(check.deps, e.run_id, e.run_attempt); if (changed) return none("run_changed", changed); @@ -761,7 +780,7 @@ async function cmdRecordCi(argv: string[]): Promise { commit: head, tree, dirty: false, wtree: tree, tests_ran: v.tests, source: "ci", runs, profiles: m.profiles, policy: loaded.label, ...(m.jobs ? { jobs: m.jobs } : {}), ...(v.unit ? { tests_unit: v.unit } : {}), ...(v.notOwed ? { not_owed: v.notOwed } : {}), ...(here ? { skipped_here: here } : {}), }; appendJsonl(paths.file, rec, { mode: 0o600 }); - console.error(`gstack-evidence: recorded label=${label} exit=0 tests_ran=${v.tests}${v.unit ? ` unit=${v.unit}` : ""} log=- source=ci runs=${runs.join(",")} policy=${loaded.label}${here ? ` skipped_here=${here.files.length} tests_ran_here=${here.tests_ran}` : ""}`); + console.error(`gstack-evidence: recorded label=${label} exit=0 tests_ran=${v.tests}${v.unit ? ` unit=${v.unit}` : ""} log=- source=ci runs=${runs.join(",")} policy=${loaded.label}${here ? ` skipped_here=${here.files.length} tests_ran_here=${here.tests_ran}${here.not_owed ? ` not_owed_here=${here.not_owed.length}` : ""}` : ""}`); if (v.notOwed) console.error(`gstack-evidence: not owed: ${v.notOwed}`); return 0; } diff --git a/changelog.d/gsta258-f-carry-skip-here.md b/changelog.d/gsta258-f-carry-skip-here.md new file mode 100644 index 000000000..81d40de00 --- /dev/null +++ b/changelog.d/gsta258-f-carry-skip-here.md @@ -0,0 +1,10 @@ +**CI carry no longer refuses a skipped file that skips here too (GST-1314).** When CI skipped every test in a file, `gstack-evidence record-ci` reruns it here through the lane's `per_file` runner, and a file that skipped everything again ended the carry with `skipped_zero_run`, so the whole free suite reran. Free Tests run 41754278952 had six such files, all env-gated or tool-gated by design (0 pass, 20 skip on a Capy machine), so every gstack /ship carry would have fallen back to the full lane. + +### Fixed +- **`gstack-evidence record-ci`:** runs the per_file remainder one file at a time. A file whose Bun summary registered tests and skipped every one here too is recorded as not owed (`skipped_here.not_owed` in the CI row, `not_owed_here=` on the `LANE:` line) instead of refusing. A file that registers no test here still refuses with `skipped_zero_run`, even beside a file that ran, and so does a file whose log hit the 2 MB cap, since a cut log's last summary may not be the run's. A failing file's `skipped_failed` names it. +- **`per_file` paths** reach the runner anchored with `./`, because Bun reads a bare relative path as a substring filter (`test/x.test.ts` also matched `browse/test/x.test.ts`). +- **gstack's `bun run test` lane** declares `"per_file": "bun --no-env-file test --timeout=30000 {files}"`, so the runner is on main before the Free Tests writer starts listing skipped files (#3979). `--no-env-file` keeps a local `.env` from switching an env-gated file on. +- **Docs:** `docs/pregate.md#skipped-files` describes the per-file runs, the not-owed case and the anchoring. + +### Tests +- `test/ship-ci-carry.test.ts`: run 41754278952's six remainder files, each printing its own skip-only Bun summary, carry with `not_owed_here=6` and reach the runner as `./`; a remainder that mixes a file that ran with one that registers no test refuses with `skipped_zero_run` naming that file; a remainder file that registers no test refuses; a remainder file whose log passed 2 MB after a nested `Ran 5 tests` summary refuses instead of counting as not owed. diff --git a/docs/pregate.md b/docs/pregate.md index c74e9a176..80378a859 100644 --- a/docs/pregate.md +++ b/docs/pregate.md @@ -775,20 +775,28 @@ once: them here. A /ship lane or guard whose inventory names one carries the covered files and runs only the skipped ones here, through the runner its `ship_lanes_from` entry declares as `per_file` (a command with `{files}`, - such as `"per_file": "bun test --timeout=30000 {files}"`, the same form as the - tests stage's `runner`). That run is recorded under `