From dd2a7bd8faa70eeb5c68fa00b20a38929825daef Mon Sep 17 00:00:00 2001 From: garrytan Date: Sun, 1 Nov 2026 11:22:20 +0000 Subject: [PATCH] fix: pre-landing review fixes (skill templates link on a real conflict; unit rows for linking, pending CI row) Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> --- docs/ship-policy.md | 5 +++-- lib/ship-queue-wave.ts | 10 +++++----- test/ship-queue-wave.test.ts | 19 +++++++++++++++++-- 3 files changed, 25 insertions(+), 9 deletions(-) diff --git a/docs/ship-policy.md b/docs/ship-policy.md index 1ee37831f..240db3a35 100644 --- a/docs/ship-policy.md +++ b/docs/ship-policy.md @@ -578,8 +578,9 @@ gstack-ship-queue wave settle --wave [--dry-run] # after the wave 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 + files never link PRs: anything under `docs/`, Markdown (generated skill + renders included; their `.tmpl` sources still link) 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/` diff --git a/lib/ship-queue-wave.ts b/lib/ship-queue-wave.ts index 7472a48d0..c0b77911c 100644 --- a/lib/ship-queue-wave.ts +++ b/lib/ship-queue-wave.ts @@ -79,12 +79,12 @@ 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); /** - * 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. + * Hub files: prose (docs/**, Markdown, 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. Skill templates (.md.tmpl) are source and link on a real conflict. */ -export const isHubFile = (file: string) => file.startsWith('docs/') || /\.(md|md\.tmpl|jsonl?)$/.test(file); +export const isHubFile = (file: string) => file.startsWith('docs/') || /\.(md|jsonl?)$/.test(file); /** * Group PRs into waves: PRs whose changes to a common code file conflict always travel together diff --git a/test/ship-queue-wave.test.ts b/test/ship-queue-wave.test.ts index 66d9b5ac4..c288574ee 100644 --- a/test/ship-queue-wave.test.ts +++ b/test/ship-queue-wave.test.ts @@ -12,7 +12,7 @@ import { spawnSync } from 'node:child_process'; import * as fs from 'node:fs'; import * as path from 'node:path'; import { EXIT } from '../lib/headless-artifacts'; -import { WAVE_HELP, areaKey, groupWaves, type PlanPr } from '../lib/ship-queue-wave'; +import { WAVE_HELP, areaKey, groupWaves, isHubFile, type PlanPr } from '../lib/ship-queue-wave'; import { cleanup, runBin } from './helpers/restamp-fixture'; import { line, openPr, queue, readyReceipt, shippedBody, trainFixture, type TrainFixture } from './helpers/ship-queue-train-fixture'; @@ -66,6 +66,19 @@ describe('areas and grouping', () => { ]); expect(solo).toEqual(['7']); }); + + test('groupWaves links PRs through a shared code file or skill template only when the merge conflicts there, never through a hub file (GST-770)', () => { + const pr = (n: string, files: string[]): PlanPr => ({ pr: n, head: n.repeat(40).slice(0, 40), files, lines: 10 }); + const areaOf = (f: string) => areaKey(f, d => d === 'ship'); + const linked = (files: string[], conflicts?: (a: PlanPr, b: PlanPr) => string[] | null) => + groupWaves([pr('1', ['lib/pregate-a.ts', 'test/pregate-a.test.ts', ...files]), pr('2', ['lib/review-b.ts', 'test/review-b.test.ts', ...files])], areaOf, 2, 6, conflicts).groups.map(g => g.prs.map(p => p.pr)); + expect(linked(['lib/ship-a.ts'])).toEqual([['1', '2']]); + expect(linked(['lib/ship-a.ts'], () => ['lib/ship-a.ts'])).toEqual([['1', '2']]); + expect(linked(['lib/ship-a.ts'], () => [])).toEqual([]); + expect(linked(['ship/SKILL.md.tmpl'], () => ['ship/SKILL.md.tmpl'])).toEqual([['1', '2']]); + for (const hub of ['docs/troubleshooting.md', 'ship/SKILL.md', 'scripts/harness-version.json']) expect(linked([hub], () => [hub])).toEqual([]); + expect([isHubFile('docs/x.ts'), isHubFile('TODOS.md'), isHubFile('test/fixtures/x.json'), isHubFile('ship/SKILL.md.tmpl'), isHubFile('lib/x.ts')]).toEqual([true, true, true, false, false]); + }); }); describe('wave build', () => { @@ -278,9 +291,11 @@ describe('ready and train build read the wave', () => { 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'); + pr('10', 'pregate'); + fx.write(d => { d.ci[d.prs['10']!.headRefOid] = [{ name: 'free-tests', status: 'in_progress', conclusion: null }]; }); 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(plan.out.split('\n').filter(l => l.startsWith('NOT_GREEN:')).map(l => l.replace(/ at [0-9a-f]{12} .*/, ''))).toEqual(['NOT_GREEN: #9 (red', 'NOT_GREEN: #10 (pending']); 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)',