From ec438fe8b1b583cf1cf4fccb2ded3d1edef3a6b0 Mon Sep 17 00:00:00 2001 From: garrytan Date: Mon, 26 Oct 2026 02:07:23 +0000 Subject: [PATCH] fix: pre-landing review fixes (ready at the stamp commit, screen on a stale merge ref, ship-run.json) - ready accepts the pin's receipt at a slot's stamp commit (plus the slot-2 merge of main), as verify --train already did; a real edit stays STALE - ready --screen refuses QUEUE_SCREEN_STALE_REF when refs/pull//merge is missing or merges an older head; a finished screen that planned 0 cases is never followed, and train build seats that slot unscreened (GSTA-95 on #3431) - gstack-ship-receipt result writes ship-run.json (a /ship record, not a run v1 manifest), validates --head, and prints status=incomplete on gh failures - bindReceipt takes an optional pr and renders one-line SHIP_RECEIPT lines - tests for every state-matrix cell the review named; the train fixture's merge ref is a real merge commit Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> --- CHANGELOG.md | 11 +-- bin/gstack-doctor-components.sh | 1 + bin/gstack-ship-receipt | 22 ++++-- bin/gstack-skill-start | 2 +- docs/ship-policy.md | 24 +++++- docs/troubleshooting.md | 22 ++++++ docs/unattended.md | 16 ++-- lib/result-codes-release.ts | 5 ++ lib/ship-queue-train-github.ts | 15 ++++ lib/ship-queue-train-ready.ts | 15 +++- lib/ship-queue-train.ts | 15 +++- lib/ship-queue.ts | 34 ++++++++- lib/ship-receipt.ts | 13 ++-- test/gstack-ship-queue.test.ts | 37 +++++++++ test/gstack-ship-receipt.test.ts | 97 +++++++++++++++++++++++- test/helpers/ship-queue-train-fixture.ts | 10 ++- test/ship-queue-train.test.ts | 73 +++++++++++++++++- 17 files changed, 368 insertions(+), 44 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index eeecf5921..e9389a9ff 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,19 +2,20 @@ ## [Unreleased] -**The merge queue now refuses a PR that never ran /ship.** Garry, 2026-10-25: "we should really be using the gstack ship skill before merging PRs." `gstack-ship-queue ready` printed `SHIP_RECEIPT: none in the PR body` as information and returned ok, so every PR in the pool was READY'd without a /ship run. The receipt /ship writes into the PR body is now a precondition at every queue step, with no flag around it. +**The merge queue now refuses a PR that never ran /ship.** Garry, 2026-10-25: "we should really be using the gstack ship skill before merging PRs." `gstack-ship-queue ready` printed `SHIP_RECEIPT: none in the PR body` as information and returned ok, so every PR in the pool was READY'd without a /ship run. The receipt /ship writes into the PR body is now a precondition at `ready`, `train build` and `verify --train`, with no flag around it. ### Changed -- **`gstack-ship-queue ready`** refuses with `QUEUE_RECEIPT_MISSING` when the PR body has no valid ```gstack-ship-receipt``` block and with `QUEUE_RECEIPT_STALE` when the receipt's `head` is not the head being READY'd (or its `pr` is another PR). The informational `SHIP_RECEIPT:` line still prints in the block. +- **`gstack-ship-queue ready`** refuses with `QUEUE_RECEIPT_MISSING` when the PR body has no valid ```gstack-ship-receipt``` block and with `QUEUE_RECEIPT_STALE` when the receipt's `head` is not the head being READY'd (or its `pr` is another PR). The re-READY after a slot's stamp push accepts the receipt written for the pin when HEAD is the pin plus stamp files, or (from slot 2 on) the pin merged with main plus stamp files. The informational `SHIP_RECEIPT:` line still prints in the block, and it is one line whatever the receipt's strings contain (control characters are collapsed, so a crafted `base` cannot add a forged `MERGE_CONDITION:` line to `verify --train`'s output). +- **`gstack-ship-queue ready --screen` refuses a stale merge ref** with the new `QUEUE_SCREEN_STALE_REF` (GSTA-95 on #3431): GitHub stops recomputing `refs/pull//merge` while a PR conflicts with the base (a CHANGELOG-only conflict after a stamp counts), so a screen dispatched there planned 0 cases, reported success and `ready` printed `SCREEN_DISPATCH:` as if a screen ran. `ready` now compares the merge ref's head parent with the PR head before dispatching (a missing merge ref carries the same code), never adopts a finished screen whose record says it planned 0 cases, and `train build` prints `SCREEN: # run planned 0 cases; counted as not-run` and seats that slot unscreened. - **`gstack-ship-queue train build`** refuses a pin whose PR has no current receipt with the same codes, naming the PR, before any write, push or dispatch, `--dry-run` included. **`verify --train`** refuses the slot the same way before `MERGE_CONDITION`, and accepts a receipt written for the slot's pinned head when the current head is its stamp commit (the stamp changes only stamp files, or merges main into the pin from slot 2 on), since no /ship runs on a stamp. -- **`gstack-ship-receipt result`** (new) is /ship's terminal step: it binds the posted receipt to HEAD, writes `/ship/runs/-/{run.json,ship-receipt.md}` and prints `GSTACK_RESULT: skill=ship status=complete run=`, or `status=refused run=-` with the receipt code, so a parent's merge check reads `run.json` instead of parsing the PR body. The ship skill's new Step 22 runs it after the pr-body and handoff sections. +- **`gstack-ship-receipt result`** (new) is /ship's terminal step: it binds the posted receipt to HEAD, writes `/ship/runs/-/{ship-run.json,ship-receipt.md}` (a /ship record, not a `run` v1 manifest: `gstack-artifact validate` does not apply) and prints `GSTACK_RESULT: skill=ship status=complete run=`, `status=refused run=-` with the receipt code, or `status=incomplete run=-` when gh cannot read the PR, so a parent's merge check reads `ship-run.json` instead of parsing the PR body. The ship skill's new Step 22 runs it after the pr-body and handoff sections. - **Unattended /ship may update an existing PR's body.** `gstack-skill-start`'s unattended block and `UNATTENDED_SHIP:` line, the ship skill's Step 18 and `docs/unattended.md` now say: when an open PR for the branch exists and HEAD is its pushed head, Step 18 takes the pr-body section's existing-PR path (body with the receipt first, and title); it never runs `pr-create` and never pushes a commit; anything else takes the handoff section. Before this the block said the receipt lived only in `.gstack/tmp/ship-receipt.md` while rule 22 and `ready` read it from the PR body. - **`INSTALL_UNVERIFIED` with no verdict names the directory to install from.** An installer run from the skill's own folder records a verdict for no project, so /ship launched from the project found none and the fix said only "run the installer on this machine" (GSTA-85). Both twins (`lib/install-verdict.ts`, `bin/gstack-install-verdict.sh`) now say `run … scripts/install.sh --for ship from : the verdict is keyed by the project directory the installer runs in`. -- **Docs:** `docs/ship-policy.md#ship-before-ready` (the lane runs /ship before every READY and after every push; the stamp keeps the pin's receipt), the two codes in `docs/troubleshooting.md`, `docs/unattended.md` (/ship now prints the result line), and the ship skill's pr-body section says what refuses without a receipt. +- **Docs:** `docs/ship-policy.md#ship-before-ready` (the lane runs /ship before every READY and after every push; the stamp keeps the pin's receipt; the receipt is a process gate, not an authenticity check), the three codes in `docs/troubleshooting.md`, `docs/unattended.md` (/ship now prints the result line), and the ship skill's pr-body section says what refuses without a receipt. - **Gate edit (size budgets):** `test/helpers/carve-guards.ts` raises ship's `maxSkeletonBytes` 81,000 → 81,600 and `maxSizeRatio` 1.6690 → 1.6740 for Step 22 and the Step 18 routing, offset in part by tightening Steps 14, 16, 18 and 20 prose (measured skeleton 81,535, union 314,027; the 2026-10-14 tightening had left 2 bytes of headroom). No threshold, trial count or case kind changed. ### Tests -- `test/ship-queue-train.test.ts`: READY, train build (dry run and live) and verify --train refuse a missing, invalid, other-head and other-PR receipt and admit the current one; the pinned-head receipt admits a stamp commit. `test/gstack-ship-queue.test.ts` covers the per-PR READY block. `test/gstack-ship-receipt.test.ts` covers `result` from a PR and from the handoff file. The train fixture's `openPr` now writes a receipt for the head it opens (`shipped: false` leaves the body empty). `test/install-verdict.test.ts` pins the GSTA-85 fix in both twins; `test/gstack-skill-start.test.ts` pins the new unattended /ship block. +- `test/ship-queue-train.test.ts`: READY, train build (dry run and live) and verify --train refuse a missing, invalid, other-head and other-PR receipt and admit the current one; the pinned-head receipt admits a stamp commit; verify refuses `QUEUE_PROOF_MISSING` when gh cannot read the PR; `--screen` refuses a lagging or missing merge ref and never follows a 0-case screen. The train fixture's `refs/pull//merge` is now a real merge commit (`mergeRefCommit`), as GitHub keeps it. `test/gstack-ship-queue.test.ts`: `ready` at a stamp commit (slot 1, and slot 2 with main merged in) accepts the pin's receipt. `test/gstack-ship-receipt.test.ts`: the `bindReceipt` head-matching table (12-character floor, other PR, one-line `SHIP_RECEIPT:`), `result --body` refusals, `--head`, a non-git cwd and gh failures. The train fixture's `openPr` now writes a receipt for the head it opens (`shipped: false` leaves the body empty). `test/install-verdict.test.ts` pins the GSTA-85 fix in both twins; `test/gstack-skill-start.test.ts` pins the new unattended /ship block. ## [1.91.224.0] - 2026-10-25 diff --git a/bin/gstack-doctor-components.sh b/bin/gstack-doctor-components.sh index eb470c992..f14038703 100644 --- a/bin/gstack-doctor-components.sh +++ b/bin/gstack-doctor-components.sh @@ -279,6 +279,7 @@ gstack_result_anchor() { QUEUE_TRAIN_FROZEN) printf '%s' 'queue-train-frozen' ;; QUEUE_RECEIPT_MISSING) printf '%s' 'queue-receipt-missing' ;; QUEUE_RECEIPT_STALE) printf '%s' 'queue-receipt-stale' ;; + QUEUE_SCREEN_STALE_REF) printf '%s' 'queue-screen-stale-ref' ;; READY_SCREEN_CHOICE_REQUIRED) printf '%s' 'ready-screen-choice-required' ;; PREGATE_TIMING_ASSERTION) printf '%s' 'pregate-timing-assertion' ;; APPROVAL_CLASSES_INVALID) printf '%s' 'approval-classes-invalid' ;; diff --git a/bin/gstack-ship-receipt b/bin/gstack-ship-receipt index 85423da0f..774fd20d4 100755 --- a/bin/gstack-ship-receipt +++ b/bin/gstack-ship-receipt @@ -17,7 +17,9 @@ * fenced block (and SHIP_RECEIPT: on stderr), writes it to --out, * and with --body makes it the first section of that PR body file. * `result` is /ship's terminal step: it binds the receipt to HEAD, writes the - * run record under /ship/runs/ and prints GSTACK_RESULT: skill=ship. + * run record under /ship/runs/ (ship-run.json: a /ship record, not a + * `run` v1 manifest, so `gstack-artifact validate` does not apply to it) and + * prints GSTACK_RESULT: skill=ship. A gh failure prints status=incomplete. * Query commands: no GSTACK_RESULT line. Logic: lib/ship-receipt.ts. */ import { spawnSync } from 'node:child_process'; @@ -42,7 +44,7 @@ const HELP = `gstack-ship-receipt — the \`\`\`gstack-ship-receipt\`\`\` block read --body parse the block from a file result --pr [--repo ] | --body [--head ] [--out ] /ship's terminal line: binds the receipt to HEAD (QUEUE_RECEIPT_MISSING / QUEUE_RECEIPT_STALE), - writes /ship/runs/-/{run.json,ship-receipt.md} and prints + writes /ship/runs/-/{ship-run.json,ship-receipt.md} and prints GSTACK_RESULT: skill=ship status=complete run= (refused: status=refused run=-) census --repo --since [--cohort ] the P7 adoption number over merged PRs (deduplicated by run and thread) @@ -209,6 +211,12 @@ function cmdResult(rest: string[], json: boolean): number { return EXIT.refused; }; if (!head) { process.stderr.write('gstack-ship-receipt result: --head is required outside a git repository\n'); return EXIT.usage; } + if (!/^[0-9a-f]{7,64}$/i.test(head)) { process.stderr.write(`gstack-ship-receipt result: --head must be a hex commit id, got ${JSON.stringify(head.slice(0, 40))}\n`); return EXIT.usage; } + const incomplete = (why: string) => { + process.stderr.write(`gstack-ship-receipt result: ${why}\n`); + process.stdout.write(json ? JSON.stringify({ skill: 'ship', status: 'incomplete', run: '-', message: why }) + '\n' : 'GSTACK_RESULT: skill=ship status=incomplete run=-\n'); + return EXIT.fail; + }; let body: string; let source: string; if (bodyFile) { @@ -218,12 +226,12 @@ function cmdResult(rest: string[], json: boolean): number { } else if (pr) { const repo = flag(rest, '--repo'); const r = gh(['pr', 'view', pr, ...(repo ? ['--repo', repo] : []), '--json', 'body']); - if (!r.ok) { process.stderr.write(`gstack-ship-receipt result: gh pr view failed: ${r.err}\n`); return EXIT.fail; } - try { body = JSON.parse(r.out).body ?? ''; } catch { process.stderr.write('gstack-ship-receipt result: gh returned no JSON body\n'); return EXIT.fail; } + if (!r.ok) return incomplete(`gh pr view failed: ${r.err.trim()}`); + try { body = JSON.parse(r.out).body ?? ''; } catch { return incomplete('gh returned no JSON body'); } source = `PR #${pr}${repo ? ` (${repo})` : ''}`; } else { process.stderr.write(`gstack-ship-receipt result: --pr or --body is required\n${HELP}\n`); return EXIT.usage; } - const bound = bindReceipt(body, pr ?? String(extractReceipt(body).receipt?.pr ?? '-'), [head]); - if (!bound.ok) return refused(bound.code, bound.message.replace(/^PR #-'s /, "the receipt's ")); + const bound = bindReceipt(body, pr, [head]); + if (!bound.ok) return refused(bound.code, bound.message); const stamp = new Date().toISOString().replace(/[:.]/g, '-'); const dir = flag(rest, '--out') ?? path.join(resolveStateRoot(), 'ship', 'runs', `${stamp}-${head.slice(0, 12)}`); fs.mkdirSync(dir, { recursive: true }); @@ -234,7 +242,7 @@ function cmdResult(rest: string[], json: boolean): number { queue_mode: bound.receipt.queue_mode ?? null, session_kind: bound.receipt.session_kind ?? process.env.GSTACK_SESSION_KIND ?? null, receipt: receiptPath, receipt_source: source, branch: repoRoot ? git(['rev-parse', '--abbrev-ref', 'HEAD'], repoRoot) ?? null : null, created_at: new Date().toISOString(), }; - fs.writeFileSync(path.join(dir, 'run.json'), JSON.stringify(run, null, 2) + '\n'); + fs.writeFileSync(path.join(dir, 'ship-run.json'), JSON.stringify(run, null, 2) + '\n'); process.stderr.write(bound.line + '\n'); process.stdout.write(json ? JSON.stringify({ ...run, run: dir }) + '\n' : `GSTACK_RESULT: skill=ship status=complete run=${dir}\n`); return EXIT.ok; diff --git a/bin/gstack-skill-start b/bin/gstack-skill-start index f83ef59d0..da1d011b7 100755 --- a/bin/gstack-skill-start +++ b/bin/gstack-skill-start @@ -648,7 +648,7 @@ if [ "$_SESSION_KIND" = "unattended" ]; then case "$SKILL_NAME" in autoplan) _UA_RECORD="This skill writes a run directory through gstack-autoplan (plan.md, review-record.md, tasks.jsonl, decisions.jsonl, findings.jsonl, timing.json, run.json, gate.json): append each recommended choice to decisions.jsonl with kind \"auto\" and each pending one with kind \"approval\" and status \"pending\". The final approval gate is written as a gate list (gstack-gate render) and this run ENDS there with \`GSTACK_RESULT: skill=autoplan status=gate_pending run=\`." ;; eval-plan) _UA_RECORD="This run ENDS at the Step 4 spend approval, left pending, with \`GSTACK_RESULT: skill=eval-plan status=gate_pending run=\`, where is the pilot's --out directory. No helper writes a gate.json for this skill yet, so the parent reads the pilot report and spend ledger in ." ;; - ship) _UA_RECORD="This skill ends with \`gstack-ship-receipt result\` (Step 22), which writes /ship/runs/-/run.json and prints \`GSTACK_RESULT: skill=ship status=complete run=\`; a run that stops earlier prints \`GSTACK_RESULT: skill=ship status=refused run=-\` itself. The one publication allowed here: when an open PR for this branch exists and HEAD is its pushed head, update that PR's body with the receipt through the pr-body section's existing-PR path (never pr-create, never a push); otherwise the handoff section writes the receipt to .gstack/tmp/ship-receipt.md and the parent reads it there." ;; + ship) _UA_RECORD="This skill ends with \`gstack-ship-receipt result\` (Step 22), which writes /ship/runs/-/ship-run.json (a /ship record, not a run manifest) and prints \`GSTACK_RESULT: skill=ship status=complete run=\`; a run that stops earlier prints \`GSTACK_RESULT: skill=ship status=refused run=-\` itself. The one publication allowed here: when an open PR for this branch exists and HEAD is its pushed head, update that PR's body with the receipt through the pr-body section's existing-PR path (never pr-create, never a push); otherwise the handoff section writes the receipt to .gstack/tmp/ship-receipt.md and the parent reads it there." ;; review) _UA_RECORD="$_UA_NONE The parent reads the review-log rows (\`gstack-review-read\`)." ;; qa) _UA_RECORD="$_UA_NONE The parent reads the \`qa-report-*.md\` file." ;; *) _UA_RECORD="$_UA_NONE The parent reads this skill's own record (docs/unattended.md)." ;; diff --git a/docs/ship-policy.md b/docs/ship-policy.md index 497765fc6..4e9add1be 100644 --- a/docs/ship-policy.md +++ b/docs/ship-policy.md @@ -332,7 +332,18 @@ slot. The queue enforces this at every step from the PR body alone: stamp files (and, from slot 2 on, merges main into the pin), so no /ship runs on it: the receipt written for the pinned head stays the binding one. `verify --train` accepts a receipt whose `head` is either the slot's pinned head or -the current head; any other head is `QUEUE_RECEIPT_STALE`. +the current head, and the re-READY after the stamp push (`ready` at the stamp +commit) accepts the receipt for the pin when HEAD is the pin plus stamp files, +or (from slot 2 on) the pin merged with main plus stamp files; any other head +is `QUEUE_RECEIPT_STALE`. + +This is a process gate, not an authenticity check: the receipt is a block in +the PR body, which the PR author writes, so the codes prove that /ship was run +and recorded for this head, not that nobody typed the block by hand. Fork heads +are already refused (`QUEUE_TRAIN_UNTRUSTED`). The `ship-run.json` that +`gstack-ship-receipt result` writes under the lane's state root is a +normalized copy of that same block bound to the head it was run at, not an +authenticity proof either. **1. Lane: READY with a screen choice.** Under a train policy READY needs an explicit choice: @@ -340,7 +351,7 @@ explicit choice: ```bash gstack-ship-queue ready --pr 42 # prints SCREEN: lines and the recommendation, refuses gstack-ship-queue ready --pr 42 --no-screen # READY receipt; the PR rides unscreened -gstack-ship-queue ready --pr 42 --screen [--broad] # dispatches one paid screen on the PR's merge SHA +gstack-ship-queue ready --pr 42 --screen [--broad] # dispatches one paid screen on the PR's merge SHA (QUEUE_SCREEN_STALE_REF when GitHub's merge ref lags the head) ``` Without a choice it prints the `SCREEN:` selection (with @@ -356,7 +367,14 @@ dispatches `gate_workflow` with `screen_pr`, `screen_merge_sha` (the 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 -it. +it. Before dispatching, `ready` fetches the merge ref and requires its second +parent to be the PR head: a missing or lagging merge ref (GitHub stops +recomputing it while the PR conflicts with the base, CHANGELOG-only conflicts +included) is `QUEUE_SCREEN_STALE_REF`, because a screen dispatched there plans +no cases and reports success. A finished screen whose record planned 0 cases +is never followed as a screen: `ready --screen` dispatches a fresh one and +`train build` prints `SCREEN: # run planned 0 cases; counted as +not-run` and seats the slot unscreened. **2. Coordinator: build.** Name the pins exactly as READY reported them: diff --git a/docs/troubleshooting.md b/docs/troubleshooting.md index 1f43b0929..a32a2ec97 100644 --- a/docs/troubleshooting.md +++ b/docs/troubleshooting.md @@ -4053,6 +4053,28 @@ written for the pinned head is the binding one and `verify --train` accepts it. `gstack-ship-queue ready --pr --screen|--no-screen`. A lane never stamps to fix this: the stamp comes only when the slot is called. + +### `QUEUE_SCREEN_STALE_REF` + +**Meaning.** `gstack-ship-queue ready --screen` dispatches the screen against +GitHub's `refs/pull//merge`, and that ref is missing or still merges an +older head than the PR's pushed head. GitHub stops recomputing the merge ref +while the PR conflicts with the base (a CHANGELOG-only conflict after a stamp +counts), so a screen dispatched there checks out the old merge, sees itself +superseded, plans 0 cases and reports success. GSTA-95 hit this on #3431: +`ready` printed `SCREEN_DISPATCH:` as if a screen ran. `ready` now compares +the merge ref's second parent with the PR head before dispatching, and a +finished screen whose record says it planned 0 cases is never adopted as a +screen: `ready --screen` dispatches a fresh one and `train build` prints +`SCREEN: # run planned 0 cases; counted as not-run` and seats the slot +unscreened. + +**Fix.** Resolve the conflict with the base branch (or wait for GitHub to +recompute the merge ref), push, and run `gstack-ship-queue ready --pr +--screen` again. Under crew rule 14 a CHANGELOG-only conflict is left to the +train, so a lane that cannot get a fresh merge ref READYs `--no-screen` only +after its own focused paid cases passed (rule 6). + ### `READY_SCREEN_CHOICE_REQUIRED` diff --git a/docs/unattended.md b/docs/unattended.md index ba1b06b9b..7d594110e 100644 --- a/docs/unattended.md +++ b/docs/unattended.md @@ -83,15 +83,17 @@ Not every workflow writes this line or a run directory today: - **`/ship`** ends with `gstack-ship-receipt result --pr ` (or `--body .gstack/tmp/ship-receipt.md` after the handoff section). It binds the receipt to the branch head, writes `/ship/runs/-/` - (`run.json` with `head`, `pr`, `base`, `version`, `queue_mode`, + (`ship-run.json` with `head`, `pr`, `base`, `version`, `queue_mode`, `session_kind`, `receipt` as the path of the copied `ship-receipt.md`, and - `receipt_source`) and prints `GSTACK_RESULT: skill=ship status=complete - run=`. A receipt that is missing or names another head prints + `receipt_source`; it is a /ship record, not the `run` v1 manifest below, so + read it directly: `gstack-artifact validate` does not apply) and prints + `GSTACK_RESULT: skill=ship status=complete run=`. When gh cannot read + the PR it prints `GSTACK_RESULT: skill=ship status=incomplete run=-` (exit 1). A receipt that is missing or names another head prints `GSTACK_RESULT: skill=ship status=refused run=-` with `QUEUE_RECEIPT_MISSING` or `QUEUE_RECEIPT_STALE` on stderr (exit 3); a run that stops before publication prints the same refused line itself. A parent's merge check is then mechanical: take `run=` from the last result line, read - `run.json`, and compare its `head` to the PR head. `gstack-ship-queue ready`, + `ship-run.json`, and compare its `head` to the PR head. `gstack-ship-queue ready`, `train build` and `verify --train` make the same comparison from the PR body ([ship-policy.md](ship-policy.md#ship-before-ready)), which is why an unattended /ship may update the body of a PR that already exists: when an @@ -150,8 +152,10 @@ JSON Schema for any of `findings`, `decisions`, `tasks`, `timing`, `run`, ## How a parent consumes a run -1. **Decide whether the run finished.** `gstack-artifact validate /run.json` - is the deterministic completion check. It exits `0` and prints +1. **Decide whether the run finished.** For an `/autoplan` run directory, + `gstack-artifact validate /run.json` + is the deterministic completion check (a `/ship` `run=` directory holds + `ship-run.json`, read directly as described above). It exits `0` and prints `ARTIFACT_VALID: run= status=` only when the manifest parses and matches its schema, every required phase and each reviewer voice has a terminal outcome, every listed artifact exists inside the run directory with diff --git a/lib/result-codes-release.ts b/lib/result-codes-release.ts index e2d0edaa4..01eebe8ec 100644 --- a/lib/result-codes-release.ts +++ b/lib/result-codes-release.ts @@ -143,6 +143,11 @@ export const RELEASE_RESULT_CODES = { summary: 'the PR body’s /ship receipt is for another head than the one being READY’d or merged (a push after /ship, or a receipt copied from another PR)', fix: 'rerun /ship on the current head so the receipt names it, then gstack-ship-queue ready again; a slot’s stamp commit keeps the receipt written for its pinned head', }, + QUEUE_SCREEN_STALE_REF: { + anchor: 'queue-screen-stale-ref', + summary: 'GitHub’s refs/pull//merge is missing or still merges an older head than the PR’s: the PR conflicts with the base (a CHANGELOG-only conflict counts), so a screen dispatched on it would plan no cases and report success', + fix: 'resolve the conflict with the base branch (or wait for GitHub to recompute the merge ref), push, then gstack-ship-queue ready --pr --screen again; a lane never READYs on a screen that planned nothing', + }, READY_SCREEN_CHOICE_REQUIRED: { anchor: 'ready-screen-choice-required', summary: 'the base policy turns trains on, so READY needs an explicit screen choice: --screen (pay for a diff-relevant screen now) or --no-screen', diff --git a/lib/ship-queue-train-github.ts b/lib/ship-queue-train-github.ts index b96c359f8..39d47f363 100644 --- a/lib/ship-queue-train-github.ts +++ b/lib/ship-queue-train-github.ts @@ -69,6 +69,21 @@ export function downloadJson(ctx: QueueContext, slug: string, run: number, name: } /** The named check run on a commit (the newest when several); null when absent or unreadable. */ +/** + * A finished screen run that planned no cases is not a screen: evals.yml plans + * 0 cases and reports success when the merge ref it was dispatched for was + * superseded (GSTA-95 on #3431). Returns the reason when the run is hollow, + * null when it is live, still running, or its census cannot be read. + */ +export function hollowScreen(ctx: QueueContext, slug: string, run: number, artifact: string, scratch: string): string | null { + const view = runView(ctx, slug, run); + if (!view || view.status !== 'completed') return null; + const rec = downloadJson(ctx, slug, run, artifact, scratch) as { census?: { planned?: { rule?: unknown; judge?: unknown } } } | null; + const planned = rec?.census?.planned; + if (!planned || typeof planned.rule !== 'number' || typeof planned.judge !== 'number') return null; + return planned.rule + planned.judge === 0 ? `run ${run} planned 0 cases (concluded ${view.conclusion ?? 'unknown'})` : null; +} + export function checkRun(ctx: QueueContext, sha: string, name: string): CheckInfo | null { const r = json<{ check_runs?: Array<{ name?: string; status?: string; conclusion?: string | null }> }>( ctx.gh(['api', `repos/{owner}/{repo}/commits/${sha}/check-runs?check_name=${encodeURIComponent(name)}`])); diff --git a/lib/ship-queue-train-ready.ts b/lib/ship-queue-train-ready.ts index 43f24fc29..6fa321b81 100644 --- a/lib/ship-queue-train-ready.ts +++ b/lib/ship-queue-train-ready.ts @@ -11,12 +11,14 @@ * replay (lib/gate-cost.ts) and printed as pending until it lands. */ import { randomBytes } from 'node:crypto'; +import * as os from 'node:os'; +import * as path from 'node:path'; import { DISPATCH_INPUTS, awaitNonceRun, dedupeKey, dispatchWorkflow, findDuplicateRun, listDispatchRuns, makeNonce, type WorkflowDispatch } from '../scripts/eval-bg'; import { EXIT } from './headless-artifacts'; import { ready, type QueueContext } from './ship-queue'; import { classifyTrainHead } from './version-gate-train'; import { git } from './ship-queue-gate'; -import { execFor, repoSlug } from './ship-queue-train-github'; +import { execFor, hollowScreen, repoSlug } from './ship-queue-train-github'; import { loadReady, nowIso, persist, readyPath, type ReadyReceipt, type ScreenChoice } from './ship-queue-train-record'; import { outOk, outPending, outRefused, trainEnv, type TrainEnv, type TrainOut } from './ship-queue-train'; @@ -85,10 +87,11 @@ async function dispatchScreen(env: TrainEnv, o: ReadyOptions, mergeSha: string, if (!slug) return outRefused({ code: 'QUEUE_PROOF_MISSING', message: 'gh repo view could not name the repository' }, [], `${Q} ready --pr ${o.pr} --screen`, 'none'); const exec = execFor(env.ctx); const existing = findDuplicateRun(listDispatchRuns(dispatch.workflow, exec, slug) ?? [], dedupeKey('pr', mergeSha, baseSha, scope)); - if (existing) return { choice: { choice: 'screen', policy, nonce: /eval-bg (\S+?)\]/.exec(existing.displayTitle)?.[1] ?? nonce, run: existing.databaseId, merge_sha: mergeSha }, line: `SCREEN_DISPATCH: following run ${existing.databaseId} (same merge SHA, base and policy; no duplicate)` }; + const hollow = existing ? hollowScreen(env.ctx, slug, existing.databaseId, env.train.record_artifact, path.join(os.tmpdir(), 'gstack-screen-scratch')) : null; + if (existing && !hollow) return { choice: { choice: 'screen', policy, nonce: /eval-bg (\S+?)\]/.exec(existing.displayTitle)?.[1] ?? nonce, run: existing.databaseId, merge_sha: mergeSha }, line: `SCREEN_DISPATCH: following run ${existing.databaseId} (same merge SHA, base and policy; no duplicate)` }; const found = dispatchWorkflow(dispatch, env.ctx.base, exec, slug) ? await awaitNonceRun(dispatch.workflow, nonce, exec, slug) : null; if (!found) return outPending({ code: 'QUEUE_TRAIN_DISPATCH_UNKNOWN', message: `no ${dispatch.workflow} run named ${nonce} appeared after the screen dispatch` }, [], `${Q} ready --pr ${o.pr} --screen${o.broad ? ' --broad' : ''} (adopts the run by its key once it appears)`, 'none'); - return { choice: { choice: 'screen', policy, nonce, run: found.databaseId, merge_sha: mergeSha }, line: `SCREEN_DISPATCH: ${dispatch.workflow} run ${found.databaseId} ${found.url} nonce ${nonce}` }; + return { choice: { choice: 'screen', policy, nonce, run: found.databaseId, merge_sha: mergeSha }, line: `SCREEN_DISPATCH: ${dispatch.workflow} run ${found.databaseId} ${found.url} nonce ${nonce}${hollow ? ` (not following ${hollow}: a screen that planned nothing is not a screen)` : ''}` }; } export async function trainReady(ctx: QueueContext, o: ReadyOptions): Promise { @@ -114,7 +117,11 @@ export async function trainReady(ctx: QueueContext, o: ReadyOptions): Promise): { const f = git(ctx.repoRoot, ['fetch', '--no-tags', '-q', env.remote, ...specs]); if (!f.ok) return outRefused({ code: 'QUEUE_PROOF_MISSING', message: `git fetch of the PR heads failed: ${f.err.slice(0, 160)}` }, [], 'none', 'none'); const slots: TrainSlot[] = []; + const lines: string[] = []; + let slug: string | null | undefined; for (const p of pins) { const info = prInfo(ctx, p.pr); if ('error' in info) return outRefused({ code: 'QUEUE_PROOF_MISSING', message: info.error }, [], 'none', 'none'); @@ -298,9 +302,13 @@ function admitSlots(env: TrainEnv, pins: Array<{ pr: string; sha: string }>): { if (!shipped.ok) return outRefused({ code: shipped.code, message: shipped.message }, [shipped.line], `#${p.pr}'s lane runs /ship, then ${Q} ready --pr ${p.pr} --screen|--no-screen; then ${Q} train build --prs `, 'none'); const ready = loadReady(env.repo.keys, p.pr, info.headRefOid); if (!ready) return outRefused({ code: 'QUEUE_NOT_READY', message: `no READY receipt for #${p.pr} at ${info.headRefOid.slice(0, 12)} (local state or ${'refs/notes/gstack-trains'})` }, [], `the lane runs ${Q} ready --pr ${p.pr} --screen|--no-screen`, 'none'); - slots.push({ pr: p.pr, head: info.headRefOid, ready_at: ready.ready_at, screen: ready.screen }); + let screen = ready.screen; + if (screen.choice === 'screen' && screen.run && slug === undefined) slug = repoSlug(ctx); + const hollow = screen.choice === 'screen' && screen.run && slug ? hollowScreen(ctx, slug, screen.run, env.train.record_artifact, path.join(os.tmpdir(), 'gstack-screen-scratch')) : null; + if (hollow) { lines.push(`SCREEN: #${p.pr} ${hollow}; counted as not-run, the slot rides unscreened`); screen = { ...screen, choice: 'no-screen' }; } + slots.push({ pr: p.pr, head: info.headRefOid, ready_at: ready.ready_at, screen }); } - return { slots, lines: [] }; + return { slots, lines }; } /** Start a new train from --prs, or find the live train with the same pins and base. */ @@ -318,6 +326,7 @@ async function newTrain(env: TrainEnv, o: BuildOptions): Promise<{ t: TrainRecor if ('exit' in admitted) return admitted; const order = orderSlots(admitted.slots, o.repair ? 1 : env.train.max_size, explicit); const lines = [ + ...admitted.lines, `ORDER: ${order.train.map((s, i) => `${i + 1}:#${s.pr}(${s.screen.choice === 'screen' ? 'screened' : 'unscreened'})`).join(' ')}${explicit ? ' (explicit --order)' : ''}`, ...(order.held.length ? [`HELD: ${order.held.map(s => '#' + s.pr).join(',')} (${order.split ? 'unscreened PRs form their own train' : `over the train size ${env.train.max_size}`})`] : []), ]; diff --git a/lib/ship-queue.ts b/lib/ship-queue.ts index 151b5d55b..788e984a6 100644 --- a/lib/ship-queue.ts +++ b/lib/ship-queue.ts @@ -24,7 +24,7 @@ import * as path from 'node:path'; import { readPregateFile, verifyPregate, PREGATE_OUT_REL } from './pregate/index'; import { ensureTmpExcluded, restamp, versionAtRef } from './restamp'; import type { ResultCodeName } from './result-codes'; -import { bindReceipt } from './ship-receipt'; +import { bindReceipt, extractReceipt } from './ship-receipt'; import { isUnavailable, loadPolicyFromBase, type LoadedPolicy } from './ship-policy'; import { QUEUE_DIR_REL, applyStampPatch, commitForTree, currentAttempt, git, listAttempts, recomputeMergeTree, sha256, writeJsonAtomic, @@ -60,6 +60,36 @@ function treeReceipt(ctx: QueueContext, policy: LoadedPolicy, gated: string, cur return 'error' in r ? { code: 'QUEUE_PROOF_MISSING', message: r.error } : r; } +/** + * The heads a READY accepts a /ship receipt for: HEAD, plus the receipt's own + * head when HEAD is that commit with only stamp files changed on top (the + * slot's stamp commit, docs/ship-policy.md#ship-before-ready), or, from slot + * 2 on, the stamp over a merge of the base into it: /ship ran on the pin, the + * stamp changes nothing it checked, and verify --train binds the same pair + * against the train's own prefix. Any other receipt head stays + * QUEUE_RECEIPT_STALE. + */ +function acceptedReceiptHeads(ctx: QueueContext, policy: LoadedPolicy, head: string, body: string): string[] { + const ex = extractReceipt(body); + const r = ex.receipt; + if (!r || ex.errors.length || typeof r.head !== 'string' || !/^[0-9a-f]{12,64}$/i.test(r.head) || head.startsWith(r.head)) return [head]; + const root = ctx.repoRoot; + const full = git(root, ['rev-parse', '--verify', '--quiet', `${r.head}^{commit}`]); + if (!full.ok || !git(root, ['merge-base', '--is-ancestor', full.out, head]).ok) return [head]; + const stampOnly = (from: string, to: string) => { const t = from === to ? null : treeReceipt(ctx, policy, from, to); return !t || (!('message' in t) && t.tree === 'same-modulo-stamps'); }; + if (stampOnly(full.out, head)) return [full.out, head]; + const baseSha = git(root, ['rev-parse', `origin/${ctx.base}`]).out; + const onBase = (sha: string) => !!sha && git(root, ['merge-base', '--is-ancestor', sha, baseSha]).ok; + for (const merge of [head, git(root, ['rev-parse', '--verify', '--quiet', `${head}^1`]).out]) { + if (!merge || !stampOnly(merge, head)) continue; + const parents = git(root, ['rev-list', '--parents', '-n', '1', merge]).out.split(' ').slice(1); + if (parents.length !== 2) continue; + const pinSide = onBase(parents[0]!) ? parents[1]! : onBase(parents[1]!) ? parents[0]! : ''; + if (pinSide && git(root, ['merge-base', '--is-ancestor', full.out, pinSide]).ok && stampOnly(full.out, pinSide)) return [full.out, head]; + } + return [head]; +} + const CI_PAGE = 100; const CI_MAX_PAGES = 10; @@ -142,7 +172,7 @@ export function ready(ctx: QueueContext, o: { pr: string; pregate?: string; gate pregateLine = `PREGATE: ${read.file.verdict} tree=${read.file.tree.slice(0, 12)} head=${(read.file.head ?? '').slice(0, 12)} checks=${read.file.checks.length}${v.ok ? '' : ` — ${v.message} (${v.code})`}`; } else pregateLine = `PREGATE: unreadable (${read.errors[0]?.message ?? 'invalid'})`; } - const receipt = bindReceipt(info.body ?? '', o.pr, [head]); + const receipt = bindReceipt(info.body ?? '', o.pr, acceptedReceiptHeads(ctx, policy, head, info.body ?? '')); const attempt = currentAttempt(ctx.repoRoot, o.pr); const lines = [ `READY #${o.pr} ${head} STAMP_VERSION=${version}`, diff --git a/lib/ship-receipt.ts b/lib/ship-receipt.ts index 64858bf89..df6c346b7 100644 --- a/lib/ship-receipt.ts +++ b/lib/ship-receipt.ts @@ -63,21 +63,22 @@ export type ReceiptBinding = * receipt written for the pin stays the binding one). `line` is the * informational `SHIP_RECEIPT:` line the READY block prints either way. */ -export function bindReceipt(body: string, pr: string, accepted: readonly string[]): ReceiptBinding { +export function bindReceipt(body: string, pr: string | undefined, accepted: readonly string[]): ReceiptBinding { + const subject = pr === undefined ? "the receipt's" : `PR #${pr}'s`; const ex = extractReceipt(body); - if (!ex.found) return { ok: false, code: 'QUEUE_RECEIPT_MISSING', message: `PR #${pr}'s body carries no ship receipt`, line: 'SHIP_RECEIPT: none in the PR body' }; + if (!ex.found) return { ok: false, code: 'QUEUE_RECEIPT_MISSING', message: `${subject} body carries no ship receipt`, line: 'SHIP_RECEIPT: none in the PR body' }; if (!ex.receipt || ex.errors.length) { const why = ex.errors[0] ? `${ex.errors[0].message} at ${ex.errors[0].path}` : 'invalid'; - return { ok: false, code: 'QUEUE_RECEIPT_MISSING', message: `PR #${pr}'s ship receipt is invalid (${why})`, line: 'SHIP_RECEIPT: invalid in the PR body' }; + return { ok: false, code: 'QUEUE_RECEIPT_MISSING', message: `${subject} ship receipt is invalid (${why})`, line: 'SHIP_RECEIPT: invalid in the PR body' }; } const r = ex.receipt; const line = receiptSummaryLine(r); const head = accepted.find(h => h === r.head || (r.head.length >= 12 && h.startsWith(r.head))); if (!head) { const want = accepted.map(h => h.slice(0, 12)).join(' or '); - return { ok: false, code: 'QUEUE_RECEIPT_STALE', message: `PR #${pr}'s ship receipt is for ${r.head.slice(0, 12)}, not the head ${want} being merged`, line }; + return { ok: false, code: 'QUEUE_RECEIPT_STALE', message: `${subject} ship receipt is for ${r.head.slice(0, 12)}, not the head ${want} being merged`, line }; } - if (r.pr !== undefined && String(r.pr) !== pr) return { ok: false, code: 'QUEUE_RECEIPT_STALE', message: `PR #${pr}'s ship receipt names pr=${r.pr}, not this PR`, line }; + if (pr !== undefined && r.pr !== undefined && String(r.pr) !== pr) return { ok: false, code: 'QUEUE_RECEIPT_STALE', message: `${subject} ship receipt names pr=${r.pr}, not this PR`, line }; return { ok: true, receipt: r, head, line }; } @@ -112,7 +113,7 @@ export function receiptSummaryLine(r: ShipReceipt): string { if (r.session_kind) parts.push(`session_kind=${r.session_kind}`); if (r.artifacts_consumed) parts.push(`artifacts_consumed=${r.artifacts_consumed}`); if (r.approval) parts.push(`approval=${approvalSummary(r.approval)}`); - return `SHIP_RECEIPT: ${parts.join(' ')}`; + return `SHIP_RECEIPT: ${parts.join(' ').replace(/[\x00-\x1f\x7f]+/g, ' ')}`; } // --------------------------------------------------------------------------- diff --git a/test/gstack-ship-queue.test.ts b/test/gstack-ship-queue.test.ts index edf31105e..931306540 100644 --- a/test/gstack-ship-queue.test.ts +++ b/test/gstack-ship-queue.test.ts @@ -295,6 +295,43 @@ describe('ready', () => { fs.appendFileSync(path.join(repo.work, 'pr-a.txt'), 'dirty\n'); expect(queue(repo, ['ready', '--pr', '1'], shim(moved, green)).err).toMatch(/uncommitted tracked changes: pr-a\.txt.*\(QUEUE_NOT_READY\)/); }, 180_000); + + // Value: protects=the re-READY after a slot's stamp push accepts the receipt /ship wrote for the pinned head, and only then; + // fails_when=ready binds the receipt to HEAD alone (every stamped READY turns QUEUE_RECEIPT_STALE) or accepts any ancestor's receipt; + // why_new=the READY test above binds receipts to the exact head only; seam=none; no_claim=real gh and CI (shims; unit only) + test('ready at the stamp commit accepts the receipt written for the pinned head; a real edit on top of the pin is stale', () => { + const repo = fixture(); + const pin = makePr(repo, 'pr-s'); + fs.writeFileSync(path.join(repo.work, 'VERSION'), '1.2.4.0\n'); + repo.git('commit', '-qam', 'v1.2.4.0 stamp'); + const stamped = repo.git('rev-parse', 'HEAD'); + const shim = (prHead: string, body: string) => ghQueueShim(repo, { prs: { '1': { headRefOid: prHead, url: 'https://github.com/o/r/pull/1', body } }, checks: { [prHead]: green } }); + const ok = queue(repo, ['ready', '--pr', '1', '--pregate', pregateFor(repo, stamped)], shim(stamped, shippedBody('1', pin))); + expect([ok.status, ok.err]).toEqual([EXIT.ok, '']); + expect(ok.out.split('\n')[0]).toBe(`READY #1 ${stamped} STAMP_VERSION=1.2.4.0`); + expect(ok.out).toContain(`SHIP_RECEIPT: pr=1 head=${pin.slice(0, 12)} version=- base=main`); + repo.git('checkout', '-q', 'origin/main', '-B', 'main-moves'); + fs.writeFileSync(path.join(repo.work, 'other.txt'), 'another PR landed\n'); + repo.git('add', 'other.txt'); + repo.git('commit', '-qm', 'other PR'); + repo.git('push', '-q', 'origin', 'HEAD:main'); + repo.git('fetch', '-q', 'origin'); + repo.git('checkout', '-q', 'pr-s'); + repo.git('merge', '-q', '--no-edit', 'origin/main'); + fs.writeFileSync(path.join(repo.work, 'VERSION'), '1.2.5.0\n'); + repo.git('commit', '-qam', 'v1.2.5.0 stamp'); + const slot2 = repo.git('rev-parse', 'HEAD'); + const merged = queue(repo, ['ready', '--pr', '1', '--pregate', pregateFor(repo, slot2)], shim(slot2, shippedBody('1', pin))); + expect([merged.status, merged.err]).toEqual([EXIT.ok, '']); + expect(merged.out).toContain(`SHIP_RECEIPT: pr=1 head=${pin.slice(0, 12)} version=- base=main`); + repo.git('reset', '-q', '--hard', stamped); + fs.appendFileSync(path.join(repo.work, 'pr-s.txt'), 'real change after /ship\n'); + repo.git('commit', '-qam', 'more'); + const edited = repo.git('rev-parse', 'HEAD'); + const stale = queue(repo, ['ready', '--pr', '1', '--pregate', pregateFor(repo, edited)], shim(edited, shippedBody('1', pin))); + expect(stale.status).toBe(EXIT.refused); + expect(stale.err).toMatch(new RegExp(`PR #1's ship receipt is for ${pin.slice(0, 12)}, not the head ${edited.slice(0, 12)} being merged.*\\(QUEUE_RECEIPT_STALE\\)`)); + }, 180_000); }); describe('receipt digest', () => { diff --git a/test/gstack-ship-receipt.test.ts b/test/gstack-ship-receipt.test.ts index 8bec45350..ff1b4550f 100644 --- a/test/gstack-ship-receipt.test.ts +++ b/test/gstack-ship-receipt.test.ts @@ -6,9 +6,10 @@ */ import { afterEach, describe, expect, test } from 'bun:test'; import * as fs from 'node:fs'; +import * as os from 'node:os'; import * as path from 'node:path'; import { EXIT } from '../lib/headless-artifacts'; -import { computeCensus, extractReceipt, placeReceiptFirst, receiptSummaryLine, renderReceiptBlock } from '../lib/ship-receipt'; +import { bindReceipt, computeCensus, extractReceipt, placeReceiptFirst, receiptSummaryLine, renderReceiptBlock } from '../lib/ship-receipt'; import { cleanup, ghShim, makeFixtureRepo, makePr, recordAndBundle, restamp, runBin, type FixtureRepo } from './helpers/restamp-fixture'; const repos: FixtureRepo[] = []; @@ -165,7 +166,8 @@ describe('gstack-ship-receipt result (/ship terminal line, GBRA-108)', () => { const dir = m![1]!; expect(dir.startsWith(path.join(state, 'ship', 'runs'))).toBe(true); expect(dir.endsWith(`-${head.slice(0, 12)}`)).toBe(true); - const run = JSON.parse(fs.readFileSync(path.join(dir, 'run.json'), 'utf8')); + const run = JSON.parse(fs.readFileSync(path.join(dir, 'ship-run.json'), 'utf8')); + expect(fs.existsSync(path.join(dir, 'run.json'))).toBe(false); expect(run).toMatchObject({ schema_version: 1, skill: 'ship', status: 'complete', head, pr: 5, base: 'main', queue_mode: 'stamp-at-merge', receipt: path.join(dir, 'ship-receipt.md'), receipt_source: 'PR #5' }); expect(extractReceipt(fs.readFileSync(run.receipt, 'utf8')).receipt).toMatchObject({ pr: 5, head }); expect(ok.err.trim()).toBe(`SHIP_RECEIPT: pr=5 head=${head.slice(0, 12)} version=- base=main`); @@ -184,11 +186,100 @@ describe('gstack-ship-receipt result (/ship terminal line, GBRA-108)', () => { const handoff = runBin(repo, 'gstack-ship-receipt', ['result', '--body', bodyFile, '--out', path.join(repo.root, 'run-out')], env); expect(handoff.status).toBe(EXIT.ok); expect(handoff.out.trim()).toBe(`GSTACK_RESULT: skill=ship status=complete run=${path.join(repo.root, 'run-out')}`); - expect(JSON.parse(fs.readFileSync(path.join(repo.root, 'run-out', 'run.json'), 'utf8'))).toMatchObject({ head, pr: null, receipt_source: bodyFile, session_kind: 'unattended' }); + expect(JSON.parse(fs.readFileSync(path.join(repo.root, 'run-out', 'ship-run.json'), 'utf8'))).toMatchObject({ head, pr: null, receipt_source: bodyFile, session_kind: 'unattended' }); expect(runBin(repo, 'gstack-ship-receipt', ['result', '--body', path.join(repo.root, 'absent.md')], env).status).toBe(EXIT.refused); expect(runBin(repo, 'gstack-ship-receipt', ['result'], env).status).toBe(EXIT.usage); expect(fs.readdirSync(path.join(state, 'ship', 'runs')).length).toBe(1); }); + + // Value: protects=a --body handoff refusal names "the receipt", never "PR #-", and a gh failure is EXIT.fail, not a refusal; + // fails_when=the PR #-'s rewrite stops matching or gh errors are mapped to status=refused; why_new=the result test covers + // only the --pr refusals, an absent file and a bound handoff; seam=none; no_claim=real gh (the shim stands in; unit only) + test('a --body file with no block or a stale head is refused in the receipt\'s own words, and a gh failure is a plain failure, not a refusal', () => { + const repo = fixture(); + const head = repo.git('rev-parse', 'HEAD'); + const env = { ...ghShim(repo, {}), GSTACK_STATE_ROOT: path.join(repo.root, 'state') }; + const dir = path.join(repo.work, '.gstack/tmp'); + fs.mkdirSync(dir, { recursive: true }); + const noBlock = path.join(dir, 'no-block.md'); + fs.writeFileSync(noBlock, 'Handoff prose without a receipt.\n'); + const missing = runBin(repo, 'gstack-ship-receipt', ['result', '--body', noBlock], env); + expect(missing.status).toBe(EXIT.refused); + expect(missing.out.trim()).toBe('GSTACK_RESULT: skill=ship status=refused run=-'); + expect(missing.err).toContain("the receipt's body carries no ship receipt"); + expect(missing.err).not.toContain('PR #-'); + const staleFile = path.join(dir, 'stale.md'); + fs.writeFileSync(staleFile, renderReceiptBlock({ schema_version: 1, head: 'c'.repeat(40), base: 'main' } as any)); + const stale = runBin(repo, 'gstack-ship-receipt', ['result', '--body', staleFile], env); + expect(stale.status).toBe(EXIT.refused); + expect(stale.err).toContain(`the receipt's ship receipt is for cccccccccccc, not the head ${head.slice(0, 12)} being merged`); + const invalidFile = path.join(dir, 'invalid.md'); + fs.writeFileSync(invalidFile, '```gstack-ship-receipt\n{"schema_version":1}\n```\n'); + const invalid = runBin(repo, 'gstack-ship-receipt', ['result', '--body', invalidFile], env); + expect([invalid.status, invalid.out.trim()]).toEqual([EXIT.refused, 'GSTACK_RESULT: skill=ship status=refused run=-']); + expect(invalid.err).toMatch(/the receipt's ship receipt is invalid \(.*\).*\(QUEUE_RECEIPT_MISSING/); + const otherPrEnv = { ...env, ...ghShim(repo, { 8: { body: placeReceiptFirst('Prose.\n', { schema_version: 1, pr: 99, head, base: 'main' } as any) } }) }; + const otherPr = runBin(repo, 'gstack-ship-receipt', ['result', '--pr', '8'], otherPrEnv); + expect([otherPr.status, otherPr.out.trim()]).toEqual([EXIT.refused, 'GSTACK_RESULT: skill=ship status=refused run=-']); + expect(otherPr.err).toContain("PR #8's ship receipt names pr=99, not this PR"); + const pinned = path.join(dir, 'pinned.md'); + fs.writeFileSync(pinned, renderReceiptBlock({ schema_version: 1, head: 'c'.repeat(40), base: 'main' } as any)); + const override = runBin(repo, 'gstack-ship-receipt', ['result', '--body', pinned, '--head', 'c'.repeat(40), '--out', path.join(repo.root, 'pinned-out')], env); + expect([override.status, override.out.trim()]).toEqual([EXIT.ok, `GSTACK_RESULT: skill=ship status=complete run=${path.join(repo.root, 'pinned-out')}`]); + expect(JSON.parse(fs.readFileSync(path.join(repo.root, 'pinned-out', 'ship-run.json'), 'utf8')).head).toBe('c'.repeat(40)); + const badHead = runBin(repo, 'gstack-ship-receipt', ['result', '--body', pinned, '--head', '../../etc'], env); + expect([badHead.status, badHead.out]).toEqual([EXIT.usage, '']); + expect(badHead.err).toContain('--head must be a hex commit id'); + const noGit = fs.mkdtempSync(path.join(os.tmpdir(), 'gstack-receipt-nogit-')); + const outside = runBin(repo, 'gstack-ship-receipt', ['result', '--body', pinned], env, noGit); + expect([outside.status, outside.err.trim()]).toEqual([EXIT.usage, 'gstack-ship-receipt result: --head is required outside a git repository']); + fs.rmSync(noGit, { recursive: true, force: true }); + const ghDown = runBin(repo, 'gstack-ship-receipt', ['result', '--pr', '9'], env); + expect(ghDown.status).toBe(EXIT.fail); + expect(ghDown.err).toContain('gh pr view failed'); + expect(ghDown.out.trim()).toBe('GSTACK_RESULT: skill=ship status=incomplete run=-'); + expect(JSON.parse(runBin(repo, 'gstack-ship-receipt', ['result', '--pr', '9', '--json'], env).out)).toMatchObject({ skill: 'ship', status: 'incomplete', run: '-' }); + expect(fs.existsSync(path.join(repo.root, 'state', 'ship', 'runs'))).toBe(false); + }); +}); + +describe('bindReceipt head matching (rule 22)', () => { + // Value: protects=a receipt binds only to a full or 12+ character prefix of an accepted head, and names the PR it was written for; + // fails_when=the 12-character floor is dropped (head "a" would bind any head) or the pr check is removed; + // why_new=the queue, train and result tests bind full 40-character heads only; seam=none; no_claim=gh and CLI wiring (pure function) + test('binds a full head or a 12+ character prefix, refuses shorter prefixes, other heads and another PR\'s receipt', () => { + const head = 'a1b2c3d4e5f6a7b8c9d0e1f2a3b4c5d6e7f8a9b0'; + const other = 'f'.repeat(40); + const body = (h: string, pr?: number) => placeReceiptFirst('Prose.\n', { schema_version: 1, head: h, base: 'main', ...(pr === undefined ? {} : { pr }) } as any); + const rows: Array<[string, string, string[], string]> = [ + ['full head', head, [head], 'ok'], + ['12-char prefix', head.slice(0, 12), [head], 'ok'], + ['11-char prefix', head.slice(0, 11), [head], 'QUEUE_RECEIPT_STALE'], + ['7-char prefix', head.slice(0, 7), [head], 'QUEUE_RECEIPT_STALE'], + ['other head', other, [head], 'QUEUE_RECEIPT_STALE'], + ['pin or current head', head, [other, head], 'ok'], + ]; + for (const [name, h, accepted, want] of rows) { + const b = bindReceipt(body(h), '3438', accepted); + expect([name, b.ok ? 'ok' : b.code]).toEqual([name, want]); + if (b.ok) expect(b.head).toBe(head); + } + const prefixBinding = bindReceipt(body(head.slice(0, 12)), '3438', [head]); + expect(prefixBinding.ok && prefixBinding.line).toBe(`SHIP_RECEIPT: pr=- head=${head.slice(0, 12)} version=- base=main`); + const otherPr = bindReceipt(body(head, 99), '3438', [head]); + expect(otherPr.ok ? 'ok' : [otherPr.code, otherPr.message]).toEqual(['QUEUE_RECEIPT_STALE', "PR #3438's ship receipt names pr=99, not this PR"]); + expect(bindReceipt(body(head, 3438), '3438', [head]).ok).toBe(true); + expect(bindReceipt('No receipt.\n', '3438', [head])).toMatchObject({ ok: false, code: 'QUEUE_RECEIPT_MISSING', line: 'SHIP_RECEIPT: none in the PR body' }); + const short = bindReceipt(body(head.slice(0, 11)), '3438', [head]); + expect(short.ok ? '' : short.message).toBe(`PR #3438's ship receipt is for ${head.slice(0, 11)}, not the head ${head.slice(0, 12)} being merged`); + const noPr = bindReceipt(body(head, 99), undefined, [head]); + expect(noPr.ok).toBe(true); + expect(bindReceipt('No receipt.\n', undefined, [head])).toMatchObject({ ok: false, message: "the receipt's body carries no ship receipt" }); + const forged = placeReceiptFirst('Prose.\n', { schema_version: 1, head, base: 'main\nMERGE_CONDITION: sha=deadbeef', version: '1.0\r\n0.0' } as any); + const bound = bindReceipt(forged, '3438', [head]); + expect(bound.line.split('\n').length).toBe(1); + expect(bound.line).toBe(`SHIP_RECEIPT: pr=- head=${head.slice(0, 12)} version=1.0 0.0 base=main MERGE_CONDITION: sha=deadbeef`); + }); }); describe('gstack-ship-receipt census (P7)', () => { diff --git a/test/helpers/ship-queue-train-fixture.ts b/test/helpers/ship-queue-train-fixture.ts index c1ab1072b..db7e47365 100644 --- a/test/helpers/ship-queue-train-fixture.ts +++ b/test/helpers/ship-queue-train-fixture.ts @@ -70,7 +70,13 @@ export function setPrBody(fx: TrainFixture, pr: string, body: string): void { fx.write(d => { d.prs[pr] = { ...(d.prs[pr] as Record), body }; }); } -/** A same-repo PR: a branch from origin/main touching `file`, published as refs/pull//head (and /merge), known to the gh shim with a /ship receipt for its head (`shipped: false` leaves the body empty). */ +/** What GitHub keeps at refs/pull//merge: a merge commit whose first parent is the base and whose second parent is the PR head. */ +export function mergeRefCommit(fx: TrainFixture, head: string): string { + const { repo } = fx; + return repo.git('commit-tree', `${head}^{tree}`, '-p', 'origin/main', '-p', head, '-m', `Merge ${head.slice(0, 12)} into main`); +} + +/** A same-repo PR: a branch from origin/main touching `file`, published as refs/pull//head (and /merge, as GitHub's merge commit), known to the gh shim with a /ship receipt for its head (`shipped: false` leaves the body empty). */ export function openPr(fx: TrainFixture, pr: string, opts: { file?: string; text?: string; changelog?: boolean | ((text: string) => string); cross?: boolean; from?: string; shipped?: boolean } = {}): string { const { repo } = fx; repo.git('checkout', '-q', opts.from ?? 'origin/main', '-B', `pr-${pr}`); @@ -84,7 +90,7 @@ export function openPr(fx: TrainFixture, pr: string, opts: { file?: string; text repo.git('add', '-A'); repo.git('commit', '-q', '-m', `feat #${pr}`); const head = repo.git('rev-parse', 'HEAD'); - repo.git('push', '-q', '-f', 'origin', `HEAD:refs/pull/${pr}/head`, `HEAD:refs/pull/${pr}/merge`); + repo.git('push', '-q', '-f', 'origin', `HEAD:refs/pull/${pr}/head`, `${mergeRefCommit(fx, head)}:refs/pull/${pr}/merge`); fx.write(d => { d.prs[pr] = { headRefOid: head, isCrossRepository: !!opts.cross, state: 'OPEN', headRefName: `pr-${pr}`, url: `https://github.com/fixture/repo/pull/${pr}`, body: opts.shipped === false ? '' : shippedBody(pr, head) }; }); return head; } diff --git a/test/ship-queue-train.test.ts b/test/ship-queue-train.test.ts index 8bf91f174..36ac86991 100644 --- a/test/ship-queue-train.test.ts +++ b/test/ship-queue-train.test.ts @@ -19,7 +19,7 @@ import { TRAIN_HELP } from '../lib/ship-queue-train-cli'; import { BASE_POLICY, cleanup, runBin } from './helpers/restamp-fixture'; import { coordinatorClone } from './helpers/ship-queue-fixture'; import { - COMPLETE, FAIL, GREEN_GATE, PASS, RED_GATE, TRAIN_BLOCK, TRAIN_POLICY, dispatches, githubOrigin, line, nonceOf, openPr, queue, readyReceipt, repoKey, setPrBody, shippedBody, trainFixture, trainJson, urlKey, + COMPLETE, FAIL, GREEN_GATE, PASS, RED_GATE, TRAIN_BLOCK, TRAIN_POLICY, dispatches, githubOrigin, line, mergeRefCommit, nonceOf, openPr, queue, readyReceipt, repoKey, setPrBody, shippedBody, trainFixture, trainJson, urlKey, type Rule, type TrainFixture, } from './helpers/ship-queue-train-fixture'; @@ -177,9 +177,11 @@ describe('ready under a train policy: the screen choice', () => { test('--screen dispatches one screen with the PR, its merge SHA and the policy; a repeat adopts it; a broad screen never adopts the narrow one', () => { const { fx, head, pregate } = readyFixture(); + const mergeSha = fx.repo.git('ls-remote', 'origin', 'refs/pull/1/merge').split(/\s/)[0]!; + expect(mergeSha).not.toBe(head); const dry = queue(fx, ['ready', '--pr', '1', '--pregate', pregate, '--screen', '--dry-run']); expect(dry.status).toBe(EXIT.ok); - expect(line(dry.out, 'SCREEN_DISPATCH:')).toMatch(new RegExp(`^SCREEN_DISPATCH: evals\\.yml --ref main evals_all=false expected_sha=${head} nonce=pr\\.screen\\.narrow-\\S+ base_ref=main base_sha=[0-9a-f]{40} screen_pr=1 screen_merge_sha=${head} screen_policy=narrow \\(dry run: not dispatched\\)$`)); + expect(line(dry.out, 'SCREEN_DISPATCH:')).toMatch(new RegExp(`^SCREEN_DISPATCH: evals\\.yml --ref main evals_all=false expected_sha=${mergeSha} nonce=pr\\.screen\\.narrow-\\S+ base_ref=main base_sha=[0-9a-f]{40} screen_pr=1 screen_merge_sha=${mergeSha} screen_policy=narrow \\(dry run: not dispatched\\)$`)); expect(dispatches(fx)).toEqual([]); fx.write(d => { d.rules['evals.yml'] = { status: 'in_progress', conclusion: null }; }); const first = queue(fx, ['ready', '--pr', '1', '--pregate', pregate, '--screen']); @@ -192,6 +194,52 @@ describe('ready under a train policy: the screen choice', () => { expect(dispatches(fx).map(d => (d.inputs as any).screen_policy)).toEqual(['narrow', 'broad']); expect(queue(fx, ['ready', '--pr', '1', '--broad']).status).toBe(EXIT.usage); }, 120_000); + + // Value: protects=--screen never dispatches against a merge ref GitHub has not recomputed for the PR head (GSTA-95 on #3431: a stale ref made evals.yml plan 0 cases and report success); + // fails_when=the merge ref's second parent is not compared with the PR head, or a missing merge ref keeps the old QUEUE_NOT_READY code; why_new=the --screen test above only sees a fresh merge ref; seam=none; no_claim=GitHub's real recompute timing (a bare origin stands in) + test('--screen refuses QUEUE_SCREEN_STALE_REF when refs/pull//merge lags the PR head or is missing, dispatching nothing; a recomputed merge ref admits it', () => { + const { fx, head, pregate } = readyFixture(); + const { repo } = fx; + fs.appendFileSync(path.join(repo.work, 'pr-1.txt'), 'pushed after the merge ref was computed\n'); + repo.git('commit', '-qam', 'more'); + const moved = repo.git('rev-parse', 'HEAD'); + repo.git('push', '-q', '-f', 'origin', 'HEAD:refs/pull/1/head'); + fx.write(d => { (d.prs['1'] as any).headRefOid = moved; (d.prs['1'] as any).body = shippedBody('1', moved); d.ci[moved] = [{ status: 'completed', conclusion: 'success' }]; }); + const pregate2 = path.join(repo.root, 'pregate2.json'); + fs.writeFileSync(pregate2, JSON.stringify({ schema_version: 1, tree: runBin(repo, 'gstack-wtree', []).out.trim(), head: moved, verdict: 'pass', checks: [{ id: 'unit', status: 'pass' }] })); + const stale = queue(fx, ['ready', '--pr', '1', '--pregate', pregate2, '--screen']); + expect(stale.status).toBe(EXIT.refused); + expect(stale.err).toMatch(new RegExp(`refs/pull/1/merge \\([0-9a-f]{12}\\) merges ${head.slice(0, 12)}, not the PR head ${moved.slice(0, 12)}.*\\(QUEUE_SCREEN_STALE_REF\\)`)); + expect(line(stale.out, 'NEXT:')).toBe('NEXT: resolve the conflict with main (or wait for GitHub to recompute refs/pull/1/merge), push, then gstack-ship-queue ready --pr 1 --screen; a screen dispatched on a stale merge ref plans no cases'); + repo.git('push', '-q', 'origin', ':refs/pull/1/merge'); + const missing = queue(fx, ['ready', '--pr', '1', '--pregate', pregate2, '--screen']); + expect(missing.status).toBe(EXIT.refused); + expect(missing.err).toMatch(/has no refs\/pull\/1\/merge.*\(QUEUE_SCREEN_STALE_REF\)/); + expect(dispatches(fx)).toEqual([]); + expect(fs.existsSync(path.join(fx.state, 'ship-queue', 'ready'))).toBe(false); + repo.git('push', '-q', '-f', 'origin', `${mergeRefCommit(fx, moved)}:refs/pull/1/merge`); + const fresh = queue(fx, ['ready', '--pr', '1', '--pregate', pregate2, '--screen', '--dry-run']); + expect([fresh.status, fresh.err]).toEqual([EXIT.ok, '']); + expect(line(fresh.out, 'SCREEN_DISPATCH:')).toContain('(dry run: not dispatched)'); + }, 120_000); + + // Value: protects=a finished screen that planned 0 cases never counts as a screen: ready dispatches a fresh one instead of adopting it and train build seats the slot unscreened with a SCREEN: line; + // fails_when=the census is not read (a hollow run is followed, or ORDER says screened); why_new=no test reads a screen run's record; seam=none; no_claim=evals.yml's real artifact (the shim's record stands in) + test('a screen run that planned 0 cases is not followed by ready and is counted as not-run by train build', () => { + const { fx, head, pregate } = readyFixture(); + fx.write(d => { d.rules['evals.yml'] = { status: 'completed', conclusion: 'success', record: { verdict: 'GREEN', census: { complete: true, planned: { rule: 0, judge: 0 }, executed: { rule: 0, judge: 0 } }, panels: [] } }; }); + const first = queue(fx, ['ready', '--pr', '1', '--pregate', pregate, '--screen']); + expect(first.status).toBe(EXIT.ok); + expect(line(first.out, 'SCREEN_DISPATCH:')).toMatch(/^SCREEN_DISPATCH: evals\.yml run 100 /); + const again = queue(fx, ['ready', '--pr', '1', '--pregate', pregate, '--screen']); + expect(again.status).toBe(EXIT.ok); + expect(line(again.out, 'SCREEN_DISPATCH:')).toMatch(/^SCREEN_DISPATCH: evals\.yml run 101 .* \(not following run 100 planned 0 cases \(concluded success\): a screen that planned nothing is not a screen\)$/); + expect(dispatches(fx, 'evals.yml').length).toBe(2); + const build = queue(fx, ['train', 'build', '--prs', `1@${head}`, '--dry-run']); + expect(build.status).toBe(EXIT.ok); + expect(line(build.out, 'SCREEN:')).toBe('SCREEN: #1 run 101 planned 0 cases (concluded success); counted as not-run, the slot rides unscreened'); + expect(line(build.out, 'ORDER:')).toBe('ORDER: 1:#1(unscreened)'); + }, 120_000); }); describe('train build', () => { @@ -353,6 +401,10 @@ describe('train build', () => { const stale = queue(fx, ['train', 'build', '--prs', `1@${h1},2@${h2}`, '--dry-run']); expect(stale.status).toBe(EXIT.refused); expect(stale.err).toMatch(new RegExp(`PR #2's ship receipt is for aaaaaaaaaaaa, not the head ${h2.slice(0, 12)} being merged.*\\(QUEUE_RECEIPT_STALE\\)`)); + setPrBody(fx, '2', '```gstack-ship-receipt\n{"schema_version":1}\n```\n'); + expect(queue(fx, ['train', 'build', '--prs', `1@${h1},2@${h2}`, '--dry-run']).err).toMatch(/PR #2's ship receipt is invalid \(.*\).*\(QUEUE_RECEIPT_MISSING\)/); + setPrBody(fx, '2', shippedBody('1', h2)); + expect(queue(fx, ['train', 'build', '--prs', `1@${h1},2@${h2}`, '--dry-run']).err).toMatch(/PR #2's ship receipt names pr=1, not this PR.*\(QUEUE_RECEIPT_STALE\)/); expect(remoteRefs(fx)).toBe(refsBefore); expect(dispatches(fx)).toEqual([]); expect(fs.existsSync(path.join(fx.state, 'ship-queue', 'trains'))).toBe(false); @@ -675,8 +727,25 @@ describe('verify, freeze, repair and clear', () => { expect(trainJson(fx, nonce).merged).toEqual([]); setPrBody(fx, '1', ''); expect(v().err).toContain('(QUEUE_RECEIPT_MISSING)'); + setPrBody(fx, '1', '```gstack-ship-receipt\n{"schema_version":1}\n```\n'); + const invalid = v(); + expect(invalid.err).toMatch(/PR #1's ship receipt is invalid \(.*\).*\(QUEUE_RECEIPT_MISSING\)/); + expect(invalid.out).not.toContain('MERGE_CONDITION:'); + setPrBody(fx, '1', shippedBody('2', h1)); + const otherPr = v(); + expect(otherPr.err).toMatch(/PR #1's ship receipt names pr=2, not this PR.*\(QUEUE_RECEIPT_STALE\)/); + expect(otherPr.out).not.toContain('MERGE_CONDITION:'); + expect(trainJson(fx, nonce).merged).toEqual([]); setPrBody(fx, '1', shippedBody('1', stamped)); expect(v().out).toContain(`SHIP_RECEIPT: pr=1 head=${stamped.slice(0, 12)} version=- base=main (binds the current head)`); + const saved = fx.read().prs['1']; + fx.write(d => { delete d.prs['1']; }); + const unreadable = v(); + expect(unreadable.status).toBe(EXIT.refused); + expect(unreadable.err).toContain('(QUEUE_PROOF_MISSING)'); + expect(unreadable.out).not.toContain('MERGE_CONDITION:'); + fx.write(d => { d.prs['1'] = saved; }); + expect(trainJson(fx, nonce).merged).toEqual([]); }, 300_000); test('from slot 2 on, a head that merges main into the pin verifies when its tree is the slot\'s prefix modulo stamps', () => {