mirror of
https://github.com/garrytan/gstack.git
synced 2026-11-05 16:34:00 +01:00
ship-queue verify: build the freeze record from the one GitHub read that decided to freeze
freezeQueue took a second prState read per unverified slot and filed anything but an exact MERGED under unmerged, so a slot that read MERGED at stopped() and was unreadable a moment later was recorded as unmerged (the record half of train 24's #3453 misfile). stopped() now reads each unverified slot once and passes those states in; a MERGED slot stays merged, and a slot unreadable beside a merged one is kept out of unmerged and named in the reason. The fixture's gh shim gains a per-PR vanish count so a test can change GitHub's answer between two reads. Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com>
This commit is contained in:
1 parent
73a678eb89
commit
5905c768f7
6 files changed
+49
-13
No files matched your search
+1
-1
@@ -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<i> modulo stamps, or differs from T<i> 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 <sha12>'s CHANGELOG.md plus #<n>'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 #<n> 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<i> modulo stamps, or differs from T<i> 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 <sha12>'s CHANGELOG.md plus #<n>'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 #<n> 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 (<file>: 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 <since>..<base> 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 <version> entry below: "<old>" should read "<new>".` (a dropped line "no longer applies", a line added between the parent's "Addition to the <version> 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.
|
||||
|
||||
|
||||
+5
-1
@@ -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 #<n> merged on GitHub could not be read")
|
||||
and asks to be rerun once `gh pr view` answers; an unreadable state is never
|
||||
|
||||
@@ -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
|
||||
(`#<n> 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.
|
||||
|
||||
|
||||
@@ -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<FreezeRecord> {
|
||||
async function freezeQueue(env: TrainEnv, t: TrainRecord, reason: string, mainSha: string, states: ReadonlyMap<string, string>): Promise<FreezeRecord> {
|
||||
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<TrainOut> {
|
||||
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 <repair-pr>@<sha>`, run);
|
||||
}
|
||||
|
||||
|
||||
@@ -47,6 +47,8 @@ export interface ShimData {
|
||||
ci: Record<string, unknown[]>; rules: Record<string, Rule>; artifact: string; log: Array<Record<string, unknown>>;
|
||||
/** Commit statuses (the legacy status API) at a sha, beside the check runs in `ci`. */
|
||||
statuses?: Record<string, Array<{ state: string; context: string }>>;
|
||||
/** 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<string, number>;
|
||||
}
|
||||
|
||||
export interface TrainFixture { repo: FixtureRepo; state: string; dataFile: string; env: Record<string, string>; 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); } });",
|
||||
|
||||
@@ -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<i-1> 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();
|
||||
|
||||
Reference in new issue
Block a user