diff --git a/bin/gstack-doctor-components.sh b/bin/gstack-doctor-components.sh index af036cfe7..1a29a9c1d 100644 --- a/bin/gstack-doctor-components.sh +++ b/bin/gstack-doctor-components.sh @@ -317,6 +317,7 @@ gstack_result_anchor() { QUEUE_SCREEN_NOT_RUN) printf '%s' 'queue-screen-not-run' ;; QUEUE_TRAIN_STACK_RED) printf '%s' 'queue-train-stack-red' ;; QUEUE_POST_MERGE_RED) printf '%s' 'queue-post-merge-red' ;; + QUEUE_READY_NOTE_UNPUSHED) printf '%s' 'queue-ready-note-unpushed' ;; QUEUE_SCREEN_RED) printf '%s' 'queue-screen-red' ;; READY_SCREEN_CHOICE_REQUIRED) printf '%s' 'ready-screen-choice-required' ;; PREGATE_TIMING_ASSERTION) printf '%s' 'pregate-timing-assertion' ;; diff --git a/changelog.d/capy-make-ship-queue-ready.md b/changelog.d/capy-make-ship-queue-ready.md new file mode 100644 index 000000000..c82f422aa --- /dev/null +++ b/changelog.d/capy-make-ship-queue-ready.md @@ -0,0 +1,4 @@ +--- +important: blocker +--- +- **Under a merge-train policy, `gstack-ship-queue ready` no longer reports READY when its note never reached origin.** `land --pin` reads the READY note from origin's `refs/notes/gstack-trains`, but `ready` exited 0 even when it couldn't push that note, so a lane sent READY and the land was refused with `QUEUE_NOT_READY` (#4245 lost a land round this way). After pushing, `ready` now fetches origin's notes ref back and prints `notes pushed` only when origin holds the note. If three tries fail, or the push exits 0 and origin still lacks the note, it refuses with `QUEUE_READY_NOTE_UNPUSHED` and keeps the local receipt, so rerunning the same `ready` command once the push works records it (GST-2242). diff --git a/crew/lib/ship-queue-train-ready.ts b/crew/lib/ship-queue-train-ready.ts index 29b8d7e78..8a37d9d9f 100644 --- a/crew/lib/ship-queue-train-ready.ts +++ b/crew/lib/ship-queue-train-ready.ts @@ -174,6 +174,19 @@ async function dispatchScreen(env: TrainEnv, o: ReadyOptions, slug: string | nul return { choice: { choice: 'screen', policy, nonce, run: found.databaseId, merge_sha: mergeSha }, lines: [`SCREEN_DISPATCH: ${dispatch.workflow} run ${found.databaseId} ${found.url} nonce ${nonce}${notFollowing}`] }; } +/** + * Record the READY receipt and require origin to hold its note: `land --pin` reads the note on origin, so a receipt + * written only here is not READY (reports/GSTA-461.md item 19). The local receipt stays for the rerun to push. + */ +function recordReceipt(env: TrainEnv, receipt: ReadyReceipt, lines: string[], again: string): TrainOut | null { + const file = readyPath(env.repo.key, receipt.pr, receipt.head); + const p = persist(env.ctx.repoRoot, [receipt], { remote: env.remote }); + lines.push(`READY_RECEIPT: ${file} (${p.detail})`); + if (p.pushed) return null; + return outRefused({ code: 'QUEUE_READY_NOTE_UNPUSHED', message: `the READY receipt for #${receipt.pr} at ${receipt.head.slice(0, 12)} is written locally, but ${env.remote} does not hold its note on refs/notes/gstack-trains (${p.detail}); land reads that note, so this head is not READY` }, + lines, `fix the notes push to ${env.remote}, then ${again}`, file); +} + const screenLine = (c: ScreenChoice) => `screen: ${c.choice}${c.policy ? ` policy=${c.policy} nonce=${c.nonce} run=${c.run ?? '-'}${c.verdict ? ` verdict=${c.verdict.outcome}` : ''}` : ''}`; /** @@ -214,8 +227,8 @@ function conditionalReady(env: TrainEnv, o: ReadyOptions, lines: string[]): Trai lines.unshift(`READY #${o.pr} ${head} CONDITIONAL`, 'screen: no-screen'); lines.push(`BASE_RED: ${red.cases.join(', ')} base=${red.base.slice(0, 12)} fixer=#${base_red.fixer} expires=${red.expires_at} (CI and the pregate ran on the broken base; the train gates every prefix and lands this PR only through an update commit with green checks)`); if (o.dryRun) return outOk([...lines, 'DRY-RUN: no READY receipt written'], `${Q} ready --pr ${o.pr} --no-screen --base-red --fixer ${base_red.fixer}`, 'none'); - const p = persist(ctx.repoRoot, [receipt], { remote: env.remote }); - lines.push(`READY_RECEIPT: ${readyPath(env.repo.key, o.pr, head)} (${p.detail})`); + const refused = recordReceipt(env, receipt, lines, `${Q} ready --pr ${o.pr} --no-screen --base-red --fixer ${base_red.fixer}`); + if (refused) return refused; return outOk(lines, `send the READY block; the coordinator builds the train with fixer #${base_red.fixer} at slot 1: ${Q} train build --prs ${base_red.fixer}@,${o.pr}@${head}`, readyPath(env.repo.key, o.pr, head)); } @@ -299,8 +312,8 @@ export async function trainReady(ctx: QueueContext, o: ReadyOptions): Promise { while (fixtures.length) cleanup(fixtures.pop()!.repo); }); + +function readyFixture() { + const fx = trainFixture(); + fixtures.push(fx); + const head = openPr(fx, '1'); + fx.write(d => { d.ci[head] = [{ status: 'completed', conclusion: 'success' }]; }); + const tree = runBin(fx.repo, 'gstack-wtree', []).out.trim(); + const pregate = path.join(fx.repo.root, 'pregate.json'); + fs.writeFileSync(pregate, JSON.stringify({ schema_version: 1, tree, head, verdict: 'pass', checks: [{ id: 'unit', status: 'pass' }] })); + return { fx, head, pregate }; +} + +function hook(fx: TrainFixture, name: 'pre-receive' | 'post-receive', body: string): void { + fs.writeFileSync(path.join(fx.repo.origin, 'hooks', name), `#!/bin/sh\n${body}\n`, { mode: 0o755 }); +} + +const remoteNotes = (fx: TrainFixture) => fx.repo.git('ls-remote', 'origin', NOTES).split(/\s+/)[0] ?? ''; +const localNotes = (fx: TrainFixture) => fx.repo.git('rev-parse', '--verify', '--quiet', NOTES); + +describe('ready refuses a READY note that never reached origin', () => { + // Value: protects=a lane never reports READY while origin lacks the note land reads; fails_when=ready ignores persist's pushed:false and exits 0 with "notes pushed"; why_new=no case pushes ready's note to a remote that rejects it; seam=a pre-receive hook in the fixture's bare origin; no_claim=GitHub's own rejection messages + test('origin rejects refs/notes/*: ready exits refused with QUEUE_READY_NOTE_UNPUSHED and prints no success', () => { + const { fx, head, pregate } = readyFixture(); + hook(fx, 'pre-receive', 'while read old new ref; do case "$ref" in refs/notes/*) echo "notes refused by fixture" >&2; exit 1;; esac; done; exit 0'); + const r = queue(fx, ['ready', '--pr', '1', '--pregate', pregate, '--no-screen']); + expect(r.status).toBe(EXIT.refused); + expect(r.err).toContain('(QUEUE_READY_NOTE_UNPUSHED)'); + expect(r.out).not.toContain('notes pushed'); + expect(line(r.out, 'GSTACK_RESULT:')).toContain('status=refused'); + expect(line(r.out, 'NEXT:')).toContain('gstack-ship-queue ready --pr 1 --no-screen'); + expect(remoteNotes(fx)).toBe(''); + expect(fs.existsSync(path.join(fx.state, 'ship-queue', 'ready', repoKey(fx), `1-${head}.json`))).toBe(true); + }, 120_000); + + // Value: protects=success is printed only after origin is read back holding the note; fails_when=a push that exits 0 without moving origin's ref passes as pushed; why_new=the case above covers a push git reports as failed; seam=a post-receive hook that deletes the ref after accepting it, standing in for a proxy that drops the update; no_claim=which proxy dropped #4245's push + test('a push that exits 0 but leaves origin without the note is refused too', () => { + const { fx, pregate } = readyFixture(); + hook(fx, 'post-receive', `git update-ref -d ${NOTES} 2>/dev/null; exit 0`); + const r = queue(fx, ['ready', '--pr', '1', '--pregate', pregate, '--no-screen']); + expect(r.status).toBe(EXIT.refused); + expect(r.err).toContain('(QUEUE_READY_NOTE_UNPUSHED)'); + expect(r.out).not.toContain('notes pushed'); + expect(remoteNotes(fx)).toBe(''); + }, 120_000); + + // Value: protects=the normal push still succeeds and origin holds exactly the local notes tip; fails_when=the confirmation read refuses a push that did land; why_new=hold case for the two refusals above; seam=none; no_claim=a real GitHub remote + test('hold: a normal push succeeds, prints notes pushed, and origin holds the local notes tip', () => { + const { fx, pregate } = readyFixture(); + const r = queue(fx, ['ready', '--pr', '1', '--pregate', pregate, '--no-screen']); + expect(r.status).toBe(EXIT.ok); + expect(line(r.out, 'READY_RECEIPT:')).toEndWith(`(notes pushed to origin ${NOTES})`); + expect(remoteNotes(fx)).toBe(localNotes(fx)); + }, 120_000); + + // Value: protects=rerunning ready once origin accepts notes pushes the kept receipt and succeeds; fails_when=the refusal's NEXT command cannot recover; why_new=the documented fix for QUEUE_READY_NOTE_UNPUSHED; seam=the pre-receive hook removed between runs; no_claim=none beyond the fixture + test('rerunning ready after origin accepts notes again pushes the note and succeeds', () => { + const { fx, pregate } = readyFixture(); + hook(fx, 'pre-receive', 'exit 1'); + expect(queue(fx, ['ready', '--pr', '1', '--pregate', pregate, '--no-screen']).status).toBe(EXIT.refused); + fs.rmSync(path.join(fx.repo.origin, 'hooks', 'pre-receive')); + const again = queue(fx, ['ready', '--pr', '1', '--pregate', pregate, '--no-screen']); + expect(again.status).toBe(EXIT.ok); + expect(remoteNotes(fx)).toBe(localNotes(fx)); + }, 120_000); +}); diff --git a/docs/ship-policy.md b/docs/ship-policy.md index 0912b6415..17eb8c464 100644 --- a/docs/ship-policy.md +++ b/docs/ship-policy.md @@ -790,7 +790,11 @@ narrow screen, except `setup`, which no paid run executes: a narrow screen derives it like any other file. The nonce key carries the purpose and the policy, so a repeat follows the same run and a broad screen never adopts a narrow one. The READY block gains `screen: …`, and the receipt is written -locally and to the notes ref. `--dry-run` prints the dispatch without sending +locally and to the notes ref. `READY_RECEIPT:` says `notes pushed` only after +`ready` reads origin's notes ref back and finds the note; after three tries +without it, `ready` refuses with +[`QUEUE_READY_NOTE_UNPUSHED`](troubleshooting.md#queue-ready-note-unpushed), +because `land` reads the note on origin. `--dry-run` prints the dispatch without sending it. Before dispatching, `ready` fetches the merge ref and requires its second parent to be the PR head and its first parent the current base: a missing, lagging or unfetchable merge ref (GitHub stops recomputing it while the PR diff --git a/docs/troubleshooting.md b/docs/troubleshooting.md index e6591d73a..3a4609292 100644 --- a/docs/troubleshooting.md +++ b/docs/troubleshooting.md @@ -6502,6 +6502,27 @@ CHANGELOG drops or renames a released heading (the message then names the CHANGELOG). `train prune --stale` deletes the scratch branches of merged or closed PRs. + +### `QUEUE_READY_NOTE_UNPUSHED` + +**Meaning.** `gstack-ship-queue ready` wrote the READY receipt in the local +state root, but origin does not hold its note on `refs/notes/gstack-trains`. +`ready` pushes the notes ref, then fetches it back and requires origin's tip +to be the local tip or to contain it. It tries three times, fetching and +merging origin's notes between tries. Either the push failed (a rejected ref, +credentials, the network) or it exited 0 and origin still read back without +the note, as when a proxy drops the update. `land --pin` reads the READY note +on origin and refuses the pin with `QUEUE_NOT_READY` without it, so `ready` refuses +here instead of printing `notes pushed`. Before this check, ready exited 0 and +the land failed instead (#4245). Don't send the READY block. + +**Fix.** The `READY_RECEIPT:` line ends with the push's error. Fix what +refused it, then run the `NEXT:` line, which is the same `ready` command +without `--pregate`, `--gate-exit` or `--gate-log`: add back any of those you +passed the first time. It keeps the first `ready_at`, follows a screen run already dispatched at the +same merge SHA, and pushes the kept receipt. To see the raw error, run +`git push origin refs/notes/gstack-trains:refs/notes/gstack-trains`. + ### `QUEUE_SCREEN_RED` diff --git a/lib/result-codes-release.ts b/lib/result-codes-release.ts index e5655583f..40db17e36 100644 --- a/lib/result-codes-release.ts +++ b/lib/result-codes-release.ts @@ -314,6 +314,11 @@ export const RELEASE_RESULT_CODES = { summary: 'a push workflow on the merged commit (free tests, the Windows subset, the main backstop) concluded failure, timed out or was cancelled after the train merged it; a neutral or skipped check is not red', fix: 'read the red run (gstack-ship-queue verify --post-merge prints one CHECK: line per workflow), repair on a new PR or roll back the merge; gstack-ship-queue train status names the freeze if the backstop filed one', }, + QUEUE_READY_NOTE_UNPUSHED: { + anchor: 'queue-ready-note-unpushed', + summary: 'ready wrote its READY receipt locally, but after three tries origin does not hold the note on refs/notes/gstack-trains (the push failed, or it exited 0 and origin read back without the note), so land would refuse the pin; the head is not READY', + fix: 'read the push error on the READY_RECEIPT line, fix what refused it (credentials, a hook or rule on refs/notes/*, the network), then rerun the printed gstack-ship-queue ready command, adding back any --pregate, --gate-exit or --gate-log you passed; it keeps ready_at and pushes the kept receipt', + }, QUEUE_SCREEN_RED: { anchor: 'queue-screen-red', summary: 'the PR head’s dispatched screen run finished RED (its gate binding says so; a case that failed twice is real), so the head is not READY and a train will not seat it', diff --git a/scripts/lib/windows-curation.ts b/scripts/lib/windows-curation.ts index d94c6db81..0851e3488 100644 --- a/scripts/lib/windows-curation.ts +++ b/scripts/lib/windows-curation.ts @@ -330,6 +330,10 @@ export const KNOWN_WINDOWS_INCOMPATIBLE: Array<{ file: string; reason: string }> file: 'crew/test/ship-queue-train.test.ts', reason: 'drives bin/gstack-ship-queue against the stateful #!/bin/sh gh shim in test/helpers/ship-queue-train-fixture.ts; on windows-latest spawnSync(\'gh\') resolves the runner\'s gh.exe instead of the shim (windows-free-shard run 39152702056)', }, + { + file: 'crew/test/ship-queue-ready-notes-push.test.ts', + reason: 'drives bin/gstack-ship-queue ready against the #!/bin/sh gh shim in test/helpers/ship-queue-train-fixture.ts, like crew/test/ship-queue-train.test.ts; on windows-latest spawnSync(\'gh\') resolves the runner\'s gh.exe instead (windows-scoped-shard run 42711409624)', + }, { file: 'crew/test/ship-queue-train-bisect.test.ts', reason: 'split from crew/test/ship-queue-train.test.ts (GSTA-151): drives bin/gstack-ship-queue against the same #!/bin/sh gh shim in test/helpers/ship-queue-train-fixture.ts',