From e3f14bd89ad1c763314075bc55fa0fa174acbf02 Mon Sep 17 00:00:00 2001 From: garrytan Date: Sun, 1 Nov 2026 11:09:06 +0000 Subject: [PATCH] fix(ship-queue): wave plan links PRs only on a real code conflict and leaves out READY heads whose CI is not green (GST-770) Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> --- docs/ship-policy.md | 12 +++++++--- lib/ship-queue-wave.ts | 46 ++++++++++++++++++++++++++---------- test/ship-queue-wave.test.ts | 42 ++++++++++++++++++++++++++++++++ 3 files changed, 84 insertions(+), 16 deletions(-) diff --git a/docs/ship-policy.md b/docs/ship-policy.md index 64cd2cf4b..1ee37831f 100644 --- a/docs/ship-policy.md +++ b/docs/ship-policy.md @@ -573,9 +573,15 @@ gstack-ship-queue wave settle --wave [--dry-run] # after the wave ``` - **When.** `wave plan` counts the live READY PRs (open, head equal to their - READY receipt). Over the trigger (6) it prints `WAVE_ADVISED` and suggested - groups of 3 to 6: PRs that change a common file always share a group (their - conflicts are resolved once), each group takes its largest PR's functional + READY receipt, CI green at that head; a READY head whose CI is red or still + running prints a `NOT_GREEN` line and stays out of the plan). Over the + trigger (6) it prints `WAVE_ADVISED` and suggested groups of 3 to 6: PRs + whose changes to a common code file conflict (`git merge-tree` cannot merge + the two heads) always share a group, so that conflict is resolved once. Hub + files never link PRs: anything under `docs/`, Markdown and its `.tmpl` + sources, and JSON ledgers and registries. Most PRs append to them, and + linking through them once chained a 55-PR queue into a single area + (GST-770). Each group takes its largest PR's functional area (a skill directory, the first word of a `lib/`, `bin/` or `test/` module name, or `docs`), smaller groups ride solo, and the integrator is the PR with the most changed lines. `train build` handed more than 6 pins diff --git a/lib/ship-queue-wave.ts b/lib/ship-queue-wave.ts index 73aec5177..7472a48d0 100644 --- a/lib/ship-queue-wave.ts +++ b/lib/ship-queue-wave.ts @@ -24,7 +24,7 @@ import { EXIT, formatResult } from './headless-artifacts'; import { RESULT_CODES } from './result-codes'; import { versionAtRef } from './restamp'; import { globToRegExp, type ShipPolicy } from './ship-policy'; -import type { QueueContext } from './ship-queue'; +import { ciAtHead, type QueueContext } from './ship-queue'; import { git, sha256, type QueueFailure } from './ship-queue-gate'; import { fetchBase, mergeCommit, outFailed, outOk, outRefused, parsePins, trainEnv, type TrainEnv, type TrainOut } from './ship-queue-train'; import { prInfo, type PrInfo } from './ship-queue-train-github'; @@ -79,20 +79,34 @@ export function dominantArea(files: string[], areaOf: (f: string) => string): st const largestFirst = (a: PlanPr, b: PlanPr) => b.lines - a.lines || Number(a.pr) - Number(b.pr); /** - * Group PRs into waves: PRs that change a common file always travel together (one integrator - * resolves their conflicts once), each cluster takes its largest PR's area, clusters of one - * area are packed in PR order into waves of at most max, and a wave below min rides solo. - * The integrator is the PR with the most changed lines. + * Hub files: prose (docs/**, Markdown and its .tmpl sources, generated SKILL.md renders) and JSON + * ledgers and registries. Most PRs append to them, so their conflicts are kept-both-sides edits + * that any train slot resolves; linking PRs through them chained a 55-PR queue into one area + * (GST-770). They never link PRs into a wave. */ -export function groupWaves(prs: PlanPr[], areaOf: (f: string) => string, min = WAVE_MIN, max = WAVE_MAX): { groups: WaveGroup[]; solo: string[] } { +export const isHubFile = (file: string) => file.startsWith('docs/') || /\.(md|md\.tmpl|jsonl?)$/.test(file); + +/** + * Group PRs into waves: PRs whose changes to a common code file conflict always travel together + * (one integrator resolves the conflict once); hub files never link. conflicts(a, b) names the + * files a merge of the two heads cannot resolve, or null when unknown, which counts every shared + * file as conflicting. Each cluster takes its largest PR's area, clusters of one area are packed + * in PR order into waves of at most max, and a wave below min rides solo. The integrator is the + * PR with the most changed lines. + */ +export function groupWaves(prs: PlanPr[], areaOf: (f: string) => string, min = WAVE_MIN, max = WAVE_MAX, conflicts: (a: PlanPr, b: PlanPr) => string[] | null = () => null): { groups: WaveGroup[]; solo: string[] } { const sorted = [...prs].sort((a, b) => Number(a.pr) - Number(b.pr)); const root = new Map(sorted.map(p => [p.pr, p.pr])); const find = (x: string): string => { const r = root.get(x)!; return r === x ? x : find(r); }; - const owner = new Map(); - for (const p of sorted) for (const f of p.files) { - const o = owner.get(f); - if (o && find(o) !== find(p.pr)) root.set(find(p.pr), find(o)); else if (!o) owner.set(f, p.pr); - } + const sharers = new Map(); + for (const p of sorted) for (const f of p.files) if (!isHubFile(f)) sharers.set(f, [...(sharers.get(f) ?? []), p]); + const clash = new Map(); + for (const [f, list] of sharers) list.forEach((a, i) => list.slice(i + 1).forEach(b => { + const key = `${a.pr} ${b.pr}`; + if (!clash.has(key)) clash.set(key, conflicts(a, b)); + const hit = clash.get(key); + if ((!hit || hit.includes(f)) && find(a.pr) !== find(b.pr)) root.set(find(b.pr), find(a.pr)); + })); const clusters = new Map(); for (const p of sorted) clusters.set(find(p.pr), [...(clusters.get(find(p.pr)) ?? []), p]); const byArea = new Map(); @@ -250,15 +264,21 @@ export function wavePlan(ctx: QueueContext, o: { trigger: number; min: number; m const info = prInfo(ctx, r.pr); if ('error' in info) { unreadable.push(`#${r.pr} (${info.error})`); continue; } if (info.state !== 'OPEN' || info.headRefOid !== r.head || info.isCrossRepository || info.headRefName.startsWith('wave/')) continue; + const ci = ciAtHead(ctx, r.head); + if (ci.line.startsWith('unknown')) { unreadable.push(`#${r.pr} (CI ${ci.line})`); continue; } + if (!ci.green) { lines.push(`NOT_GREEN: #${r.pr} (${ci.line}; left out until CI is green at the READY head)`); continue; } if (!ensureCommit(env, r.head, r.pr)) { unreadable.push(`#${r.pr} (head ${short(r.head)} not fetchable)`); continue; } const c = changed(env, r.head); if (!c) { unreadable.push(`#${r.pr} (diff unreadable)`); continue; } live.push({ pr: r.pr, head: r.head, ...c }); } if (unreadable.length) return outFailed(fail('QUEUE_PROOF_MISSING', `cannot read ${unreadable.length} READY PR(s): ${unreadable.join('; ')}; a plan from a partial queue would mislead`), lines, `${Q} wave plan (after gh and git can read every PR)`, 'none'); - lines.push(`QUEUE: ${live.length} live READY PR(s) (open, head equal to its READY receipt); wave trigger ${o.trigger}`); + lines.push(`QUEUE: ${live.length} live READY PR(s) (open, head equal to its READY receipt, CI green there); wave trigger ${o.trigger}`); if (live.length <= o.trigger) return outOk([...lines, `WAVE: not needed (${live.length} <= ${o.trigger}); build trains of single PRs`], `${Q} train build --prs `, 'none'); - const plan = groupWaves(live, areaOf(env), o.min, o.max); + const plan = groupWaves(live, areaOf(env), o.min, o.max, (a, b) => { + const m = git(env.ctx.repoRoot, ['merge-tree', '--write-tree', '--name-only', '--no-messages', a.head, b.head]); + return m.ok ? [] : m.status === 1 ? m.out.split('\n').slice(1).filter(Boolean) : null; + }); lines.push(`WAVE_ADVISED: ${live.length} READY PRs over the wave trigger ${o.trigger}; group by area into waves of ${o.min}-${o.max} (Garry R46)`); for (const g of plan.groups) { lines.push(`WAVE_GROUP: area=${g.area} prs=${g.prs.map(p => '#' + p.pr).join(',')} integrator=#${g.integrator} (most changed lines)`); diff --git a/test/ship-queue-wave.test.ts b/test/ship-queue-wave.test.ts index e2132fd7c..66d9b5ac4 100644 --- a/test/ship-queue-wave.test.ts +++ b/test/ship-queue-wave.test.ts @@ -235,6 +235,7 @@ describe('ready and train build read the wave', () => { const pins = ['1', '2', '3', '4', '5', '6', '7'].map(pr => { const h = openPr(fx, pr, { file: pr === '7' ? 'docs/seven.md' : `lib/ship-${pr}.ts`, changelog: false }); readyReceipt(fx, pr, h); + fx.write(d => { d.ci[h] = [{ name: 'free-tests', status: 'completed', conclusion: 'success' }]; }); return `${pr}@${h}`; }); const build = queue(fx, ['train', 'build', '--prs', pins.join(','), '--dry-run']); @@ -246,6 +247,47 @@ describe('ready and train build read the wave', () => { expect(line(plan.out, 'SOLO:')).toContain('#7'); expect(line(queue(fx, ['wave', 'plan', '--trigger', '7']).out, 'WAVE:')).toContain('not needed'); }, 180_000); + + test('wave plan links PRs only on a real code conflict, so hub docs and ledgers do not chain areas together, and leaves out READY heads whose CI is not green (GST-770)', () => { + const fx = make(); + const seed = { 'docs/troubleshooting.md': '# Troubleshooting\n', 'test/fixtures/ledger.json': '{}\n', 'lib/ship-a.ts': 'export const a = 0;\n' }; + fx.repo.git('checkout', '-q', 'origin/main', '-B', 'seed'); + for (const [f, t] of Object.entries(seed)) { fs.mkdirSync(path.dirname(path.join(fx.repo.work, f)), { recursive: true }); fs.writeFileSync(path.join(fx.repo.work, f), t); } + fx.repo.git('add', '-A'); + fx.repo.git('commit', '-q', '-m', 'seed hub files'); + fx.repo.git('push', '-q', 'origin', 'HEAD:main'); + fx.repo.git('fetch', '-q', 'origin'); + const pr = (n: string, area: string, extra: Record = {}, ci = 'success') => { + fx.repo.git('checkout', '-q', 'origin/main', '-B', `pr-${n}`); + const files = { [`lib/${area}-${n}.ts`]: `export const x${n} = 1;\n`, [`test/${area}-${n}.test.ts`]: `// #${n}\n`, + 'docs/troubleshooting.md': `# Troubleshooting\n\n## #${n}\n`, 'test/fixtures/ledger.json': `{ "pr": ${n} }\n`, ...extra }; + for (const [f, t] of Object.entries(files)) fs.writeFileSync(path.join(fx.repo.work, f), t); + fx.repo.git('add', '-A'); + fx.repo.git('commit', '-q', '-m', `feat #${n}`); + const head = fx.repo.git('rev-parse', 'HEAD'); + fx.repo.git('push', '-q', '-f', 'origin', `HEAD:refs/pull/${n}/head`); + fx.write(d => { + d.prs[n] = { headRefOid: head, isCrossRepository: false, state: 'OPEN', headRefName: `pr-${n}`, url: `https://github.com/fixture/repo/pull/${n}`, body: shippedBody(n, head) }; + d.ci[head] = [{ name: 'free-tests', status: 'completed', conclusion: ci }]; + }); + readyReceipt(fx, n, head); + }; + pr('1', 'ship', { 'lib/ship-a.ts': Array.from({ length: 40 }, (_, i) => `export const a${i} = 'one';\n`).join('') }); + pr('2', 'ship'); + pr('3', 'ship'); + for (const n of ['4', '5', '6']) pr(n, 'pregate'); + pr('8', 'review', { 'lib/ship-a.ts': "export const a = 'eight';\n" }); + pr('9', 'pregate', {}, 'failure'); + const plan = queue(fx, ['wave', 'plan']); + expect(plan.status).toBe(EXIT.ok); + expect(line(plan.out, 'NOT_GREEN:')).toMatch(/^NOT_GREEN: #9 \(red at [0-9a-f]{12} /); + expect(line(plan.out, 'QUEUE:')).toContain('7 live READY PR(s)'); + expect(plan.out.split('\n').filter(l => l.startsWith('WAVE_GROUP:'))).toEqual([ + 'WAVE_GROUP: area=pregate prs=#4,#5,#6 integrator=#4 (most changed lines)', + 'WAVE_GROUP: area=ship prs=#1,#8,#2,#3 integrator=#1 (most changed lines)', + ]); + expect(line(plan.out, 'SOLO:')).toBe(''); + }, 180_000); }); describe('wave settle', () => {