From ada2accea064f814071330f97f61c03db84180a8 Mon Sep 17 00:00:00 2001 From: garrytan Date: Tue, 27 Oct 2026 16:22:49 +0000 Subject: [PATCH] Review pass 2: an unreadable GitHub state at a failed slot refuses, never "nothing merged" (GSTA-129) stopped() read prState's 'unreadable' as "not merged": with nothing in t.merged, a slot-1 squash already on main while gh pr view failed answered "nothing of the train merged, so the queue is not frozen". It now refuses with QUEUE_PROOF_MISSING ("whether # merged on GitHub could not be read, so whether the queue must freeze is unknown") and asks for the verify to be rerun; no freeze, no backstop. Raised independently by five pass-2 reviewers. Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> --- CHANGELOG.md | 4 ++-- lib/ship-queue-train-verify.ts | 8 ++++++-- test/ship-queue-train.test.ts | 15 ++++++++++++--- 3 files changed, 20 insertions(+), 7 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 5335b2208..9054cec8a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,12 +5,12 @@ **A merge the pre-merge verify accepted no longer freezes the queue, and a slot stamp touches only the stamp files.** Train 24 (2026-10-27) merged #3434 and #3453, then froze on #3453's post-merge check and paid a $31 backstop: the pre-merge verify judged the lane's CHANGELOG as main's plus the pin's own entry and said MERGE_CONDITION, the post-merge check compared main to T2 section by section, and T2's one `[Unreleased]` (both stacked entries) had become two released sections. In both slots `gstack-restamp` also listed `lib/ship-policy.ts` as a fifth stamp file, because the lane's uncommitted base merge made #3511's `DOCS_CAPS_KEY_VERSION` look like the branch's own tentative key. And `mergeChangelogs` refused the stacked child outright, so the lane's hand resolve dropped #3453's two wording fixes to the released 1.91.276.0 entry. ### Fixed -- **`gstack-ship-queue verify --train`** (`lib/ship-queue-train-verify.ts`): one CHANGELOG rule before and after the merge. Post-merge, main confirms when its tree is T modulo stamps, or differs from T in the CHANGELOG alone while being its own first parent's CHANGELOG (the main the pre-merge verify saw) plus the PR's own entry, the rule `mergedIn` applied pre-merge; the `main:` line says "except CHANGELOG.md, main 's CHANGELOG.md plus #'s own entry". The own lines are the pin's authored CHANGELOG lines or the lines `mergeChangelogs` writes for that pin against main, so a tool-written correction bullet passes. A freeze lists a slot GitHub reports MERGED as merged (its reason says "merged on GitHub without a verified tree"), never under `unmerged`, so the rebuild after the repair does not seat it again; and a first slot GitHub reports MERGED whose squash fails the post-merge check freezes the queue instead of answering "nothing of the train merged" while main already holds the unverified squash. A main whose CHANGELOG equals its parent's is not "the parent's plus the PR's own entry" (`ownEntryAboveMain`: "no entry of the PR's own above it"), so a squash that lost the entry the gate carried is refused. +- **`gstack-ship-queue verify --train`** (`lib/ship-queue-train-verify.ts`): one CHANGELOG rule before and after the merge. Post-merge, main confirms when its tree is T modulo stamps, or differs from T in the CHANGELOG alone while being its own first parent's CHANGELOG (the main the pre-merge verify saw) plus the PR's own entry, the rule `mergedIn` applied pre-merge; the `main:` line says "except CHANGELOG.md, main 's CHANGELOG.md plus #'s own entry". The own lines are the pin's authored CHANGELOG lines or the lines `mergeChangelogs` writes for that pin against main, so a tool-written correction bullet passes. A freeze lists a slot GitHub reports MERGED as merged (its reason says "merged on GitHub without a verified tree"), never under `unmerged`, so the rebuild after the repair does not seat it again; and a first slot GitHub reports MERGED whose squash fails the post-merge check freezes the queue instead of answering "nothing of the train merged" while main already holds the unverified squash; when GitHub's answer for a slot cannot be read, the verify refuses (`QUEUE_PROOF_MISSING`, "whether # merged on GitHub could not be read") and asks to be rerun, never "nothing merged". A main whose CHANGELOG equals its parent's is not "the parent's plus the PR's own entry" (`ownEntryAboveMain`: "no entry of the PR's own above it"), so a squash that lost the entry the gate carried is refused. - **`gstack-restamp` stamp_paths** (`lib/restamp.ts`): a line the base added since the pin's fork point is the base's, whether the base merge is committed or still staged; `branchAddedLines` leaves it out and the stage detail says "N line(s) the base added since the fork point are the base's, not this branch's, and stay (: N)". #3514's receipt bound (`stampOnlyLine`) refuses such a move after the fact only while the moved version is older than the base's at the build; a key at the base version passes it, so the restamp is the guard. A base diff git cannot run is a refusal ("cannot diff .. for the stamp_paths stage"), never an empty "nothing added" set that would move the base's lines again. - **`mergeChangelogs`** (`lib/changelog-check.ts`): a stacked child whose parent squash-merged and was released since the merge base (`carryReleasedParent`) is a merge, not a hand resolve. The parent's lines leave the child's `[Unreleased]`; each line the child changed inside them becomes `- Correction to the entry below: "" should read "".` (a dropped line "no longer applies", a line added between the parent's "Addition to the entry below") under the child's own `### Gate integrity`, appended to one it already has. The released entry stays main's byte for byte. Without the merge base the shape is GSTA-67's dropped heading and stays null. `mergeChangelogsWithNotes` returns the carried corrections; the slot one-liner in docs/ship-policy.md prints them. `mergeStep` (train build) passes the merge base, so a child of an already-released parent seats; from slot 2 on, where the prefix already holds an earlier slot's `[Unreleased]` and the sections are unioned, the union takes the carried child, so the gated tree never repeats the released parent's text under the child's heading. ### Tests -- `test/ship-queue-train.test.ts`: a stacked child's merged-in head the pre-merge verify accepted confirms after its squash merge (froze before), with the correction bullet accepted pre-merge; a post-merge CHANGELOG holding a line the pin never wrote still freezes, and the freeze lists the slot GitHub reports MERGED under `merged`; a slot-1 squash GitHub reports MERGED that fails the post-merge check freezes; a stacked child behind another slot is gated on the carried CHANGELOG (the released parent's text appears once, under its own heading, with the child's corrections in the child's entry). `test/restamp-base-merge.test.ts` (new): train 24's shape (pin forked before the base's key, base merge staged), for a key naming an older release and for one at the base version; the dry run lists the four stamp files plus the branch's own key file only, committed or staged, and the receipt bound is asserted for both keys. `test/changelog-merge-damage.test.ts`: the real a78ce08a/3e4240a7/d87eec95 CHANGELOGs (`test/fixtures/merge-train-3453/`) merge to main's sections unchanged plus #3453's entry with the two corrections; without the base they stay null; shape controls for an unedited, a dropped, an added and an empty child. +- `test/ship-queue-train.test.ts`: a stacked child's merged-in head the pre-merge verify accepted confirms after its squash merge (froze before), with the correction bullet accepted pre-merge; a post-merge CHANGELOG holding a line the pin never wrote still freezes, and the freeze lists the slot GitHub reports MERGED under `merged`; a slot-1 squash GitHub reports MERGED that fails the post-merge check freezes, and an unreadable GitHub state refuses; a stacked child behind another slot is gated on the carried CHANGELOG (the released parent's text appears once, under its own heading, with the child's corrections in the child's entry). `test/restamp-base-merge.test.ts` (new): train 24's shape (pin forked before the base's key, base merge staged), for a key naming an older release and for one at the base version; the dry run lists the four stamp files plus the branch's own key file only, committed or staged, and the receipt bound is asserted for both keys. `test/changelog-merge-damage.test.ts`: the real a78ce08a/3e4240a7/d87eec95 CHANGELOGs (`test/fixtures/merge-train-3453/`) merge to main's sections unchanged plus #3453's entry with the two corrections; without the base they stay null; shape controls for an unedited, a dropped, an added and an empty child. ### Docs - docs/ship-policy.md § Run a merge train: the one CHANGELOG rule, the stamp-files rule (never a fifth), the stacked-child-after-squash resolution, and that an unstamped head changes a release output by regenerating it at its own VERSION (the Version Gate has accepted that since 1.91.237.0; #3434 reverted its digest change against the older gate). docs/captain.md § Merge trains and captain/templates/slot-call.md carry the stamp-files rule (the files the slot call names: `STAMP_FILES`, plus the file carrying a `*_KEY_VERSION` the PR introduced when the call names it; a file the restamp lists that the call did not name is left out and reported) and the stacked-child rule. diff --git a/lib/ship-queue-train-verify.ts b/lib/ship-queue-train-verify.ts index ec8ff4135..3c5037900 100644 --- a/lib/ship-queue-train-verify.ts +++ b/lib/ship-queue-train-verify.ts @@ -143,11 +143,15 @@ async function freezeQueue(env: TrainEnv, t: TrainRecord, reason: string, mainSh * Interrupted publication: freeze when a slot merged, verified or merged on * GitHub without a verified tree (slot 1's own squash failing the post-merge * check leaves main unverified with nothing in `merged` yet); otherwise a - * plain refusal (nothing was published). + * plain refusal (nothing was published). A GitHub state that cannot be read + * is neither: the verify refuses and asks to be rerun, never "nothing merged". */ async function stopped(env: TrainEnv, t: TrainRecord, code: 'QUEUE_TRAIN_TREE_MISMATCH' | 'QUEUE_TRAIN_MOVED', message: string, mainSha: string, lines: string[]): Promise { const run = trainFile(t.nonce); - const landed = t.merged.length > 0 || t.slots.some(s => prState(env.ctx, s.pr) === 'MERGED'); + const states = t.merged.length ? [] : t.slots.map(s => ({ pr: s.pr, state: prState(env.ctx, s.pr) })); + const landed = t.merged.length > 0 || states.some(s => s.state === 'MERGED'); + const unreadable = landed ? [] : states.filter(s => s.state === 'unreadable').map(s => `#${s.pr}`); + if (unreadable.length) return outRefused({ code: 'QUEUE_PROOF_MISSING', message: `${message}; whether ${unreadable.join(', ')} merged on GitHub could not be read, so whether the queue must freeze is unknown` }, lines, `${Q} verify --pr ${t.slots[0]!.pr} --train ${t.nonce} (once gh pr view answers)`, run); if (!landed) return outFailed({ code, message: `${message}; nothing of train ${t.nonce} merged, so the queue is not frozen` }, lines, `${Q} train build ${prsArgs(t.slots, t.explicit_order)} (a new train on current main)`, run); const f = activeFreeze(env.repo.keys) ?? await freezeQueue(env, t, message, mainSha); return outFailed({ code, message: `${message}; the queue is frozen` }, [...lines, ...frozenLines(f)], `${Q} train build --repair --prs @`, run); diff --git a/test/ship-queue-train.test.ts b/test/ship-queue-train.test.ts index 8c139e96b..0a4d106bf 100644 --- a/test/ship-queue-train.test.ts +++ b/test/ship-queue-train.test.ts @@ -1022,8 +1022,8 @@ describe('verify, freeze, repair and clear', () => { expect(JSON.parse(fs.readFileSync(freezeFile, 'utf8'))).toMatchObject({ merged: ['1', '2'], unmerged: [] }); }, 300_000); - // Value: protects=a first slot GitHub reports MERGED whose squash fails the post-merge check freezes the queue; fails_when=stopped() reads an empty merged list as "nothing merged" while main already holds the unverified squash (red-team review of GSTA-129); why_new=the test above freezes on slot 2 with slot 1 verified; seam=the fixture's gh pr view state; no_claim=the backstop's outcome - test("slot 1 merged on GitHub but not confirmed: the queue freezes with #1 merged and #2 unmerged instead of saying nothing merged", () => { + // Value: protects=a first slot GitHub reports MERGED whose squash fails the post-merge check freezes the queue, and a GitHub state that cannot be read refuses instead of reading as not merged; fails_when=stopped() reads an empty merged list, or an unreadable gh answer, as "nothing merged" while main already holds the unverified squash (review of GSTA-129, passes 1 and 2); why_new=the test above freezes on slot 2 with slot 1 verified; seam=the fixture's gh pr view state; no_claim=the backstop's outcome + test("slot 1 merged on GitHub but not confirmed: an unreadable GitHub state refuses, and a MERGED one freezes with #1 merged and #2 unmerged instead of saying nothing merged", () => { const fx = make(); const { h2, nonce } = twoPrTrain(fx); expect(queue(fx, ['train', 'verdict', '--train', nonce]).status).toBe(EXIT.ok); @@ -1032,7 +1032,16 @@ describe('verify, freeze, repair and clear', () => { const v = (pr: string) => queue(fx, ['verify', '--pr', pr, '--train', nonce], {}, coord); const smuggled = fx.repo.git('show', `${t.prefixes[0].commit}:CHANGELOG.md`).replace('# Changelog\n', '# Changelog\n\n## [1.2.4.0] - 2026-10-18\n\n- #1 change\n- added after the gate\n') + '\n'; const broken = publishMain(fx, t.prefixes[1].commit, '1.2.4.0', { file: 'CHANGELOG.md', text: smuggled }); - fx.write(d => { (d.prs['1'] as any).state = 'MERGED'; }); + // While GitHub's answer for #1 cannot be read, the verify neither freezes nor says "nothing merged": it refuses and asks to be rerun. + const row = fx.read().prs['1']; + fx.write(d => { delete d.prs['1']; }); + const unreadable = v('1'); + expect(unreadable.status).toBe(EXIT.refused); + expect(unreadable.err).toContain(`main ${broken.slice(0, 12)} is neither T0 nor T1 modulo stamps`); + expect(unreadable.err).toMatch(/; whether #1 merged on GitHub could not be read, so whether the queue must freeze is unknown; .*\(QUEUE_PROOF_MISSING\)/); + expect(unreadable.out).toContain(`NEXT: gstack-ship-queue verify --pr 1 --train ${nonce} (once gh pr view answers)`); + expect(fs.existsSync(path.join(fx.state, 'ship-queue', 'freeze', `${repoKey(fx)}.json`))).toBe(false); + fx.write(d => { d.prs['1'] = { ...row, state: 'MERGED' }; }); const frozen = v('1'); expect(frozen.status).toBe(EXIT.fail); expect(frozen.err).toContain(`main ${broken.slice(0, 12)} is neither T0 nor T1 modulo stamps`);