diff --git a/CHANGELOG.md b/CHANGELOG.md index 9b6ba320c..7428eff95 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -16,6 +16,83 @@ ### Tests - Three free fixtures (the two docsync captures and the design-floor menu) registered in `FREE_FIXTURES`; probe sets for the hook-actor arm, the two-attempt dispatch contract, the branch-listing grammar and the Scope Check owners; the pass-1 and pass-2 review fixes each carry a probe that fails on the previous helper. +## [1.91.280.0] - 2026-10-27 + +**Three free-tier tests measured something other than what they assert, and the Ubicloud quota wait shaved its own budget.** The daily flaky pass (GSTA-128) traced the free tier's open timing reds to measurement defects, not product ones: `quota_preflight` in `scripts/ubicloud/ubi-runner.sh` pinned its deadline to the wall clock before the first usage call, so a slow CLI spawn on a loaded runner printed "waiting up to 2s more" where the test expects the full 3 s; the pregate-remote teardown test read the `.down` file as soon as it existed, before the stub's `echo` had written the line; and the design-lint linearity check timed one wall-clock run of the 8000-label lint against a 500 ms floor on a shared runner. Under eight busy loops on four cores the quota case went from 8 of 12 green to 12 of 12. No ms bound, threshold, trial count or retry changed. + +### Fixed +- **`scripts/ubicloud/ubi-runner.sh`**: the quota wait budget is a countdown of the seconds spent sleeping for room. Usage reads between sleeps do not eat it, the first log line names the whole `UBI_QUOTA_WAIT`, `UBI_QUOTA_WAIT=0` still refuses on the first poll, and the wait still releases when usage drops. + +### Tests +- `test/ubicloud-pregate-remote.test.ts` waits for the written teardown line (`exit …` or `term …` ending in a newline), not for the file to exist. +- `test/design-lint-fifth-cycle.test.ts` measures the lint's own CPU time, fastest of three runs, against the unchanged `max(2.5 × t4000, 500 ms)` bound. A quadratic regression at 8000 labels would still measure about 690 ms CPU, above the floor, so the check keeps catching what it was written for. +- `test/ubicloud-runner.test.ts` attaches the runner's stderr to the hung-setup watchdog assertion, so the next red of that case names its cause. A new quota case proves the countdown. A usage read that takes 1.2 s keeps the printed 2 s budget (origin/main prints 1 s), a 70 s wait sleeps 30 s twice, and the wait releases when the peer VM disappears. + +### Gate integrity +- Four hunks listed, three read, none a relaxation. The teardown wait is stricter, the watchdog assertion is unchanged with a message added, and the linearity measure keeps its bound (measurement above). + +## [1.91.279.0] - 2026-10-27 + +**A licence line or a changelog entry no longer buys a full paid gate, the review loop stops re-reporting findings the owners have already ruled below the floor, and three follow-ups from the Blast radius ship land.** #3499: every PR that touched `NOTICE.md`, `LICENSE`, `licenses/` or `ETHOS.md` restored every gate case and judge because no paid case was mapped to them (#3493's docs-only PR planned 82 cases, about $30, for a notice and a changelog entry). #3504 and #3485: the same classes of finding (a schema lagging its writer, a helper nobody calls, a re-dispatch, a refusal one step late) reached the owners again on every ship after being judged below the floor once. GSTA-121's ship left three gaps. The Captain's merge-ask template and `docs/skills.md` lacked the `## Blast radius` item the PR body now carries, an 11-digit GitHub Actions run id after a run word was redacted from an approval record as a phone number, and the remote free gate failed with `PREGATE_REMOTE_INFRA` when a reviewer ran git in the checkout while the gate tarred `.git`. + +### Added +- **`RELEASE_METADATA_FILES` in `scripts/test-pr-profile.ts`** (`NOTICE.md`, `LICENSE`, `licenses/**`): release metadata no paid case reads, exempt like `CHANGELOG.md` and `VERSION` already were in `knownNonBehaviorFile`; a PR of only these files selects no paid case and keeps mode `pr`; `captain/templates/**` (the crew templates no paid case reads, found as the one unknown dependency of this PR's own selection) joins `FREE_ONLY_PR_FILES`. `ETHOS.md` is registered under the cases and judge of the two prompts that read it (`/office-hours`, `/plan-ceo-review`) in `test/helpers/touchfiles-data.ts`, so an ETHOS edit selects those cases (three gate cases and one judge today) instead of the whole gate; `SECURITY.md` replaces it as the tests' example of an unregistered file. The "unknown dependency restores the full gate" rule itself is unchanged. +- **`docs/review-policy.md#rulings-below-the-floor`**: six finding classes put to the floor by #3485 and #3504 and ruled below it (own-artifact hardening, unwired or unread, bounded repeat work, conservative refusal, artifact presentation, test gap), each with what it looks like and why it fails the floor; a finding in one is a note named by its ruling from pass 1 on. The review army's lead judgment (`scripts/resolvers/review-army.ts`, rendered into both `review-army.md` sections) gains the one-line **On record** ground that applies the table; its other grounds were tightened by the same byte count so the generated review SKILL.md stays at its pins. The floor ("would fail a test, break production, or lose data or secrets") is unchanged. +- **`docs/TESTING_INTERNALS.md` Windows free lane**: the command that shows whether one test is in the curated Windows subset and why not, and which list (`KNOWN_WINDOWS_SAFE`, `WINDOWS_PROBE_SAFE`, `KNOWN_WINDOWS_INCOMPATIBLE`) moves it. +- **`docs/skills.md` `/ship` "The PR body"**: the four Capy sections in order (What changed, Why, Blast radius, Approvals), how to write the Blast radius item, and what `gstack-prose-lint` gates. `captain/templates/merge-ask.md` carries `## Blast radius` between Why and Approvals; `bin/gstack-pr-shots`'s example caption and the `capy-pr` section heading name it too. + +### Fixed +- **`lib/redact-patterns.ts` `isCiRunIdentifier`**: an 11-digit GitHub Actions run id right after a run word on the same line (`screen 40477638211 GREEN`, `gate=…`, `rerun passed …`, `run #…`) is not `pii.phone.e164`, in every scanner (`gstack-approvals add`, `gstack-decision-trail`, the pregate secrets check, `gstack-post`); the run-word regex moved there from `lib/gstack-post.ts`, which imports it for its chained-id allowlist. A 10-digit national number after a run word, a bare 11-digit number, an E.164 `+` number and `job`, `id`, `workflow` or `attempt` before one still report; a national number written without its `+` right after a run word reads as a run id, the accepted residual of the ruling that a run id is not a phone number (`gstack-post` has read it that way since GSTA-121). +- **`scripts/ubicloud/ubi-runner.sh pack`**: the live `.git` is never tarred. The refs are fetched into a fresh repository through git's transport (`--update-shallow`, so a shallow CI or Capy checkout packs), the `remote.*` and `branch.*` config is replayed so `origin/` and each branch's upstream resolve on the VM, a detached HEAD gets a `detached-` branch, the index is copied with the blobs staged against HEAD (a VM `gstack-wtree` can read a staged file), and a pack that fails removes its temporary repository; a review subagent's `git diff` or a trial merge's `worktree add` during the first minute of a remote gate no longer makes tar exit 1 (`PREGATE_REMOTE_INFRA`). The linked-worktree path uses the same snapshot. +- **`lib/pregate/import-graph.ts`**: a relative string literal (`'../ship/sections/pr-body.md.tmpl'` through `new URL(…, import.meta.url)` or `readFileSync`) is a dependency edge from the file that names it, like a relative import; `test/ship-publication-gates.test.ts` is now selected when the pr-body template changes (the remote free gate missed it in GSTA-121's ship). A literal that climbs out of the repository names nothing. +- **`scripts/lib/windows-curation.ts`**: `test/pregate-remote-stamp-only.test.ts` is `KNOWN_WINDOWS_INCOMPATIBLE` (it expects the Linux-host lanes verdict and skips itself on win32), as #3485's follow-up asked. + +### Tests +- `test/paid-free-boundary.test.ts`: release metadata alone and beside `CHANGELOG.md` keeps mode `pr` with no paid case; `ETHOS.md` with `NOTICE.md` selects the two skills' gate cases and judge and nothing else. `test/pr-dependencies.test.ts`, `test/paid-pr-profile.test.ts`, `test/exercised-map.test.ts`: `SECURITY.md` as the unregistered example. +- `test/redact-context-exemptions.test.ts`: seven run-word rows with run 40477638211 and five controls; `test/approval-record.test.ts`: an approval whose words cite `screen 40477638211 GREEN` is recorded verbatim, a phone number still refuses the row; `test/pregate-allowlist.test.ts`: its run-id fixture reads `job `, which the engine does not exempt, so the allowlist mechanism it tests still has a finding to silence. +- `test/ubicloud-runner.test.ts`: pack of a plain checkout under twenty seconds of index, worktree and ref churn, three times; the unpacked snapshot has the same HEAD, `origin/main`, remote URL, status and `gstack-wtree` fingerprint, a detached HEAD lands on `detached-`, a pack with no index exits 1 and leaves nothing in `TMPDIR`, and a branch's upstream survives the snapshot. +- `test/pregate-import-graph-paths.test.ts`: relative literals through `new URL` and `readFileSync`, one and two directories up, and a literal that escapes the repository. + +### Gate integrity +- No threshold, timeout, trial count or case kind changed. The paid selection narrows only for `NOTICE.md`, `LICENSE`, `licenses/**`, `captain/templates/**` and `ETHOS.md`; every other unmapped file still restores the full gate. Generated SKILL.md sizes stay at their pins and the ship parity union stays under its 1.7500 cap; no allowlist or cap moved. + +## [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). + +### Changed +- **`scripts/resolvers/preamble/generate-ask-user-format.ts`**: office-hours' `Q` form and "a free-text reply answers the most recent unanswered `Q`" move into one `### Open-ended questions` subsection appended to the AskUserQuestion section. The form's template and Q-numbering text are unchanged; its lead-in now also carries what the Layout line used to say, "use this form instead of a `D` brief" and the free-text routing rule. Other skills render byte-identically. +- **`scripts/resolvers/preamble-hoist.ts`**: a skill variant may add subsections after the shared ones. It keeps only its changed and added subsections, and its note says "Only these subsections replace the shared ones or, under a new heading, add to its end". Variants without additions keep the existing note. +- **plan-eng-review keeps its versions** of the shared Tool resolution, fallback, Format, Self-check, Voice, plan-mode invocation and Plan Status Footer text, because each differs on purpose (the Scope gate, D-numbering after the target menu, the em-dash rule for composed prose, the selected-report footer, its own self-check). Its cap is unchanged. + +### Tests +- `test/helpers/skill-startup.ts` applies added subsections when it builds the effective text, so the hoisted-equals-inline check covers office-hours' new layout. Its `scripts/harness-version.json` pin moves to the new blob, recorded non-behavioral at HARNESS_VERSION 6 with no pass-rate reset: `readSkillStartup`, `readSharedSections` and `readSkillEffective` return the same bytes as the pinned helper for every tier 2+ render without added subsections (735 across all hosts) and for office-hours' SKILL.md before this change, so office-hours' fixture input changes only because its prompt did. +- `test/preamble-hoist.test.ts` checks, on every host and tier, that a cut-down variant carries the "add to its end" note exactly when it adds a subsection, and that `readSkillEffective` puts a changed subsection in place and appends added ones in order. +- `test/helpers/carve-guards.ts`: office-hours' parity cap goes down from 1.2279 to 1.1796, its union as measured before the main merge (154,653 bytes; 154,624 now, 1.1793). After the v1.91.270.0 merge, `test/fixtures/context-budget.json` is recaptured and office-hours' ceiling goes down from 20,792 to 19,139. No cap or pin goes up. + +### Gate integrity +- Corrections to the shared-preamble entry below: its post-v1.91.260.0 bullet should say the size allowlist carries 31 pin raises from #3433 and #3481 (23 and 8), not 30, and that `alwaysOnTotal` sits above main's unchanged ceiling (7,458) by the measured growth, so "all stay below main's pre-hoist values" holds for the per-skill rows only (`gstack` is back at main's 5,326 since the v1.91.265.0 merge). Its post-v1.91.265.0 bullet's "measured 7,542" for `alwaysOnTotal` is the raw bill; the ratchet's bill, which excludes test fixtures and aliases, measures 7,398.5 against the 7,542 ceiling. Its post-v1.91.270.0 bullet's "every row that rises matches main's own recapture" holds for the non-hoisted rows only (`alwaysOnTotal`, `capy/skill/gstack`, `diagram`, `gstack`, `gstack-upgrade`); the hoisted rows that rose (`eval-plan` 4,887, `landing-report`, `plan-tune`, `retro`, `ship`) are that branch's own recapture with main's growth included, and main measures them unhoisted (`eval-plan` 10,581). + ## [1.91.276.0] - 2026-10-27 **Tier 2+ skills load the shared preamble once per session instead of once per invocation.** Every tier 2+ SKILL.md carried the same ~23 KB of preamble (AskUserQuestion format, voice, writing style, completeness, completion protocol and more), so each skill invocation re-read text the conversation already held. That text now renders once per host into `sections/preamble.md` beside each tier 2+ SKILL.md. The SKILL.md keeps a short "Shared preamble (load first)" pointer naming the file and its id (a hash of the shared text before host rewrites), and asks for the Read unless the skill's own earlier Read of that file, headed by that id, is still in context, so other text that merely carries the id cannot stand in for the shared rules. Measured with Anthropic token counting: review's SKILL.md drops from 21,884 to 16,103 tokens, ship from 22,688 to 16,907, office-hours from 27,100 to 23,085 and design-review from 39,040 to 33,261. The shared file is 5,953 tokens, so a skill's first invocation in a session costs about 170 tokens more than before (about 1,900 for office-hours, about 3,800 for plan-eng-review) and every later invocation of that skill saves about 5,800 (4,000 for office-hours, 2,200 for plan-eng-review). The skip is tied to the skill's own copy of the file, so the first invocation of each different skill pays the Read again. A /autoplan run's four review-skill loads carry 17,941 fewer SKILL.md tokens (191,871 → 173,930), and /autoplan and the inline /office-hours loader skip each loaded skill's "Shared preamble (load first)" section, because the parent already holds the shared preamble. diff --git a/VERSION b/VERSION index 3c9e25f22..3d95d3946 100644 --- a/VERSION +++ b/VERSION @@ -1 +1 @@ -1.91.276.0 +1.91.280.0 diff --git a/agents-digest/gstack-AGENTS.md b/agents-digest/gstack-AGENTS.md index 89640315f..15ac5e105 100644 --- a/agents-digest/gstack-AGENTS.md +++ b/agents-digest/gstack-AGENTS.md @@ -1,4 +1,4 @@ -# gstack digest v1.91.276.0 — regenerate/re-copy after upgrading gstack +# gstack digest v1.91.280.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-shots b/bin/gstack-pr-shots index a41052903..653d816d2 100755 --- a/bin/gstack-pr-shots +++ b/bin/gstack-pr-shots @@ -33,7 +33,7 @@ const HELP = `gstack-pr-shots — before/after image of a change for a PR body ( [--title ] [--name ] [--out ] [--json] Example: - gstack-pr-shots --path ship/SKILL.md --heading "Step 19: Create PR/MR" --caption "PR body opens with What changed, Why and Approvals under Capy" + gstack-pr-shots --path ship/SKILL.md --heading "Step 19: Create PR/MR" --caption "PR body opens with What changed, Why, Blast radius and Approvals under Capy" ${exitTableHelp()}`; 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/captain/templates/merge-ask.md b/captain/templates/merge-ask.md index b477437f1..568a0a287 100644 --- a/captain/templates/merge-ask.md +++ b/captain/templates/merge-ask.md @@ -10,7 +10,10 @@ it before training the PR. ## Why - + + +## Blast radius + ## Approvals diff --git a/capy/skill/gstack/captain.md b/capy/skill/gstack/captain.md index cd07f93d0..28564d917 100644 --- a/capy/skill/gstack/captain.md +++ b/capy/skill/gstack/captain.md @@ -126,9 +126,12 @@ Every PR you send READY on, and the READY report itself, has: With no browser that line says `unavailable`: screenshot the printed `shot.html` with your own browser. Upload each PNG with Capy's PR asset tool (`tools.pr.uploadAsset`), only for public repositories, and embed the URL it returns with the caption as alt text. -2. **Why.** The cause and the effect the user will see, in two or three sentences. -3. **Approvals.** `gstack-approvals render`. -4. **Evidence.** Checks green at the head, gate or train results, flaky cases. +2. **Why.** The problem in the person's words first, then the cause and the effect the user will see, + in two or three sentences. +3. **Blast radius.** What else the change touches and the one fact that makes that safe, with the + evidence-ladder rung that fact reached. +4. **Approvals.** `gstack-approvals render`. +5. **Evidence.** Checks green at the head, gate or train results, flaky cases. The READY report attaches the key image (Capy message attachment), so the Captain can show it inline. /ship does all of this when `GSTACK_CAPY=1` (its `capy-pr` section). Outside Capy, the PR body is unchanged. diff --git a/docs/TESTING_INTERNALS.md b/docs/TESTING_INTERNALS.md index cc35d83fb..549999ec0 100644 --- a/docs/TESTING_INTERNALS.md +++ b/docs/TESTING_INTERNALS.md @@ -1097,8 +1097,10 @@ PR selection (`scripts/test-pr-profile.ts`) classifies every changed file. A fil mapped by a paid case's touchfiles selects that case. These files are exempt and select nothing: docs, any non-paid `*.test.ts(x)`, `FREE_ONLY_PR_FILES` (the free and paid duration seeds, `scripts/ubicloud/**`, every workflow that runs no -paid evals, and free-only tools), and the fixtures in `FREE_FIXTURES` -(`test/helpers/free-fixtures-data.ts`, each fixture with its free consumers). +paid evals, `captain/templates/**`, and free-only tools), the release metadata +in `RELEASE_METADATA_FILES` (`NOTICE.md`, `LICENSE`, `licenses/**`), and the +fixtures in `FREE_FIXTURES` (`test/helpers/free-fixtures-data.ts`, each fixture +with its free consumers). Unmapped files under `DERIVABLE_PREFIXES` (bin/, lib/, scripts/, browse/, design/, make-pdf/, hosts/, extension/, model-overlays/, agents/, test/helpers/, capy/) are not unknown: `scripts/pr-dependencies.ts` derives the cases whose @@ -1236,7 +1238,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 @@ -1501,6 +1508,20 @@ summary). Files a probe proved safe despite a `/tmp` literal are force-included `WINDOWS_CURATED_FLOOR`; when Windows-safe tests are deleted on purpose, lower it in the same PR with a reason. +To see whether one test is in the subset, and why not when it is excluded: + +```bash +bun run scripts/test-free-shards.ts --windows-only --list | grep test/.test.ts +``` + +A test a content pattern excludes wrongly joins the subset through +`KNOWN_WINDOWS_SAFE` (with its reason) or, for a `/tmp` literal a probe proved +safe, `WINDOWS_PROBE_SAFE`. A test that fails on `windows-latest` for a reason +the patterns cannot see (it expects the Linux-host lanes verdict, skips itself +on win32, drives a POSIX signal path) leaves it through a +`KNOWN_WINDOWS_INCOMPATIBLE` entry naming the reason; rerun `--list` to confirm +the file moved. + Native qualification campaigns (Windows cookie extraction, Windows launch diagnostics, Dia on macOS) run only on dispatch of `native-qualification.yml`: diff --git a/docs/captain.md b/docs/captain.md index faba03ec7..deae58020 100644 --- a/docs/captain.md +++ b/docs/captain.md @@ -232,7 +232,7 @@ recommendation at full scope. `gstack-owner-brief render --captain` prints it fr autoplan run. **C. The merge ask** ([template](../captain/templates/merge-ask.md)): captioned -before/after images, why, the Approvals table, and the evidence. /ship writes the PR +before/after images, why, blast radius, the Approvals table, and the evidence. /ship writes the PR body in this shape on Capy; the controller's merge report shows the key image inline. ## The digest diff --git a/docs/pregate.md b/docs/pregate.md index 80f72bd3d..975e20f5f 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 @@ -564,7 +566,10 @@ that reads them ships. The worked gbrain configuration is in The `secrets` check cannot be downgraded, and that is right for a key. It also refused 1,800 per-trial dummy tokens in eval receipts (`"key": "sk-trial-"` -reads as `env.kv`), an 11-digit Actions run id read as a phone number, and +reads as `env.kv`), an 11-digit Actions run id read as a phone number (one +after a run word such as `run`, `screen` or `gate` is exempt in the scanner +since the GSTA-121 follow-up; the allowlist still covers one after any other +word), and excalidraw.com's public Firebase key that every rebuild of a bundle re-adds (harness-evals #81/#83, GSTA-102, GSTA-106). `allowlist` in the policy names those, each with its scope and its reason: diff --git a/docs/review-policy.md b/docs/review-policy.md index a9b44f990..046220eb6 100644 --- a/docs/review-policy.md +++ b/docs/review-policy.md @@ -39,6 +39,31 @@ floor above; everything else is INFORMATIONAL. Data loss, a trust-boundary bypass and uncapped spend stay forced CRITICAL (the sideways sweep rule); no lane, specialist, dedup or saved decision downgrades them. + +### Rulings below the floor + +The floor is a test, and these classes have been put to it, each more than +once, and failed it (#3485 from the /ship of #3484, #3504 from the /ship of +#3487). A finding in one of them is a **note** from the first pass on, named +by its ruling in the PR body's `## Review notes`; it never earns a fix round +or another review pass, and the lead judgment stage drops it from the defect +count with the pointer. The floor itself does not move, so a finding that also +loses data, crosses a trust boundary or spends without a cap stays CRITICAL +whatever class it resembles. + +| Ruling | What it looks like | Why it is below the floor | +|---|---|---| +| **own-artifact hardening** | A check missing between two things the repository's own tooling writes: a screen verdict not compared with the receipt it belongs to; a receipt schema that does not declare a key the writer records; a policy loader that defaults when the base branch's own file is invalid; a size read that follows a symlink inside the repository. | The only writer is the repository's maintainers through its tools; no caller inside the intended case reaches the gap, and no test fails. A writer outside that trust boundary makes it a bypass, and CRITICAL. | +| **unwired or unread** | A helper with no caller, a field written and never read, a duplicated helper, a header comment naming a step that does not run it, a key-version constant that moved before any release carried the old value. | Nothing takes the path, so nothing breaks; it is dead code to delete or wire in its own change. | +| **bounded repeat work** | A re-dispatch on every call, a redundant screen after main moves, an orphaned scratch branch, a refusal that could come one step earlier in the sequence, a settle that could wait one more check. | The operation completes, nothing is lost, and the cost is one more bounded run; uncapped spend is the sideways sweep's CRITICAL, not this class. | +| **conservative refusal** | Stale state scoped wider than its cause: a RED verdict that keeps refusing a head after main moved, a pending result that keeps refusing after it settled. | A refusal costs a rerun and never passes something wrong; a wrong pass would be the floor. | +| **artifact presentation** | A string from an artifact printed verbatim into a parsed line (whitespace, control characters); a classifier that reads a non-zero exit code beside a non-RED verdict as RED. | Display and classification of the repository's own output; it forges no parsed field. | +| **test gap** | A state-matrix cell without a test, a coverage percentage, a weak or flaky test, a fixture that shares the coordinator's environment. | Category `testing`, below the floor by definition above; a gap is work for the test audit, not a defect in the diff. | + +A reviewer that wants a class promoted argues it against the floor with a +reaching input, in the finding, and the owner rules; the table records the +rulings, it does not close them. + ## `NO FINDINGS` is a result A reviewer that finds nothing answers `NO FINDINGS` and nothing else. That is a @@ -171,7 +196,7 @@ trailers, appended }`. ## Where this is enforced -- `scripts/resolvers/review-army.ts`: the floor and `NO FINDINGS` in every specialist and Red Team prompt (rendered into `review/sections/review-army.md` and `ship/sections/review-army.md`); in `/ship` every specialist and Red Team prompt it renders carries its diff base as a literal sha the parent writes in (`DIFF_BASE=`: the merge base on pass 1, `RANGE_BASE` from pass 2 on), because a reviewer is its own process and never sees the parent's shell variables; the fan-out batches above. +- `scripts/resolvers/review-army.ts`: the floor and `NO FINDINGS` in every specialist and Red Team prompt (rendered into `review/sections/review-army.md` and `ship/sections/review-army.md`), and the lead judgment's "On record" line that turns a [ruled class](#rulings-below-the-floor) into a note; in `/ship` every specialist and Red Team prompt it renders carries its diff base as a literal sha the parent writes in (`DIFF_BASE=`: the merge base on pass 1, `RANGE_BASE` from pass 2 on), because a reviewer is its own process and never sees the parent's shell variables; the fan-out batches above. - `ship/sections/review-army.md` Step 9: passes, `RANGE_BASE`, classify, defer, render, the cap check before any edit, the Step 11 late-defer row, scratch files under `.gstack/tmp/`, the cap decisions; `ship/sections/adversarial.md` Step 11: the outside voice reads the same range and scopes queued fixes the same way. - `lib/review-scope.ts`, `bin/gstack-review-scope`: `classify`, `defer`, `render`, `summary`; refusals are `REVIEW_SCOPE_INPUT` and `REVIEW_SCOPE_RANGE` ([troubleshooting.md](troubleshooting.md#review-scope-input)). - `lib/ship-policy.ts`: the `review_scope` key and its `ship: review_scope …` consequence line. 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/skills.md b/docs/skills.md index 66827cd33..24ab4a175 100644 --- a/docs/skills.md +++ b/docs/skills.md @@ -735,6 +735,17 @@ Every `/ship` run builds a code path map from your diff, searches for correspond A lot of branches die when the interesting work is done and only the boring release work is left. Humans procrastinate that part. AI should not. +### The PR body + +`/ship` writes the PR body from the diff, the receipt and the review, and a reader who was not in the session decides the merge from it. On Capy (`CAPY_PR: on`) the body opens with four sections, in this order, before the standard Summary, Test plan and Documentation sections: + +1. **`## What changed`**: one to three captioned before/after images from `gstack-pr-shots` or a UI screenshot, highest-signal first, with a collapsed text version under each. +2. **`## Why`**: two or three sentences. The first states the problem in the person's words (the issue, the message, the failing run), then the cause and the user-visible effect. +3. **`## Blast radius`**: what else the change touches and the one fact that makes that safe, with the evidence-ladder rung that fact reached (a test that exercises the shared path, a grep over every caller, a measured run). A change to a shared helper, a generated file, a selection list or a policy default always has one; a leaf fix says so in one line. +4. **`## Approvals`**: the table from `gstack-approvals render`, verbatim. + +Before the body is composed, `gstack-prose-lint` reads the Why, Summary and Blast radius drafts (`--sections 'Why,Summary,Blast radius' --gate`) and the CHANGELOG entry (`--base origin/ --gate`), and a `PROSE_LINT_FOUND` line names the sentence to rewrite; the rest of the body is advisory. The Captain's merge-ask template (`captain/templates/merge-ask.md`) carries the same four sections, so a READY report and its PR say the same thing. + ### Versions: where `/ship` looks, and when it ships without one `/ship` bumps a version only when the project says where the version lives. It checks, in order: 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..a1089de32 100644 --- a/docs/troubleshooting.md +++ b/docs/troubleshooting.md @@ -2373,7 +2373,12 @@ exact tokens it finds in these positions: `gates`, `backstop`, `bisect`, `rerun`, `dispatched`, `dispatch`, `failed`, `passed` (`failed 40269992077 | rerun passed 40287239206`, `gate 40323989742`, `run #19368712345`); a generic word such as `id`, - `job`, `attempt` or `workflow` exempts nothing; + `job`, `attempt` or `workflow` exempts nothing. This position is exempt in + the scanner itself (`isCiRunIdentifier` in `lib/redact-patterns.ts`), so + `gstack-approvals add`, `gstack-decision-trail` and the pregate secrets + check accept it too, not only `gstack-post`. A national number written + without its `+` right after a run word reads as a run id there; that is the + accepted residual of the ruling that a run id is not a phone number; - every 11-digit id chained after an allowlisted one through `, `, ` and `, ` & `, ` or ` or ` / `, optionally with a `(#1234)` between (`screen 40269992077 (#3434) and 40275443393 (#3453)`). @@ -2708,11 +2713,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 +4187,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 +4228,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 +4711,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/gstack-post.ts b/lib/gstack-post.ts index 0bd643598..80e991e09 100644 --- a/lib/gstack-post.ts +++ b/lib/gstack-post.ts @@ -21,6 +21,7 @@ */ import { createHash } from "crypto"; import { scan, type Finding, type RepoVisibility } from "./redact-engine"; +import { CI_RUN_WORD_BEFORE } from "./redact-patterns"; import { RECEIPT_FENCE } from "./ship-receipt"; import { canonicalRemote } from "./remote-identity"; import { parseChangelog } from "./changelog-check"; @@ -329,8 +330,6 @@ function repoReleaseVersions(env: PostEnv): string[] { return [...versions]; } -/** A run word with an optional ':' '#' '=' right before an 11-digit token: `run 19368712345`, `gate=40323989742`, `rerun passed 40287239206`. */ -const RUN_WORD_BEFORE = /\b(?:runs?|screens?|gates?|backstop|bisect|rerun|dispatch(?:ed)?|failed|passed)\s*[:#=]?\s*$/i; /** Between two 11-digit tokens of one list: `, `, ` and `, ` & `, ` or `, ` / `, with an optional `(#1234)` after the first. */ const RUN_ID_CHAIN = /^(?:\s*\(#\d+\))?\s*(?:,|and|&|or|\/)\s*$/i; @@ -344,7 +343,9 @@ const RUN_ID_CHAIN = /^(?:\s*\(#\d+\))?\s*(?:,|and|&|or|\/)\s*$/i; * word on the same line (run, runs, screen, screens, gate, gates, backstop, * bisect, rerun, dispatched, dispatch, failed, passed, with an optional ':' * '#' '=' between: `failed 40269992077`, `gate 40323989742`, - * `run #19368712345`; generic words such as id, job, attempt and workflow + * `run #19368712345`; the engine's isCiRunIdentifier exempts that position on + * its own since the run-id fix for approval records, and the same regex drives + * the chain below; generic words such as id, job, attempt and workflow * exempt nothing, since `customer id 14155551234` is a number), plus every 11-digit id chained * after an allowlisted one through ", ", " and ", " & ", " or " or " / ", * optionally with a "(#1234)" between (`screen 40269992077 (#3434) and @@ -361,7 +362,7 @@ function receiptDigitTokens(texts: string[]): string[] { let chainFrom = -1; for (const m of line.matchAll(/(?= 0 && RUN_ID_CHAIN.test(line.slice(chainFrom, at))); + const listed = CI_RUN_WORD_BEFORE.test(line.slice(0, at)) || (chainFrom >= 0 && RUN_ID_CHAIN.test(line.slice(chainFrom, at))); if (listed) tokens.add(m[0]); chainFrom = listed ? at + m[0].length : -1; } 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/pregate/import-graph.ts b/lib/pregate/import-graph.ts index 22b90e4fb..19176e94b 100644 --- a/lib/pregate/import-graph.ts +++ b/lib/pregate/import-graph.ts @@ -49,7 +49,11 @@ const scanner = new Bun.Transpiler({ loader: 'tsx' }); // `( ) [ ] +` appear in framework route paths: Next.js route groups `(app)`, // dynamic segments `[id]` / `[[...slug]]`, SvelteKit `+page.svelte`. A literal // that matches more text only gains an edge when it resolves to a repo file. -const QUOTED = /['"`]([A-Za-z0-9_@][A-Za-z0-9_./@()[\]+-]*)['"`]/g; +// A relative literal (`'../ship/sections/pr-body.md.tmpl'` read through +// `new URL(…, import.meta.url)` or `readFileSync`) resolves from the file that +// names it, like a relative import; without it a test reading a template by +// relative path had no edge to the template (GSTA-121's test/ship-publication-gates.test.ts). +const QUOTED = /['"`]((?:\.\.?\/)*[A-Za-z0-9_@][A-Za-z0-9_./@()[\]+-]*)['"`]/g; const JOIN = /\bjoin\(([^()]*)\)/g; const RESOLVE_EXT = ['', '.ts', '.tsx', '.js', '.jsx', '.mjs', '.cjs', '.mts', '.cts', '/index.ts', '/index.tsx', '/index.js', '/index.jsx', '/index.mjs']; // A statement starts a line or follows `;` or a compound statement's `:`. The module spec is diff --git a/lib/redact-patterns.ts b/lib/redact-patterns.ts index 6ece3ff4b..9c820900c 100644 --- a/lib/redact-patterns.ts +++ b/lib/redact-patterns.ts @@ -328,16 +328,28 @@ export function isNumericMetadataValue(span: string, match: RegExpExecArray): bo * command. Only the position decides: a run/job flag (`--run 37235771700`, * `--job=111534615007`), a `gh run view|watch|rerun|download|cancel` * argument, an `actions/runs/` or `/job/` URL segment, or a - * `run_id` / `job_id` key. The same digits anywhere else still report. + * `run_id` / `job_id` key. An 11-digit id (every GitHub Actions run id since + * 2025) also counts right after a run word on the same line, the way a ship + * receipt, a READY block or an approval record names one (`screen 40477638211 + * GREEN`, `gate=40323989742`, `rerun passed 40287239206`, `run #19368712345`); + * generic words (id, job, attempt, workflow) exempt nothing, since `customer id + * 14155551234` is a number. A national number written without its `+` right + * after a run word (`failed 14155551234`) is the accepted residual of the + * owner's ruling that a run id is not a phone number; gstack-post's chained-id + * allowlist reads the same position the same way. The same digits anywhere + * else still report. */ const CI_ID_POSITION_BEFORE = /(?:--(?:run|job)(?:-id)?(?:[ \t]+|[ \t]*=[ \t]*)|\bgh[ \t]+run[ \t]+(?:view|watch|rerun|download|cancel)[ \t]+|\/actions\/runs\/|\/jobs?\/|(?:run|job)_id["']?[ \t]*[:=][ \t]*["']?)$/i; +/** A run word with an optional ':' '#' '=' right before an 11-digit token; shared with gstack-post's chained-id allowlist. */ +export const CI_RUN_WORD_BEFORE = /\b(?:runs?|screens?|gates?|backstop|bisect|rerun|dispatch(?:ed)?|failed|passed)\s*[:#=]?\s*$/i; export function isCiRunIdentifier(span: string, match: RegExpExecArray): boolean { if (!/^\d+$/.test(span)) return false; const input = match.input ?? ""; const { start } = spanBounds(match); const lineStart = input.lastIndexOf("\n", start - 1) + 1; - return CI_ID_POSITION_BEFORE.test(input.slice(Math.max(lineStart, start - 80), start)); + const before = input.slice(Math.max(lineStart, start - 80), start); + return CI_ID_POSITION_BEFORE.test(before) || (span.length === 11 && CI_RUN_WORD_BEFORE.test(before)); } /** 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/office-hours/SKILL.md b/office-hours/SKILL.md index 612835c5f..800ed32ac 100644 --- a/office-hours/SKILL.md +++ b/office-hours/SKILL.md @@ -82,38 +82,11 @@ If `SKILL_PREFIX` is `true` use `/gstack-*` names; paths stay `~/.claude/skills/ ## AskUserQuestion Format -Only these subsections replace the shared ones; the rest of this section is shared. +Only these subsections replace the shared ones or, under a new heading, add to its end; the rest of this section is shared. -### When AskUserQuestion is unavailable or a call fails +### Open-ended questions -Tell these apart: - -1. **Auto-decide denial (NOT a failure).** The result contains `[plan-tune auto-decide] →