From f25b22b0d4625ada579b168894ca9b424d6ce1a3 Mon Sep 17 00:00:00 2001 From: garrytan Date: Mon, 26 Oct 2026 16:12:57 +0000 Subject: [PATCH] train build --dry-run: never persist or push the read screen verdict admitSlots recorded a verdict that differed from the receipt and pushed refs/notes/gstack-trains before the dryRun branch printed 'nothing written, pushed or dispatched'. It now takes dryRun and skips the persist, matching trainReady's guard. Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> --- lib/ship-queue-train.ts | 6 +++--- test/ship-queue-train-ops.test.ts | 17 +++++++++++++++++ 2 files changed, 20 insertions(+), 3 deletions(-) diff --git a/lib/ship-queue-train.ts b/lib/ship-queue-train.ts index 4c44c8567..48c13cd80 100644 --- a/lib/ship-queue-train.ts +++ b/lib/ship-queue-train.ts @@ -296,7 +296,7 @@ export function parseOrder(spec: string | undefined, pins: Array<{ pr: string }> } /** The slots a new train would carry: trusted, unmoved, READY PRs, in slot order. */ -function admitSlots(env: TrainEnv, pins: Array<{ pr: string; sha: string }>): { slots: TrainSlot[]; lines: string[] } | TrainOut { +function admitSlots(env: TrainEnv, pins: Array<{ pr: string; sha: string }>, dryRun: boolean): { slots: TrainSlot[]; lines: string[] } | TrainOut { const { ctx } = env; const specs = pins.map(p => `+refs/pull/${p.pr}/head:refs/gstack/train/pr-${p.pr}`); const f = git(ctx.repoRoot, ['fetch', '--no-tags', '-q', env.remote, ...specs]); @@ -320,7 +320,7 @@ function admitSlots(env: TrainEnv, pins: Array<{ pr: string; sha: string }>): { let screen = ready.screen; if (screen.choice === 'screen' && screen.run && slug === undefined) slug = repoSlug(ctx); const read = screen.choice === 'screen' && screen.run ? (slug ? screenVerdict(ctx, slug, screen.run) : { kind: 'unknown' as const, detail: 'the repository slug is unreadable' }) : null; - if (read && screen.run && recordVerdict(ready.screen, read)) { ready.updated_at = nowIso(); persist(ctx.repoRoot, [ready], { remote: env.remote }); } + if (read && screen.run && recordVerdict(ready.screen, read) && !dryRun) { ready.updated_at = nowIso(); persist(ctx.repoRoot, [ready], { remote: env.remote }); } if (read?.kind === 'red') { return outRefused({ code: 'QUEUE_SCREEN_RED', message: `#${p.pr}'s screen run ${screen.run} is RED on ${read.red.join(', ') || 'its gate binding'} (${read.detail}); a red head is not seated` }, [...lines, ...verdictLines(p.pr, screen.run!, read)], without ? `${Q} train build --prs ${without}` : 'none', 'none'); @@ -345,7 +345,7 @@ async function newTrain(env: TrainEnv, o: BuildOptions): Promise<{ t: TrainRecor frozenLines(freeze), `${Q} train build --repair --prs @`, 'none'); const explicit = parseOrder(o.order, pins); if (explicit && 'error' in explicit) return outRefused({ code: 'QUEUE_PROOF_MISSING', message: explicit.error }, [], 'none', 'none'); - const admitted = admitSlots(env, pins); + const admitted = admitSlots(env, pins, o.dryRun); if ('exit' in admitted) return admitted; const order = orderSlots(admitted.slots, o.repair ? 1 : env.train.max_size, explicit); const lines = [ diff --git a/test/ship-queue-train-ops.test.ts b/test/ship-queue-train-ops.test.ts index 7efa76aeb..99a40a35c 100644 --- a/test/ship-queue-train-ops.test.ts +++ b/test/ship-queue-train-ops.test.ts @@ -105,6 +105,23 @@ describe('the screen verdict comes from the run', () => { // Value: protects=a screen that concluded success with no slice (18 of 41 PR screens on 2026-10-26, superseded by a push before the plan) is NOT-RUN at verdict, not a screen and not a stale ref; // fails_when=verify --screen calls it GREEN, HOLLOW or QUEUE_SCREEN_STALE_REF, or ready follows it; why_new=the hollow outcome reused the stale-ref code; seam=the gh shim's run with no plan artifact; no_claim=why evals.yml planned nothing + // Value: protects=train build --dry-run (with or without --stack-tests) writes nothing: the screen verdict it reads is printed, not recorded in the READY receipt or pushed to refs/notes/gstack-trains, as its DRY-RUN line says; + // fails_when=admitSlots persists the read verdict under --dry-run (the notes ref moves and the receipt gains a verdict before any real build); why_new=the other dry-run tests only check the trains dir; seam=none; no_claim=a verdict that was already recorded by ready or verify + test('train build --dry-run prints the screen verdict it read without recording it in the receipt or pushing the notes ref', () => { + const { fx, head, pregate } = readyFixture(); + fx.write(d => { d.rules['evals.yml'] = GREEN_SCREEN; }); + expect(queue(fx, ['ready', '--pr', '1', '--pregate', pregate, '--screen']).status).toBe(EXIT.ok); + expect(receiptOf(fx, '1', head).screen.verdict).toBeUndefined(); + const notes = fx.repo.git('ls-remote', 'origin', 'refs/notes/gstack-trains'); + const build = queue(fx, ['train', 'build', '--prs', `1@${head}`, '--dry-run']); + expect([build.status, line(build.out, 'SCREEN:'), line(build.out, 'DRY-RUN:')]).toEqual([EXIT.ok, 'SCREEN: #1 run 100 GREEN (flaky: c-flaky)', 'DRY-RUN: nothing written, pushed or dispatched']); + expect(receiptOf(fx, '1', head).screen.verdict).toBeUndefined(); + expect(fx.repo.git('ls-remote', 'origin', 'refs/notes/gstack-trains')).toBe(notes); + const verify = queue(fx, ['verify', '--pr', '1', '--screen']); + expect([verify.status, receiptOf(fx, '1', head).screen.verdict?.outcome]).toEqual([EXIT.ok, 'GREEN']); + expect(fx.repo.git('ls-remote', 'origin', 'refs/notes/gstack-trains')).not.toBe(notes); + }, 120_000); + test('a screen run that concluded success with no slice is NOT-RUN: verify --screen refuses with QUEUE_SCREEN_NOT_RUN, the receipt records NOT_RUN, and ready --screen dispatches afresh', () => { const { fx, head, pregate } = readyFixture(); fx.write(d => { d.rules['evals.yml'] = { status: 'completed', conclusion: 'success', plan: null }; });