diff --git a/CHANGELOG.md b/CHANGELOG.md index 9241d1d95..749fc6f56 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,7 +5,7 @@ **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; 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-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; the record is built from the one GitHub read that decided to freeze, so a slot that read MERGED stays merged when GitHub stops answering a moment later, and a slot unreadable beside a merged one is kept out of `unmerged` and named in the reason ("unreadable on GitHub, kept out of unmerged until read"); 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. diff --git a/docs/ship-policy.md b/docs/ship-policy.md index 8b1750528..2f24ee4cf 100644 --- a/docs/ship-policy.md +++ b/docs/ship-policy.md @@ -620,7 +620,11 @@ backstop run. A slot GitHub reports MERGED counts as merged in that record, verified tree or not, so the rebuild after the repair never seats it again (the reason says "merged on GitHub without a verified tree"); a first slot whose squash GitHub reports MERGED but whose tree fails the post-merge check -freezes the same way, since main already holds the unverified squash. When +freezes the same way, since main already holds the unverified squash. The +record comes from the one GitHub read that decided to freeze, never a second +one: a slot that read MERGED stays merged even when GitHub stops answering +before the record is written, and a slot unreadable beside a merged one is +kept out of `unmerged` and named in the reason until it can be read. When whether a slot merged on GitHub cannot be read, verify refuses (`QUEUE_PROOF_MISSING`, "whether # merged on GitHub could not be read") and asks to be rerun once `gh pr view` answers; an unreadable state is never diff --git a/docs/troubleshooting.md b/docs/troubleshooting.md index 6746440df..eded67d2d 100644 --- a/docs/troubleshooting.md +++ b/docs/troubleshooting.md @@ -4349,7 +4349,12 @@ prefix no gate covered on its own. The `QUEUE_TRAIN_FROZEN:` line lists the merged PRs (a slot GitHub reports MERGED counts as merged, verified tree or not, and the reason then says `merged on GitHub without a verified tree`, so the repair's rebuild never seats it again), main's tree and the backstop run -dispatched on main's SHA. The +dispatched on main's SHA. The record is built from the one GitHub read that +decided to freeze: a slot that read MERGED stays merged even when `gh pr view` +stops answering a moment later, and a slot whose state could not be read when +another slot had merged is kept out of `unmerged` and named in the reason +(`# unreadable on GitHub, kept out of unmerged until read`), so the +rebuild after the repair never seats a PR that may already be in. The freeze lives in local state and on `refs/notes/gstack-trains`. Ordinary trains and other trains' merges are refused until a repair lands. diff --git a/lib/ship-queue-train-verify.ts b/lib/ship-queue-train-verify.ts index 3c5037900..5cf30b561 100644 --- a/lib/ship-queue-train-verify.ts +++ b/lib/ship-queue-train-verify.ts @@ -112,17 +112,23 @@ function published(env: TrainEnv, t: TrainRecord, i: number, main: string): { sa /** * Freeze the queue: the record goes to local state and the notes ref before * anything remote, then the backstop gate is dispatched on main's sha. + * `states` is the one GitHub read of every unverified slot, made by the + * caller: a slot that read MERGED is merged, verified tree or not, and a slot + * that could not be read is left out of `unmerged` and named in the reason, + * so the rebuild after the repair never seats a PR that may already be in. */ -async function freezeQueue(env: TrainEnv, t: TrainRecord, reason: string, mainSha: string): Promise { +async function freezeQueue(env: TrainEnv, t: TrainRecord, reason: string, mainSha: string, states: ReadonlyMap): Promise { const now = nowIso(); - // A slot GitHub reports MERGED is merged, verified or not: the freeze must not list it as unmerged, or the rebuild after the repair would seat it again. const unverified = t.slots.filter(s => !t.merged.some(m => m.pr === s.pr)); - const landed = unverified.filter(s => prState(env.ctx, s.pr) === 'MERGED').map(s => s.pr); + const landed = unverified.filter(s => states.get(s.pr) === 'MERGED').map(s => s.pr); + const unreadable = unverified.filter(s => states.get(s.pr) === 'unreadable').map(s => s.pr); + const prs = (list: string[]) => list.map(p => '#' + p).join(', '); const f: FreezeRecord = { schema: 1, kind: 'freeze', repo_key: env.repo.key, train: t.nonce, merged: [...t.merged.map(m => m.pr), ...landed], - unmerged: unverified.filter(s => !landed.includes(s.pr)).map(s => ({ pr: s.pr, head: s.head })), + unmerged: unverified.filter(s => !landed.includes(s.pr) && !unreadable.includes(s.pr)).map(s => ({ pr: s.pr, head: s.head })), main: { sha: mainSha, tree: git(env.ctx.repoRoot, ['rev-parse', `${mainSha}^{tree}`]).out }, - reason: landed.length ? `${reason}; ${landed.map(p => '#' + p).join(', ')} merged on GitHub without a verified tree` : reason, backstop: null, repair: null, + reason: [reason, landed.length ? `${prs(landed)} merged on GitHub without a verified tree` : '', unreadable.length ? `${prs(unreadable)} unreadable on GitHub, kept out of unmerged until read` : ''].filter(Boolean).join('; '), + backstop: null, repair: null, created_at: now, cleared_at: null, cleared_by: null, updated_at: now, }; persist(env.ctx.repoRoot, [f], { remote: env.remote }); @@ -148,12 +154,13 @@ async function freezeQueue(env: TrainEnv, t: TrainRecord, reason: string, mainSh */ 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 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}`); + // One GitHub read per unverified slot decides both the freeze and its record. + const states = new Map(t.slots.filter(s => !t.merged.some(m => m.pr === s.pr)).map(s => [s.pr, prState(env.ctx, s.pr)] as const)); + const landed = t.merged.length > 0 || [...states.values()].includes('MERGED'); + const unreadable = landed ? [] : [...states].filter(([, state]) => state === 'unreadable').map(([pr]) => `#${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); + const f = activeFreeze(env.repo.keys) ?? await freezeQueue(env, t, message, mainSha, states); return outFailed({ code, message: `${message}; the queue is frozen` }, [...lines, ...frozenLines(f)], `${Q} train build --repair --prs @`, run); } diff --git a/test/helpers/ship-queue-train-fixture.ts b/test/helpers/ship-queue-train-fixture.ts index 04d85ef65..7a7ffd589 100644 --- a/test/helpers/ship-queue-train-fixture.ts +++ b/test/helpers/ship-queue-train-fixture.ts @@ -47,6 +47,8 @@ export interface ShimData { ci: Record; rules: Record; artifact: string; log: Array>; /** Commit statuses (the legacy status API) at a sha, beside the check runs in `ci`. */ statuses?: Record>; + /** Per PR, how many more `gh pr view` calls answer before the PR reads as not found (a GitHub answer that changes between two reads). */ + vanish?: Record; } export interface TrainFixture { repo: FixtureRepo; state: string; dataFile: string; env: Record; read: () => ShimData; write: (fn: (d: ShimData) => void) => void } @@ -156,7 +158,7 @@ function shimSource(): string { "const rev = r => { const x = cp.spawnSync('git', ['--git-dir', d.origin, 'rev-parse', '--verify', '--quiet', r], { encoding: 'utf8', timeout: 30000 }); return x.status === 0 ? x.stdout.trim() : ''; };", "d.log.push({ cmd: a.slice(0, 2).join(' '), args: a });", "if (a[0] === 'repo' && a[1] === 'view') out({ nameWithOwner: d.repo });", - "if (a[0] === 'pr' && a[1] === 'view') { const p = d.prs[a[2]]; if (!p) fail('no pull request found'); out(p); }", + "if (a[0] === 'pr' && a[1] === 'view') { const p = d.prs[a[2]]; const left = d.vanish && d.vanish[a[2]]; if (left !== undefined) { if (left <= 0) fail('no pull request found'); d.vanish[a[2]] = left - 1; } if (!p) fail('no pull request found'); out(p); }", "if (a[0] === 'workflow' && a[1] === 'run') {", " const w = a[2]; const ref = opt('--ref'); const inputs = {};", " a.forEach((x, i) => { if (x === '-f') { const kv = a[i + 1]; const k = kv.slice(0, kv.indexOf('=')); inputs[k] = kv.slice(kv.indexOf('=') + 1); } });", diff --git a/test/ship-queue-train.test.ts b/test/ship-queue-train.test.ts index cbf3f3a3d..7aa71fd8b 100644 --- a/test/ship-queue-train.test.ts +++ b/test/ship-queue-train.test.ts @@ -1051,6 +1051,24 @@ describe('verify, freeze, repair and clear', () => { expect(JSON.parse(fs.readFileSync(path.join(fx.state, 'ship-queue', 'freeze', `${repoKey(fx)}.json`), 'utf8'))).toMatchObject({ merged: ['1'], unmerged: [{ pr: '2', head: h2 }] }); }, 300_000); + // Value: protects=the freeze record is built from the same GitHub read that decided to freeze, so a slot that read MERGED stays merged even when GitHub stops answering a moment later; fails_when=freezeQueue reads GitHub again and files anything but an exact MERGED under unmerged (the misfile that listed train 24's merged #3453 as unmerged, GSTA-129 defect 1); why_new=the test above holds GitHub's answer fixed across the verify; seam=the fixture's gh pr view vanishing after one answer; no_claim=the backstop's outcome + test("a slot that read MERGED when the verify decided to freeze is recorded as merged even when GitHub stops answering before the record is written", () => { + const fx = make(); + const { h2, nonce } = twoPrTrain(fx); + expect(queue(fx, ['train', 'verdict', '--train', nonce]).status).toBe(EXIT.ok); + const t = trainJson(fx, nonce); + const coord = coordinatorClone(fx.repo); + 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'; + 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'; d.vanish = { '1': 1 }; }); + const frozen = queue(fx, ['verify', '--pr', '1', '--train', nonce], {}, coord); + expect(frozen.status).toBe(EXIT.fail); + expect(frozen.err).toContain('"- added after the gate"); the queue is frozen;'); + expect(fx.read().vanish).toEqual({ '1': 0 }); + expect(line(frozen.out, 'freeze:')).toMatch(/; #1 merged on GitHub without a verified tree; unmerged #2; repair none admitted$/); + expect(JSON.parse(fs.readFileSync(path.join(fx.state, 'ship-queue', 'freeze', `${repoKey(fx)}.json`), 'utf8'))).toMatchObject({ merged: ['1'], unmerged: [{ pr: '2', head: h2 }] }); + }, 300_000); + // Value: protects=a post-merge CHANGELOG difference is excused only when main's parent is T modulo stamps, so a foreign commit between the verified head and the squash freezes; fails_when=published() skips or inverts the parent check; why_new=the two tests above reach the CHANGELOG rule only with a parent that is T1 modulo stamps; seam=none; no_claim=a parent differing from T1 in the CHANGELOG alone test("a post-merge CHANGELOG difference is not excused when main's parent is not the verified prefix modulo stamps: the queue freezes naming the parent", () => { const fx = make();