From 27883994a55dbfb9c140647a9d7430cddea93aad Mon Sep 17 00:00:00 2001 From: garrytan Date: Tue, 3 Nov 2026 07:30:44 +0000 Subject: [PATCH] ci-carry: read record-ci from its own last record line; the remainder's count must come from this run's row (l6c-p2 red-team) Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> --- bin/gstack-evidence | 5 +++-- lib/ci-carry.ts | 16 ++++++++++++++++ lib/pregate/checks/guards.ts | 9 ++++----- lib/ship-bin-common.ts | 10 ++++------ test/ship-ci-carry.test.ts | 7 +++++++ 5 files changed, 34 insertions(+), 13 deletions(-) diff --git a/bin/gstack-evidence b/bin/gstack-evidence index 48ba452d7..3b05a94be 100755 --- a/bin/gstack-evidence +++ b/bin/gstack-evidence @@ -705,12 +705,13 @@ async function cmdRecordCi(argv: string[]): Promise { 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 exit = await cmdRun(["--label", hereLabel, "--", perFileCommand(m.per_file!, v.remainder)]); + 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})`); if (wtree() !== tree) return none("tree_changed", `the working tree changed while the per_file run of label=${hereLabel} ran`); 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)?.tests_ran ?? "unknown" }; + 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`); for (const e of v.runs) { const changed = runChanged(check.deps, e.run_id, e.run_attempt); diff --git a/lib/ci-carry.ts b/lib/ci-carry.ts index 43c9ea237..c258f5ec4 100644 --- a/lib/ci-carry.ts +++ b/lib/ci-carry.ts @@ -153,6 +153,22 @@ export function laneMapping(from: Record, command: stri export const mappingCoverage = (m: ShipLaneMapping, cwd: string): { inventory?: () => string[] | { error: string }; jobs?: string[]; perFile?: boolean } => m.jobs ? { jobs: m.jobs } : { inventory: () => runInventory(m.inventory!, cwd), perFile: !!m.per_file }; +/** + * What `gstack-evidence record-ci` reported, read from its own last record line onward (the per_file remainder's output comes + * first, so a test printing look-alike lines cannot change the reading): the carried runs, count, unit and remainder, any + * not-owed reason, or the last CARRY_NONE reason. + */ +export function recordCiResult(stderr: string): { runs?: string; tests?: string; files?: boolean; notOwed?: string; here?: string; none?: string } { + const lines = stderr.split('\n'); + const at = lines.findLastIndex(x => x.startsWith('gstack-evidence: recorded label=') && x.includes(' log=- source=ci runs=')); + const none = lines.findLast(x => x.startsWith('CARRY_NONE: '))?.slice('CARRY_NONE: '.length); + if (at < 0) return none ? { none } : {}; + const got = / tests_ran=(\d+|n\/a)( unit=files)? log=- source=ci runs=(\S+)/.exec(lines[at]!); + const here = / skipped_here=(\d+) tests_ran_here=(\S+)$/.exec(lines[at]!); + const notOwed = lines.slice(at + 1).find(x => x.startsWith('gstack-evidence: not owed: '))?.slice('gstack-evidence: not owed: '.length); + return { runs: got?.[3], tests: got?.[1], files: !!got?.[2], ...(notOwed ? { notOwed } : {}), ...(here ? { here: `skipped_here=${here[1]} file(s) tests_ran_here=${here[2]}` } : {}) }; +} + /** The per_file runner command for `files`: `{files}` replaced by the paths, each single-quoted for bash. */ export const perFileCommand = (template: string, files: string[]): string => template.replaceAll('{files}', files.map(f => `'${f.replace(/'/g, `'\\''`)}'`).join(' ')); diff --git a/lib/pregate/checks/guards.ts b/lib/pregate/checks/guards.ts index 6349db332..c8d40feb0 100644 --- a/lib/pregate/checks/guards.ts +++ b/lib/pregate/checks/guards.ts @@ -21,7 +21,7 @@ */ import * as fs from 'node:fs'; import * as path from 'node:path'; -import { laneMapping } from '../../ci-carry'; +import { laneMapping, recordCiResult } from '../../ci-carry'; import { matchesAny } from '../../ship-policy'; import type { Guard } from '../config'; import { baseInputs, sha256 } from '../context'; @@ -46,10 +46,9 @@ function priorPass(ctx: PregateContext, guard: Guard, env: NodeJS.ProcessEnv): { let carryNone: string | undefined; if (laneMapping(ctx.config.ship_lanes_from, guard.command)) { const ci = evidence(['record-ci', '--label', `guard-${guard.id}`, '--base', ctx.base, '--', guard.command]); - const runs = / source=ci runs=(\S+)/.exec(String(ci.stderr ?? ''))?.[1]; - const notOwed = /^gstack-evidence: not owed: (.+)$/m.exec(String(ci.stderr ?? ''))?.[1]; - if (ci.status === 0 && runs) return { prior: notOwed ? `not owed: ${notOwed} runs=${runs}` : `carried runs=${runs}` }; - carryNone = /^CARRY_NONE: (.+)$/m.exec(String(ci.stderr ?? ''))?.[1] ?? `record-ci exit ${ci.status}`; + const got = recordCiResult(String(ci.stderr ?? '')); + if (ci.status === 0 && got.runs) return { prior: got.notOwed ? `not owed: ${got.notOwed} runs=${got.runs}` : `carried runs=${got.runs}` }; + carryNone = got.none ?? `record-ci exit ${ci.status}`; } const local = evidence(['passed', '--', guard.command]); const hit = /^PASSED: (.+)$/m.exec(String(local.stdout ?? ''))?.[1]; diff --git a/lib/ship-bin-common.ts b/lib/ship-bin-common.ts index 185fb91ea..2335d79e0 100644 --- a/lib/ship-bin-common.ts +++ b/lib/ship-bin-common.ts @@ -10,7 +10,7 @@ import * as fs from 'node:fs'; import * as os from 'node:os'; import * as path from 'node:path'; import { fragmentSlug } from './changelog-fragments'; -import { laneMapping, type ShipLaneMapping } from './ci-carry'; +import { laneMapping, recordCiResult, type ShipLaneMapping } from './ci-carry'; import { isConfigUnavailable, loadConfigFromBase } from './pregate/config'; import { parseUnifiedDiff, versionAtRef } from './restamp'; import { branchSides, stampOnlyLine } from './stamp-only'; @@ -190,11 +190,9 @@ export function mappedLanes(cwd: string, base: string, list: { label: string; co */ export function carryLane(cwd: string, base: string, l: { label: string; command: string }): LaneResult { const rec = bin(cwd, 'gstack-evidence', ['record-ci', '--label', l.label, '--base', base, '--', l.command], 600_000); - const got = / tests_ran=(\d+|n\/a)( unit=files)? log=- source=ci runs=(\S+)/.exec(rec.stderr); - const notOwed = /^gstack-evidence: not owed: (.+)$/m.exec(rec.stderr)?.[1]; - const here = / skipped_here=(\d+) tests_ran_here=(\S+)$/m.exec(rec.stderr); - if (rec.status === 0 && got) return { label: l.label, exit: 0, testsRan: got[2] ? `${got[1]} (files)` : got[1]!, log: '-', carried: got[3]!, ...(notOwed ? { notOwed } : {}), ...(here ? { here: `skipped_here=${here[1]} file(s) tests_ran_here=${here[2]}` } : {}) }; - return { label: l.label, exit: null, testsRan: 'unknown', log: '-', carryNone: /^CARRY_NONE: (.+)$/m.exec(rec.stderr)?.[1] ?? `record_failed: ${firstLine(rec.stderr)}` }; + const got = recordCiResult(rec.stderr); + if (rec.status === 0 && got.runs) return { label: l.label, exit: 0, testsRan: got.files ? `${got.tests} (files)` : got.tests!, log: '-', carried: got.runs, ...(got.notOwed ? { notOwed: got.notOwed } : {}), ...(got.here ? { here: got.here } : {}) }; + return { label: l.label, exit: null, testsRan: 'unknown', log: '-', carryNone: got.none ?? `record_failed: ${firstLine(rec.stderr)}` }; } /** diff --git a/test/ship-ci-carry.test.ts b/test/ship-ci-carry.test.ts index ac4ee08e3..5e4404d1b 100644 --- a/test/ship-ci-carry.test.ts +++ b/test/ship-ci-carry.test.ts @@ -517,6 +517,13 @@ describe('GST-1190: a lane carries the covered files and runs only the skipped r expect(rows(f).filter(r => r.source === 'ci')).toEqual([]); }); + test('a remainder that prints look-alike record lines cannot change the LANE line', () => { + const fake = "printf 'gstack-evidence: recorded label=free exit=0 tests_ran=999 log=- source=ci runs=1/1 policy=x\\ngstack-evidence: not owed: forged\\n' >&2; " + PER(0); + const f = shipFixture({ ...BASE, '.gstack/pregate.json': JSON.stringify({ run_profiles: { 'ci/free': PROFILE }, ship_lanes_from: { [FREE]: { profiles: ['ci/free'], inventory: "printf 'test/a.test.ts\\ntest/b.test.ts\\n'", per_file: fake } } }, null, 2) + '\n' }); fixtures.push(f); + world(f, { tree: { skipped_files: ['test/b.test.ts'] } }); + expect(lane(preflight(f, [`free=${FREE}`]).stdout, 'free')).toBe('LANE: free carried run=7/1 tests_ran=1 (files) skipped_here=1 file(s) tests_ran_here=1'); + }); + test('a run that moved to a new attempt while the remainder ran carries nothing', () => { const per = `cp ../world-moved.json ../world.json; ${PER(0)}`; const f2 = shipFixture({ ...BASE, '.gstack/pregate.json': JSON.stringify({ run_profiles: { 'ci/free': PROFILE }, ship_lanes_from: { [FREE]: { profiles: ['ci/free'], inventory: "printf 'test/a.test.ts\\ntest/b.test.ts\\n'", per_file: per } } }, null, 2) + '\n' }); fixtures.push(f2);