From 1f246e3505eac788abf570d1e3f552ef939bee7f Mon Sep 17 00:00:00 2001 From: Garry Tan Date: Tue, 27 Oct 2026 08:11:13 -0700 Subject: [PATCH] v1.91.278.0 fix(ship-queue): readiness takes dispatched Free Tests, stamp paths accept tentative key versions, ready and train build read the base overlap, pr-wait --exclude, restamp prints the corrected order (#3514) * fix(ship-queue): readiness takes dispatched Free Tests, prose check reads key=value pairs, stamp paths accept tentative key versions, ready and train build read the base overlap, pr-wait --exclude, restamp prints the corrected order Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> * fix: pre-landing review fixes Pass 1 of /ship's review on #3514: the prose check skips a key: value pair's value whatever the whitespace, the restamp recompute keeps a merged-not-in-base predecessor and says to fetch, stampOnlyLine applies the version-width bound, pr-wait --exclude excludes nothing while the required checks are unreadable, baseOverlap reports a failed git read as unknown and gives no verdict at train build, readiness lets a pull_request run outrank a dispatched one, and the READY conflict refusal follows the crew template's rule 14 instead of a crew-local rule. Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> * docs: stamp rule for tentative key versions, key=value prose claims, QUEUE_STALE merged-not-in-base fix Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> --------- Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> --- CHANGELOG.md | 20 +++++++ VERSION | 2 +- agents-digest/gstack-AGENTS.md | 2 +- bin/gstack-pr-wait | 2 +- bin/gstack-ship-receipt | 2 +- docs/TESTING_INTERNALS.md | 7 ++- docs/pregate.md | 4 +- docs/ship-policy.md | 37 ++++++++++-- docs/standalone-binaries.md | 2 +- docs/troubleshooting.md | 51 ++++++++++++----- lib/pr-wait-github.ts | 14 +++-- lib/pr-wait.ts | 64 +++++++++++++++++---- lib/restamp.ts | 69 ++++++++++++++++++++--- lib/ship-queue-train-ready.ts | 14 ++++- lib/ship-queue-train.ts | 50 ++++++++++++++-- lib/ship-receipt-prose.ts | 18 ++++-- lib/ship-receipt.ts | 25 +++++--- lib/tree-receipt.ts | 11 ++-- package.json | 2 +- scripts/eval-readiness.ts | 26 +++++++-- scripts/train-gate.ts | 28 +++++++-- test/eval-readiness.test.ts | 33 +++++++++++ test/gstack-restamp-queue.test.ts | 33 ++++++++++- test/gstack-ship-receipt-docs.test.ts | 17 ++++++ test/pr-wait.test.ts | 44 ++++++++++++++- test/ship-queue-train-ops.test.ts | 11 ++++ test/ship-queue-train.test.ts | 41 ++++++++++++++ test/ship-receipt-prose.test.ts | 22 ++++++++ test/train-gate.test.ts | 34 +++++++++++ test/tree-receipt-stamp-sensitive.test.ts | 52 ++++++++++++++++- 30 files changed, 653 insertions(+), 84 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index c5d367fdd..7a08871d7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,25 @@ # Changelog +## [1.91.278.0] - 2026-10-27 + +**The merge train's tooling stops failing the lanes for things the tooling itself could see.** A head whose only conflict with main was the CHANGELOG got its Free Tests through the dispatch route and the paid screen still called it NOT-RUN; the receipt-versus-prose check read the canonical `REVIEW_LOOP` line as a false mismatch; a PR that writes a new `*_KEY_VERSION` constant as a tentative next-minor version (the convention #3511 introduces) could not clear the train's stamp-only receipt; a pooled pin learned that main had moved onto its files only when the train was built; a watcher had no way to leave a stuck non-required check out of its verdict; and a lane that passed the slot call's remaining order to `gstack-restamp` without its own PR got the same stale list back as the fix. Backlog items from GSTA-119, #3485, #3495 and the 2026-10-27 slot calls. + +### Fixed +- **`scripts/eval-readiness.ts`** (#3508): readiness accepts a `workflow_dispatch` Free Tests run at the exact head beside GitHub's `pull_request` run, so a head the dispatch route of docs/ship-policy.md tested starts its paid screen. GitHub's `pull_request` run decides whenever one exists (it built the merge ref; a dispatch builds the head alone, so a newer green dispatch never overrides a red `pull_request` run and an extra red or running dispatch never holds a head its `pull_request` CI cleared); the newest dispatched run decides only for a head with none, a red one blocks, and a `push` run still counts for nothing. +- **`lib/ship-receipt-prose.ts`** (#3513): a number that is the value of a `key=` or `key:` pair is that key's claim alone, whatever the whitespace around the separator, so `REVIEW_LOOP: passes=2 blocking_found=1 deferred=0 noted=4 fixed=8 scope=frozen` and `passes: 2 blocking_found: 1` parse as five claims that all match the receipt instead of a `2 blocking_found` claim. +- **Stamp paths under the next-minor key convention** (#3512): the tree receipt (`lib/tree-receipt.ts`), the carry (`lib/ship-receipt.ts` `stampOnlyText`) and the train's tree-modulo-stamps digest (`scripts/train-gate.ts` `stampFreeTree`) accept a stamp_paths line whose stamp-form version moved to the released version whatever it was before, not only when it was the base's version. The receipt keeps the stamp_paths stage's own bounds (`lib/restamp.ts` `stampOnlyLine`): a moved version older than the gated head's is a released key moved by hand, and one of another width (a 3-digit pin in a 4-digit repository) is never a stamp candidate, so both stay `changed`. The digest collapses every stamp-form mention, so a tentative key and a base-version mention on a base-side line digest the same before and after the slot stamp and `train-backstop` finds the record that covers main. +- **`gstack-restamp --expect-order`**: the `QUEUE_STALE` refusal prints the corrected command instead of the stale list: this PR first when the list left it out (with the message saying every entry read as a predecessor), PRs merged into the local base dropped, an unmerged predecessor kept ahead with a note that it must merge first, and a predecessor GitHub calls merged that the local base does not carry yet kept ahead too, with `fix: git fetch origin ` (dropping it would let the recompute, pinned to that local base, allocate the version it already took). The base-moved refusal carries `--pr` too. + +### Added +- **`gstack-ship-queue ready` and `train build` read gstack-pr-wait's base-overlap event from git** (handoff GSTA-119; the crew template's sibling trial merge, done by the tool). `ready` prints `BASE_OVERLAP: moved to and touches files in this PR: ; the train's merge onto is clean` (or the conflicting files) when the base moved onto the PR's files since it forked, and under `--no-screen` a conflict is `QUEUE_TRAIN_CONFLICT` at READY rather than at the train, with the template's fix (report it to the controller; merge the base on its word). A git read that fails prints `conflicts unknown` and refuses nothing. `train build` runs the scratch merges before the fill rule. A seated pin whose files the base moved onto gets `# BASE_OVERLAP: moved to and touches files in this PR: ` (the chain's merge onto the pin's own prefix is the verdict), a pin that does not merge is `QUEUE_TRAIN_CONFLICT` while the train is still filling, and a clean pool prints `MERGE_CHECK: T1..T merge clean onto @` ahead of `QUEUE_TRAIN_WAITING`, so the waiting verdict is also a dry-run merge (#3485). +- **`gstack-pr-wait --exclude `** (#3495 follow-up): a named non-required check is left out of the verdict as `gstack-ci-gate --exclude` does, so it neither holds `--until green` while pending nor blocks when it fails; a required check cannot be excluded and the first poll logs the refusal, and while a required-check source cannot be read nothing is excluded (the set may be short), as `gstack-ci-gate` refuses its overrides. + +### Tests +- `test/eval-readiness.test.ts`: a dispatched-only head decides green, red and running, a `pull_request` run outranks a dispatched one either way; the runs listing is no longer narrowed to `pull_request`. `test/ship-receipt-prose.test.ts`: the exact `REVIEW_LOOP` line with `passes=2 blocking_found=1` and with `passes=1 blocking_found=0`, the `key: value` spelling with whitespace, plus a real disagreement on the same line. `test/tree-receipt-stamp-sensitive.test.ts`, `test/train-gate.test.ts`, `test/gstack-ship-receipt-docs.test.ts`: a tentative key moved by the restamp is a stamp in the receipt, the digest and the carry; a renamed key, a released key moved by hand or a mention of another width is not. `test/gstack-restamp-queue.test.ts`: the corrected `--expect-order` in each refusal, the own-PR-first lesson and a merged predecessor the local base does not carry. `test/pr-wait.test.ts`: `--exclude` parsing, pending and failed excluded checks, a refused required name, an unreadable required set, and the flag through `prWaitMain`. `test/ship-queue-train-ops.test.ts` and `test/ship-queue-train.test.ts`: the `BASE_OVERLAP:` lines at READY and at build, the `--no-screen` conflict refusal, and `MERGE_CHECK:` beside `QUEUE_TRAIN_WAITING`. + +### Gate integrity +- No threshold, timeout, trial count or case kind changed. No skill prose changed. + ## [1.91.277.0] - 2026-10-27 **office-hours stops carrying its own copy of the shared AskUserQuestion subsections.** After the shared-preamble hoist, office-hours still inlined whole copies of the shared `### When AskUserQuestion is unavailable or a call fails` and `### Format` subsections, because each held one office-hours addition: the `Q` open-question form. A session's first office-hours invocation read those subsections twice, once from the shared file and once from SKILL.md. The `Q` form and its routing sentence now render as office-hours' own `### Open-ended questions` subsection at the end of the AskUserQuestion section, so the shared subsections apply as they are and office-hours' SKILL.md drops 6,328 bytes (its parity union goes from 160,952 to 154,624, under the pre-hoist 1.18 cap). diff --git a/VERSION b/VERSION index fce37150c..ec63d14a2 100644 --- a/VERSION +++ b/VERSION @@ -1 +1 @@ -1.91.277.0 +1.91.278.0 diff --git a/agents-digest/gstack-AGENTS.md b/agents-digest/gstack-AGENTS.md index fbf62f29c..109a4f394 100644 --- a/agents-digest/gstack-AGENTS.md +++ b/agents-digest/gstack-AGENTS.md @@ -1,4 +1,4 @@ -# gstack digest v1.91.277.0 — regenerate/re-copy after upgrading gstack +# gstack digest v1.91.278.0 — regenerate/re-copy after upgrading gstack Behavioral rules from gstack (https://github.com/garrytan/gstack), compressed for agent hosts without a full skill install. The full skills add workflows, diff --git a/bin/gstack-pr-wait b/bin/gstack-pr-wait index a26655a41..285046d13 100755 --- a/bin/gstack-pr-wait +++ b/bin/gstack-pr-wait @@ -2,7 +2,7 @@ /** * gstack-pr-wait — watch one pull request and print one line per real event. * - * gstack-pr-wait [--repo owner/name] [--until green|ready|any-change] [--interval SECONDS] [--timeout MINUTES] + * gstack-pr-wait [--repo owner/name] [--until green|ready|any-change] [--interval SECONDS] [--timeout MINUTES] [--exclude CHECK,...] * * Exit 0 when the --until condition is met or the PR merged, 1 when it needs * action (PR_WAIT_BLOCKED), 2 on bad input, a duplicate watcher or a local diff --git a/bin/gstack-ship-receipt b/bin/gstack-ship-receipt index 533262e14..3614720be 100755 --- a/bin/gstack-ship-receipt +++ b/bin/gstack-ship-receipt @@ -446,7 +446,7 @@ function cmdCarry(rest: string[], json: boolean): number { const oldVersion = versionAt(from); const newVersion = versionAt(head); const allMetadataGlobs = [...CARRY_METADATA_GLOBS, ...extra.metadata]; - const changed = changedRaw.split('\n').filter(Boolean).map(p => ({ path: p, class: classifyCarryPath(p, extra, matchesAny(allMetadataGlobs, p) ? { before: blobAt(from, p), after: blobAt(head, p), oldVersion, newVersion } : undefined) })); + const changed = changedRaw.split('\n').filter(Boolean).map(p => ({ path: p, class: classifyCarryPath(p, extra, matchesAny(allMetadataGlobs, p) ? { before: blobAt(from, p), after: blobAt(head, p), oldVersion, newVersion, templates: policy?.stamp_patterns ?? [] } : undefined) })); // Pathspecs by class: tests and metadata are excluded from the product patch; the test patch is the tests alone. // A metadata-listed path whose change was not a stamp stays in the product patch: every glob that matches it is dropped from the exclusion. const excludeGlobs = (globs: readonly string[]) => globs.map(g => `:(exclude,glob)${g}`); diff --git a/docs/TESTING_INTERNALS.md b/docs/TESTING_INTERNALS.md index cc35d83fb..d85b1656d 100644 --- a/docs/TESTING_INTERNALS.md +++ b/docs/TESTING_INTERNALS.md @@ -1236,7 +1236,12 @@ GitHub-hosted restore never sees (until GSTA-48 every restore missed). **Readiness (R1).** Paid slices for a PR head start only when the PR has been quiet for the window (repository variable `EVALS_QUIET_MINUTES`, default 10, measured from the run's creation) and Free Tests has concluded success on the -same head SHA. The `readiness` job (`scripts/eval-readiness.ts`) sleeps out the +same head SHA, as GitHub's `pull_request` run or, for a head with none, as a +`workflow_dispatch` run on the branch (the dispatch route of docs/ship-policy.md +for a head whose only conflict with the base is the CHANGELOG, which gets no +`pull_request` CI; the `pull_request` run built the merge ref, so it decides +whenever it exists, a dispatched run decides only without one, and a red +dispatched run blocks like any other, #3508). The `readiness` job (`scripts/eval-readiness.ts`) sleeps out the window with no API calls, since a newer push cancels the run anyway, then polls Free Tests every minute for up to 80 minutes. Free Tests red on the head fails the run without paid work; re-run Free Tests, then re-run this workflow (a diff --git a/docs/pregate.md b/docs/pregate.md index 80f72bd3d..a1bf4619e 100644 --- a/docs/pregate.md +++ b/docs/pregate.md @@ -456,7 +456,9 @@ receipt for H when the two trees differ only as a stamp does: that does not account for every path `git diff-tree` lists between the two trees. The version rule is the tree receipt's: a `stamp_files`, `stamp_paths` or `release_outputs` hunk is a stamp - only as the rewrite from the gated tree's version to the head's (so two trees + only as the rewrite to the head's version from the gated tree's version, or + from a newer tentative version of the repo's width that the stamp moved (a + `*_KEY_VERSION` written as the next minor, #3512) (so two trees at the same version refuse such a hunk as `other`), while the built-in version-file, manifest, lockfile and CHANGELOG rules compare content and do not check the direction of the version move. The obligation stays uncleared; rerun diff --git a/docs/ship-policy.md b/docs/ship-policy.md index 72306e5a0..552730afa 100644 --- a/docs/ship-policy.md +++ b/docs/ship-policy.md @@ -425,7 +425,18 @@ conflicts with the base, CHANGELOG-only conflicts included) is replaced by a scratch merge of the head onto the base, pushed as `screen/-` (the `SCREEN_MERGE:` line says which); a screen dispatched on a stale ref would plan no cases and report success. Only a real conflict with the base (or a -remote whose refs cannot be listed) is `QUEUE_SCREEN_STALE_REF`. A finished screen that planned no cases (no +remote whose refs cannot be listed) is `QUEUE_SCREEN_STALE_REF`. Under either +choice `ready` first reads what `gstack-pr-wait`'s base-overlap event reports, +from git: when the base moved since the head forked and its new commits touch +files the PR changes, `BASE_OVERLAP: moved to and touches files +in this PR: ; the train's merge onto is clean` (or `conflicts +beyond the CHANGELOG and the stamp files ()`), the train's own merge +rules run as a dry run in object storage; a git read that fails prints +`conflicts unknown` and refuses nothing. A conflict under `--no-screen` is +`QUEUE_TRAIN_CONFLICT` at READY, since the train would stop at this PR: the +lane reports it to the controller and, on its word (crew template rule 14), +merges the base into the pin, resolves, pushes and runs /ship before READY, +instead of learning it when the train is built. A finished screen that planned no cases (no `paid-plan` artifact, which a superseded run never uploads, or a plan manifest with nothing planned) is never followed as a screen: `ready --screen` dispatches a fresh one and `train build` prints `SCREEN: # run planned @@ -453,7 +464,14 @@ and waits (`--free-timeout `, default 2700) for a green through eval-bg's nonce. A train fills at `max_size` READY PRs or when its oldest READY has waited `fill_hours`, whichever comes first; until then `QUEUE_TRAIN_WAITING` names the time, and `--urgent` dispatches a one-PR -train at once. gstack's own block fills at 3 PRs or one hour (it was 5 PRs or +train at once. The scratch merges run before the fill rule: every `train +build` prints `# BASE_OVERLAP: moved to and touches files in +this PR: ` for a seated pin whose files the base moved onto since it +forked (the chain's merge onto the pin's own prefix is the verdict), refuses +`QUEUE_TRAIN_CONFLICT` for a pin that does not merge, and otherwise prints +`MERGE_CHECK: T1..T merge clean onto @`, so a +`QUEUE_TRAIN_WAITING` result is also a clean dry-run merge of the pool (the +crew template's sibling trial merge, done by the tool). gstack's own block fills at 3 PRs or one hour (it was 5 PRs or three hours until 2026-10-26, which left READY PRs waiting most of a day). The same pins on the same base return the live train instead of a second one; `train build --train ` resumes a train from its record. Refusals, all before any spend: `QUEUE_TRAIN_UNTRUSTED` (a fork @@ -708,7 +726,9 @@ integers under `review_loop`, `gate` and `ci`: `passes=2`, `passes: 2` and several counters, `passed` under `gate` and `ci` or `pass` under every gate lane, matches when the number equals any of them; an underscore in a key matches `_`, a space or `-`; a name inside a longer word, `3 pass-through`, is -not a claim). A body that quotes a different number is +not a claim; a number that is the value of a `key=` or `key:` pair is that +key's claim alone, so `passes=2 blocking_found=1` is two claims, never a +`2 blocking_found` one, #3513). A body that quotes a different number is `RECEIPT_PROSE_MISMATCH` (exit 1) and the `PROSE:` line names each field with both values; prose that quotes no counter is not a claim. Data PRs (evals, measurements, sweeps) run it before READY so the body's numbers are the run's. @@ -798,7 +818,11 @@ dependency, a script or a new key makes them product). A path the policy's `stamp_files`, `stamp_paths` or `release_outputs` name is metadata only while its change is a stamp, the rule the tree receipt applies to the same paths: a manifest by its version key, any other file when its new text reads as the old -once the new release version is replaced by the old one. A logic change in a +once the new release version is replaced by the old one, or when every changed +line is a stamp-form rewrite to the new version of the old version or of a +newer tentative one the stamp moved (a `*_KEY_VERSION` written as the next +minor, #3512; a version older than the old one or of another width is a hand +edit). A logic change in a stamped source file is product. Lockfiles are product unless the policy names them. A base merge contributes no product change of its own. A receipt carried twice keeps the reviewed head as @@ -940,7 +964,10 @@ only, where `gstack-version-bump` writes; a dependency entry is `changed`), the CHANGELOG (a heading move or re-heading, compared as an order-independent multiset of entries), `stamp_paths` and `release_outputs` (every removed/added pair must be an exact `(was X)` / `since: X` / `vX` rewrite (a quote around X in the first two forms included) with -X the gated version and the replacement the current one, so a dependency pin +X the gated version, or a newer version of the repo's width that the +stamp_paths stage moves (a tentative `*_KEY_VERSION` written as the next +minor, #3512; a version older than the gated one, or of another width, is a +hand edit), and the replacement the current one, so a dependency pin that happens to equal the old version is `changed`; `stamp_patterns` templates count as stamp forms too). With `stamp_files` set, a file outside it is `changed (stamp_files: not in the repo's stamp_files allowlist)` whatever diff --git a/docs/standalone-binaries.md b/docs/standalone-binaries.md index 9307428c4..2aab76988 100644 --- a/docs/standalone-binaries.md +++ b/docs/standalone-binaries.md @@ -18,7 +18,7 @@ Beyond the slash-command skills, gstack ships standalone CLIs for workflows that | `gstack-review-log` | **Review-pass receipts** — `--start ` captures the working-tree fingerprint before a diff review; `'' --finish ` consumes that single-use, repository/branch/skill-scoped receipt. Binding requires matching start/end content and reviewer-reported `completed:true` and `converged:true`; it is not independent proof that a model read the source. `--check-shared-libs ` reads a current finding as JSON on stdin and checks prior Skip decisions against the actual branch, capture and eligible raw source blobs. It returns `reusable:false` when proof is missing or unsafe, including older records without logger-versioned coverage. Final review logging computes shared-code fingerprints and coverage rather than trusting supplied proof. | | `gstack-review-read` | **Review freshness** — emits review records with computed `review_freshness.status` and `reason`: CURRENT, STALE, or UNVERIFIED for diff reviews. `/ship` and `/land-and-deploy` use the same grade; a matching commit alone never certifies a diff review. [Dashboard rules](skills.md#review-readiness-dashboard). | | `gstack-evidence` | **Verification-evidence ledger** — `run --label -- ` transparently wraps any test command (the child's exit code always passes through) and records what ran against which working-tree fingerprint; `check` grades each label FRESH/STALE/MISSING with `--expect-cmd`, `--max-age`, and `--allow-paths` binding. /ship and /land-and-deploy cite fresh evidence instead of re-running suites. Per-run logs are 0600, capped at 2MB, pruned after 30 days; the ledger and logs stay machine-local by design. | -| `gstack-pr-wait` | **PR event watcher** — `gstack-pr-wait [--repo owner/name] [--until green\|ready\|any-change] [--interval 20] [--timeout 60]` polls one pull request through `gh` and prints one stdout line per real event (a check failed with its link, all checks finished, a comment or review from someone other than the author, a review bot finished on the head, the base moved and its new commits touch this PR's files, BEHIND or DIRTY, merged or closed), with progress on stderr, so a monitor wakes the agent once per event instead of on a timer. `green` gates on every check on the head (an advisory bot's review is never waited for, its checks gate like any other), as `gstack-ci-gate` does; a check the base branch requires (rulesets and classic branch protection) is pending until it appears, never green by omission; two identical polls confirm a check set is complete. Exit 0 when the condition is met, 1 when it needs action or `--timeout` passes (`PR_WAIT_BLOCKED`), 2 on bad input, a second watcher for the same PR or a local error (`PR_WAIT_REFUSED`). State under `/pr-wait/`, baseline discarded after 6 hours. Review bots are configuration: `gstack-config set pr_wait_gating_bots greptile,macroscope`, `pr_wait_advisory_bots coderabbit` (advisory bots are reported, never waited on). `/land-and-deploy` Step 3 is one call of it. Design ported from tstack's pr-wait (MIT, Theo Browne; NOTICE.md). [Exit codes](troubleshooting.md#gstack-pr-wait). | +| `gstack-pr-wait` | **PR event watcher** — `gstack-pr-wait [--repo owner/name] [--until green\|ready\|any-change] [--interval 20] [--timeout 60] [--exclude CHECK,...]` polls one pull request through `gh` and prints one stdout line per real event (a check failed with its link, all checks finished, a comment or review from someone other than the author, a review bot finished on the head, the base moved and its new commits touch this PR's files, BEHIND or DIRTY, merged or closed), with progress on stderr, so a monitor wakes the agent once per event instead of on a timer. `green` gates on every check on the head (an advisory bot's review is never waited for, its checks gate like any other), as `gstack-ci-gate` does; a check the base branch requires (rulesets and classic branch protection) is pending until it appears, never green by omission; two identical polls confirm a check set is complete; `--exclude` leaves a named non-required check out of the verdict as `gstack-ci-gate --exclude` does (a required name is refused and gates). Exit 0 when the condition is met, 1 when it needs action or `--timeout` passes (`PR_WAIT_BLOCKED`), 2 on bad input, a second watcher for the same PR or a local error (`PR_WAIT_REFUSED`). State under `/pr-wait/`, baseline discarded after 6 hours. Review bots are configuration: `gstack-config set pr_wait_gating_bots greptile,macroscope`, `pr_wait_advisory_bots coderabbit` (advisory bots are reported, never waited on). `/land-and-deploy` Step 3 is one call of it. Design ported from tstack's pr-wait (MIT, Theo Browne; NOTICE.md). [Exit codes](troubleshooting.md#gstack-pr-wait). | | `gstack-issue-guard` | **Tracker-text trust envelope** — fetches GitHub issue/PR text (`issue `, `pr-body`, `pr-comments`, or `--stdin`) and wraps it in a labeled envelope so agents treat it as data: injection-shaped lines get labeled even through fullwidth and invisible-character evasion, and forged envelope banners are defused. Every tracker-text ingress in gstack routes through it, enforced by a CI scanner. | | `gstack-ios-qa-daemon` | **iOS QA daemon** — Mac-side broker between an agent and a connected iPhone or iPad over USB CoreDevice. Loopback by default; `--tailnet` opens a Tailscale-facing listener with identity-gated capability tiers. Single-instance via flock on `~/.gstack/ios-qa-daemon.pid`. See [docs/howto-ios-testing-with-gstack.md](howto-ios-testing-with-gstack.md). | | `gstack-ios-qa-mint` | **iOS allowlist manager** — owner-grant CLI for the tailnet allowlist. `grant`/`revoke`/`list` against `~/.gstack/ios-qa-allowlist.json` (mode 0600). Remote agents never auto-allowlist; this is the explicit-intent path. | diff --git a/docs/troubleshooting.md b/docs/troubleshooting.md index e4199286f..04255e7bb 100644 --- a/docs/troubleshooting.md +++ b/docs/troubleshooting.md @@ -2708,11 +2708,19 @@ code (you already do when you run its tests), or drop the release tool. **Meaning.** `gstack-restamp` was told the base SHA (`--expect-base`) or the merge order (`--expect-order`) the allocation assumed, and one of them moved: the base advanced, a predecessor is not merged, or a successor merged first. -The JSON result carries `expected`, `actual` and `recompute`. +`--expect-order` lists the lane's own PR (`--pr`) first, then the remaining +order; a list that leaves it out reads every entry as an unmerged predecessor, +and the message says so. The JSON result carries `expected`, `actual` and +`recompute`. -**Fix.** Run the printed `recompute` command, which recomputes the version -from the current base, then merge with the server-side head condition -(`MERGE_CONDITION: sha=`). +**Fix.** Run the printed `recompute` command: it carries the corrected order +(this PR first, merged PRs dropped, an unmerged predecessor kept ahead because +it must merge first) and recomputes the version from the current base; then +merge with the server-side head condition (`MERGE_CONDITION: sha=`). A +predecessor GitHub calls merged that the local base does not carry yet +(`#=merged-not-in-base`) is kept ahead too, with `fix: git fetch origin +`: fetch, then rerun, so the recompute, pinned to that local base, never +allocates the version the predecessor already took. ### `RESTAMP_VERSION_SOURCE` @@ -4174,13 +4182,19 @@ with the new pin (`train build --prs @,…`). ### `QUEUE_TRAIN_CONFLICT` **Meaning.** Merging a PR onto the prefix before it conflicts outside the -version files and the CHANGELOG. The `QUEUE_TRAIN_CONFLICT:` line names the -PR, the conflicting files and the PRs merged before it. Nothing was -dispatched and no branch was pushed. +version files and the CHANGELOG. From `train build`, the +`QUEUE_TRAIN_CONFLICT:` line names the PR, the conflicting files and the PRs +merged before it, and the check runs before the fill rule, so a filling train +finds it too. From `ready --no-screen`, the prefix is the base itself: the +`BASE_OVERLAP:` line names the files the base moved onto since the head +forked, and the train would stop at this PR (under `--screen` the same +conflict is `QUEUE_SCREEN_STALE_REF`). Nothing was dispatched and no branch +was pushed. -**Fix.** Run the printed `train build --prs `. The PR's -lane merges main (with the earlier PRs once they land) and reports READY -again. +**Fix.** From `train build`, run the printed `train build --prs `. The PR's lane reports the conflict to the controller and, on its +word (crew template rule 14), merges main into the pin (with the earlier PRs +once they land), resolves, pushes, runs /ship and reports READY again. ### `QUEUE_TRAIN_FREE_RED` @@ -4209,7 +4223,11 @@ run carrying the nonce, or marks the dispatch absent; only then does **Meaning.** The train has fewer PRs than `train.max_size` and its oldest READY PR has waited less than `train.fill_hours`, so it does not dispatch -yet. The message prints when it will. +yet. The message prints when it will. The scratch merges already ran: the +`MERGE_CHECK: T1..T merge clean onto @` line above it says every +seated pin merges in slot order (a pin that does not is `QUEUE_TRAIN_CONFLICT` +instead), and a `# BASE_OVERLAP:` line names a pin whose files the base +moved onto since it forked. **Fix.** Add READY PRs, rerun after the printed time, or pass `--urgent` for a one-PR train. @@ -4688,14 +4706,21 @@ reason }` and carry the `Documentation exception: accepted by via ## gstack-pr-wait `gstack-pr-wait [--repo owner/name] [--until green|ready|any-change] -[--interval SECONDS] [--timeout MINUTES]` watches one pull request and prints +[--interval SECONDS] [--timeout MINUTES] [--exclude CHECK,...]` watches one pull request and prints one stdout line per real event, prefixed `#`; progress and recoverable errors go to stderr. `/land-and-deploy` Step 3 runs it with `--until green`. Exit 0 means the `--until` condition is met or the PR merged; exit 1 and 2 end with one of the two codes below. `green` gates on every check on the head, as `gstack-ci-gate` does, so the two never disagree; a check the base branch requires (rulesets and classic branch protection) counts as pending -until it appears. A 403 on the classic +until it appears. `--exclude ` leaves a named non-required check +out of the verdict, as `gstack-ci-gate --exclude` does: it neither holds +`green` while pending nor blocks when it fails, and the first poll logs +`excluded from the verdict: `; a required check cannot be excluded +(`exclude refused for required check `) and gates, and while a +required-check source cannot be read nothing is excluded (`exclude refused for +: the required checks could not be read`), as `gstack-ci-gate` refuses +its overrides. A 403 on the classic protection read (a token without admin scope) is logged and skipped, and the rulesets read still names the required checks. The review-bot lists are configuration: `gstack-config set pr_wait_gating_bots greptile,macroscope` diff --git a/lib/pr-wait-github.ts b/lib/pr-wait-github.ts index fa8eda600..e999e9cf5 100644 --- a/lib/pr-wait-github.ts +++ b/lib/pr-wait-github.ts @@ -193,24 +193,28 @@ export function fetchChecks(env: GhEnv, repo: string, pr: number): Map { +export function requiredChecks(env: GhEnv, repo: string, base: string): { names: Set; known: boolean } { const sources: Array<[string, string]> = [ [`repos/${repo}/rules/branches/${base}`, '.[] | select(.type == "required_status_checks") | .parameters.required_status_checks[].context'], [`repos/${repo}/branches/${base}/protection/required_status_checks`, '.contexts[]?, .checks[]?.context'], ]; const names = new Set(); + let known = true; for (const [path, jq] of sources) { try { for (const name of ghLines(env, ['api', path, '--jq', jq])) names.add(name); } catch (error) { const message = error instanceof Error ? error.message : String(error); - if (!/HTTP 404/.test(message)) env.log(`could not read required checks from ${path}: ${message}`); + if (/HTTP 404/.test(message)) continue; + known = false; + env.log(`could not read required checks from ${path}: ${message}`); } } - return names; + return { names, known }; } /** Files the PR changes, from `gh pr diff --name-only`. */ diff --git a/lib/pr-wait.ts b/lib/pr-wait.ts index 557b90906..b9462a1de 100644 --- a/lib/pr-wait.ts +++ b/lib/pr-wait.ts @@ -46,12 +46,22 @@ export interface PrWaitEnv { pidAlive(pid: number): boolean; } -export interface PrWaitOptions { target?: string; repo?: string; until: Until; interval: number; timeout: number } +export interface PrWaitOptions { target?: string; repo?: string; until: Until; interval: number; timeout: number; excludes: string[] } export interface BotLists { gating: string[]; advisory: string[] } interface State { reported: string[]; summaries: Record; base_oid: string | null; merge_state: string | null } -export const PR_WAIT_USAGE = `gstack-pr-wait [--repo owner/name] [--until green|ready|any-change] [--interval SECONDS] [--timeout MINUTES] +/** + * The base-overlap event, one shape for the watcher and for `gstack-ship-queue + * ready` / `train build`, which read the same fact from git (handoff + * GSTA-119): the base moved and its new commits touch files this PR changes. + */ +export function baseOverlapLine(base: string, baseOid: string, touched: readonly string[]): string { + const more = touched.length > 5 ? ` and ${touched.length - 5} more` : ''; + return `${base} moved to ${baseOid.slice(0, 7)} and touches files in this PR: ${touched.slice(0, 5).join(', ')}${more}`; +} + +export const PR_WAIT_USAGE = `gstack-pr-wait [--repo owner/name] [--until green|ready|any-change] [--interval SECONDS] [--timeout MINUTES] [--exclude CHECK,...] Watches one pull request and prints one line per real event on stdout (prefixed #), so a monitor wakes the agent once per event. Progress and recoverable @@ -69,6 +79,10 @@ comes from the URL or the current checkout. --timeout minutes before a watch that met nothing ends as PR_WAIT_BLOCKED, naming what is still pending (default 60; 0 waits without limit) +--exclude check names (comma-separated, repeatable) left out of the verdict, + as gstack-ci-gate --exclude does: a named check neither holds green + while pending nor blocks when it fails. A check the base branch + requires cannot be excluded; the name is logged as refused and gates Events: a check failed (name and link); all checks finished on the head (one summary line); a comment, review, thread or thread reply from someone @@ -101,7 +115,7 @@ lock, which the next watcher for the PR takes over once the pid is gone. Docs: docs/troubleshooting.md#gstack-pr-wait`; export function parsePrWaitArgs(argv: string[]): PrWaitOptions | { help: true } | { error: string } { - const opts: PrWaitOptions = { until: 'ready', interval: 20, timeout: DEFAULT_TIMEOUT_MINUTES }; + const opts: PrWaitOptions = { until: 'ready', interval: 20, timeout: DEFAULT_TIMEOUT_MINUTES, excludes: [] }; for (let i = 0; i < argv.length; i++) { const flag = argv[i]; if (flag === '--help' || flag === '-h') return { help: true }; @@ -124,6 +138,12 @@ export function parsePrWaitArgs(argv: string[]): PrWaitOptions | { help: true } opts.timeout = Number(value); continue; } + if (flag === '--exclude') { + const names = (argv[++i] ?? '').split(',').map(s => s.trim()).filter(Boolean); + if (!names.length) return { error: '--exclude needs one or more check names, comma-separated' }; + for (const name of names) if (!opts.excludes.includes(name)) opts.excludes.push(name); + continue; + } if (flag.startsWith('-') && !/^-?\d+$/.test(flag)) return { error: `unknown argument ${flag}` }; if (opts.target !== undefined) return { error: `unexpected argument ${flag}` }; opts.target = flag; @@ -185,12 +205,14 @@ export class Watcher { /** First run for this PR: record what exists and report nothing. */ baseline: boolean; private required: Set | null = null; + /** False when a required-check source could not be read: the set may be short, so no --exclude name is excluded. */ + private requiredKnown = true; private prevSnapshot: string | null = null; private prFiles = new Map>(); private pending: string[] = []; private readonly gh: GhEnv; - constructor(readonly env: PrWaitEnv, readonly repo: string, readonly pr: number, readonly until: Until, readonly interval: number, readonly bots: BotLists, readonly timeoutMinutes = DEFAULT_TIMEOUT_MINUTES) { + constructor(readonly env: PrWaitEnv, readonly repo: string, readonly pr: number, readonly until: Until, readonly interval: number, readonly bots: BotLists, readonly timeoutMinutes = DEFAULT_TIMEOUT_MINUTES, readonly excludes: readonly string[] = []) { const stem = path.join(stateDir(env.vars), `${repo.replace('/', '__')}__${pr}`); this.stateFile = `${stem}.json`; this.lockFile = `${stem}.lock`; @@ -295,11 +317,22 @@ export class Watcher { return count === null ? '' : `; ${count} commit${count === '1' ? '' : 's'} behind ${base}`; } - private evaluateChecks(checks: Map, head: string) { + /** + * `--exclude` names that are not required checks: the ones the verdict leaves out, as gstack-ci-gate does. While the + * required set is unknown (a source could not be read) nothing is excluded, as gstack-ci-gate refuses its overrides. + */ + private excluded(): Set { + return new Set(this.requiredKnown ? this.excludes.filter(name => !this.required?.has(name)) : []); + } + + private evaluateChecks(all: Map, head: string) { const required = this.required ?? new Set(); // Every check on the head gates, as gstack-ci-gate does, so land-and-deploy's two gates never // disagree; a required check gates even before it appears. An advisory bot's review is never - // waited for, but its check is a check like any other. + // waited for, but its check is a check like any other. A --exclude name that is not required + // is left out of the verdict; a required one cannot be excluded and gates. + const excluded = this.excluded(); + const checks = new Map([...all].filter(([name]) => !excluded.has(name))); const watched = checks; const gate = new Set([...required, ...watched.keys()]); const bucketOf = (name: string) => checks.get(name)?.bucket ?? 'pending'; @@ -392,10 +425,7 @@ export class Watcher { const oldBase = this.state.base_oid; if (pr.baseOid && oldBase && pr.baseOid !== oldBase && !this.baseline) { const touched = this.baseOverlap(head, oldBase, pr.baseOid); - if (touched.length > 0) { - const more = touched.length > 5 ? ` and ${touched.length - 5} more` : ''; - this.emit(`base:${pr.baseOid}`, `${base} moved to ${pr.baseOid.slice(0, 7)} and touches files in this PR: ${touched.slice(0, 5).join(', ')}${more}`); - } + if (touched.length > 0) this.emit(`base:${pr.baseOid}`, baseOverlapLine(base, pr.baseOid, touched)); } if (pr.baseOid) this.state.base_oid = pr.baseOid; const mergeState = pr.mergeStateStatus; @@ -418,9 +448,19 @@ export class Watcher { if (pr.state === 'CLOSED') return this.finish(PR_WAIT_EXIT.blocked, [`closed without merging: ${pr.url}`]); const base = pr.baseRefName; if (this.required === null) { - this.required = requiredChecks(this.gh, this.repo, base); + const read = requiredChecks(this.gh, this.repo, base); + this.required = read.names; + this.requiredKnown = read.known; const gate = [...this.required].sort().join(', ') || 'none set'; this.log(`watching ${this.repo}#${this.pr} until ${this.until}; required checks: ${gate}; every check on the head gates`); + if (this.excludes.length && !this.requiredKnown) { + this.log(`exclude refused for ${this.excludes.join(', ')}: the required checks could not be read, so every check gates`); + } else if (this.excludes.length) { + const refused = this.excludes.filter(name => this.required!.has(name)); + const dropped = this.excludes.filter(name => !this.required!.has(name)); + if (dropped.length) this.log(`excluded from the verdict: ${dropped.join(', ')}`); + if (refused.length) this.log(`exclude refused for required check${refused.length === 1 ? '' : 's'} ${refused.join(', ')}: a required check gates`); + } } const checks = fetchChecks(this.gh, this.repo, this.pr); const head = pr.headRefOid; @@ -530,7 +570,7 @@ export function prWaitMain(argv: string[], env: PrWaitEnv): number { if ('error' in parsed) return refuse(env, parsed.error); const target = resolveTarget(env, parsed); if ('error' in target) return refuse(env, target.error); - const watcher = new Watcher(env, target.repo, target.pr, parsed.until, parsed.interval, botLists(env.vars), parsed.timeout); + const watcher = new Watcher(env, target.repo, target.pr, parsed.until, parsed.interval, botLists(env.vars), parsed.timeout, parsed.excludes); // Exit 1 always carries a PR_WAIT_BLOCKED line, so a local fault (an unwritable state root, a full disk) is a refusal, never a verdict. try { const lock = watcher.lock(); diff --git a/lib/restamp.ts b/lib/restamp.ts index 852a99ed8..666d8c73f 100644 --- a/lib/restamp.ts +++ b/lib/restamp.ts @@ -128,12 +128,33 @@ function queueOrderCheck(o: RestampOptions, baseSha: string): void { actual[pr] = merged ? (inBase ? 'merged' : 'merged-not-in-base') : String(info.state ?? 'unknown').toLowerCase(); } const stale = [...ahead.filter(pr => actual[pr] !== 'merged'), ...behind.filter(pr => actual[pr]?.startsWith('merged'))]; - if (stale.length) { - throw new Refusal('QUEUE_STALE', `queue order moved: ${stale.map(pr => `#${pr}=${actual[pr]}`).join(', ')}`, { - expected: { order: o.expectOrder, base: o.expectBase ?? null }, actual: { prs: actual, base: baseSha }, - recompute: `gstack-restamp --next --base ${o.base} --expect-base ${baseSha}${mine ? ` --pr ${mine}` : ''} --expect-order ${o.expectOrder.join(',')}`, - }); - } + if (!stale.length) return; + // The recompute command is the corrected order, never the stale list again: this PR first (a list that left it out read + // every entry as a predecessor), PRs merged into the local base dropped, an unmerged predecessor kept ahead because it + // still has to merge first. A predecessor GitHub calls merged that the local base does not carry stays ahead too: the + // recompute pins --expect-base to that local base, and dropping the PR would let it allocate the version the + // predecessor already took (pre-landing review, pass 1); the lane fetches the base and the entry reads merged. + const corrected = correctedOrder(o.expectOrder, mine, actual); + const waitingOn = ahead.filter(pr => actual[pr] !== 'merged' && actual[pr] !== 'merged-not-in-base'); + const unfetched = ahead.filter(pr => actual[pr] === 'merged-not-in-base'); + const why = mine && idx < 0 ? `; --pr ${mine} is not in --expect-order, so every listed PR read as a predecessor: list this PR first, then the remaining order` + : [waitingOn.length ? `${waitingOn.map(pr => `#${pr}`).join(', ')} must merge before this PR stamps` : '', + unfetched.length ? `${unfetched.map(pr => `#${pr}`).join(', ')} merged but the local ${o.base} does not carry the merge yet: git fetch origin ${o.base}, then rerun` : ''] + .filter(Boolean).map(s => `; ${s}`).join(''); + throw new Refusal('QUEUE_STALE', `queue order moved: ${stale.map(pr => `#${pr}=${actual[pr]}`).join(', ')}${why}`, { + expected: { order: o.expectOrder, base: o.expectBase ?? null }, actual: { prs: actual, base: baseSha }, + recompute: `gstack-restamp --next --base ${o.base} --expect-base ${baseSha.slice(0, 12)}${mine ? ` --pr ${mine}` : ''} --expect-order ${corrected.join(',')}`, + ...(unfetched.length ? { fix: `git fetch origin ${o.base}` } : {}), + }); +} + +/** + * The --expect-order to run next: this PR first when the list left it out, every PR merged into the local base dropped, + * the rest (open PRs and merges the local base does not carry yet) in their order. + */ +export function correctedOrder(order: readonly string[], mine: string | null, actual: Record): string[] { + const rest = order.filter(pr => actual[pr] !== 'merged'); + return mine && !order.includes(mine) ? [mine, ...rest] : rest; } interface Plan { @@ -148,7 +169,7 @@ function planVersions(o: RestampOptions, loaded: LoadedPolicy, policyLabel: stri if (o.expectBase && !baseSha.out.startsWith(o.expectBase)) { throw new Refusal('QUEUE_STALE', `base moved: expected ${o.expectBase}, ${baseRef} is ${baseSha.out}`, { expected: { base: o.expectBase }, actual: { base: baseSha.out }, - recompute: `gstack-restamp --next --base ${o.base} --expect-base ${baseSha.out.slice(0, 12)}${o.expectOrder?.length ? ` --expect-order ${o.expectOrder.join(',')}` : ''}`, + recompute: `gstack-restamp --next --base ${o.base} --expect-base ${baseSha.out.slice(0, 12)}${o.pr ? ` --pr ${o.pr}` : ''}${o.expectOrder?.length ? ` --expect-order ${correctedOrder(o.expectOrder, o.pr ?? null, {}).join(',')}` : ''}`, }); } queueOrderCheck(o, baseSha.out); @@ -313,6 +334,40 @@ export function tentativeMentions(added: Map>, forkVersion: return [...found]; } +/** The versions one line mentions in a stamp form (the built-in forms and the policy's `stamp_patterns` templates). */ +export function stampMentions(text: string, templates: readonly string[] = []): Set { + const found = new Set(); + for (const re of [...BUILT_IN_MENTIONS, ...templates.map(templateMention)]) for (const m of text.matchAll(re)) found.add(m.groups!.v!); + return found; +} + +/** + * A removed/added line pair that a restamp alone explains: every version the + * removed line mentions in a stamp form and the added line no longer does + * moves to `newV`, and the rewritten line is the added line byte for byte. + * The moved versions are the ones the stamp_paths stage moves, not only the + * old version `oldV` (the gated head's): a tentative `*_KEY_VERSION = ''` never equals the base version at the slot and is a stamp all the + * same (#3512). The stage's own bound applies: a tentative mention is newer + * than the version the branch was written against, so a moved version older + * than `oldV` (a released key moved by hand) is not a stamp, and neither is + * one of another width (`tentativeMentions`: a 3-digit dependency pin in a + * 4-digit repo is never a candidate, so moving it is a hand edit). A line that + * moves nothing, or changes anything beyond those mentions, is not one either. + */ +export function stampOnlyLine(removed: string, added: string, oldV: string, newV: string, templates: readonly string[] = []): boolean { + if (removed === added) return false; + const floor = parseVersion(oldV); + const width = versionWidth(oldV); + const after = stampMentions(added, templates); + const moved = [...stampMentions(removed, templates)].filter(v => v !== newV && !after.has(v)); + if (!moved.length || !floor) return false; + if (moved.some(v => { const p = parseVersion(v); return !p || versionWidth(v) !== width || cmpVersion(p, floor) < 0; })) return false; + let text = removed; + for (const v of moved) text = rewriteStamps(text, v, newV, templates).text; + return text === added; +} + /** The version at the branch's original fork point: the first parent of its oldest own commit (base when it has none). */ export function forkVersion(repoRoot: string, baseRef: string, fallback: string): string { const first = git(repoRoot, ['rev-list', '--reverse', '--first-parent', `${baseRef}..HEAD`]).out.split('\n')[0]?.trim(); diff --git a/lib/ship-queue-train-ready.ts b/lib/ship-queue-train-ready.ts index 736b74809..bbb4cbda0 100644 --- a/lib/ship-queue-train-ready.ts +++ b/lib/ship-queue-train-ready.ts @@ -28,7 +28,7 @@ import { dispatchCi } from './ship-queue-dispatch-ci'; import { recordVerdict, screenVerdict, verdictLines, type ScreenOutcome } from './ship-queue-screen'; import { execFor, repoSlug } from './ship-queue-train-github'; import { loadReady, nowIso, persist, readyPath, rebuildFromNotes, type ReadyReceipt, type ScreenChoice } from './ship-queue-train-record'; -import { fetchBase, mergeCommit, outOk, outPending, outRefused, trainEnv, type TrainEnv, type TrainOut } from './ship-queue-train'; +import { baseOverlap, fetchBase, mergeCommit, outOk, outPending, outRefused, trainEnv, type TrainEnv, type TrainOut } from './ship-queue-train'; const Q = 'gstack-ship-queue'; @@ -181,6 +181,18 @@ export async function trainReady(ctx: QueueContext, o: ReadyOptions): Promise !r.ok)?.out.split('\n')[0] || 'git read failed'; + return { ...none, unknown: true, line: `BASE_OVERLAP: could not compare ${base}@${baseSha.slice(0, 12)} with ${head.slice(0, 12)} (${why}); conflicts unknown` }; + } + const changed = new Set(theirs.out.split('\n').filter(Boolean)); + const touched = ours.out.split('\n').filter(f => f && changed.has(f)).sort(); + if (!touched.length) return { ...none, line: `BASE: ${base} moved to ${baseSha.slice(0, 7)} since this PR forked; its new commits touch no file this PR changes` }; + if (!verdict) return { touched, conflicts: null, unknown: false, line: `BASE_OVERLAP: ${baseOverlapLine(base, baseSha, touched)}` }; + const m = mergeStep(repoRoot, baseSha, head, policy); + const conflicts = 'conflicts' in m ? m.conflicts : null; + const said = conflicts ? `the train's merge onto ${base} conflicts beyond the CHANGELOG and the stamp files (${conflicts.join(', ')})` : `the train's merge onto ${base} is clean`; + return { touched, conflicts, unknown: false, line: `BASE_OVERLAP: ${baseOverlapLine(base, baseSha, touched)}; ${said}` }; +} + const TRAIN_IDENTITY = { GIT_AUTHOR_NAME: 'gstack-ship-queue', GIT_AUTHOR_EMAIL: 'ship-queue@gstack.local', GIT_COMMITTER_NAME: 'gstack-ship-queue', GIT_COMMITTER_EMAIL: 'ship-queue@gstack.local', GIT_AUTHOR_DATE: '2000-01-01T00:00:00Z', GIT_COMMITTER_DATE: '2000-01-01T00:00:00Z' }; @@ -353,12 +387,15 @@ async function newTrain(env: TrainEnv, o: BuildOptions): Promise<{ t: TrainRecor `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}`})`] : []), ]; - const due = fillDue(order.train, env.train, nowIso(), o.urgent || o.repair); - if (due && !o.dryRun) return outPending({ code: 'QUEUE_TRAIN_WAITING', message: `${order.train.length} of ${env.train.max_size} PRs; the oldest READY dispatches at ${due}` }, lines, `${Q} train build ${prsArgs(order.train, !!explicit)} (after ${due}, or with more PRs, or --urgent for one)`, 'none'); const baseSha = git(env.ctx.repoRoot, ['rev-parse', `${env.remote}/${env.ctx.base}`]).out; const key = sha256(JSON.stringify({ base: baseSha, slots: order.train.map(s => `${s.pr}@${s.head}`) })); - const live = listTrains(env.repo.keys).find(t => t.key === key && !TERMINAL_STATES.includes(t.state) && t.state !== 'red' && t.state !== 'unattributed'); - if (live) return { t: Object.assign(live, { held: order.held.map(s => s.pr) }), lines }; + // The scratch merges run before the fill rule (#3485): a QUEUE_TRAIN_WAITING result is also a clean dry-run merge of + // every seated pin, and a pin the base moved onto is named (files only; the chain below is the merge verdict) before the + // train is refused for it. + for (const s of order.train) { + const overlap = baseOverlap(env.ctx.repoRoot, env.ctx.base, baseSha, s.head, env.policy.policy, false); + if (overlap.touched.length || overlap.unknown) lines.push(`#${s.pr} ${overlap.line}`); + } const chain = buildChain(env.ctx.repoRoot, baseSha, order.train, env.policy.policy); if ('stoppedAt' in chain) { const without = order.train.filter(s => s.pr !== chain.stoppedAt); @@ -366,6 +403,11 @@ async function newTrain(env: TrainEnv, o: BuildOptions): Promise<{ t: TrainRecor `QUEUE_TRAIN_CONFLICT: train ${key.slice(0, 12)} stopped at #${chain.stoppedAt} (conflicts: ${chain.conflicts.join(', ')}) after merging ${chain.merged.map(p => '#' + p).join(',') || '(none)'}; nothing dispatched, no branch pushed; fix: ${Q} train build ${prsArgs(without, !!explicit)}`], without.length ? `${Q} train build ${prsArgs(without, !!explicit)}` : 'none', 'none'); } + lines.push(`MERGE_CHECK: T1..T${order.train.length} merge clean onto ${env.ctx.base}@${baseSha.slice(0, 12)} (dry run in object storage; nothing pushed)`); + const due = fillDue(order.train, env.train, nowIso(), o.urgent || o.repair); + if (due && !o.dryRun) return outPending({ code: 'QUEUE_TRAIN_WAITING', message: `${order.train.length} of ${env.train.max_size} PRs; the oldest READY dispatches at ${due}` }, lines, `${Q} train build ${prsArgs(order.train, !!explicit)} (after ${due}, or with more PRs, or --urgent for one)`, 'none'); + const live = listTrains(env.repo.keys).find(t => t.key === key && !TERMINAL_STATES.includes(t.state) && t.state !== 'red' && t.state !== 'unattributed'); + if (live) return { t: Object.assign(live, { held: order.held.map(s => s.pr) }), lines }; const head = chain.prefixes.at(-1)!; const nonce = makeNonce('gate', head.commit, baseSha, randomBytes(3).toString('hex'), { purpose: 'train' }); const now = nowIso(); diff --git a/lib/ship-receipt-prose.ts b/lib/ship-receipt-prose.ts index 471cc8e26..6762fae62 100644 --- a/lib/ship-receipt-prose.ts +++ b/lib/ship-receipt-prose.ts @@ -34,7 +34,9 @@ const ESCAPE = /[.*+?^${}()|[\]\\]/g; * is not a `pass` claim), compared with the receipt. A leaf key shared by * several counters (`pass` under every gate lane, `passed` under gate and ci) * names any of them: the claim matches when its number equals one of those - * counters, and a mismatch names the first of them. + * counters, and a mismatch names the first of them. A number that is the value + * of a `key=` or `key:` pair is that key's claim alone, never the start of a + * ` ` claim about the next key (#3513). */ export function checkReceiptProse(receipt: Record, body: string): ProseCheck { const prose = body.replace(BLOCK_RE, ''); @@ -43,12 +45,20 @@ export function checkReceiptProse(receipt: Record, body: string const leaf = field.split('.').at(-1)!; byLeaf.set(leaf, [...(byLeaf.get(leaf) ?? []), { field, value }]); } + const pattern = (leaf: string) => leaf.replace(ESCAPE, '\\$&').replaceAll('_', '[ _-]'); + // The number-then-name branch must not start on a `key=value` or `key: value` pair's value: in `passes=2 blocking_found=1` + // and `passes: 2 blocking_found: 1` the 2 is passes' claim, not a "2 blocking_found" one. The offsets of every counter + // key's value are collected first, whatever the whitespace around the separator, and a claim starting there is skipped. + const values = new Set(); + for (const leaf of byLeaf.keys()) for (const m of prose.matchAll(new RegExp(`(? c.value === n)) mismatches.push({ field: counters[0]!.field, prose: n, receipt: counters[0]!.value }); diff --git a/lib/ship-receipt.ts b/lib/ship-receipt.ts index c24251dc1..298484cd4 100644 --- a/lib/ship-receipt.ts +++ b/lib/ship-receipt.ts @@ -11,6 +11,7 @@ import { approvalSummary } from './approval-citations'; import { renderIssueLines } from './issue-links'; import { matchesAny } from './ship-policy'; import { stampFileKind, versionStampOnly } from './pregate/stamp-only'; +import { stampOnlyLine } from './restamp'; export const RECEIPT_FENCE = 'gstack-ship-receipt'; const KEY_ORDER: Array = [ @@ -165,22 +166,32 @@ export interface CarryPlanInput { export interface CarryRecord { from: string; reason: string; product_patch_id: string; changed_paths: string[]; merge_commits: number; range_diff: string; covers: string[]; reruns: string[] } export type CarryPlan = { ok: true; carry: CarryRecord } | { ok: false; code: 'RECEIPT_CARRY_PRODUCT_CHANGED' | 'RECEIPT_CARRY_REFUSED'; message: string }; -/** The file's text at the reviewed head and at HEAD, with the release version each head carries (from `VERSION`, else `package.json`). */ -export interface CarryChange { before: string | null; after: string | null; oldVersion?: string | null; newVersion?: string | null } +/** + * The file's text at the reviewed head and at HEAD, with the release version each head carries (from `VERSION`, else + * `package.json`) and the base policy's `stamp_patterns` templates. + */ +export interface CarryChange { before: string | null; after: string | null; oldVersion?: string | null; newVersion?: string | null; templates?: readonly string[] } /** * A policy-named stamp path's change is a stamp when the new text reads as the - * old text once every new version in it is the old one: the rule the tree receipt - * applies to the same `stamp_paths`, `stamp_files` and `release_outputs` before - * the train accepts a stamp commit. No version change, or a file missing on - * either side, is not a stamp. + * old text once every new version in it is the old one, or when every changed + * line is a stamp-form rewrite to the new version (`stampOnlyLine`: the old + * version or a newer tentative `*_KEY_VERSION` the slot stamp moved, #3512): the rule + * the tree receipt applies to the same `stamp_paths`, `stamp_files` and + * `release_outputs` before the train accepts a stamp commit. No version + * change, or a file missing on either side, is not a stamp. */ export function stampOnlyText(change: CarryChange): boolean { const { before, after, oldVersion, newVersion } = change; if (before === null || after === null || !oldVersion || !newVersion || oldVersion === newVersion) return false; // Digit-bounded: replacing 1.2.3 inside 11.2.3 or 1.2.34 would read a different version's stamp as this one's. const escaped = newVersion.replace(/[.*+?^${}()|[\]\\]/g, '\\$&'); - return after.replace(new RegExp(`(? i).filter(i => was[i] !== now[i]); + return changed.length > 0 && changed.every(i => stampOnlyLine(was[i]!, now[i]!, oldVersion, newVersion, change.templates ?? [])); } /** diff --git a/lib/tree-receipt.ts b/lib/tree-receipt.ts index 12ce5641a..2451bcae6 100644 --- a/lib/tree-receipt.ts +++ b/lib/tree-receipt.ts @@ -15,7 +15,7 @@ import * as fs from 'node:fs'; import * as path from 'node:path'; import { normalizeChangelog } from './changelog-check'; import { compareIdentity, executionIdentity, normalizeLockfile, readBundle, type EvidenceBundle, type ExecutionIdentity } from './evidence-bundle'; -import { parseUnifiedDiff, rewriteStamps, versionAtRef, type FileDiff, type GateAheadRecord } from './restamp'; +import { parseUnifiedDiff, stampOnlyLine, versionAtRef, type FileDiff, type GateAheadRecord } from './restamp'; import type { ResultCodeName } from './result-codes'; import { matchesAny, type ShipPolicy } from './ship-policy'; @@ -49,15 +49,18 @@ function blob(cwd: string, treeish: string, rel: string): string | null { return r.status === 0 ? r.stdout : null; } -/** Every removed/added pair differs only by the exact stamp rewrites (old -> new). */ +/** + * Every removed/added pair differs only by stamp rewrites to the new version: + * the old version, or a tentative one (newer than it) the stamp_paths stage + * moved (#3512), each in a stamp form (`stampOnlyLine`). + */ function stampOnlyHunks(hunks: FileDiff['hunks'], oldV: string, newV: string, templates: readonly string[]): { why: string; line: number } | null { for (const h of hunks) { if (h.removed.length !== h.added.length) return { why: `${h.removed.length} line(s) removed, ${h.added.length} added`, line: h.at }; for (let i = 0; i < h.removed.length; i++) { const r = h.removed[i]!; const a = h.added[i]!; - const rewritten = rewriteStamps(r, oldV, newV, templates); - if (rewritten.count === 0 || rewritten.text !== a) return { why: `${JSON.stringify(r.trim().slice(0, 60))} -> ${JSON.stringify(a.trim().slice(0, 60))}`, line: h.at + i }; + if (!stampOnlyLine(r, a, oldV, newV, templates)) return { why: `${JSON.stringify(r.trim().slice(0, 60))} -> ${JSON.stringify(a.trim().slice(0, 60))}`, line: h.at + i }; } } return null; diff --git a/package.json b/package.json index 7fcbbf58c..c42be4052 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "gstack", - "version": "1.91.277", + "version": "1.91.278", "description": "Garry's Stack — Claude Code skills + fast headless browser. One repo, one install, entire AI engineering workflow.", "license": "MIT", "type": "module", diff --git a/scripts/eval-readiness.ts b/scripts/eval-readiness.ts index 81f208092..01b768047 100644 --- a/scripts/eval-readiness.ts +++ b/scripts/eval-readiness.ts @@ -5,7 +5,8 @@ * head's paid slices start only after (1) the PR has been quiet for the * window (no newer push since this run was created; `vars.EVALS_QUIET_MINUTES`, * default 10; the `evals-no-debounce` label sets it to 0) and (2) Free Tests - * (free-tests.yml) has concluded success on the same head SHA. Selection, + * (free-tests.yml) has concluded success on the same head SHA, as GitHub's + * pull_request run or as a workflow_dispatch run on the branch. Selection, * trials and verdict rules are unchanged: only the start time moves. * * A newer push cancels the run through the workflow's cancel-in-progress @@ -40,9 +41,25 @@ export type ReadinessDecision = | { kind: 'superseded'; by: string } | { kind: 'blocked'; reason: string }; -/** The newest Free Tests pull_request run on this exact head decides; a rerun updates that run in place. */ +/** + * The Free Tests run events that count as this head's CI, in order of + * precedence: GitHub's own pull_request run (it builds the merge ref, the tree + * a merge would test), then a workflow_dispatch run on the branch (the + * dispatch route in docs/ship-policy.md for a head whose only conflict with + * the base is the CHANGELOG, which GitHub builds no merge ref for and so never + * runs pull_request CI on; #3508). A dispatched run builds the head alone, so + * it decides only for a head with no pull_request run: it never overrides a red + * pull_request run (rerun that run), and a lane's extra dispatch never holds a + * head its pull_request CI cleared. A push run is a branch build, not this + * head's gate. + */ +export const FREE_TESTS_EVENTS: readonly string[] = ['pull_request', 'workflow_dispatch']; + +/** The newest Free Tests run on this exact head under the first event present decides; a rerun updates that run in place. */ export function freeTestsState(runs: FreeTestsRun[], headSha: string): FreeTestsState { - const newest = runs.filter(run => run.head_sha === headSha && run.event === 'pull_request').sort((a, b) => b.id - a.id)[0]; + const atHead = runs.filter(run => run.head_sha === headSha); + const event = FREE_TESTS_EVENTS.find(e => atHead.some(run => run.event === e)); + const newest = atHead.filter(run => run.event === event).sort((a, b) => b.id - a.id)[0]; if (!newest) return { state: 'missing' }; if (newest.status !== 'completed') return { state: 'running', runId: newest.id }; if (newest.conclusion === 'success') return { state: 'success', runId: newest.id }; @@ -111,8 +128,9 @@ export function waitForReadiness(o: WaitOptions): { decision: ReadinessDecision; if (quietUntil > now()) sleep(quietUntil - now()); let lastError = ''; for (;;) { + // Every event at the head; freeTestsState keeps the pull_request and workflow_dispatch runs. const listed = ghJson<{ workflow_runs?: FreeTestsRun[] }>(gh, ['api', '-X', 'GET', `repos/${o.repo}/actions/workflows/free-tests.yml/runs`, - '-f', `head_sha=${o.headSha}`, '-f', 'event=pull_request', '-f', 'per_page=20']); + '-f', `head_sha=${o.headSha}`, '-f', 'per_page=20']); const pr = ghJson<{ head?: { sha?: string } }>(gh, ['api', `repos/${o.repo}/pulls/${o.pr}`]); lastError = !listed.ok ? listed.error : !pr.ok ? pr.error : ''; if (!lastError && listed.ok && pr.ok) { diff --git a/scripts/train-gate.ts b/scripts/train-gate.ts index 8c38c2fd0..b5b433feb 100644 --- a/scripts/train-gate.ts +++ b/scripts/train-gate.ts @@ -26,7 +26,7 @@ import * as fs from 'node:fs'; import * as path from 'node:path'; import { normalizeChangelog } from '../lib/changelog-check'; import { normalizeLockfile } from '../lib/evidence-bundle'; -import { rewriteStamps, versionAtRef } from '../lib/restamp'; +import { stampMentions, rewriteStamps, versionAtRef } from '../lib/restamp'; import { loadPolicyFromHead, matchesAny, type ShipPolicy } from '../lib/ship-policy'; import type { BisectPrefixRecord, TrainGateRecord } from '../lib/ship-queue-train-verdict'; import type { CasePanel } from '../lib/ship-queue-train-record'; @@ -132,20 +132,38 @@ export function fullGateCases(rootDir = ROOT): Map { return plannedCases(buildRunManifest({ tier: 'gate', profile: 'full', sliceCount: 1, evalsAll: true, env: { EVALS_ALL: '1' }, rootDir }), rootDir); } +/** + * A release output or stamp path without its stamps: every version mentioned + * in a stamp form (the built-in forms and the policy's `stamp_patterns` + * templates) collapses to one placeholder. Normalising only the commit's own + * version left a tentative `*_KEY_VERSION = ''` as written on the + * unstamped prefix and normalised once the slot stamp moved it to the released + * version, so the two digests differed and train-backstop found no covering + * record for main after that train (#3512), and a base-version mention on a + * line the base already carried was normalised on the prefix and kept on the + * stamped main. Every stamp-form mention collapsing makes the digest blind to + * which version a stamp names; the receipt (`stampOnlyLine`) is what judges a + * stamp commit's content. + */ +export function stampFreeText(text: string, templates: readonly string[]): string { + let out = text; + for (const v of [...stampMentions(text, templates)].sort((a, b) => b.length - a.length)) out = rewriteStamps(out, v, '\0VERSION', templates).text; + return out; +} + /** * A commit's tree modulo stamps: every path with its mode and blob, except the * files a restamp rewrites, which contribute their content without the stamp * (the version file nothing, package.json without its version, lockfiles * without their root version, the CHANGELOG with its headings normalized, - * release outputs and stamp paths with their own version replaced). Two - * commits that differ only by a restamp have the same digest. + * release outputs and stamp paths with every stamp-form version mention + * replaced). Two commits that differ only by a restamp have the same digest. */ export function stampFreeTree(git: Git, commit: string, repoRoot: string, policy: ShipPolicy): string | { error: string } { const listed = git(['ls-tree', '-r', '-z', '--full-tree', commit]); if (!listed.ok) return { error: `cannot list ${commit.slice(0, 12)} (${listed.out.split('\n')[0]})` }; const at = versionAtRef(repoRoot, commit); const versionRel = 'rel' in at ? at.rel : 'VERSION'; - const version = 'version' in at ? at.version : null; const lines: string[] = []; for (const item of listed.out.split('\0').filter(Boolean)) { const [meta, rel] = [item.slice(0, item.indexOf('\t')), item.slice(item.indexOf('\t') + 1)]; @@ -161,7 +179,7 @@ export function stampFreeTree(git: Git, commit: string, repoRoot: string, policy if (stamp === 'manifest') { try { const parsed = JSON.parse(text); delete parsed.version; text = JSON.stringify(parsed); } catch { /* not JSON: kept verbatim */ } } else if (stamp === 'lockfile') text = normalizeLockfile(base, text); else if (stamp === 'changelog') text = normalizeChangelog(text); - else if (version) text = rewriteStamps(text, version, '\0VERSION', policy.stamp_patterns).text; + else text = stampFreeText(text, policy.stamp_patterns); lines.push(`${rel}\0${mode}\0${createHash('sha256').update(text).digest('hex')}`); } return createHash('sha256').update(lines.sort().join('\n')).digest('hex'); diff --git a/test/eval-readiness.test.ts b/test/eval-readiness.test.ts index b4133414a..7297edcca 100644 --- a/test/eval-readiness.test.ts +++ b/test/eval-readiness.test.ts @@ -53,6 +53,25 @@ describe('readiness decision', () => { expect(freeTestsState([free(3, 'completed', 'cancelled')], HEAD)).toEqual({ state: 'failed', runId: 3, conclusion: 'cancelled' }); }); + // #3508: a head whose only conflict with main is the CHANGELOG gets no pull_request CI (GitHub has no merge ref to build); + // the dispatch route runs Free Tests through workflow_dispatch on the branch, and readiness must read that run. + test('a dispatched-only head: the workflow_dispatch Free Tests run at the exact head decides, green or red, newest first; a pull_request run outranks it', () => { + const dispatched = (id: number, status: string, conclusion: string | null, over: Partial = {}) => free(id, status, conclusion, { event: 'workflow_dispatch', ...over }); + expect(freeTestsState([dispatched(40448356650, 'completed', 'success')], HEAD)).toEqual({ state: 'success', runId: 40448356650 }); + expect(freeTestsState([dispatched(5, 'completed', 'failure')], HEAD)).toEqual({ state: 'failed', runId: 5, conclusion: 'failure' }); + expect(freeTestsState([dispatched(5, 'in_progress', null)], HEAD)).toEqual({ state: 'running', runId: 5 }); + // A dispatched run on another head, or a push run on this one, is not this head's gate. + expect(freeTestsState([dispatched(5, 'completed', 'success', { head_sha: NEWER })], HEAD)).toEqual({ state: 'missing' }); + expect(freeTestsState([free(5, 'completed', 'success', { event: 'push' })], HEAD)).toEqual({ state: 'missing' }); + // A pull_request run at the head decides when one exists, whatever its age: it built the merge ref, a dispatch built the + // head alone, so a newer green dispatch never overrides a red pull_request run (rerun that run instead) and a red or + // running extra dispatch never holds a head its pull_request CI cleared (pre-landing review, pass 1). + expect(freeTestsState([free(1, 'completed', 'failure'), dispatched(2, 'completed', 'success')], HEAD)).toEqual({ state: 'failed', runId: 1, conclusion: 'failure' }); + expect(freeTestsState([dispatched(1, 'completed', 'success'), free(2, 'completed', 'failure')], HEAD)).toEqual({ state: 'failed', runId: 2, conclusion: 'failure' }); + expect(freeTestsState([free(1, 'completed', 'success'), dispatched(2, 'in_progress', null)], HEAD)).toEqual({ state: 'success', runId: 1 }); + expect(freeTestsState([free(1, 'completed', 'success'), dispatched(2, 'completed', 'failure')], HEAD)).toEqual({ state: 'success', runId: 1 }); + }); + test('ready needs both a green Free Tests on the head and an elapsed window; a moved head is superseded', () => { const green = { state: 'success', runId: 7 } as const; expect(readinessDecision({ headSha: HEAD, prHead: HEAD, free: green, quietLeftMs: 0 }).kind).toBe('ready'); @@ -80,6 +99,20 @@ describe('waitForReadiness', () => { expect(lines.join('\n')).toContain('Free Tests run 5 passed'); }); + test('a dispatched-only head starts the gate: the runs listing is not narrowed to pull_request (#3508)', () => { + const { gh, calls } = fakeGh([{ free: [free(40448356650, 'completed', 'success', { event: 'workflow_dispatch' })] }]); + const c = clock(CREATED + 3 * 3600_000); + const argsSeen: string[][] = []; + const spy: GhRunner = (args, timeout) => { argsSeen.push(args); return gh(args, timeout); }; + const { decision, lines } = waitForReadiness({ ...base, gh: spy, now: c.now, sleep: c.sleep }); + expect(decision.kind).toBe('ready'); + expect(lines.join('\n')).toContain('Free Tests run 40448356650 passed'); + const listing = argsSeen.find(args => args.some(a => a.endsWith('free-tests.yml/runs')))!; + expect(listing).toContain(`head_sha=${HEAD}`); + expect(listing.some(a => a.startsWith('event='))).toBe(false); + expect(calls.filter(call => call.endsWith('free-tests.yml/runs'))).toHaveLength(1); + }); + test('a re-run keeps the original creation time, so an elapsed window is not waited again', () => { const { gh } = fakeGh([{ free: [free(5, 'completed', 'success')] }]); const c = clock(CREATED + 3 * 3600_000); diff --git a/test/gstack-restamp-queue.test.ts b/test/gstack-restamp-queue.test.ts index 7cfb9b012..4592f3f43 100644 --- a/test/gstack-restamp-queue.test.ts +++ b/test/gstack-restamp-queue.test.ts @@ -469,7 +469,7 @@ describe('gate-ahead (--after), predecessor movement and queue staleness', () => const j = JSON.parse(r.out); expect(j).toMatchObject({ status: 'refused', code: 'QUEUE_STALE', expected: { base: assumed.slice(0, 12) } }); expect(j.actual.base).toBe(repo.git('rev-parse', 'origin/main')); - expect(j.recompute).toMatch(/^gstack-restamp --next --base main --expect-base [0-9a-f]{12}$/); + expect(j.recompute).toMatch(/^gstack-restamp --next --base main --expect-base [0-9a-f]{12} --pr 2$/); expect(repo.git('status', '--porcelain')).toBe(''); const text = restamp(repo, ['--next', '--pr', '2', '--expect-base', assumed.slice(0, 12)]); expect(text.err).toMatch(/base moved.*\(QUEUE_STALE\)\nrecompute: gstack-restamp --next/); @@ -495,9 +495,38 @@ describe('gate-ahead (--after), predecessor movement and queue staleness', () => const stale = restamp(repo, ['--next', '--pr', '2', '--expect-order', '1,2,3', '--json'], ghShim(repo, { '1': merged, '3': merged })); expect(stale.status).toBe(EXIT.refused); expect(JSON.parse(stale.out)).toMatchObject({ code: 'QUEUE_STALE', actual: { prs: { '1': 'merged', '3': 'merged' } } }); - expect(JSON.parse(stale.out).recompute).toContain('--expect-order 1,2,3'); + // The recompute is the corrected order (merged PRs dropped), never the stale list again. + expect(JSON.parse(stale.out).recompute).toMatch(/ --pr 2 --expect-order 2$/); const unmerged = restamp(repo, ['--next', '--pr', '2', '--expect-order', '1,4,2', '--json'], ghShim(repo, { '1': merged, '4': open })); expect(JSON.parse(unmerged.out)).toMatchObject({ code: 'QUEUE_STALE', actual: { prs: { '4': 'open' } } }); + expect(JSON.parse(unmerged.out).message).toContain('#4 must merge before this PR stamps'); + expect(JSON.parse(unmerged.out).recompute).toMatch(/ --expect-order 4,2$/); + // A predecessor GitHub calls merged that the local base does not carry yet (the slot call came before the lane fetched) + // stays ahead in the recompute: dropping it would let --next allocate from the stale base the version it already took. + const unfetched = { state: 'MERGED', mergeCommit: { oid: repo.git('rev-parse', 'pr-b') }, headRefOid: 'y' }; + const stale2 = restamp(repo, ['--next', '--pr', '2', '--expect-order', '1,4,2', '--json'], ghShim(repo, { '1': merged, '4': unfetched })); + expect(JSON.parse(stale2.out)).toMatchObject({ code: 'QUEUE_STALE', actual: { prs: { '4': 'merged-not-in-base' } }, fix: 'git fetch origin main' }); + expect(JSON.parse(stale2.out).message).toContain('#4 merged but the local main does not carry the merge yet: git fetch origin main, then rerun'); + expect(JSON.parse(stale2.out).recompute).toMatch(/ --expect-order 4,2$/); + }); + + // The slot-call lesson (GSTA-123, 2026-10-27): a lane given "remaining order 3487" ran --expect-order 3487 without its own + // PR, so every entry read as an unmerged predecessor and the refusal repeated the same list as its recompute. + test('--expect-order without the lane\'s own PR: the refusal says so and the recompute lists this PR first, then the remaining order', () => { + const repo = fixture(); + makePr(repo, 'pr-a'); + makePr(repo, 'pr-b'); + repo.git('checkout', '-q', 'pr-a'); + const open = { state: 'OPEN', mergeCommit: null, headRefOid: 'x' }; + const r = restamp(repo, ['--next', '--pr', '1', '--expect-order', '2,3', '--json'], ghShim(repo, { '2': open, '3': open })); + expect(r.status).toBe(EXIT.refused); + const j = JSON.parse(r.out); + expect(j).toMatchObject({ code: 'QUEUE_STALE', actual: { prs: { '2': 'open', '3': 'open' } } }); + expect(j.message).toContain('--pr 1 is not in --expect-order, so every listed PR read as a predecessor: list this PR first, then the remaining order'); + expect(j.recompute).toMatch(/^gstack-restamp --next --base main --expect-base [0-9a-f]{12} --pr 1 --expect-order 1,2,3$/); + // The corrected command passes against the same gh answers. + expect(restamp(repo, ['--next', '--pr', '1', '--expect-order', '1,2,3', '--dry-run'], ghShim(repo, { '2': open, '3': open })).status).toBe(EXIT.ok); + expect(repo.git('status', '--porcelain')).toBe(''); }); }); diff --git a/test/gstack-ship-receipt-docs.test.ts b/test/gstack-ship-receipt-docs.test.ts index 53c65b665..e889e56ae 100644 --- a/test/gstack-ship-receipt-docs.test.ts +++ b/test/gstack-ship-receipt-docs.test.ts @@ -201,6 +201,23 @@ describe('carry follow-ups: git floor, named refusal, branch-touched walk, digit expect(change('1.2.34\n', '1.2.44\n')).toBe(false); }); + // #3512: the carry classifies a stamp path as metadata only while its change is a stamp; a tentative key version the slot + // stamp moved (next minor -> released) was product, so `carry` refused RECEIPT_CARRY_PRODUCT_CHANGED on a stamp commit. + test('stampOnlyText accepts a tentative key version moved to the new version, and refuses a released one moved by hand or any other edit', () => { + const templates = ["_KEY_VERSION = '{version}'"]; + const change = (before: string, after: string) => stampOnlyText({ before, after, oldVersion: '1.2.3.0', newVersion: '1.2.4.0', templates }); + const keys = (docs: string, old: string, extra = '') => `export const DOCS_KEY_VERSION = '${docs}';\nexport const OLD_KEY_VERSION = '${old}';\n${extra}`; + expect(change(keys('1.3.0.0', '1.2.2.0'), keys('1.2.4.0', '1.2.2.0'))).toBe(true); + expect(change(keys('1.2.3.0', '1.2.2.0'), keys('1.2.4.0', '1.2.2.0'))).toBe(true); + expect(change(keys('1.3.0.0', '1.2.2.0'), keys('1.2.4.0', '1.2.4.0'))).toBe(false); + expect(change(keys('1.3.0.0', '1.2.2.0'), keys('1.2.4.0', '1.2.2.0', 'export const EXTRA = 1;\n'))).toBe(false); + // Without the template the constant is not a stamp form, so the move is a product change. + expect(stampOnlyText({ before: keys('1.3.0.0', '1.2.2.0'), after: keys('1.2.4.0', '1.2.2.0'), oldVersion: '1.2.3.0', newVersion: '1.2.4.0' })).toBe(false); + // A mention of another width is never a stamp candidate (tentativeMentions), so moving it by hand is a product change. + expect(change(keys('1.3.0', '1.2.2.0'), keys('1.2.4.0', '1.2.2.0'))).toBe(false); + expect(stampOnlyText({ before: 'left-pad since: 1.2.3\n', after: 'left-pad since: 1.2.4.0\n', oldVersion: '1.2.3.0', newVersion: '1.2.4.0' })).toBe(false); + }); + test('planCarry names at most CARRY_NAMED_PATHS paths', () => { const many = Array.from({ length: CARRY_NAMED_PATHS + 3 }, (_, i) => `src/f${i}.ts`); const plan = planCarry({ from: 'a'.repeat(40), head: 'b'.repeat(40), changed: many.map(p => ({ path: p, class: 'product' as const })), productPatchId: { from: 'x', head: 'y' }, testPatchId: { from: '', head: '' }, changedProduct: many, mergeCommits: 0, rangeDiff: '' }); diff --git a/test/pr-wait.test.ts b/test/pr-wait.test.ts index 54689d8a0..5d91a8059 100644 --- a/test/pr-wait.test.ts +++ b/test/pr-wait.test.ts @@ -12,7 +12,7 @@ import { tmpdir } from 'node:os'; import { join } from 'node:path'; import { spawn, spawnSync } from 'node:child_process'; import type { GhResult } from '../lib/ci-gate'; -import { DEFAULT_ADVISORY_BOTS, DEFAULT_GATING_BOTS, PR_WAIT_EXIT, Watcher, botLists, parsePrTarget, parsePrWaitArgs, prWaitMain, stateDir, type PrWaitEnv, type PrWaitOptions } from '../lib/pr-wait'; +import { DEFAULT_ADVISORY_BOTS, DEFAULT_GATING_BOTS, DEFAULT_TIMEOUT_MINUTES, PR_WAIT_EXIT, Watcher, botLists, parsePrTarget, parsePrWaitArgs, prWaitMain, stateDir, type PrWaitEnv, type PrWaitOptions } from '../lib/pr-wait'; import { GhError, runGh } from '../lib/pr-wait-github'; import { RESULT_CODES } from '../lib/result-codes'; @@ -360,7 +360,9 @@ describe('arguments and the gh layer', () => { expect(parsePrTarget('abc')).toMatchObject({ error: "'abc' is not a PR number or PR URL" }); expect(parsePrTarget('0')).toMatchObject({ error: "'0' is not a valid PR number" }); expect(parsePrTarget('42', 'nope')).toMatchObject({ error: "'nope' is not owner/name" }); - expect(parsePrWaitArgs(['42', '--until', 'green', '--interval', '2'])).toEqual({ target: '42', until: 'green', interval: 5, timeout: 60 }); + expect(parsePrWaitArgs(['42', '--until', 'green', '--interval', '2'])).toEqual({ target: '42', until: 'green', interval: 5, timeout: 60, excludes: [] }); + expect(parsePrWaitArgs(['42', '--exclude', 'quality-gate, canary', '--exclude', 'canary'])).toMatchObject({ excludes: ['quality-gate', 'canary'] }); + expect(parsePrWaitArgs(['42', '--exclude', ' , '])).toMatchObject({ error: expect.stringContaining('--exclude needs') }); expect(parsePrWaitArgs(['42', '--timeout', '0'])).toMatchObject({ timeout: 0 }); expect(parsePrWaitArgs(['42', '--timeout', 'soon'])).toMatchObject({ error: expect.stringContaining('--timeout needs') }); expect(parsePrWaitArgs(['--until', 'soon'])).toMatchObject({ error: expect.stringContaining('--until needs') }); @@ -421,6 +423,44 @@ describe('pass-1 review fixes: gates agree, bounded waits, bot identity, local f expect(lines()).toEqual(['#42 check failed: quality-gate https://ci.example/quality-gate', expect.stringContaining('(PR_WAIT_BLOCKED;')]); }); + // #3495 follow-up (ruled by GStack Minder, 2026-10-26): gstack-ci-gate --exclude has a watcher equivalent, so a non-required + // check stuck in pending (a canary, a deploy preview) no longer holds --until green for the whole timeout. + test('--exclude leaves a named non-required check out of the verdict: pending does not hold green, failed does not block; a required name is refused and gates', () => { + const excluded = (script: Script, ...names: string[]) => { + const e = env(fakeGh(script)); + return new Watcher(e, REPO, 42, 'green', 20, botLists(e.vars), DEFAULT_TIMEOUT_MINUTES, names); + }; + const checks = ok(checksJson(['free-tests', 'pass'], ['windows-free-tests', 'pass'], ['quality-gate', 'pending'])); + const held = excluded({ rulesets: RULESETS, checks }, 'other-check'); + expect(pollUntilDone(held, 4)).toBeNull(); + expect(lines()).toEqual([]); + const w = excluded({ rulesets: RULESETS, checks }, 'quality-gate'); + expect(pollUntilDone(w)).toBe(PR_WAIT_EXIT.met); + expect(lines()).toEqual(['#42 green on aaaaaaa: 2 pass; required 2/2 green; no greptile or macroscope yet; 0 unresolved threads; merge state CLEAN; 0 commits behind main']); + expect(stderr.join('')).toContain('excluded from the verdict: quality-gate'); + stdout = []; stderr = []; + const red = excluded({ rulesets: RULESETS, checks: ok(checksJson(['free-tests', 'pass'], ['windows-free-tests', 'pass'], ['quality-gate', 'fail'])) }, 'quality-gate'); + expect(pollUntilDone(red)).toBe(PR_WAIT_EXIT.met); + expect(lines()).toEqual([expect.stringMatching(/^#42 green on aaaaaaa: 2 pass;/)]); + stdout = []; stderr = []; + const refused = excluded({ rulesets: RULESETS, checks: ok(checksJson(['free-tests', 'fail'], ['windows-free-tests', 'pass'])) }, 'free-tests'); + expect(refused.poll()).toBe(PR_WAIT_EXIT.blocked); + expect(lines()[0]).toBe('#42 required check failed: free-tests https://ci.example/free-tests'); + expect(stderr.join('')).toContain('exclude refused for required check free-tests: a required check gates'); + stdout = []; stderr = []; + // When a required-check source cannot be read the set may be short, so nothing is excluded (gstack-ci-gate refuses its + // overrides the same way): the excluded failure gates and the first poll says why. + const unknown = excluded({ rulesets: err('gh: HTTP 403: rate limited', 1), checks: ok(checksJson(['free-tests', 'pass'], ['windows-free-tests', 'pass'], ['quality-gate', 'fail'])) }, 'quality-gate'); + expect(unknown.poll()).toBe(PR_WAIT_EXIT.blocked); + expect(lines()[0]).toBe('#42 check failed: quality-gate https://ci.example/quality-gate'); + expect(stderr.join('')).toContain('exclude refused for quality-gate: the required checks could not be read, so every check gates'); + stdout = []; stderr = []; + // Through main: the flag reaches the watcher. + const e = env(fakeGh({ rulesets: RULESETS, checks })); + expect(prWaitMain(['42', '--repo', REPO, '--until', 'green', '--exclude', 'quality-gate'], e)).toBe(PR_WAIT_EXIT.met); + expect(lines()).toEqual([expect.stringMatching(/^#42 green on aaaaaaa: 2 pass;/)]); + }); + test('a failed bot check is a failed check, gating or advisory, as gstack-ci-gate counts it; never a completed review', () => { const gating = watcher({ pr: prJson({ reviews: [] }), checks: ok(checksJson(['build', 'pass'], ['greptile-review', 'fail'])) }); expect(pollUntilDone(gating)).toBe(PR_WAIT_EXIT.blocked); diff --git a/test/ship-queue-train-ops.test.ts b/test/ship-queue-train-ops.test.ts index e2af535fe..61df3d0dd 100644 --- a/test/ship-queue-train-ops.test.ts +++ b/test/ship-queue-train-ops.test.ts @@ -157,6 +157,8 @@ describe('#3465: the screen is planned on a scratch merge when GitHub\'s merge r expect([dry.status, dry.err]).toEqual([EXIT.ok, '']); const main = repo.git('rev-parse', 'origin/main'); expect(line(dry.out, 'SCREEN_MERGE:')).toMatch(new RegExp(`^SCREEN_MERGE: scratch merge [0-9a-f]{12} of #1@${head.slice(0, 12)} onto main@${main.slice(0, 12)} \\(refs/pull/1/merge [0-9a-f]{12} merges onto [0-9a-f]{12}, not main@${main.slice(0, 12)}\\); would push screen/1-${head.slice(0, 12)} \\(dry run\\)$`)); + // gstack-pr-wait's base-overlap event, read from git at READY (handoff GSTA-119): main's stamp touched the CHANGELOG this PR also changes. + expect(line(dry.out, 'BASE_OVERLAP:')).toBe(`BASE_OVERLAP: main moved to ${main.slice(0, 7)} and touches files in this PR: CHANGELOG.md; the train's merge onto main is clean`); expect(repo.git('ls-remote', 'origin', 'refs/heads/screen/*')).toBe(''); const real = queue(fx, ['ready', '--pr', '1', '--pregate', pregate, '--screen']); expect([real.status, real.err]).toEqual([EXIT.ok, '']); @@ -195,6 +197,15 @@ describe('#3465: the screen is planned on a scratch merge when GitHub\'s merge r expect(conflict.err).toMatch(/#1 conflicts with main@[0-9a-f]{12} beyond the CHANGELOG and the stamp files \(pr-1\.txt\); GitHub's merge ref lags too \(origin has no refs\/pull\/1\/merge\).*\(QUEUE_SCREEN_STALE_REF\)/); expect(next(conflict.out)).toBe('NEXT: resolve the conflict with main, push, run /ship, then gstack-ship-queue ready --pr 1 --screen'); expect(dispatches(fx, 'evals.yml').length).toBe(1); + const main2 = repo.git('rev-parse', 'origin/main'); + expect(line(conflict.out, 'BASE_OVERLAP:')).toBe(`BASE_OVERLAP: main moved to ${main2.slice(0, 7)} and touches files in this PR: CHANGELOG.md, pr-1.txt; the train's merge onto main conflicts beyond the CHANGELOG and the stamp files (pr-1.txt)`); + // Without a screen there is no scratch merge to refuse at, so READY itself refuses the conflict the train would stop at; + // the fix follows the crew template's rule 14 (the base is merged on the controller's word, never by the lane alone). + const noScreen = queue(fx, ['ready', '--pr', '1', '--pregate', pregate, '--no-screen']); + expect(noScreen.status).toBe(EXIT.refused); + expect(noScreen.err).toMatch(new RegExp(`#1 conflicts with main@${main2.slice(0, 12)} beyond the CHANGELOG and the stamp files \\(pr-1\\.txt\\); the train would stop at this PR.*\\(QUEUE_TRAIN_CONFLICT\\)`)); + expect(next(noScreen.out)).toBe('NEXT: report the conflict to the controller; on its word merge main into the pin (never rebase), resolve, push, run /ship, then gstack-ship-queue ready --pr 1 --screen|--no-screen'); + expect(fs.existsSync(path.join(fx.state, 'ship-queue', 'ready', repoKey(fx), `1-${head}.json`))).toBe(true); }, 120_000); }); diff --git a/test/ship-queue-train.test.ts b/test/ship-queue-train.test.ts index 318a25657..295c280e0 100644 --- a/test/ship-queue-train.test.ts +++ b/test/ship-queue-train.test.ts @@ -16,6 +16,7 @@ import { spawnSync } from 'node:child_process'; import * as fs from 'node:fs'; import * as path from 'node:path'; import { EXIT } from '../lib/headless-artifacts'; +import { baseOverlap } from '../lib/ship-queue-train'; 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'; @@ -352,6 +353,44 @@ describe('train build', () => { expect(dispatches(fx)).toEqual([]); }, 120_000); + // Handoff GSTA-119 / #3485: a pooled pin the base moved onto is named with the files, and the conflict is found while the + // train is still filling instead of at dispatch time; a base move onto files the PR leaves alone is silent. + test('a pin whose files main moved onto gets the base-overlap line; a conflict with main is QUEUE_TRAIN_CONFLICT at slot 1 even while the train fills', () => { + const fx = make(); + const h1 = openPr(fx, '1', { file: 'same.txt', text: 'one\n' }); + const h2 = openPr(fx, '2'); + readyReceipt(fx, '1', h1, { at: '2026-10-18T11:00:00.000Z' }); + readyReceipt(fx, '2', h2, { at: '2026-10-18T11:00:00.000Z' }); + const { repo } = fx; + repo.git('checkout', '-q', '-B', 'mainsim', 'origin/main'); + fs.writeFileSync(path.join(repo.work, 'same.txt'), 'main\n'); + fs.writeFileSync(path.join(repo.work, 'unrelated.txt'), 'main\n'); + repo.git('add', '-A'); + repo.git('commit', '-qm', 'main moves onto same.txt'); + repo.git('push', '-q', 'origin', 'HEAD:refs/heads/main'); + repo.git('checkout', '-q', 'pr-1'); + const main = repo.git('rev-parse', 'origin/main'); + const r = queue(fx, ['train', 'build', '--prs', `2@${h2},1@${h1}`]); + expect(r.status).toBe(EXIT.fail); + // At build the line is the files alone: the chain merges each pin onto its prefix, and that merge is the verdict. + expect(line(r.out, '#1 BASE_OVERLAP:')).toBe(`#1 BASE_OVERLAP: main moved to ${main.slice(0, 7)} and touches files in this PR: same.txt`); + expect(r.out).not.toContain('#2 BASE'); + expect(line(r.out, 'QUEUE_TRAIN_CONFLICT:')).toMatch(new RegExp(`stopped at #1 \\(conflicts: same\\.txt\\) after merging (\\(none\\)|#2); nothing dispatched, no branch pushed; fix: gstack-ship-queue train build --prs 2@${h2}$`)); + expect(r.out).not.toContain('MERGE_CHECK:'); + expect(remoteRefs(fx)).not.toContain('refs/heads/train/'); + expect(dispatches(fx)).toEqual([]); + const without = queue(fx, ['train', 'build', '--prs', `2@${h2}`]); + expect(without.status).toBe(EXIT.refused); + expect(without.err).toContain('(QUEUE_TRAIN_WAITING)'); + expect(line(without.out, 'MERGE_CHECK:')).toBe(`MERGE_CHECK: T1..T1 merge clean onto main@${main.slice(0, 12)} (dry run in object storage; nothing pushed)`); + // A git read that fails is never "no overlap" (pre-landing review, pass 1): the comparison is reported as unknown. + const unknown = baseOverlap(repo.work, 'main', main, 'f'.repeat(40), TRAIN_POLICY as any); + expect(unknown).toMatchObject({ unknown: true, touched: [], conflicts: null }); + expect(unknown.line).toMatch(new RegExp(`^BASE_OVERLAP: could not compare main@${main.slice(0, 12)} with ffffffffffff \\(.+\\); conflicts unknown$`)); + // At build the overlap line carries the files alone; the verdict is the chain's. + expect(baseOverlap(repo.work, 'main', main, h1, TRAIN_POLICY as any, false).line).toBe(`BASE_OVERLAP: main moved to ${main.slice(0, 7)} and touches files in this PR: same.txt`); + }, 120_000); + test("GSTA-67: a PR whose CHANGELOG drops a released heading (#3345's pin 1eb7092) is a CHANGELOG conflict, never merged with the damage", () => { const fx = make(); const h1 = openPr(fx, '1'); @@ -423,6 +462,8 @@ describe('train build', () => { expect(filling.status).toBe(EXIT.refused); expect(filling.err).toMatch(/1 of 5 PRs; the oldest READY dispatches at 2026-10-18T14:00:00\.000Z.*\(QUEUE_TRAIN_WAITING\)/); expect(result(filling.out)).toBe('GSTACK_RESULT: skill=ship-queue status=incomplete run=none'); + // #3485: the scratch merges run before the fill rule, so a WAITING result also proves the seated pins merge clean. + expect(line(filling.out, 'MERGE_CHECK:')).toBe(`MERGE_CHECK: T1..T1 merge clean onto main@${fx.repo.git('rev-parse', 'origin/main').slice(0, 12)} (dry run in object storage; nothing pushed)`); fx.write(d => { d.rules['evals.yml'] = GREEN_GATE; }); const urgent = queue(fx, ['train', 'build', '--prs', `3@${h3}`, '--urgent']); expect(urgent.status).toBe(EXIT.ok); diff --git a/test/ship-receipt-prose.test.ts b/test/ship-receipt-prose.test.ts index f352e5765..3e00f8913 100644 --- a/test/ship-receipt-prose.test.ts +++ b/test/ship-receipt-prose.test.ts @@ -48,6 +48,28 @@ describe('checkReceiptProse', () => { expect(bad.mismatches).toEqual([{ field: 'gate.unit.pass', prose: 40, receipt: 41 }]); expect(checkReceiptProse(receipt, 'Gate passed: 41. CI passed=41.').mismatches).toEqual([]); }); + + // #3513: the canonical REVIEW_LOOP line /ship and `gstack-ship-queue ready` print. Before the fix the ` ` branch + // started on passes' value and read `2 blocking_found` as a blocking_found claim of 2 (PROSE: mismatch ... body=2 receipt=1). + test('a key=value pair\'s value is that key\'s claim alone: the REVIEW_LOOP line parses as five claims that all match', () => { + const loop = { ...receipt, review_loop: { passes: 2, blocking_found: 1, deferred: 0, noted: 4, fixed: 8, scope: 'frozen' } }; + const line = 'REVIEW_LOOP: passes=2 blocking_found=1 deferred=0 noted=4 fixed=8 scope=frozen'; + const ok = checkReceiptProse(loop, line); + expect(ok).toEqual({ checked: 5, mismatches: [] }); + expect(proseLine(ok)).toBe('PROSE: 5 receipt counter claims in the body, all match'); + const one = { ...receipt, review_loop: { passes: 1, blocking_found: 0, deferred: 0, noted: 0, fixed: 0, scope: 'frozen' } }; + expect(checkReceiptProse(one, 'REVIEW_LOOP: passes=1 blocking_found=0 deferred=0 noted=0 fixed=0 scope=frozen')).toEqual({ checked: 5, mismatches: [] }); + // A real disagreement on the same line is still caught, and named for the key that carries it. + expect(checkReceiptProse(loop, 'REVIEW_LOOP: passes=2 blocking_found=3 deferred=0 noted=4 fixed=8 scope=frozen').mismatches) + .toEqual([{ field: 'review_loop.blocking_found', prose: 3, receipt: 1 }]); + // Prose claims in the number-then-name spelling keep working when they do not follow a pair. + expect(checkReceiptProse(loop, 'The loop ran 2 passes and found 1 blocking_found item.')).toEqual({ checked: 2, mismatches: [] }); + // The `key: value` spelling with whitespace is the same pair (pre-landing review, pass 1): the value never starts the + // next key's claim, while a non-counter word before the number (`Review: 2 passes`) leaves the claim as it was. + expect(checkReceiptProse(loop, 'REVIEW_LOOP: passes: 2 blocking_found: 1 deferred: 0 noted: 4 fixed: 8')).toEqual({ checked: 5, mismatches: [] }); + expect(checkReceiptProse(loop, 'Gate passed: 41 failed: 0')).toEqual({ checked: 2, mismatches: [] }); + expect(checkReceiptProse(loop, 'Review: 2 passes; passes = 2 fixed = 8.')).toEqual({ checked: 3, mismatches: [] }); + }); }); describe('gstack-ship-receipt read --check-prose', () => { diff --git a/test/train-gate.test.ts b/test/train-gate.test.ts index e0dfa4cd7..32757a46d 100644 --- a/test/train-gate.test.ts +++ b/test/train-gate.test.ts @@ -224,6 +224,40 @@ describe('prefix trees modulo stamps', () => { expect(stampFreeTree(git, 'f'.repeat(40), repo.work, policy)).toHaveProperty('error'); }); + // #3512: a stamp path carrying a tentative `*_KEY_VERSION = ''` (gstack's own convention since #3511) digests the + // same before and after the slot stamp moves it, so train-backstop finds the record that covers main; a base-version mention + // on a line the base already carried digests the same at the prefix (where it equals VERSION) and on the stamped main. + test('stampFreeTree collapses every stamp-form mention, not only the commit\'s version: a moved tentative key and a kept released one both digest equal', () => { + const git = gitIn(repo.work); + const policy = { ...loadPolicyFromHead(repo.work).policy, stamp_paths: ['docs/**/*.md', 'lib/policy.ts'], stamp_patterns: ["_KEY_VERSION = '{version}'"] }; + const policyFile = (text: string) => { fs.mkdirSync(path.join(repo.work, 'lib'), { recursive: true }); fs.writeFileSync(path.join(repo.work, 'lib', 'policy.ts'), text); }; + // The unstamped prefix: VERSION 1.2.3.0, a new key written as the tentative 1.3.0.0, a released key at 1.2.3.0 (the base's version). + repo.git('checkout', '-q', '-B', 'key-prefix', sha.main!); + policyFile("export const DOCS_KEY_VERSION = '1.3.0.0';\nexport const REVIEW_KEY_VERSION = '1.2.3.0';\nexport const OLD_KEY_VERSION = '1.2.2.0';\n"); + repo.git('add', '-A'); repo.git('commit', '-q', '-m', 'prefix with a tentative key'); + const prefix = repo.git('rev-parse', 'HEAD'); + // Main after the slot stamp: VERSION 1.2.4.0, the tentative moved to the released version, the released key as written. + fs.writeFileSync(path.join(repo.work, 'VERSION'), '1.2.4.0\n'); + policyFile("export const DOCS_KEY_VERSION = '1.2.4.0';\nexport const REVIEW_KEY_VERSION = '1.2.3.0';\nexport const OLD_KEY_VERSION = '1.2.2.0';\n"); + repo.git('commit', '-qam', 'stamp 1.2.4.0'); + const stamped = repo.git('rev-parse', 'HEAD'); + expect(stampFreeTree(git, stamped, repo.work, policy)).toBe(stampFreeTree(git, prefix, repo.work, policy)); + // A byte beyond a stamp-form mention still changes the digest. + policyFile("export const DOCS_KEY_VERSION = '1.2.4.0';\nexport const REVIEW_KEY_VERSION = '1.2.3.0';\nexport const OLD_KEY_VERSION = '1.2.2.0';\nexport const EXTRA = 1;\n"); + repo.git('commit', '-qam', 'a code byte'); + expect(stampFreeTree(git, repo.git('rev-parse', 'HEAD'), repo.work, policy)).not.toBe(stampFreeTree(git, prefix, repo.work, policy)); + // The receipt agrees with the digest on the stamp, and refuses a released key moved by hand (older than the gated version; + // a mention equal to it is the base's own version, which the stamp_paths stage moves on a branch-added line). + const tree = (current: string) => (buildReceipt({ repoRoot: repo.work, gated: prefix, current, policy, policyLabel: 'fixture' }) as { tree: string; residual: string[] }).tree; + expect(tree(stamped)).toBe('same-modulo-stamps'); + repo.git('checkout', '-q', '-B', 'hand-move', prefix); + fs.writeFileSync(path.join(repo.work, 'VERSION'), '1.2.4.0\n'); + policyFile("export const DOCS_KEY_VERSION = '1.2.4.0';\nexport const REVIEW_KEY_VERSION = '1.2.3.0';\nexport const OLD_KEY_VERSION = '1.2.4.0';\n"); + repo.git('commit', '-qam', 'stamp plus a released key moved by hand'); + expect(tree(repo.git('rev-parse', 'HEAD'))).toBe('changed'); + repo.git('checkout', '-q', 'squash'); + }); + test('T1..Tk come from the train head by first parents; a head that is not a train is refused', () => { const git = gitIn(repo.work); const policy = loadPolicyFromHead(repo.work).policy; diff --git a/test/tree-receipt-stamp-sensitive.test.ts b/test/tree-receipt-stamp-sensitive.test.ts index dfe462f9c..245c11962 100644 --- a/test/tree-receipt-stamp-sensitive.test.ts +++ b/test/tree-receipt-stamp-sensitive.test.ts @@ -8,7 +8,8 @@ import { afterEach, describe, expect, test } from 'bun:test'; import * as fs from 'node:fs'; import * as path from 'node:path'; import { EXIT } from '../lib/headless-artifacts'; -import { cleanup, ghShim, makeFixtureRepo, makePr, readFile, recordAndBundle, restamp, runBin, type FixtureRepo } from './helpers/restamp-fixture'; +import { stampOnlyLine } from '../lib/restamp'; +import { BASE_POLICY, cleanup, ghShim, makeFixtureRepo, makePr, readFile, recordAndBundle, restamp, runBin, type FixtureRepo } from './helpers/restamp-fixture'; const repos: FixtureRepo[] = []; const fixture = (opts?: Parameters[0]) => { const r = makeFixtureRepo({ prefix: 'tree-receipt-', ...opts }); repos.push(r); return r; }; @@ -55,6 +56,55 @@ describe('tree receipt: stamps, stamp-sensitive gates and negative fixtures', () expect(r.out).not.toContain('pr-a.txt'); }); + // #3512: under the next-minor convention a new `*_KEY_VERSION` constant is written as a tentative future version, which the + // stamp_paths stage moves to the released one at the slot. The receipt accepted a stamp_paths pair only as base -> new, so the + // moved constant classified the file `changed` and the train refused the stamp commit (QUEUE_STAMP_NOT_STAMP_ONLY). + test('a tentative key version a restamp moved is a stamp, like the base version; a line that changes more than the mention is not', () => { + const repo = fixture({ policy: { ...BASE_POLICY, stamp_paths: ['docs/**/*.md', 'lib/policy.ts'], stamp_patterns: ["_KEY_VERSION = '{version}'"] } }); + makePr(repo, 'pr-a', { expects: '1.2.4.0' }); + fs.mkdirSync(path.join(repo.work, 'lib'), { recursive: true }); + fs.writeFileSync(path.join(repo.work, 'lib', 'policy.ts'), "export const DOCS_KEY_VERSION = '1.3.0.0';\nexport const OLD_KEY_VERSION = '1.2.2.0';\n"); + repo.git('add', '-A'); repo.git('commit', '-q', '-m', 'a new policy key, tentative next minor'); + const bundle = path.join(repo.root, 'bundle.json'); + recordAndBundle(repo, repo.work, LANES, bundle); + const gatedHead = repo.git('rev-parse', 'HEAD'); + expect(restamp(repo, ['--version', '1.2.4.0']).status).toBe(EXIT.ok); + // The tentative 1.3.0.0 moved; the released 1.2.2.0 stayed. + expect(readFile(repo, 'lib/policy.ts')).toBe("export const DOCS_KEY_VERSION = '1.2.4.0';\nexport const OLD_KEY_VERSION = '1.2.2.0';\n"); + repo.git('commit', '-qam', 'stamp 1.2.4.0'); + const r = receipt(repo, ['--gated', gatedHead, '--bundle', bundle]); + expect(r.status).toBe(EXIT.ok); + expect(r.out).toMatch(/^version:\s+1\.2\.3\.0 -> 1\.2\.4\.0$/m); + expect(r.out).toMatch(/^ stamp lib\/policy\.ts \(stamp_paths\)$/m); + expect(r.out).toMatch(/^TREE: same-modulo-stamps$/m); + expect(r.out).toMatch(/^gate-reuse: eligible/m); + // Negative: the version moves and the key is renamed on the same line; a tentative value moved by hand to the wrong version. + fs.writeFileSync(path.join(repo.work, 'lib', 'policy.ts'), "export const DOCS_CAPS_KEY_VERSION = '1.2.4.0';\nexport const OLD_KEY_VERSION = '1.2.2.0';\n"); + repo.git('commit', '-qam', 'rename the key too'); + const renamed = receipt(repo, ['--gated', gatedHead, '--bundle', bundle]); + expect(renamed.status).toBe(EXIT.fail); + expect(renamed.out).toMatch(/^ changed lib\/policy\.ts \(stamp_paths: "export const DOCS_KEY_VERSION = '1\.3\.0\.0';" -> "export const DOCS_CAPS_KEY_VERSION = '1\.2\.4\.0';"\)$/m); + repo.git('reset', '-q', '--hard', 'HEAD~1'); + fs.writeFileSync(path.join(repo.work, 'lib', 'policy.ts'), "export const DOCS_KEY_VERSION = '1.2.4.0';\nexport const OLD_KEY_VERSION = '1.2.4.0';\n"); + repo.git('commit', '-qam', 'move a released key by hand'); + const wrong = receipt(repo, ['--gated', gatedHead, '--bundle', bundle]); + expect(wrong.status).toBe(EXIT.fail); + expect(wrong.out).toMatch(/^TREE: changed \(lib\/policy\.ts\)$/m); + // A mention of another width (a 3-digit pin in a 4-digit repo) is never a stamp candidate, so moving it by hand to the + // new version is a product change even though it reads newer than the gated head (pre-landing review, pass 1). + repo.git('reset', '-q', '--hard', 'HEAD~1'); + fs.writeFileSync(path.join(repo.work, 'lib', 'policy.ts'), "export const DOCS_KEY_VERSION = '1.2.4.0';\nexport const OLD_KEY_VERSION = '1.2.2.0';\nexport const PIN_KEY_VERSION = '9.9.9';\n"); + repo.git('commit', '-qam', 'a 3-digit pin'); + const pinned = repo.git('rev-parse', 'HEAD'); + fs.writeFileSync(path.join(repo.work, 'lib', 'policy.ts'), "export const DOCS_KEY_VERSION = '1.2.4.0';\nexport const OLD_KEY_VERSION = '1.2.2.0';\nexport const PIN_KEY_VERSION = '1.2.4.0';\n"); + repo.git('commit', '-qam', 'move the 3-digit pin by hand'); + expect(stampOnlyLine("export const PIN_KEY_VERSION = '9.9.9';", "export const PIN_KEY_VERSION = '1.2.4.0';", '1.2.3.0', '1.2.4.0', ["_KEY_VERSION = '{version}'"])).toBe(false); + expect(stampOnlyLine('left-pad since: 1.2.3', 'left-pad since: 1.2.4.0', '1.2.3.0', '1.2.4.0')).toBe(false); + const width = receipt(repo, ['--gated', pinned, '--bundle', bundle]); + expect(width.status).toBe(EXIT.fail); + expect(width.out).toMatch(/^TREE: changed \(lib\/policy\.ts\)$/m); + }); + test('negative fixture: a dependency pin equal to the old version is `changed`, not a stamp, even inside stamp_paths', () => { const repo = fixture(); const g = gated(repo);