From 583cc1b3725e26f3a86a030948b052ba3e73de98 Mon Sep 17 00:00:00 2001 From: garrytan Date: Sat, 31 Oct 2026 15:24:02 +0000 Subject: [PATCH] fix: train land refuses the next slot until the previous slot's post-merge checks are green Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> --- lib/ship-queue-train-land.ts | 10 +++++++++- lib/ship-queue-train-watch.ts | 2 +- test/ship-queue-train-generic.test.ts | 16 +++++++++++++--- 3 files changed, 23 insertions(+), 5 deletions(-) diff --git a/lib/ship-queue-train-land.ts b/lib/ship-queue-train-land.ts index c6d14a935..0ed0c86a4 100644 --- a/lib/ship-queue-train-land.ts +++ b/lib/ship-queue-train-land.ts @@ -23,6 +23,7 @@ import * as path from 'node:path'; import { ciAtHead, type QueueContext } from './ship-queue'; import { git, writeJsonAtomic } from './ship-queue-gate'; import { waitChecks } from './ship-queue-land'; +import { verifyPostMerge } from './ship-queue-post-merge'; import { bindTrainReceipt } from './ship-queue-train-gate'; import { checkRun, prInfo, type PrInfo } from './ship-queue-train-github'; import { activeFreeze, attemptsOf, nowIso, trainFile, type LandingRecord, type TrainRecord } from './ship-queue-train-record'; @@ -142,7 +143,7 @@ async function mergeAt(ctx: QueueContext, s: Slot, head: string, base: string): /** The check names every workflows gate asks to see green on main after each land. */ export const postMergeChecks = (env: TrainEnv): string[] => (env.train.gates ?? []).flatMap(g => (g.kind === 'workflows' ? g.post_merge_checks ?? [] : [])); -/** confirmMerged, with the post-merge checks a workflows gate declares in the NEXT: the next slot lands only once they are green. */ +/** confirmMerged, with the post-merge checks a workflows gate declares in the NEXT: train land refuses the next slot until they are green. */ function confirmed(s: Slot, mainSha: string, line: string): TrainOut { const out = confirmMerged(s.env, s.t, s.t.slots[s.i]!.pr, mainSha, s.target.index, [...s.lines, line]); const post = postMergeChecks(s.env); @@ -254,6 +255,13 @@ export async function trainLand(ctx: QueueContext, o: { train: string; pr: strin if ('error' in info) return outRefused({ code: 'QUEUE_PROOF_MISSING', message: info.error }, s.lines, rerun, run); if (info.isCrossRepository) return outRefused({ code: 'QUEUE_TRAIN_UNTRUSTED', message: `#${o.pr} is a fork head; train land pushes and merges only crew branches` }, s.lines, 'none', run); if (info.baseRefName && info.baseRefName !== ctx.base) return retargeted(s, info.baseRefName, ctx.base); + const post = postMergeChecks(env); + const prev = i > 0 ? t.merged.find(m => m.pr === t.slots[i - 1]!.pr) : undefined; + if (post.length && prev) { + const checked = await verifyPostMerge(ctx, { sha: prev.sha, checks: post, timeoutSec: 0, pollMs: 0 }); + s.lines.push(`POST_MERGE_CHECKS: #${t.slots[i - 1]!.pr} at ${prev.sha.slice(0, 12)} (${checked.failure?.code ?? 'green'})`, ...checked.lines); + if (checked.status !== 'complete') return { ...checked, lines: s.lines, next: checked.failure?.code === 'QUEUE_TRAIN_PENDING' ? `${checked.next} && ${rerun}` : checked.next, run }; + } const landing = t.landing?.[o.pr]; const conditional = !!t.slots[i]!.base_red; let mode = landing?.mode === 'update' || fs.existsSync(path.join(path.dirname(run), `land-${o.pr}.json`)) ? { mode: 'update' as const, why: 'resuming the recorded update' } : chooseMode(ctx, s, main.sha, conditional); diff --git a/lib/ship-queue-train-watch.ts b/lib/ship-queue-train-watch.ts index f91b29c52..922c2aab4 100644 --- a/lib/ship-queue-train-watch.ts +++ b/lib/ship-queue-train-watch.ts @@ -88,7 +88,7 @@ async function watchGeneric(ctx: QueueContext, env: TrainEnv, start: TrainRecord if (!post.length || !sha) continue; const checked = await verifyPostMerge(ctx, { sha, checks: post, timeoutSec: Math.max(1, Math.floor((deadline - Date.now()) / 1000)), pollMs: o.pollMs }); lines.push(`WATCH: verify --post-merge ${sha} (${checked.failure?.code ?? checked.status})`, ...checked.lines); - if (checked.status !== 'complete') return { ...checked, lines, next: `${checked.next === 'none' ? '' : `${checked.next} && `}${Q} train watch --train ${t.nonce} --land` }; + if (checked.status !== 'complete') return { ...checked, lines, next: checked.failure?.code === 'QUEUE_TRAIN_PENDING' ? `${checked.next} && ${Q} train watch --train ${t.nonce} --land` : checked.next }; } return outOk(lines, nextFor(t), run); } diff --git a/test/ship-queue-train-generic.test.ts b/test/ship-queue-train-generic.test.ts index afe0cf1e4..3596917be 100644 --- a/test/ship-queue-train-generic.test.ts +++ b/test/ship-queue-train-generic.test.ts @@ -578,7 +578,7 @@ describe('review pass 1 voices: dispatch bound, lease, output tail, post-merge c expect(held.err).toContain('holds ' + lock); } expect(fx.read().log.slice(before).some(l => l.cmd === 'workflow run' || l.cmd === 'run rerun')).toBe(false); - fs.writeFileSync(lock, JSON.stringify({ pid: spawnSync('true').pid, host: os.hostname() })); + fs.writeFileSync(lock, JSON.stringify({ pid: spawnSync('true', { timeout: 5000 }).pid, host: os.hostname() })); const v = queue(fx, ['train', 'verdict', '--train', nonce]); expect(v.status, v.err + v.out).toBe(EXIT.ok); expect(fs.existsSync(lock)).toBe(false); @@ -603,7 +603,17 @@ describe('review pass 1 voices: dispatch bound, lease, output tail, post-merge c const watched = queue(fx, ['train', 'watch', '--train', nonce, '--land', '--timeout', '20']); expect(watched.out, watched.err).toMatch(/^WATCH: verify --post-merge [0-9a-f]{40} \(QUEUE_TRAIN_PENDING\)/m); expect(fx.read().log.filter(l => l.cmd === 'pr merge')).toHaveLength(1); - expect(trainJson(fx, nonce).merged.map((m: any) => m.pr)).toEqual(['1']); + const merged = trainJson(fx, nonce).merged; + expect(merged.map((m: any) => m.pr)).toEqual(['1']); + const pending = queue(fx, ['train', 'land', '--train', nonce, '--pr', '2', '--no-wait']); + expect(pending.err, pending.out).toContain('(QUEUE_TRAIN_PENDING)'); + expect(line(pending.out, 'NEXT:')).toContain('--checks "deploy" && gstack-ship-queue train land --train ' + nonce + ' --pr 2'); + fx.write(d => { d.ci[merged[0].sha] = [{ name: 'test', status: 'completed', conclusion: 'success', id: 1, completed_at: '2026-10-26T00:00:00Z' }, { name: 'deploy', status: 'completed', conclusion: 'failure', id: 2, completed_at: '2026-10-26T00:00:00Z' }]; }); + for (const args of [['train', 'land', '--train', nonce, '--pr', '2', '--no-wait'], ['train', 'watch', '--train', nonce, '--land', '--timeout', '20']]) { + const red = queue(fx, args); + expect(red.err, red.out).toContain('(QUEUE_POST_MERGE_RED)'); + } + expect(fx.read().log.filter(l => l.cmd === 'pr merge')).toHaveLength(1); }, 300_000); }); @@ -621,7 +631,7 @@ describe('a command run another process still owns (red-team #4)', () => { expect(held.err).toContain('still running under process ' + process.pid); } expect(JSON.parse(fs.readFileSync(file, 'utf8')).state).toBe('dispatching'); - own(spawnSync('true').pid!); + own(spawnSync('true', { timeout: 5000 }).pid!); const freed = queue(fx, ['train', 'verdict', '--train', nonce]); expect(freed.err + freed.out).not.toContain('still running under process'); expect(line(freed.out, 'PREFIX: T0')).toContain('not dispatched');