mirror of
https://github.com/garrytan/gstack.git
synced 2026-10-31 14:06:50 +01:00
fix: pre-landing review fixes (runnable clear-time NEXT, stale exit only on a fetched main, open-slot freeze control, LF size-ledger fixture)
- verify's clear-time NEXT names gh pr view <n> --json state,headRefOid per unreadable slot instead of a pasteable #n@sha that views the current branch's PR - QUEUE_TRAIN_STALE needs a fetched main (never under --no-fetch) and reports a freeze restored from the notes ref instead of saying the queue is not frozen - freezeQueue docstring matches the unreadable field - new test: a train with a slot still OPEN freezes even when another train verified main's sha - test/size-ledger.test.ts fixture repo sets core.autocrlf false (Windows Free Tests shard 4 counted CRLF bytes) Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com>
This commit is contained in:
1 parent
5a94db36b1
commit
0f6ca6f254
5 files changed
+49
-16
No files matched your search
+6
-3
@@ -645,15 +645,18 @@ 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
|
||||
left out of `unmerged`, recorded under `unreadable` and printed on an
|
||||
`unreadable:` line; the `NEXT:` after the repair names it (`gh pr view
|
||||
#<n>@<sha> (unreadable at the freeze); seat each that is OPEN in a train`).
|
||||
`unreadable:` line; the `NEXT:` after the repair names it (`gh pr view <n>
|
||||
--json state,headRefOid (unreadable at the freeze: #<n>@<sha>); seat each
|
||||
that is OPEN in a train`).
|
||||
When every slot of the train already merged on GitHub and main's SHA is a
|
||||
prefix another train's verify confirmed (read after the notes ref, so a
|
||||
machine that never ran that train still sees it), the mismatch says nothing
|
||||
about this train: verify refuses with `QUEUE_TRAIN_STALE` and `NEXT:
|
||||
gstack-ship-queue train prune --stale --train <nonce>`, freezing nothing and
|
||||
dispatching no backstop (train 24's record froze the queue that way on a main
|
||||
train 28 had verified, #3519). When
|
||||
train 28 had verified, #3519). Only a verify that fetched main takes this
|
||||
exit: under `--no-fetch` the local main may be older than GitHub's, so the
|
||||
mismatch freezes as before. 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
|
||||
|
||||
@@ -4358,8 +4358,10 @@ did both on a main train 28 had verified, #3519). Other trains are read after
|
||||
the notes ref, so a machine that never ran the later train still sees it.
|
||||
|
||||
**Fix.** `gstack-ship-queue train prune --stale --train <nonce>` settles the
|
||||
record. When some slot is not merged, or no train verified main's SHA, verify
|
||||
freezes as before (`QUEUE_TRAIN_FROZEN`).
|
||||
record. When some slot is not merged, no train verified main's SHA, or verify
|
||||
ran with `--no-fetch`, verify freezes as before (`QUEUE_TRAIN_FROZEN`). A queue
|
||||
another machine already froze stays frozen; the refusal prints its freeze
|
||||
lines.
|
||||
|
||||
<a id="queue-train-frozen"></a>
|
||||
### `QUEUE_TRAIN_FROZEN`
|
||||
|
||||
@@ -114,9 +114,10 @@ function published(env: TrainEnv, t: TrainRecord, i: number, main: string): { sa
|
||||
* 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
|
||||
* (nothing reads it again; the operator seats it by hand once gh answers), so
|
||||
* the rebuild after the repair never seats a PR that may already be in.
|
||||
* that could not be read is left out of `unmerged`, recorded under
|
||||
* `unreadable` and named in the reason and in the NEXT after the repair (the
|
||||
* operator seats it by hand once gh answers), 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, states: ReadonlyMap<string, string>): Promise<FreezeRecord> {
|
||||
const now = nowIso();
|
||||
@@ -169,7 +170,7 @@ function verifiedElsewhere(env: TrainEnv, t: TrainRecord, mainSha: string): stri
|
||||
* 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<TrainOut> {
|
||||
async function stopped(env: TrainEnv, t: TrainRecord, code: 'QUEUE_TRAIN_TREE_MISMATCH' | 'QUEUE_TRAIN_MOVED', message: string, mainSha: string, lines: string[], fetched: boolean): Promise<TrainOut> {
|
||||
const run = trainFile(t.nonce);
|
||||
// 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));
|
||||
@@ -177,8 +178,11 @@ async function stopped(env: TrainEnv, t: TrainRecord, code: 'QUEUE_TRAIN_TREE_MI
|
||||
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 later = t.slots.every(s => t.merged.some(m => m.pr === s.pr) || states.get(s.pr) === 'MERGED') ? verifiedElsewhere(env, t, mainSha) : null;
|
||||
if (later) return outRefused({ code: 'QUEUE_TRAIN_STALE', message: `${message}; every slot of train ${t.nonce} merged on GitHub and main ${mainSha.slice(0, 12)} is ${later}, so this record is stale and the queue is not frozen` }, lines, `${Q} train prune --stale --train ${t.nonce}`, run);
|
||||
const later = fetched && t.slots.every(s => t.merged.some(m => m.pr === s.pr) || states.get(s.pr) === 'MERGED') ? verifiedElsewhere(env, t, mainSha) : null;
|
||||
if (later) {
|
||||
const held = activeFreeze(env.repo.keys);
|
||||
return outRefused({ code: 'QUEUE_TRAIN_STALE', message: `${message}; every slot of train ${t.nonce} merged on GitHub and main ${mainSha.slice(0, 12)} is ${later}, so this record is stale and ${held ? `freezes nothing (the queue is already frozen by train ${held.train})` : 'the queue is not frozen'}` }, held ? [...lines, ...frozenLines(held)] : lines, `${Q} train prune --stale --train ${t.nonce}`, run);
|
||||
}
|
||||
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);
|
||||
}
|
||||
@@ -210,13 +214,13 @@ export async function verifyTrain(ctx: QueueContext, o: { pr: string; train: str
|
||||
if (post.same) return confirmMerged(env, t, o.pr, main, after.index, [...lines, `main: ${main} = T${after.index} (${post.detail})`]);
|
||||
const pre = sameModuloStamps(env, before.commit, main);
|
||||
if (!pre.same) {
|
||||
return stopped(env, t, 'QUEUE_TRAIN_TREE_MISMATCH', `main ${main.slice(0, 12)} is neither T${before.index} nor T${after.index} modulo stamps (vs T${after.index}: ${post.detail})`, main, lines);
|
||||
return stopped(env, t, 'QUEUE_TRAIN_TREE_MISMATCH', `main ${main.slice(0, 12)} is neither T${before.index} nor T${after.index} modulo stamps (vs T${after.index}: ${post.detail})`, main, lines, !o.noFetch);
|
||||
}
|
||||
const headRef = o.noFetch ? '' : (git(ctx.repoRoot, ['fetch', '--no-tags', '-q', env.remote, `+refs/pull/${o.pr}/head:refs/gstack/queue/pr-${o.pr}`]).ok ? `refs/gstack/queue/pr-${o.pr}` : '');
|
||||
const head = headRef ? git(ctx.repoRoot, ['rev-parse', headRef]).out : t.slots[i]!.head;
|
||||
const pinned = head === t.slots[i]!.head ? { same: true, detail: 'the pinned head' } : sameModuloStamps(env, t.slots[i]!.head, head);
|
||||
const stampOnly = pinned.same ? pinned : mergedIn(env, t, i, head, main);
|
||||
if (!stampOnly.same) return stopped(env, t, 'QUEUE_TRAIN_MOVED', `#${o.pr} moved past its pinned head ${t.slots[i]!.head.slice(0, 12)} and is not T${after.index} modulo stamps (${head.slice(0, 12)}: ${stampOnly.detail}; against the pin it ${pinned.detail})`, main, lines);
|
||||
if (!stampOnly.same) return stopped(env, t, 'QUEUE_TRAIN_MOVED', `#${o.pr} moved past its pinned head ${t.slots[i]!.head.slice(0, 12)} and is not T${after.index} modulo stamps (${head.slice(0, 12)}: ${stampOnly.detail}; against the pin it ${pinned.detail})`, main, lines, !o.noFetch);
|
||||
const info = prInfo(ctx, o.pr);
|
||||
if ('error' in info) return outRefused({ code: 'QUEUE_PROOF_MISSING', message: info.error }, lines, `${Q} verify --pr ${o.pr} --train ${t.nonce}`, run);
|
||||
const shipped = bindReceipt(info.body, o.pr, [t.slots[i]!.head, head]);
|
||||
@@ -241,8 +245,8 @@ function confirmMerged(env: TrainEnv, t: TrainRecord, pr: string, main: string,
|
||||
persist(env.ctx.repoRoot, [freeze], { remote: env.remote });
|
||||
lines.push(`FREEZE: cleared (${freeze.cleared_by})`);
|
||||
const rebuild = freeze.unmerged.length ? `${Q} train build --prs ${pinList(freeze.unmerged)}` : '';
|
||||
const unread = (freeze.unreadable ?? []).map(u => `#${u.pr}@${u.head.slice(0, 12)}`);
|
||||
const check = unread.length ? `gh pr view ${unread.join(', ')} (unreadable at the freeze); seat each that is OPEN in a train` : '';
|
||||
const unread = freeze.unreadable ?? [];
|
||||
const check = unread.length ? `${unread.map(u => `gh pr view ${u.pr} --json state,headRefOid`).join(' && ')} (unreadable at the freeze: ${unread.map(u => `#${u.pr}@${u.head.slice(0, 12)}`).join(', ')}); seat each that is OPEN in a train` : '';
|
||||
return outOk(lines, [rebuild, check].filter(Boolean).join('; then ') || 'none', run);
|
||||
}
|
||||
return outOk(lines, nextFor(t), run);
|
||||
|
||||
@@ -1092,7 +1092,7 @@ describe('verify, freeze, repair and clear', () => {
|
||||
publishMain(fx, trainJson(fx, rn).prefixes[1].commit, '1.2.6.0');
|
||||
const cleared = queue(fx, ['verify', '--pr', '3', '--train', rn], {}, coord);
|
||||
expect(cleared.out).toMatch(/^FREEZE: cleared /m);
|
||||
expect(next(cleared.out)).toBe(`NEXT: gh pr view #2@${h2.slice(0, 12)} (unreadable at the freeze); seat each that is OPEN in a train`);
|
||||
expect(next(cleared.out)).toBe(`NEXT: gh pr view 2 --json state,headRefOid (unreadable at the freeze: #2@${h2.slice(0, 12)}); seat each that is OPEN in a train`);
|
||||
}, 300_000);
|
||||
|
||||
// Value: protects=verify on a train whose every slot already merged, against a main another train verified, refuses as stale instead of freezing the queue and dispatching a backstop (#3519 c: train 24's record froze the queue at 9ccb5233, T1 of verified train 28, run 40586582766); fails_when=stopped() freezes when every slot merged and main's sha is another train's verified prefix, or refuses without that proof; why_new=every freeze test above has an unmerged or unverified main; seam=a second train record written into the state root; no_claim=what prune --stale then does (covered by the prune tests)
|
||||
@@ -1127,6 +1127,29 @@ describe('verify, freeze, repair and clear', () => {
|
||||
expect(line(frozen.out, 'QUEUE_TRAIN_FROZEN:')).toMatch(/^QUEUE_TRAIN_FROZEN: train \S+ merged #1,#2; main /);
|
||||
}, 300_000);
|
||||
|
||||
// Value: protects=a train with a slot still OPEN on GitHub freezes on a mismatch even when main's sha is another train's verified prefix; fails_when=the every-slot-merged guard before verifiedElsewhere is dropped or weakened, so a live train refuses as stale and skips the freeze; why_new=the test above only covers an all-merged train; seam=none; no_claim=prune behavior
|
||||
test("a train with a slot still open freezes on the mismatch even when another train verified main's sha", () => {
|
||||
const fx = make();
|
||||
const { 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 v = (pr: string) => queue(fx, ['verify', '--pr', pr, '--train', nonce], {}, coord);
|
||||
publishMain(fx, t.prefixes[1].commit, '1.2.4.0');
|
||||
expect(v('1').status).toBe(EXIT.ok);
|
||||
const later = publishMain(fx, t.prefixes[2].commit, '1.2.5.0', { file: 'stray.txt', text: 'a later train\n' });
|
||||
const tree = fx.repo.git('rev-parse', `${later}^{tree}`);
|
||||
const otherNonce = `${nonce}-later`;
|
||||
const other = { ...t, nonce: otherNonce, state: 'merging', prefixes: [t.prefixes[0], { index: 1, commit: later, tree, pr: '9' }], merged: [{ pr: '9', sha: later, tree, at: '2026-10-18T13:00:00.000Z' }] };
|
||||
fs.mkdirSync(path.join(fx.state, 'ship-queue', 'trains', otherNonce), { recursive: true });
|
||||
fs.writeFileSync(path.join(fx.state, 'ship-queue', 'trains', otherNonce, 'train.json'), JSON.stringify(other));
|
||||
const partial = v('2');
|
||||
expect(partial.status).toBe(EXIT.fail);
|
||||
expect(partial.err).not.toContain('(QUEUE_TRAIN_STALE)');
|
||||
expect(line(partial.out, 'QUEUE_TRAIN_FROZEN:')).toMatch(/^QUEUE_TRAIN_FROZEN: train \S+ merged #1; main /);
|
||||
expect(fs.existsSync(path.join(fx.state, 'ship-queue', 'freeze', `${repoKey(fx)}.json`))).toBe(true);
|
||||
}, 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();
|
||||
|
||||
@@ -45,6 +45,7 @@ function mainRepo(): { root: string; main: string } {
|
||||
const root = fs.mkdtempSync(path.join(os.tmpdir(), 'size-ledger-'));
|
||||
roots.push(root);
|
||||
git(root, 'init', '-q', '-b', 'main');
|
||||
git(root, 'config', 'core.autocrlf', 'false');
|
||||
write(root, 'demo/sections/a.md', '# A\n');
|
||||
write(root, 'demo/sections/b.md', '# B\n');
|
||||
write(root, 'demo/SKILL.md', '## Preamble\n' + 'x'.repeat(MAIN_UNION - 8 - 13) + '\n');
|
||||
|
||||
Reference in new issue
Block a user