diff --git a/CHANGELOG.md b/CHANGELOG.md index 6bc75c54d..6ba3eea5f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,25 @@ # Changelog +## [Unreleased] + +**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. The newest run at the head still decides, a red dispatched run still 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, so `REVIEW_LOOP: passes=2 blocking_found=1 deferred=0 noted=4 fixed=8 scope=frozen` parses 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 bound (`lib/restamp.ts` `stampOnlyLine`): a moved version older than the gated head's is a released key moved by hand and stays `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), merged PRs dropped, an unmerged predecessor kept ahead with a note that it must merge first. 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, crew rule 25). `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. `train build` runs the scratch merges before the fill rule. A seated pin whose files the base moved onto gets `# BASE_OVERLAP: …`, 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. + +### Tests +- `test/eval-readiness.test.ts`: a dispatched-only head decides green, red and running; 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`, 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 or a released key moved by hand is not. `test/gstack-restamp-queue.test.ts`: the corrected `--expect-order` in each refusal and the own-PR-first lesson. `test/pr-wait.test.ts`: `--exclude` parsing, pending and failed excluded checks, a refused required name, 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.273.0] - 2026-10-27 **A PR body opens with the problem it solves, says what else it touches, and the prose /ship publishes is checked for autopilot tells before it goes out.** A /ship PR body began with a list of what changed, so a reader had to work out the problem from the inventory; nothing in the body said what else the change could break or how sure the author was that it would not; and a CHANGELOG entry or PR body could carry the words a model writes when it is not thinking (`a vibrant tapestry of enhancements`, `not just X, but Y`, an arrow where a sentence belongs) because nothing checked prose mechanically. Garry approved the tstack and pstack absorb plans on 2026-10-26 ("Do it, send"); this release carries tstack item 4 and pstack items 2(a) and 11. 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..e51ac68fc 100644 --- a/docs/TESTING_INTERNALS.md +++ b/docs/TESTING_INTERNALS.md @@ -1236,7 +1236,11 @@ 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 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 +newest run at the head decides, 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/ship-policy.md b/docs/ship-policy.md index 3ed2f3457..65b010986 100644 --- a/docs/ship-policy.md +++ b/docs/ship-policy.md @@ -424,7 +424,16 @@ 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 conflict under `--no-screen` is +`QUEUE_TRAIN_CONFLICT` at READY, since the train would stop at this PR: the +lane trial-merges the base into the pin (crew rule 25), 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 @@ -452,7 +461,12 @@ 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: …` for a seated pin whose files the base +moved onto since it forked, 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 (crew rule 25's 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 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..5ce058aa0 100644 --- a/docs/troubleshooting.md +++ b/docs/troubleshooting.md @@ -2708,11 +2708,15 @@ 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=`). ### `RESTAMP_VERSION_SOURCE` @@ -4174,12 +4178,18 @@ 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 +**Fix.** From `train build`, run the printed `train build --prs `. The PR's lane trial-merges main into the pin (crew rule 25; with the +earlier PRs once they land), resolves, pushes, runs /ship and reports READY again. @@ -4209,7 +4219,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 +4702,18 @@ 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. 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.ts b/lib/pr-wait.ts index 557b90906..f7f5925c5 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; @@ -190,7 +210,7 @@ export class Watcher { 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 +315,19 @@ 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. */ + private excluded(): Set { + return new Set(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 +420,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; @@ -421,6 +446,12 @@ export class Watcher { this.required = requiredChecks(this.gh, this.repo, base); 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) { + 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 +561,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..1b647d640 100644 --- a/lib/restamp.ts +++ b/lib/restamp.ts @@ -128,12 +128,23 @@ 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), merged PRs dropped, an unmerged predecessor kept ahead because it still has to merge first. + const corrected = correctedOrder(o.expectOrder, mine, actual); + const waitingOn = ahead.filter(pr => actual[pr] !== 'merged' && !actual[pr]?.startsWith('merged')); + 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` : ''; + 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(',')}`, + }); +} + +/** The --expect-order to run next: this PR first when the list left it out, every merged PR dropped, the rest in their order. */ +export function correctedOrder(order: readonly string[], mine: string | null, actual: Record): string[] { + const rest = order.filter(pr => !actual[pr]?.startsWith('merged')); + return mine && !order.includes(mine) ? [mine, ...rest] : rest; } interface Plan { @@ -148,7 +159,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 +324,37 @@ 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. 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 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 || 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..1dc281869 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,15 @@ export async function trainReady(ctx: QueueContext, o: ReadyOptions): Promise new Set(git(repoRoot, ['diff', '--name-only', from, to]).out.split('\n').filter(Boolean)); + const theirs = names(fork.out, baseSha); + const touched = [...names(fork.out, head)].filter(f => theirs.has(f)).sort(); + if (!touched.length) return { touched, conflicts: null, line: `BASE: ${base} moved to ${baseSha.slice(0, 7)} since this PR forked; its new commits touch no file this PR changes` }; + const m = mergeStep(repoRoot, baseSha, head, policy); + const conflicts = 'conflicts' in m ? m.conflicts : null; + const verdict = 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, line: `BASE_OVERLAP: ${baseOverlapLine(base, baseSha, touched)}; ${verdict}` }; +} + 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 +377,14 @@ 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, rule 25): 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 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); + if (overlap.touched.length) 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 +392,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..992879146 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, ''); @@ -47,7 +49,9 @@ export function checkReceiptProse(receipt: Record, body: string let checked = 0; for (const [leaf, counters] of byLeaf) { const name = leaf.replace(ESCAPE, '\\$&').replaceAll('_', '[ _-]'); - const re = new RegExp(`(?:(? = [ @@ -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/scripts/eval-readiness.ts b/scripts/eval-readiness.ts index 81f208092..c33dd3106 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,18 @@ 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: GitHub's own + * pull_request run, and 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 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 decides, pull_request or dispatched; 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 newest = runs.filter(run => run.head_sha === headSha && FREE_TESTS_EVENTS.includes(run.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 +121,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..3f1598cfe 100644 --- a/test/eval-readiness.test.ts +++ b/test/eval-readiness.test.ts @@ -53,6 +53,21 @@ 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', () => { + 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' }); + // The newest run at the head decides whatever its event: a dispatched rerun after a red pull_request run is green. + expect(freeTestsState([free(1, 'completed', 'failure'), dispatched(2, 'completed', 'success')], HEAD)).toEqual({ state: 'success', runId: 2 }); + expect(freeTestsState([dispatched(1, 'completed', 'success'), free(2, 'completed', 'failure')], HEAD)).toEqual({ state: 'failed', runId: 2, conclusion: 'failure' }); + }); + 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 +95,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..a327b22d6 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,31 @@ 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$/); + }); + + // 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..98d18b7a4 100644 --- a/test/gstack-ship-receipt-docs.test.ts +++ b/test/gstack-ship-receipt-docs.test.ts @@ -201,6 +201,20 @@ 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); + }); + 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..f3bd509cc 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,37 @@ 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 = []; + // 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..1ca9c7cc3 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,14 @@ 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 (rule 25). + 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: trial-merge main into the pin (rule 25), 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..8baab6e97 100644 --- a/test/ship-queue-train.test.ts +++ b/test/ship-queue-train.test.ts @@ -352,6 +352,37 @@ 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); + 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; the train's merge onto main conflicts beyond the CHANGELOG and the stamp files (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)`); + }, 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 +454,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 / rule 25: 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..ccd6e0bc3 100644 --- a/test/ship-receipt-prose.test.ts +++ b/test/ship-receipt-prose.test.ts @@ -48,6 +48,23 @@ 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: [] }); + }); }); 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..c99cc61ab 100644 --- a/test/tree-receipt-stamp-sensitive.test.ts +++ b/test/tree-receipt-stamp-sensitive.test.ts @@ -8,7 +8,7 @@ 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 { 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 +55,42 @@ 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); + }); + 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);