diff --git a/CHANGELOG.md b/CHANGELOG.md index c432700aa..17e5ed851 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,89 @@ # Changelog +## [1.94.17.0] - 2026-11-03 + +- A pregate guard whose exact command already passed at this tree no longer runs again (GST-1186). When the base branch's `ship_lanes_from` maps the command, it is carried from the verified CI run through the same `gstack-evidence record-ci` path /ship lanes use (`GUARD: carried runs=…`); otherwise a counted local ledger pass of that command at the same working tree stands (CI rows count only through a live `record-ci` check) (`GUARD: reused …`, and the new read-only `gstack-evidence passed -- `). Repos whose suites are guards, like founderstar, stop rerunning about six minutes of suites per /ship. See docs/pregate.md#guard-carry. +- Known limit: `ship_lanes_from` is still read from `origin/` only, so the PR that adds a mapping cannot carry from it. +- A `ship_lanes_from` entry can name the CI jobs that run a lane instead of an inventory command: `{ "profiles": [...], "jobs": ["verify", "typecheck"] }`, exactly one of the two (GST-1189). It carries only when every named job succeeded in the newest attempt of the verified run at HEAD's tree, through the same `verifyRun` job check. A missing job refuses as `job_missing` and a skipped one as the new `job_skipped`. Lanes with no test files, like gbrain's verify and typecheck (run 41564913688) or harness-evals' `bunx tsc --noEmit`, can now carry, and they record `tests_ran=n/a`. Fake `check:` entries in `tree.json` can move to this form. A repo declaring `jobs` should raise its `min_gstack_version` to this release first, since older gstack rejects the key. See docs/pregate.md#ship-lanes-jobs. + +- A `ship_lanes_from` entry can declare `aliases`, other exact spellings of the same lane such as `env -u ANTHROPIC_API_KEY bun test tests/` (GST-1190). gstack never strips env prefixes on its own, since an `env -i` run is a different environment. +- A carried lane no longer reports the whole run's test count. An inventory lane reports how many of its files the run covered, labeled as a file count (alphaclaw's 46-test guards lane recorded 9677 before). +- `gstack-ship-preflight --tests-only --policy-from head --simulate` proves a new mapping before it lands: every line starts `SIMULATION:`, no lane runs, no evidence row is written and nothing clears. An inventory-mapped lane runs the branch's inventory command, so the dry run needs `--allow-repo-commands`. A branch's own map never carries, so the first pull request that adopts a mapping pays its full lanes. +- `gstack-evidence carried`, which the ship receipt reads, lists only rows the receipt base's policy mapped, so a run against another base never lands in a receipt. +- An unknown `.gstack/pregate.json` key under a `min_gstack_version` above the running gstack now says `needs gstack >= ; upgrade` (`PREGATE_GSTACK_TOO_OLD`). It still refuses. +- docs/pregate.md#prove-a-mapping documents the proof route, the version floors, and that `tree.json` writers using Bun's JUnit must take the root `testsuite` count. + +- A test file whose every test skipped on the CI runner no longer counts as carried (GST-1190, harness-evals: 75 tests). A `tree.json` that lists such files in `skipped_files` makes the pregate tests stage run them here. A /ship lane or guard whose inventory names one carries the covered files and runs only the skipped ones here, through the `per_file` runner its `ship_lanes_from` entry declares. The CI row is written only after that run passes with a test count above zero and the carried runs still stand (`skipped_failed`, `skipped_zero_run` or `run_changed` otherwise), and an entry without `per_file` refuses as `inventory_skipped`, naming the files. A partly skipped file still counts. An older `tree.json` without the list is read as before. gstack itself changes nothing yet: its own writer and a `per_file` entry for its `bun run test` lane switch on together in a follow-up pull request, after the pull requests now queued on older gstack have landed, because an older gstack rejects the new key in main's policy. + +- A run profile can declare `skippable: { jobs, gate, scripts }` for jobs a path filter skips (GST-1190, truther #1169). A skipped job counts only in a `pull_request` run whose gate job succeeded, with the workflow, the declared `scripts` (every file the gate reads), the paths the gate job names (also under its `working-directory`) and `package.json` when it runs a package script matching `origin/` byte for byte; the lane then prints `not owed: CI skipped at this tree (gate job , workflow unchanged)`. If the branch changed any of them, it controlled the skip, so the lane runs here. +- `gstack-ci-carry find --policy-from head --simulate` is the same dry run for one lane command, every line starting `SIMULATION:`. + +### Tests +- `test/ship-ci-carry.test.ts`: in founderstar's shape (guards only, no lanes), `check-quick` carries from CI, `play-through` reuses a same-tree ledger pass, `assets` runs, and a dirty tree carries nothing. It fails without the guard carry. +- `test/ship-ci-carry.test.ts`: a lane mapped by `jobs` carries when every named job passed at the same tree with no test files, and refuses for a missing job, a skipped job, a rerun whose newest attempt skipped the job, and another tree's run. Those five fail without the change. +- `test/ship-ci-carry.test.ts` (GST-1190): an alias carries and an undeclared env prefix does not; a narrow lane reports its covered file count, not 9677; a jobs-mapped `bunx tsc --noEmit` guard carries with `n/a`; the newer-gstack message; `--simulate` writes and clears nothing and the branch's own map never carries; `carried` drops a row another base mapped. Six of these fail without the change. +- `test/ship-ci-carry.test.ts` (GST-1190 skipped files): a fully skipped inventory file stops the lane carry, an empty list or older `tree.json` carries, and the tests stage runs only the skipped selected file. Both fail without the change. +- `test/ship-ci-carry.test.ts` (path-skipped jobs and `find --simulate`): a skip under an unchanged, green gate is not owed; a changed workflow, a changed gate script or a failed gate job runs the lane here; `find --policy-from head --simulate` prints only SIMULATION lines and writes nothing. All five fail without the change. +- `test/ship-ci-carry.test.ts` (review fixes): a gate script behind `${{ github.workspace }}`, under `working-directory` or behind a package script is compared; the gate is the job, not a same-named step; only a `pull_request` run's skip counts; a skippable profile must declare `scripts`; `passed` never counts a CI row; a path-skipped guard says not owed; `--simulate` runs a branch inventory only with `--allow-repo-commands`. Eight fail without the fixes; a newer-failure veto test covers existing behavior. +- `test/ship-ci-carry.test.ts` (per-file lane carry): the covered file carries and only the skipped file runs through `per_file`, a failing or zero-test remainder, or a run that moved to a new attempt meanwhile, records no CI row and runs the lane whole, and `per_file` needs `{files}` and an inventory entry. All five fail without the change. + +## [1.94.16.0] - 2026-11-03 + +**A declared placeholder restamped to the new version is a stamp, and a carried receipt names the head's version (GST-1026, GST-1030).** gbrain writes a PR's own BEHAVIOR_CHANGES row as `since: 'TBD'` and its `release:restamp` fills in the version. Finish refused that receipt ("behavior, tests or build inputs changed since review"), and the receipt carry and the tree receipt would have refused the same row (gbrain #8444). Separately, a receipt carried after a main merge and a restamp kept the version of the head it was written at (GGST-1). + +### Added +- **`stamp_placeholders` policy key** (default `[]`, so nothing changes for a repo that does not set it): literal placeholders such as `["TBD"]`, at most 16, one line and 32 characters each, never `{version}` and never a version. It needs `min_gstack_version` at the release that introduced it. + +### Fixed +- **`lib/stamp-only.ts`:** `stampOnlyLine` takes the policy's placeholders. On a line the branch added itself (named by its position), a placeholder inside a `stamp_patterns` template may become that template at the new version, alongside any stamp-form version move on the same line. The built-in forms, a bare `{version}` template, base lines (including one whose text duplicates a branch line) and undeclared placeholders are still refused. Finish's `stampedFiles`, the receipt carry and the tree receipt share it. +- **`bin/gstack-ship-receipt` carry:** the carried receipt's `version` is HEAD's, read the way `write` reads it, instead of the version copied from the old receipt. A head with no version source keeps the version `write --version` recorded. +- **`bin/gstack-ship-receipt` carry, pin-bump PRs:** a `package.json` the branch changed as product (a dependency pin) that both main and a restamp moved since review is compared with only its `version` field masked, at all four sides. The receipt carries when nothing else in the file moved, and still refuses when the pin or any other field changed (GBEV-3, gbrain-evals #306). + +### Tests +- `test/tree-receipt-stamp-sensitive.test.ts`, `test/gstack-ship-receipt-docs.test.ts`: the shared predicate and the carry classifier accept gbrain's `since: 'TBD'` → `since: '0.60.553.0'` row and refuse an undeclared placeholder, a base row, a duplicated base copy, a built-in form and a line no template matches. +- `test/ship-rereview-triggers.test.ts`: finish routes the declared row as metadata and stops on the same controls. +- `test/restamp-mentions.test.ts`: the tree receipt and the carry accept the declared row and refuse it undeclared or on a base row; the carried receipt records the head's version after a main merge and a restamp; a version-only restamp of an EVIDENCE.md declared in `stamp_files` carries, and an undeclared one stays product; a pin-bump PR's receipt carries after a main merge and a restamp, and refuses when the pin moved since review. +- `test/gstack-ship-policy.test.ts`: the key's default, floor and validation. + +The EVIDENCE.md refusal (GST-1030's second point) reproduces on main and is already fixed by the GST-980 own-row carry this branch builds on, so this change adds only the test for it. + +## [1.94.15.0] - 2026-11-03 + +- **A stacked PR keeps its /ship receipt after its parent squash-merges, even when the receipt was written against main (GST-1199).** `gstack-ship-receipt carry` used to refuse with `RECEIPT_CARRY_PRODUCT_CHANGED` once the parent's hunks moved into main, forcing a full re-review of unchanged code. The carry now finds the newest reviewed commit that main already holds (`git merge-tree` into main changes no product path) and compares from there: an unchanged child carries as `stack parent landed`, base edits next to the child's hunks go to `RECEIPT_CARRY_SCOPED_REVIEW`, and any other change still refuses. A parent that never landed never counts, even if the child's merge dropped its change. + +## [1.94.14.0] - 2026-11-03 + +**`review_exclude_paths` keeps code it used to exclude, and the docs say exactly what the receipt check stops (GST-972).** Three ways to get code or a credential out of review under a base pattern are closed, each with a failing-first test in `test/review-exclude.test.ts`, and the receipt docs no longer over-claim. + +### Fixed +- **Native binaries are code.** A `.node`, `.so`, `.dylib` or `.dll` file under a base pattern was excluded as binary content, and so was a compiled executable with a data extension. A binary file that starts with ELF, Mach-O (universal binaries included), PE, WebAssembly or `#!` magic is now kept as `code` whatever its extension. Binary here is git's own verdict on the bytes, read with no attributes from the tree as well as with the branch's, so a branch's `.gitattributes` (`*.png diff -binary`) cannot make an ELF read as text and skip the check (#3910). +- **A NUL byte no longer makes code excludable.** A binary file was content unless its extension was on a short source list, so `.gradle`, `.sql`, `.tf`, `.groovy`, a `.p12` key store or an extensionless private key with a NUL byte was excluded. A binary file is now content only under a known content extension (images, fonts, media, documents, compressed archives, `.bin`, `.dat` and data blobs); every other binary file is kept as `code`. +- **A branch's `.gitattributes` cannot get a file with a credential line excluded.** Marking `receipts/**` binary left the collector's added lines empty, so the secret-line keep rule saw nothing and the file was excluded. For a file git calls binary only because of its attributes, the rule now reads `git diff --text --no-textconv --no-ext-diff`, so the file stays in review and the plan lists it as `REVIEW_EXCLUDE_KEPT: secret-line`. Readers' own diffs still follow the branch's attributes and show that file as `Binary files differ`; reading such files as text in every reader's diff is a separate follow-up. +- **`gstack-ship-receipt write --review-exclude` docs are exact.** The help, `docs/troubleshooting.md`, `docs/ship-policy.md` and the queue's refusal line said a receipt write alone cannot clear `field=review_exclude`. Write checks the digest against the base policy and does not read the review ledger, so a write given the refusal's `expected=` digest passes without a review. A mismatch proves only that the `--review-exclude` argument (or the digest the receipt recorded from it) differs from the head's selection, not what the readers read. The text now says that, and that the review rerun is the fix. + +## [1.94.13.0] - 2026-11-03 + +- **A bare `--base main` reads origin/main in every review and ship bin (GST-1036).** `gstack-review-plan` now prints `REVIEW_BASE: ` and names a lagging local base on stderr. `gstack-banned-terms`, `gstack-prose-lint --base`, `gstack-docs-candidate`, `gstack-design-detect.ts scan --changed` and `gstack-pr-shots --base` no longer merge against a stale local main and report lines the branch only inherited. They all use the one resolver gstack-gate-diff already used. +- **`gstack-diff-scope` stops treating authorship files as auth (GST-1065).** `author-spec.md`, `AUTHORS`, `authority.ts` and `coauthors.ts` no longer raise a docs diff to gate class. Every other name with auth in it still does, including `auth.ts`, `/auth/`, `oauth`, `nextauthmiddleware.ts` and camelCase names like `useAuth.ts`. + +## [1.94.12.0] - 2026-11-03 + +### Fixed + +- **Six markup tricks no longer pass a remote-loading or scripted SVG off as a chart (GST-940).** `</style>` or `<!--` inside a `` let CSS that loads a remote image read as a plain drawing, and a non-ASCII namespace prefix such as `` hid a script. Any of these could get the SVG a light review. The style walker now reads the ``, comment that only an HTML parser ends early, quoted `>` or Unicode space can hide CSS. A non-ASCII character in an element or attribute name, such as a `gé:` prefix, keeps the SVG code too (GST-940); anything else, a script or `foreignObject` included, keeps it code. More than 2,000 data paths keep every one code; a `git grep` failure is `consumer_scan_failed` and fails the depth closed. The scan first lists the files that contain a candidate's @@ -384,7 +387,7 @@ and matching is token-bounded: `costume` never reads as cost. |---|---| | money | a path segment naming cost, spend, budget, ledger, billing, price, fee or invoice; or a changed line with an identifier ending in `_usd` or `Usd`, any `spend_cap` spelling, or a call to a paid provider `lib/pricing.ts` prices | | gate | a hunk the gate scan tagged: a relaxed threshold, a skipped test, a loosened check | -| auth | `SCOPE_AUTH` from `gstack-diff-scope`, on a code path with an auth, session, jwt, permission or role token in its name; a data file named for sessions (`recovered-sessions.jsonl`) outside a config location does not set `SCOPE_AUTH` | +| auth | `SCOPE_AUTH` from `gstack-diff-scope`, on a code path with an auth, session, jwt, permission or role token in its name; a data file named for sessions (`recovered-sessions.jsonl`) outside a config location does not set `SCOPE_AUTH`, and `SCOPE_AUTH` fails safe: any name token containing auth sets it (`auth.ts`, `/auth/`, `oauth`, `useAuth.ts`, `nextauthmiddleware.ts`) except the authorship words `author`, `authors`, `authored`, `authoring`, `authorship`, `authority`, `authorities`, `coauthor` and `coauthors` | | secret | a secrets, credentials, keychain or token path, or a changed line that reads a key-named environment variable (`OPENAI_API_KEY`, `GH_TOKEN`) or a credential store | An input the classifier cannot read (the diff, `gstack-diff-scope`, the gate @@ -493,6 +496,21 @@ through the docs stage. A path outside that scope and a gate-edited test still route as behavior, with `--reviewed-base` (as /ship passes it) so does a file too large for the scoped review, and without the record nothing changes. +A completed fix-check is a review, not an owner acceptance (GST-1130). Finish +reads the newest `review-fix-check` row from `gstack-review-read`. When it is a +completed clean read on committed content (freshness `CURRENT` against its own +tree) whose commit is on the branch, whose `range_base` is a branch commit +holding the reviewed tree, and for which an approved `review_loop_grant` in +`gstack-approvals` names a stop head inside `range_base..commit`, each code or test path inside `range_base..commit` +that is unchanged since routes as `fix_checked=` in `ROUTE:` and is listed on a +`FIX_CHECK: reviewed` line, a gate-edited test included. Authored docs keep the +stage 2.2 recheck, and a path edited after the fix-check still routes by the +rules above. Any other row prints +`FIX_CHECK: not counted ()` and changes nothing. `gstack-review-log` +refuses a `review-fix-check` row without boolean `completed` and `converged`, +before it consumes the start token, because without them the row reads +`UNVERIFIED` and finish cannot count it. + The receipt's summary line prints `passes= blocking_found= deferred=`, which is what a scorecard tracks (target: a median of 1 pass per PR). A receipt carried across a base merge or a test-only fix diff --git a/docs/ship-policy.md b/docs/ship-policy.md index a6fcabd18..6612a8aa3 100644 --- a/docs/ship-policy.md +++ b/docs/ship-policy.md @@ -53,12 +53,13 @@ refuses the file (`POLICY_GSTACK_TOO_OLD`). `validate` ends with | `maintainers` | GitHub logins | `[]` | Maintainer set for `/review` contributor mode; empty means repository push permission. | | `stamp_files` | globs | `[]` | The repo's stamp rule: when set, only these files may differ by a stamp (list the version file, manifests, plugin trees and the files holding the PR's own `since:` rows). It applies before every other stamp rule, so a doc `vX` mention outside it is a difference. | | `stamp_patterns` | literal templates | `[]` | At most 16 templates of at most 128 characters, each with exactly one `{version}` slot (`"version": "{version}"`, `since: '{version}'`, ``); matched literally at a version boundary, never as a regex. The restamp writer, its mention scanner and the tree receipt use them beside the built-in forms. | +| `stamp_placeholders` | literal placeholders | `[]` | At most 16 one-line literals of at most 32 characters, never `{version}` and never a version (`["TBD"]`). On a line the branch added itself, a placeholder inside a `stamp_patterns` template (`since: 'TBD'` under `since: '{version}'`) may become that template at the new version, and finish's route, the receipt carry and the tree receipt read the change as a stamp. The built-in forms never take a placeholder, and neither does a base line, even one whose text duplicates a branch line. The writer does not fill placeholders: the repo's `restamp_command` does. A newer key: set `min_gstack_version` to the release `gstack-ship-policy validate` names. Empty accepts no placeholder. | | `restamp_command` | command or `null` | `null` | The repo's own restamp, run first in the staging worktree; see [restamp_command](#restamp-command). | | `migrations` | `{ dir, id }` or `null` | `null` | The migration directory and a regex with exactly one capture group for the number in a file name (`"^v(\\d+)-"`); `gstack-pregate regate` reports `MIGRATION_COLLISION`. | | `gate_command` | command or `null` | `null` | The repo's full pre-merge gate. `gstack-ship-queue gate-ahead` and `ready` print it for the caller to run (in the background or in CI); gstack never executes it. Needs `min_gstack_version`. | | `full_e2e` | `true` \| `false` | `false` | `true`: every merge needs a passing full-gate result bound to a tree equal to the merging one modulo stamps; `gstack-ship-queue ready`, `stamp` and `verify` refuse without one (`QUEUE_GATE_MISSING`): a gate-ahead record, or the lane's `--gate-exit 0 --gate-log ` from its run on the gated head. Needs `min_gstack_version`. | | `review_scope` | `frozen` \| `diff-plus-blocking` \| `all` | `frozen` | The finding scope of `/ship`'s review loop; see [review_scope](#review-scope). `frozen` fixes the diff on pass 1, files blocking findings in unchanged code as issues, and later passes read only the range since the previous pass. `diff-plus-blocking` is the same but fixes a blocking finding in unchanged code in this PR instead of filing it. `all` is the earlier loop: every finding is fixed on every pass. Needs `min_gstack_version` when not `frozen`. | -| `review_exclude_paths` | git glob pathspecs | `[]` | Content files (data, docs, binary, `.log`) `/ship`'s reviewers leave out; read from the base only, with code, gate, config, executable and named-by-code files kept. See [review_exclude_paths](#review-exclude-paths). Needs `min_gstack_version`. | +| `review_exclude_paths` | git glob pathspecs | `[]` | Content files `/ship`'s reviewers leave out: data, docs, `.log`, and binaries under a content extension (images, fonts, archives and the like) that do not start with executable magic. Read from the base only. Code (native libraries and every other binary outside the content extensions included), gate paths, agent prose, config, executables, links, `.gstack/` and `.github/` files, gate-tagged, secret-line and named-by-code files stay in review. See [review_exclude_paths](#review-exclude-paths). Needs `min_gstack_version`. | | `min_gstack_version` | version or `null` | `null` | The oldest gstack that may read the file; see the compatibility rule above. | | `docs` | `{ caps: [{ path, max_bytes }] }` or `null` | `null` | Documentation files with a byte cap. `/ship` Step 14.5 runs `gstack-ship-receipt docs-caps --base ` (the caps come from `origin/`'s policy, never the branch's copy; `--policy ` is for a repository bootstrapping its policy) before the verification gate and prints one `DOCS_CAP: / ok\|over` line per cap; any `over` (an absent file counts) exits 1 and blocks the ship until the file is trimmed or the cap raised on the base branch (`DOCS_AUDIT_BLOCKED`). At most 64 caps, each path contained. See [the ship receipt](#ship-receipt). Needs `min_gstack_version`. | | `train` | `{ gate_workflow, free_workflow, full_gate_check, record_artifact, max_size, fill_hours, screen_selection, trial_reservation_usd, land? }`, `{ gates, land, max_size, fill_hours }` or `null` | `null` | Turns on [merge trains](#run-a-merge-train), read from the base revision only. Every field is required and an unknown field is `POLICY_INVALID`: `max_size` 1 to 6, `fill_hours` above 0 and at most 24, `screen_selection` `gstack-paid` \| `none`, `trial_reservation_usd` above 0 and at most 100. A block with `gates` ([any repository](#train-gates)) takes `max_size` 1 to 5 and needs `min_gstack_version` at least `TRAIN_GATES_KEY_VERSION`. `null` keeps the per-PR queue exactly as it is. Needs `min_gstack_version`. | @@ -1287,8 +1288,8 @@ A pattern that matches no changed file is listed as `unmatched=` on the plan lin **What is excluded, and what is kept.** A matched file is excluded only when it is content: a data file (JSON, JSONL, CSV, TSV, images), a docs-class file, a -`.log` file, or a file git reports as binary (`-\t-` in `--numstat`; readers would only see `Binary files differ`) whose bytes hold a NUL in their first 8,000, so a branch's `.gitattributes` cannot make a text file binary content. A binary file is content even under -`config/` or in a skill directory; only `policy-dir`, `gate-path`, `executable`, `link`, `gate-tagged`, or a source or script extension (`code`) keep it. +`.log` file, or a binary file: one git reports as binary (`-\t-` in `--numstat`; readers would only see `Binary files differ`) under the branch's attributes or with no attributes from the tree, so a branch's `.gitattributes` cannot make an executable read as text, and whose bytes hold a NUL in their first 8,000, so it cannot make a text file binary content either. A binary file is content only under a known content extension (images, fonts, media, documents, compressed archives, `.bin`, `.dat`, and data blobs such as `.parquet`, `.npy` or `.sqlite`) and only when its bytes do not start with an executable image's magic (ELF, Mach-O including universal binaries, PE, WebAssembly, `#!`). Under those rules it is content even under +`config/` or in a skill directory, and only `policy-dir`, `gate-path`, `executable`, `link` or `gate-tagged` keep it. Every other binary file is `code`: source with a NUL, a native library (`.node`, `.so`, `.dylib`, `.dll`), a compiled executable, a key store, a pickle, or a file with no extension. Everything else stays in every reader's diff with one reason code: | Reason | Kept because | @@ -1300,8 +1301,8 @@ Everything else stays in every reader's diff with one reason code: | `agent-prose` | it is a skill, section, resolver, template or rule file | | `config` | it is a data-shaped config file (`package.json`, `tsconfig*.json`, `config/`, dotfiles) | | `gate-tagged` | the gate scan tagged one of its hunks | -| `code` | it is code or a test, not content; a binary file with a source or script extension (`.js`, `.py`, `.sh`, `.wasm`) counts as code | -| `secret-line` | an added line reads a credential (the targeted pass's secret rule) | +| `code` | it is code or a test, not content; a binary file outside the content extensions above, or one with executable magic, counts as code | +| `secret-line` | an added line reads a credential (the targeted pass's secret rule); for a file git calls binary only because of its attributes, the added lines come from `git diff --text`, which `.gitattributes` cannot change (readers' own diffs still follow the attributes and show the kept file as binary) | | `named-by-code` | a code or test file that stays in review (not docs, a data file, a `.log` or binary, and not itself excluded) names its full repository path, at the head or the merge base | | `consumer-scan-unavailable` | the `git grep` for those names failed, so every candidate is kept; also a live binary file whose bytes could not be read | | `gate-scan-unavailable` | the gate scan failed or did not run at the head, so no hunk's tag is known and every candidate is kept | @@ -1351,11 +1352,15 @@ paths and three hashes). The `digest` hashes the declared patterns and the excluded and kept lists; `source` is provenance only. Write, carry and the queue each recompute the digest at the current base and compare it, so a base merge or a moved base that leaves the patterns and the lists unchanged still carries -and binds. A different digest means the reviewers read another selection: +and binds. A different digest means the digest write was given (or the receipt recorded) names another selection than the head has now: write refuses with `RECEIPT_INVALID field=review_exclude`, carry with `RECEIPT_CARRY_REFUSED review_exclude changed`, and the queue with `RECEIPT_INVALID field=review_exclude expected= recorded=`. The -fix is to rerun the review, never a receipt write alone. +fix is to rerun the review and write with the digest its `verify` prints. Write +checks the digest against the base policy, so a stale selection cannot be +recorded by accident, but it does not read the review ledger: a write given the +`expected=` digest passes without a review, and only the review run shows that +the reviewers read that selection. **Adopting it.** (1) Open a PR on the base that adds the patterns and `min_gstack_version` at least `REVIEW_EXCLUDE_KEY_VERSION`, and ship it like any @@ -1503,6 +1508,7 @@ receipt when the base moves: | a rebase or force-push onto a tree the receipt covers, product diff against the new base unchanged | carries (`carry.ancestry: rewritten`) | | any other rebase or force-push | a fresh /ship (`RECEIPT_CARRY_TREE_MISMATCH`) | | a stacked PR whose parent landed (squash or merge), retargeted to the base with the base merged in, HEAD's product diff exactly the one reviewed against the parent branch | carries (reason `stack parent landed`) | +| a stacked PR whose receipt was written against the base, after its parent landed and the base was merged in, HEAD's product diff exactly the reviewed range from the parent's head | carries (reason `stack parent landed`); base context next to the child's hunks is `RECEIPT_CARRY_SCOPED_REVIEW` | | a stamp on the branch's own rows in a stamp file the branch also changed as product: every changed line at a branch-added position, the file's mode unchanged, the base not touching it, the rest of the product diff unchanged | carries (reason `a stamp on the branch's own rows ()`) | **Stacked PRs (#3570).** A child PR's receipt is written against its parent @@ -1520,6 +1526,21 @@ since, or a parent that landed with changes the child never merged all leave a different diff and refuse with `RECEIPT_CARRY_PRODUCT_CHANGED`. An older receipt whose parent branch is gone refuses with `RECEIPT_CARRY_REFUSED`. +**Stacked PRs reviewed against the base (GST-1199).** A child PR that targets +the base while stacked has a receipt whose `base` is the base and whose +reviewed range holds the parent's hunks too, and the receipt records no stack +parent. Once the parent squash-merges and the base is merged into the child, +the two heads' product diffs differ by exactly the parent's hunks, which now +sit in the base. When they differ, the carry looks for the newest commit inside +the reviewed range (the reviewed head excluded, at most 100 commits) whose +product change the current base already holds: `git merge-tree` of that commit +into the base changes no product path. A conflicted product path never counts, +so a parent that never landed is never treated as one, even when the child's +merge dropped its change. The comparison then runs from that commit: an equal +patch-id carries with reason `stack parent landed`, base context next +to the child's unchanged hunks is `RECEIPT_CARRY_SCOPED_REVIEW`, and any other +difference refuses with `RECEIPT_CARRY_PRODUCT_CHANGED`. + Since the release that added `review_scope`. A `/ship` receipt names the head it reviewed, so a conflict-only merge of the base or a test-only fix used to invalidate it and cost a full re-run on identical product code. diff --git a/docs/troubleshooting.md b/docs/troubleshooting.md index 0a4c63110..95e20574f 100644 --- a/docs/troubleshooting.md +++ b/docs/troubleshooting.md @@ -3340,10 +3340,13 @@ targeted refusal, run the targeted reader on HEAD, `gstack-review-dispatch verif use this code when the receipt's review_exclude digest (or the `--review-exclude ` write was given) is not the digest HEAD has under the base policy's [`review_exclude_paths`](ship-policy.md#review-exclude-paths). The line -names `expected=` (now), `recorded=` (what the review read, `none` when it read -without exclusion) and `source=` (the base commit). The reviewers read another -selection than the one the head now has, so a receipt write alone cannot clear -it: rerun `/ship`'s review on the head (`gstack-review-dispatch pass --base `), +names `expected=` (now), `recorded=` (the `--review-exclude` argument, or the digest +the receipt recorded from it; `none` without one) and `source=` (the base commit). +A mismatch proves only that this argument differs from the selection the head has +now. Write compares the argument and does not read the review ledger, so the line +says nothing about what the readers read, and a write that passes the `expected=` digest clears +the field without a review. Do not do that. Rerun +`/ship`'s review on the head (`gstack-review-dispatch pass --base `), run `gstack-review-dispatch verify --ledger `, which prints `REVIEW_EXCLUDE_RECORD: digest=` from the briefs the readers got, then `gstack-ship-receipt write --pr --review-exclude --body `. @@ -5998,7 +6001,10 @@ the reviewed tree but the base moved under it, so the diff a reviewer would see is not the reviewed one. For a stacked PR retargeted to the base after its parent landed, it means HEAD's product diff against the base is not the range reviewed against the parent branch: the parent has not fully landed, or the -branch changed since. The reviewed verdict does not cover HEAD, so the +branch changed since. A stacked PR whose receipt was written against the base +carries from the newest reviewed commit the base already holds +([ship-policy.md](ship-policy.md#receipt-carry)); it refuses here only when no +such commit exists or the branch changed since. The reviewed verdict does not cover HEAD, so the receipt is not carried. The refusal names the changed product paths, or for a retargeted stacked PR the parent branch and the base. diff --git a/lib/gate-diff/git.ts b/lib/gate-diff/git.ts index adc46fbbf..daebe14d4 100644 --- a/lib/gate-diff/git.ts +++ b/lib/gate-diff/git.ts @@ -137,6 +137,27 @@ export function resolveBase(ref: string, cwd: string, head?: string): { ref: str return sha ? { ref, sha } : null; } +export type ResolvedBase = NonNullable>; + +/** Why resolveBase read another ref than the name given (`local main is behind origin/main; base is origin/main`); null when the name stood as given. */ +export function baseNote(name: string, base: ResolvedBase): string | null { + if (base.local === 'fallback') return `${base.unrelatedRemote ? `${base.unrelatedRemote} shares no history with HEAD` : `no remote-tracking branch for ${name}`}; base is the local ${name}, which may lag its remote`; + if (!base.local) return null; + return `${{ unrelated: `local ${name} shares no history with HEAD`, missing: `no local branch ${name}`, behind: `local ${name} is behind ${base.ref}`, ahead: `local ${name} is ahead of ${base.ref}`, diverged: `local ${name} has diverged from ${base.ref}` }[base.local]}; base is ${base.ref}`; +} + +/** + * A --base argument as the commit a review or ship bin reads (GST-1036): resolveBase against HEAD, so a bare `main` + * reads origin/main over a stale local main, with the note on stderr as `: ` when it read another ref than + * the name. An unresolvable name comes back as given, so the caller's own git error still names it. + */ +export function baseArg(bin: string, name: string, cwd: string): string { + const base = resolveBase(name, cwd, 'HEAD'); + const note = base && baseNote(name, base); + if (note) process.stderr.write(`${bin}: ${note}\n`); + return base?.sha ?? name; +} + /** A word for a printed shell line: as is when it is plain, single-quoted otherwise (ref names may hold `;`, `$`, `(`, `{1,2}`). */ function shellWord(s: string): string { return /^[A-Za-z0-9._\/@^~+:-]+$/.test(s) ? s : `'${s.replace(/'/g, "'\\''")}'`; diff --git a/lib/pr-shots.ts b/lib/pr-shots.ts index 821569dc8..0e8c4659a 100644 --- a/lib/pr-shots.ts +++ b/lib/pr-shots.ts @@ -17,6 +17,7 @@ import { spawnSync } from 'node:child_process'; import * as fs from 'node:fs'; import * as path from 'node:path'; +import { resolveBase } from './gate-diff/git'; import { findSection } from './markdown-sections'; import { scan } from './redact-engine'; import type { ResultCodeName } from './result-codes'; @@ -55,9 +56,11 @@ export function readCapture(file: string, roots: string[], what: string): string return cleanText(fs.readFileSync(real), `${what} file`); } -/** Resolve `ref` to a commit sha without letting it be read as an option. */ +/** Resolve `ref` to a commit sha without letting it be read as an option; a bare branch name reads origin's tracking branch first (resolveBase, GST-1036). */ export function resolveCommit(cwd: string, ref: string): string | ShotError { if (!ref || ref.startsWith('-') || /\s/.test(ref)) return { code: 'PR_SHOTS_INPUT_REFUSED', message: `ref ${JSON.stringify(ref)} is not a plain ref` }; + const base = resolveBase(ref, cwd, 'HEAD'); + if (base) return base.sha; const r = git(cwd, ['rev-parse', '--verify', '--quiet', '--end-of-options', `${ref}^{commit}`]); return r.status === 0 ? r.stdout.trim() : { code: 'PR_SHOTS_INPUT_NOT_FOUND', message: `base ref ${ref}` }; } diff --git a/lib/restamp.ts b/lib/restamp.ts index be2c6fc23..9eba5f88e 100644 --- a/lib/restamp.ts +++ b/lib/restamp.ts @@ -313,7 +313,7 @@ export function branchAddedLines(cwd: string, since: string, files: string[], to return out; } -const escapeRegExp = (t: string) => t.replace(/[.*+?^${}()|[\]\\]/g, '\\$&'); +export const escapeRegExp = (t: string) => t.replace(/[.*+?^${}()|[\]\\]/g, '\\$&'); const VERSION_RE = '(?\\d+(?:\\.\\d+){1,3})(?![0-9.])'; /** * The built-in stamp forms as scanners, the version in the `v` group: `(was X)`, `since: X` and `vX`. diff --git a/lib/review-collect.ts b/lib/review-collect.ts index ac16ba0d2..f5e10709f 100644 --- a/lib/review-collect.ts +++ b/lib/review-collect.ts @@ -10,6 +10,7 @@ import { spawnSync } from 'node:child_process'; import * as fs from 'node:fs'; import * as path from 'node:path'; +import { resolveBase, type ResolvedBase } from './gate-diff/git'; import { GENERATED_PATH, parseRegistry } from './regen'; import { RESULT_CODES } from './result-codes'; import { classifyPath, dataCandidate, svgCandidate, svgDrawing, type ClassInput, type DepthError, type PathLines, type RegistryInfo } from './review-depth'; @@ -31,6 +32,8 @@ export interface Collected extends ClassInput { untracked: string[]; consumed: string[] | 'all'; drawings: string[]; + /** collectLive only: the ref resolveBase read for the base name (GST-1036), null when nothing resolved. */ + base?: ResolvedBase | null; } function git(root: string, args: string[]): { ok: boolean; status: number | null; out: string; err: string } { @@ -255,16 +258,20 @@ export function collectCommit(root: string, baseSha: string, headSha: string): C return result(diff, { root: skillDirs, registry, errors, mergeBase, untracked: [], consumed, drawings }); } -/** The working tree against the merge base with origin/ (or ; a remote-qualified base reads refs/remotes/ only), untracked files counted per path; the registry is the base ref's commit. */ +/** + * The working tree against the merge base with the base resolveBase reads (lib/gate-diff/git.ts: a bare name is + * origin/ when it shares history with HEAD, else the local branch; a remote-qualified base reads + * refs/remotes/ only), untracked files counted per path; the registry is the base ref's commit. + */ export function collectLive(root: string, base: string): Collected { const errors: DepthError[] = []; - const qualified = base.startsWith('origin/') || git(root, ['remote']).out.split('\n').some(r => r && base.startsWith(`${r}/`)); - const ref = (qualified ? [`refs/remotes/${base}`] : [`refs/remotes/origin/${base}`, `refs/heads/${base}`]).find(r => git(root, ['rev-parse', '--verify', '-q', `${r}^{commit}`]).ok) ?? base; + const resolved = resolveBase(base, root, 'HEAD'); + const ref = resolved?.sha ?? base; const registry = loadRegistryAt(root, ref); const mb = git(root, ['merge-base', ref, 'HEAD']); if (!mb.ok) { errors.push({ code: 'numstat_failed', detail: oneLine(mb.err || `no merge base with ${ref}`), input: `git merge-base ${ref} HEAD`, repair: `fetch the base first (git fetch origin ${base})` }); - return result({ perPath: [], deleted: [], stampOnly: [], created: [], added: {}, removed: {} }, { root, registry, errors, mergeBase: null, untracked: [], consumed: 'all' }); + return { ...result({ perPath: [], deleted: [], stampOnly: [], created: [], added: {}, removed: {} }, { root, registry, errors, mergeBase: null, untracked: [], consumed: 'all' }), base: resolved }; } const mergeBase = mb.out.trim(); const diff = readDiff(root, [mergeBase], errors); @@ -284,5 +291,5 @@ export function collectLive(root: string, base: string): Collected { const drawings = drawingsAmong(diff.perPath, root, p => { try { return fs.statSync(path.join(root, p)).size <= DRAWING_READ_CAP ? fs.readFileSync(path.join(root, p)) : null; } catch { return null; } }); - return result(diff, { root, registry, errors, mergeBase, untracked, consumed, drawings }); + return { ...result(diff, { root, registry, errors, mergeBase, untracked, consumed, drawings }), base: resolved }; } diff --git a/lib/review-depth.ts b/lib/review-depth.ts index e8751cc8e..c337ebd98 100644 --- a/lib/review-depth.ts +++ b/lib/review-depth.ts @@ -93,7 +93,16 @@ const SVG_MARKUP = /^<(?:!--|!\[CDATA\[|\?xml\s|!DOCTYPE\s+svg(?:\s+PUBLIC\s+([" export const SVG_REFUSED = [/&(?!(?:amp|lt|gt|quot|apos);)/, /\\/, /@import/i, /url\((?!\s*#|['"]\s*#)/i, /expression\s*\(/i, /-moz-binding|\bbehavior\s*:/i]; /** A drawing's CSS, a style element's text or a style attribute's value, holds declarations only: no @ rule and no function but url(#id). */ const SVG_STYLE_OPEN = /<(?:[A-Za-z_][\w.-]*:)?style\b/gi; -const SVG_STYLE_CLOSE = /^<\/(?:[A-Za-z_][\w.-]*:)?style\s*>/i; +const SVG_STYLE_CLOSE = /^<\/(?:[A-Za-z_][\w.-]*:)?style[ \t\n\r]*>/i; +/** + * A style element's opening tag, read as quoted name="value" pairs separated by the ASCII whitespace XML and HTML share: any + * other shape (a value holding < or >, an unquoted or valueless attribute, />, another space) refuses, so it cannot hide a closing tag. + */ +const SVG_STYLE_TAG = /^<(?:[A-Za-z_][\w.-]*:)?style(?:[ \t\n\r]+[\w.:-]+[ \t\n\r]*=[ \t\n\r]*(?:"[^"<>]*"|'[^'<>]*'))*[ \t\n\r]*>/i; +/** An element name, ended by ASCII whitespace, / or >: a non-ASCII character in the name (a prefix such as gé:) refuses rather than reading as its ASCII start. */ +const SVG_TAG_NAME = /^<\/?(?:[A-Za-z_][\w.-]*:)?([A-Za-z_][\w.-]*)(?=[ \t\n\r/>]|$)/; +/** Every name= in the text, non-ASCII letters included so a prefix such as xé:href is read whole and refused. */ +const SVG_ATTR = /(?=]+)))/g; const SVG_CSS_REF = /url\(\s*(['"]?)#[\w.:-]*\1\s*\)/gi; const SVG_CSS_REFUSED = /[@(]/; /** An href or src value a drawing may hold: a same-document reference, or a raster image inline. */ @@ -102,10 +111,12 @@ const SVG_HREF = /^\s*(?:#|data:image\/(?:png|jpe?g|gif|webp);base64,[A-Za-z0-9+ /** * Whether an SVG's whole text is a drawing (GST-911): every element in SVG_ELEMENTS, no event handler, every href or src a * same-document reference or an inline raster image, CSS of declarations only, and nothing SVG_REFUSED names, read after character references are - * decoded. Every name= in the text is read, so a stray quote cannot hide a later attribute inside its value; an unquoted value - * stops at the next =, which keeps the scan linear. A style element's CSS is walked with indexOf to its closing tag, dropping - * comments and reading CDATA as CSS, so neither can hide a closing tag; any other markup inside it, or a style element that never - * closes, refuses. Anything else, an SVG that can run code included, stays code. + * decoded. Every name= in the text is read (SVG_ATTR), so a stray quote cannot hide a later attribute inside its value, and a name + * holding a non-ASCII character refuses, like an element name (SVG_TAG_NAME); an unquoted value + * stops at the next =, which keeps the scan linear. A style element's opening tag is read quote by quote (SVG_STYLE_TAG), and its CSS is walked with indexOf to its closing tag over the raw + * text, so a character reference cannot fake a closing tag or a comment, and any & in it refuses (GST-940). Comments and CDATA are + * read as CSS, so neither can hide a closing tag and an HTML-only comment end ( or --!>) exposes nothing unread; any other + * markup inside it, or a style element that never closes, refuses. Anything else, an SVG that can run code included, stays code. */ export function svgDrawing(text: string): boolean { const t = decodeCharRefs(text); @@ -113,36 +124,36 @@ export function svgDrawing(text: string): boolean { const cssDrawing = (css: string) => !SVG_CSS_REFUSED.test(css.replace(SVG_CSS_REF, '')); for (let at = 0; ;) { SVG_STYLE_OPEN.lastIndex = at; - const open = SVG_STYLE_OPEN.exec(t); + const open = SVG_STYLE_OPEN.exec(text); if (!open) break; - let i = t.indexOf('>', open.index) + 1; - if (!i) return false; + const tag = SVG_STYLE_TAG.exec(text.slice(open.index, open.index + 4096)); + if (!tag) return false; + let i = open.index + tag[0].length; let css = ''; for (;;) { - const lt = t.indexOf('<', i); + const lt = text.indexOf('<', i); if (lt === -1) return false; - css += t.slice(i, lt); - if (t.startsWith('', lt + 4) + 3; - if (i === 2) return false; - } else if (t.startsWith('', lt + 9); + css += text.slice(i, lt); + const [start, close] = text.startsWith(''] : text.startsWith(''] : [-1, '']; + if (start !== -1) { + const end = text.indexOf(close, start); if (end === -1) return false; - css += t.slice(lt + 9, end); + css += ' ' + text.slice(start, end) + ' '; i = end + 3; - } else if (SVG_STYLE_CLOSE.test(t.slice(lt, lt + 256))) { + } else if (SVG_STYLE_CLOSE.test(text.slice(lt, lt + 256))) { at = lt + 1; break; } else return false; } - if (!cssDrawing(css)) return false; + if (css.includes('&') || !cssDrawing(css)) return false; } for (let at = t.indexOf('<'); at !== -1; at = t.indexOf('<', at + 1)) { if (SVG_MARKUP.test(t.slice(at, at + 512))) continue; - const tag = /^<\/?(?:[A-Za-z_][\w.-]*:)?([A-Za-z_][\w.-]*)/.exec(t.slice(at, at + 256)); + const tag = SVG_TAG_NAME.exec(t.slice(at, at + 256)); if (!tag || !SVG_ELEMENTS.has(tag[1]!.toLowerCase())) return false; } - for (const m of t.matchAll(/(?=]+)))/g)) { + for (const m of t.matchAll(SVG_ATTR)) { + if (/[^\x00-\x7f]/.test(m[1]!)) return false; const local = m[1]!.slice(m[1]!.lastIndexOf(':') + 1).toLowerCase(); const value = m[2] ?? m[3] ?? m[4] ?? ''; if (local.startsWith('on') || ((local === 'href' || local === 'src') && !SVG_HREF.test(value)) || (local === 'style' && !cssDrawing(value))) return false; @@ -153,7 +164,7 @@ export function svgDrawing(text: string): boolean { /** A hash of the constants and patterns that decide a depth, not of this file's bytes: a comment edit keeps it, a threshold edit changes it. */ export const DEPTH_RULES = createHash('sha256').update(JSON.stringify({ RISK_RULES, SMALL_DIFF_LINES, RED_TEAM_LINES, LARGE_DIFF_LINES, FULL_PASS_CAP, LIGHT_FIXED, BATCH_SCOPED, SCOPED_RANGE_LINES, ROLE_EFFORT, SVG_ELEMENTS: [...SVG_ELEMENTS], - patterns: Object.entries({ GATE_PATH, AGENT_PROSE, TEST_PATH, RELEASE_METADATA, DOC_PATH: [DOC_PATH], DATA_PATH: [DATA_PATH], DATA_CONFIG, SVG_MARKUP: [SVG_MARKUP], SVG_REFUSED, SVG_HREF: [SVG_HREF], SVG_STYLE_OPEN: [SVG_STYLE_OPEN], SVG_STYLE_CLOSE: [SVG_STYLE_CLOSE], SVG_CSS_REF: [SVG_CSS_REF], SVG_CSS_REFUSED: [SVG_CSS_REFUSED] }).map(([k, v]) => [k, v.map(r => `/${r.source}/${r.flags}`)]), + patterns: Object.entries({ GATE_PATH, AGENT_PROSE, TEST_PATH, RELEASE_METADATA, DOC_PATH: [DOC_PATH], DATA_PATH: [DATA_PATH], DATA_CONFIG, SVG_MARKUP: [SVG_MARKUP], SVG_REFUSED, SVG_HREF: [SVG_HREF], SVG_STYLE_OPEN: [SVG_STYLE_OPEN], SVG_STYLE_CLOSE: [SVG_STYLE_CLOSE], SVG_STYLE_TAG: [SVG_STYLE_TAG], SVG_TAG_NAME: [SVG_TAG_NAME], SVG_ATTR: [SVG_ATTR], SVG_CSS_REF: [SVG_CSS_REF], SVG_CSS_REFUSED: [SVG_CSS_REFUSED] }).map(([k, v]) => [k, v.map(r => `/${r.source}/${r.flags}`)]), })).digest('hex').slice(0, 16); /** diff --git a/lib/review-evidence.ts b/lib/review-evidence.ts index 60dc4e669..f4cd47549 100644 --- a/lib/review-evidence.ts +++ b/lib/review-evidence.ts @@ -272,6 +272,9 @@ export function bindReview(rec: Record, token: string, env = proces if (env.GSTACK_STAMP_TREE) rec.tree = env.GSTACK_STAMP_TREE; if (env.GSTACK_STAMP_DIRTY) rec.dirty = env.GSTACK_STAMP_DIRTY === 'true'; stampGateFindings(rec.findings, env.GSTACK_STAMP_SESSION_KIND); + if (rec.skill === 'review-fix-check' && (typeof rec.completed !== 'boolean' || typeof rec.converged !== 'boolean')) { + throw new Error('review-log: a review-fix-check row must carry completed and converged as true or false (review-army item 6 fields; without them gstack-review-read marks the row UNVERIFIED and finish cannot count it)'); + } // A /ship pass whose findings were all fixed, or deferred or noted below the floor, left no open defect (review-army // item 6), so a claimed issues_found is recorded clean with zero counts and Step 11.5 can bind it (GST-430). Only the // logger-stamped action counts (never a caller's status), and the findings must account for every claimed issue. diff --git a/lib/review-exclude.ts b/lib/review-exclude.ts index 80533bc7d..78d9131f6 100644 --- a/lib/review-exclude.ts +++ b/lib/review-exclude.ts @@ -122,20 +122,38 @@ function select(i: ExcludeInput, patterns: readonly string[]): { byPattern: Map< return { byPattern, files }; } +/** Executable image magic: ELF, Mach-O (thin, universal and 64-bit universal; 0xcafebabe is also a Java class), PE, WebAssembly and a script's #!. */ +const NATIVE_MAGIC = [[0x7f, 0x45, 0x4c, 0x46], [0xfe, 0xed, 0xfa, 0xce], [0xfe, 0xed, 0xfa, 0xcf], [0xce, 0xfa, 0xed, 0xfe], [0xcf, 0xfa, 0xed, 0xfe], [0xca, 0xfe, 0xba, 0xbe], [0xca, 0xfe, 0xba, 0xbf], [0x4d, 0x5a], [0x00, 0x61, 0x73, 0x6d], [0x23, 0x21]]; + /** - * The matched paths git calls binary whose bytes hold a NUL in their first 8,000 (git's own test), read from the head blob, - * the file on a live tree, or the old blob for a deletion. A branch's .gitattributes can make git call a text file binary; - * only real bytes make it binary content here. + * The matched paths git calls binary by their bytes alone: `git diff --numstat` with attributes read from the empty tree, so + * a branch's .gitattributes (`diff`, `-binary`, `text`) cannot make an executable image read as text and skip the + * magic check. null when git cannot run it (a git without --attr-source); the caller then reads every matched file's bytes. */ -function binaryContent(i: ExcludeInput, files: ReadonlyMap, claimed: ReadonlySet, unread: Set): Set { +function bytesBinary(i: ExcludeInput, patterns: readonly string[]): string[] | null { + const empty = git(i.root, ['hash-object', '-t', 'tree', '--stdin'], ''); + if (!empty.ok) return null; + const range = i.head ? [i.mergeBase, i.head] : [i.mergeBase]; + const r = git(i.root, [`--attr-source=${empty.out.trim()}`, 'diff', '--numstat', '-z', '--no-renames', ...range, '--', ...patterns.map(p => `:(top,glob)${p}`)]); + return r.ok ? r.out.split('\0').filter(l => l.startsWith('-\t-\t')).map(l => l.slice(4)) : null; +} +const nativeImage = (b: Buffer) => NATIVE_MAGIC.some(m => m.every((x, k) => b[k] === x)); + +/** + * The claimed paths (git calls them binary under the branch's attributes or under none: bytesBinary) whose bytes hold a NUL + * in their first 8,000 (git's own test), read from the head blob, the file on a live tree, or the old blob for a deletion. + * A branch's .gitattributes can make git call a text file binary; only real bytes make it binary content here. A binary file that starts with executable image magic also goes in `native`. + */ +function binaryContent(i: ExcludeInput, files: ReadonlyMap, claimed: ReadonlySet, unread: Set, native: Set): Set { const out = new Set(); + const sniff = (p: string, b: Buffer) => { if (!b.includes(0)) return; out.add(p); if (nativeImage(b)) native.add(p); }; const real = (sha: string) => /^[0-9a-f]{40,64}$/.test(sha) && !/^0+$/.test(sha); const bySha = new Map(); for (const p of claimed) { const m = files.get(p); if (!m) continue; if (!i.head && m.newMode !== '000000') { - try { const fd = fs.openSync(path.join(i.root, p), 'r'); const buf = Buffer.alloc(8000); const n = fs.readSync(fd, buf, 0, 8000, 0); fs.closeSync(fd); if (buf.subarray(0, n).includes(0)) out.add(p); } catch { unread.add(p); } + try { const fd = fs.openSync(path.join(i.root, p), 'r'); const buf = Buffer.alloc(8000); const n = fs.readSync(fd, buf, 0, 8000, 0); fs.closeSync(fd); sniff(p, buf.subarray(0, n)); } catch { unread.add(p); } continue; } const sha = real(m.newSha) ? m.newSha : real(m.oldSha) ? m.oldSha : ''; @@ -144,8 +162,36 @@ function binaryContent(i: ExcludeInput, files: ReadonlyMap, cla for (const [sha, paths] of bySha) { const r = spawnSync('git', ['cat-file', 'blob', sha], { cwd: i.root, timeout: 30_000, maxBuffer: 8000 }); const read = r.status === 0 || (r.error as NodeJS.ErrnoException | undefined)?.code === 'ENOBUFS'; - const binary = read && (r.stdout ?? Buffer.alloc(0)).subarray(0, 8000).includes(0); - for (const p of paths) if (!read) unread.add(p); else if (binary) out.add(p); + const head = (r.stdout ?? Buffer.alloc(0)).subarray(0, 8000); + for (const p of paths) if (!read) unread.add(p); else sniff(p, head); + } + return out; +} + +/** + * The added lines of matched files git calls binary only because of their attributes (the bytes hold no NUL): the + * collector's added lines are empty for them, so the secret-line rule reads `git diff --text` instead, which no + * .gitattributes, textconv or external diff driver changes. One git diff per KEEP_ARG_MAX_BYTES of literal pathspecs; + * git prints one section per path in byte order, so a chunk whose sections do not line up with its sorted paths, or a + * git failure, puts every path of that chunk in `unread`. + */ +function attributeBinaryLines(i: ExcludeInput, paths: readonly string[], unread: Set): Record { + const out: Record = {}; + const range = i.head ? [i.mergeBase, i.head] : [i.mergeBase]; + const chunks: string[][] = []; + let bytes = Infinity; + for (const p of [...paths].sort((a, b) => Buffer.compare(Buffer.from(a), Buffer.from(b)))) { + const n = argBytes([p]); + if (bytes + n > KEEP_ARG_MAX_BYTES) { chunks.push([]); bytes = 0; } + chunks[chunks.length - 1]!.push(p); + bytes += n; + } + for (const chunk of chunks) { + const r = git(i.root, ['diff', '--text', '--no-textconv', '--no-ext-diff', '--no-renames', '--src-prefix=a/', '--dst-prefix=b/', '-U0', ...range, '--', ...chunk.map(literal)]); + const sections = r.ok ? r.out.split(/^(?=diff --git )/m).filter(s => s.startsWith('diff --git ')) : []; + const aligned = sections.length === chunk.length && chunk.every((p, k) => /["\\\x00-\x1f\x7f]/.test(p) || sections[k]!.startsWith(`diff --git a/${p} b/${p}\n`)); + if (!aligned) { for (const p of chunk) unread.add(p); continue; } + chunk.forEach((p, k) => { let hunk = false; out[p] = sections[k]!.split('\n').filter(l => (hunk ||= l.startsWith('@@')) && l.startsWith('+')).map(l => l.slice(1)); }); } return out; } @@ -159,17 +205,21 @@ function contentKind(i: ExcludeInput, p: string, binary: ReadonlySet): C return classifyPath(p, i.classRoot) === 'docs' ? 'docs' : null; } -/** Source and script extensions: a NUL in one does not make it excludable binary content. */ -const CODE_EXT = /\.(?:[cm]?[jt]sx?|py|rb|go|rs|java|kts?|scala|swift|exs?|php|cs|c|h|cc|cpp|hpp|mm?|dart|pl|pm|lua|sh|bash|zsh|fish|ps1|bat|cmd|vue|svelte|html?|wasm)$/i; +/** + * The only extensions a binary file may be excludable content under: images, fonts, media, documents, compressed archives + * and data blobs. Any other binary (source with a NUL, a native library or executable, a key store, a pickle, no + * extension) is code; so is one that starts with executable image magic whatever its extension. + */ +const BINARY_CONTENT_EXT = /\.(?:png|jpe?g|gif|webp|bmp|ico|avif|tiff?|heic|heif|woff2?|ttf|otf|eot|mp3|mp4|m4a|wav|ogg|oga|ogv|webm|mov|avi|mkv|flac|aac|opus|pdf|docx|xlsx|pptx|odt|ods|odp|gz|tgz|zip|tar|bz2|xz|zst|7z|bin|dat|parquet|arrow|feather|avro|orc|npy|npz|h5|hdf5|sqlite3?|db|safetensors)$/i; -/** Why a matched file is kept by its path, mode and lines alone; null for content the policy may exclude. A binary file readers would only see as "Binary files differ" is content unless its directory, mode or gate tags keep it. */ -function pathReason(i: ExcludeInput, p: string, m: Matched, binary: ReadonlySet): KeptReason | null { +/** Why a matched file is kept by its path, mode and lines alone; null for content the policy may exclude. A binary file readers would only see as "Binary files differ" is content unless its directory, mode, gate tags, extension or magic keep it. */ +function pathReason(i: ExcludeInput, p: string, m: Matched, binary: ReadonlySet, native: ReadonlySet): KeptReason | null { if (/^\.(gstack|github)\//.test(p)) return 'policy-dir'; if (isGatePath(p)) return 'gate-path'; if (m.oldMode === '100755' || m.newMode === '100755') return 'executable'; if ([m.oldMode, m.newMode].some(x => x === '120000' || x === '160000')) return 'link'; if (i.gateTagged?.has(p)) return 'gate-tagged'; - if (binary.has(p)) return CODE_EXT.test(p) ? 'code' : null; + if (binary.has(p)) return BINARY_CONTENT_EXT.test(p) && !native.has(p) ? null : 'code'; if (isAgentProse(p)) return 'agent-prose'; if (isDataConfig(p)) return 'config'; if (contentKind(i, p, binary) === null) return 'code'; @@ -227,9 +277,12 @@ export function reviewExclude(i: ExcludeInput): Exclusion { const sel = select(i, patterns); if ('error' in sel) return { state: 'refused', code: 'REVIEW_DIFF_UNREADABLE', lines: [`${i.context}: git could not list the files review_exclude_paths matches (${sel.error}); fix: git fetch origin ${base}, then rerun once; nothing was planned (REVIEW_DIFF_UNREADABLE)`] }; const unread = new Set(); - const binary = binaryContent(i, sel.files, new Set(i.perPath.filter(e => e.binary && sel.files.has(e.path)).map(e => e.path)), unread); + const native = new Set(); + const claimed = i.perPath.filter(e => e.binary && sel.files.has(e.path)).map(e => e.path); + const binary = binaryContent(i, sel.files, new Set([...claimed, ...(bytesBinary(i, patterns) ?? sel.files.keys())]), unread, native); + const text = { ...i, added: { ...i.added, ...attributeBinaryLines(i, claimed.filter(p => !binary.has(p) && !unread.has(p)), unread) } }; const reasons = new Map(); - for (const [p, m] of sel.files) reasons.set(p, unread.has(p) ? 'consumer-scan-unavailable' : pathReason(i, p, m, binary)); + for (const [p, m] of sel.files) reasons.set(p, unread.has(p) ? 'consumer-scan-unavailable' : pathReason(text, p, m, binary, native)); const candidates = new Set([...reasons].filter(([, r]) => r === null).map(([p]) => p)); const named = grepFixed(i, [...candidates].filter(p => !p.includes('\n')), candidates, [i.mergeBase]); for (const p of candidates) { diff --git a/lib/ship-bin-common.ts b/lib/ship-bin-common.ts index 2335d79e0..c76e51887 100644 --- a/lib/ship-bin-common.ts +++ b/lib/ship-bin-common.ts @@ -114,7 +114,8 @@ export function releasePaths(policy: Record | null, cwd: string, re * added since the merge base with origin/, and not added by the base since): after a lost slot and a merge of the * base its stale stamp is older than `ref`'s VERSION, so it must be at or newer than the fork point's version instead, * the bound gstack-restamp's stamp_paths stage uses (GST-895; the fork version itself since GST-963). A branch line is - * named by its position, and with a kept VERSION only such a line may move (GST-979, GST-980). + * named by its position, and with a kept VERSION only such a line may move (GST-979, GST-980); on it a declared + * `stamp_placeholders` value may also become the new version inside a `stamp_patterns` template (GST-1026). */ export function stampedFiles(policy: Record | null, cwd: string, ref: string, base: string): string[] { const globs = Array.isArray(policy?.stamp_files) ? policy.stamp_files.filter((s: unknown): s is string => typeof s === 'string') : []; @@ -124,11 +125,13 @@ export function stampedFiles(policy: Record | null, cwd: string, re let after = ''; try { after = extractVersion(fs.readFileSync(path.join(cwd, before.rel), 'utf8'), before.rel); } catch { return []; } if (!after) return []; - const templates = Array.isArray(policy?.stamp_patterns) ? policy.stamp_patterns.filter((s: unknown): s is string => typeof s === 'string') : []; + const strings = (v: unknown): string[] => (Array.isArray(v) ? v.filter((s): s is string => typeof s === 'string') : []); + const templates = strings(policy?.stamp_patterns); + const placeholders = strings(policy?.stamp_placeholders); const diff = git(cwd, ['diff', '--unified=0', '--no-color', '--no-ext-diff', '--no-textconv', '--no-renames', ref]); if (diff.status !== 0) return []; const side = branchSides(cwd, `origin/${base}`, 'HEAD', before.version, ref); - const stamp = (rel: string, removed: string, added: string, at: number): boolean => stampOnlyLine(removed, added, before.version, after, templates, side(rel), at); + const stamp = (rel: string, removed: string, added: string, at: number): boolean => stampOnlyLine(removed, added, before.version, after, templates, side(rel), at, placeholders); return parseUnifiedDiff(diff.stdout).filter(f => f.status === 'M' && !f.binary && !f.modeChanged && matchesAny(globs, f.path) && f.hunks.length > 0 && f.hunks.every(h => h.removed.length === h.added.length && h.removed.every((line, i) => stamp(f.path, line, h.added[i]!, h.removedAt[i]!)))).map(f => f.path); } diff --git a/lib/ship-policy-stamps.ts b/lib/ship-policy-stamps.ts new file mode 100644 index 000000000..6e0b144da --- /dev/null +++ b/lib/ship-policy-stamps.ts @@ -0,0 +1,35 @@ +/** + * Validation of the stamp policy keys (stamp_patterns templates and stamp_placeholders), split out of ship-policy.ts + * to keep that module under the module-size ratchet. + */ +import type { PolicyError } from './ship-policy'; +import { parseVersion } from './version-source'; + +const MAX_TEMPLATES = 16; +const MAX_TEMPLATE = 128; +const MAX_PLACEHOLDERS = 16; +const MAX_PLACEHOLDER = 32; +export const VERSION_SLOT = '{version}'; + +export function templateErrors(value: unknown, out: PolicyError[]): void { + if (!Array.isArray(value)) { out.push({ code: 'POLICY_INVALID', path: 'stamp_patterns', message: 'must be an array of literal templates' }); return; } + if (value.length > MAX_TEMPLATES) out.push({ code: 'POLICY_CONTAINMENT', path: 'stamp_patterns', message: `more than ${MAX_TEMPLATES} templates` }); + value.forEach((t, i) => { + const at = `stamp_patterns[${i}]`; + if (typeof t !== 'string') { out.push({ code: 'POLICY_INVALID', path: at, message: 'must be a string' }); return; } + if (t.split(VERSION_SLOT).length !== 2) out.push({ code: 'POLICY_INVALID', path: at, message: `${JSON.stringify(t)} must contain exactly one ${VERSION_SLOT}` }); + else if (t.length > MAX_TEMPLATE || /[\0\r\n]/.test(t)) out.push({ code: 'POLICY_CONTAINMENT', path: at, message: `must be one line of at most ${MAX_TEMPLATE} characters` }); + }); +} + +export function placeholderErrors(value: unknown, out: PolicyError[]): void { + if (!Array.isArray(value)) { out.push({ code: 'POLICY_INVALID', path: 'stamp_placeholders', message: 'must be an array of literal placeholders' }); return; } + if (value.length > MAX_PLACEHOLDERS) out.push({ code: 'POLICY_CONTAINMENT', path: 'stamp_placeholders', message: `more than ${MAX_PLACEHOLDERS} placeholders` }); + value.forEach((p, i) => { + const at = `stamp_placeholders[${i}]`; + if (typeof p !== 'string') { out.push({ code: 'POLICY_INVALID', path: at, message: 'must be a string' }); return; } + if (!p.trim() || p.length > MAX_PLACEHOLDER || /[\0\r\n]/.test(p)) out.push({ code: 'POLICY_CONTAINMENT', path: at, message: `must be one non-empty line of at most ${MAX_PLACEHOLDER} characters` }); + else if (p.includes(VERSION_SLOT)) out.push({ code: 'POLICY_INVALID', path: at, message: `${JSON.stringify(p)} must not contain ${VERSION_SLOT}` }); + else if (parseVersion(p)) out.push({ code: 'POLICY_INVALID', path: at, message: `${JSON.stringify(p)} is a version, not a placeholder` }); + }); +} diff --git a/lib/ship-policy.ts b/lib/ship-policy.ts index 0092b3be4..3fd85545c 100644 --- a/lib/ship-policy.ts +++ b/lib/ship-policy.ts @@ -18,6 +18,8 @@ import * as path from 'node:path'; import { defaultBaseBranch, noDefaultBaseLine } from './default-base'; import { RESULT_CODES, type ResultCodeName } from './result-codes'; import { cmpVersion, parseVersion } from './version-source'; +import { placeholderErrors, templateErrors } from './ship-policy-stamps'; +export { VERSION_SLOT } from './ship-policy-stamps'; export const SHIP_POLICY_PATH = '.gstack/ship-policy.json'; export const SHIP_POLICY_SCHEMA = 'https://github.com/garrytan/gstack/docs/ship-policy.md#schema-v1'; @@ -64,6 +66,8 @@ export interface ShipPolicy { stamp_files: string[]; /** Literal stamp templates with one `{version}` slot, beside the built-in `(was X)`, `since: X`, `vX`; writer and classifier share them. */ stamp_patterns: string[]; + /** Placeholders (`TBD`) a restamp may replace with the new version inside a `stamp_patterns` template, on the branch's own lines only. */ + stamp_placeholders: string[]; /** The repo's own restamp, run first in the staging worktree with the target version; gstack's version writer is skipped. */ restamp_command: string | null; /** Migration numbering: `{ dir, id }` where `id` is a regex with one capture group over the file name. */ @@ -161,6 +165,7 @@ export const DEFAULT_POLICY: ShipPolicy = { maintainers: [], stamp_files: [], stamp_patterns: [], + stamp_placeholders: [], restamp_command: null, migrations: null, gate_command: null, @@ -193,6 +198,7 @@ export const POLICY_EXPLANATIONS: Record = { maintainers: 'GitHub logins treated as maintainers by /review contributor mode; empty uses repository push permission.', stamp_files: 'The repo’s stamp rule: globs of the only files that may differ by a stamp (version file, manifests, plugin trees, the PR’s own since: rows). Empty keeps gstack’s per-file rules; set, it applies before every rule.', stamp_patterns: 'Literal templates with exactly one {version} slot (at most 16, 128 characters each), for example `"version": "{version}"` or `since: \'{version}\'`; the restamp writer and the tree receipt use them beside (was X), since: X and vX.', + stamp_placeholders: 'Literal placeholders (at most 16, one line, 32 characters each, never `{version}` or a version), for example `["TBD"]`: a line the branch added that reads a placeholder in a stamp_patterns template (`since: \'TBD\'`) may become that template at the new version, and finish, the receipt carry and the tree receipt read it as a stamp. Empty (default) accepts no placeholder.', restamp_command: 'The repo’s own restamp, run first in gstack-restamp’s staging worktree (a clean checkout of HEAD) with the target version as its argument and in GSTACK_RESTAMP_VERSION; gstack’s version writer is skipped, output confinement still applies. Needs a clean caller tree and --allow-repo-commands.', migrations: '{ dir, id }: the migration directory and a regex with one capture group for the number in a file name; `gstack-pregate regate` reports MIGRATION_COLLISION against the base branch and every open PR.', gate_command: 'The repo’s full pre-merge gate command. gstack-ship-queue gate-ahead and ready print it for the caller to run (in the background or in CI); gstack never executes it. null when the repo has none.', @@ -236,6 +242,7 @@ export const BUMP_NEVER_KEY_VERSION = '1.91.353.0'; export const BASE_RED_KEY_VERSION = '1.91.374.0'; /** `train.gates` and `train.land` are values of the older train key, so compatErrors checks them by value, as for bump: never. */ export const TRAIN_GATES_KEY_VERSION = '1.92.0.0'; +export const STAMP_PLACEHOLDERS_KEY_VERSION = '1.94.16.0'; export const REVIEW_EXCLUDE_KEY_VERSION = '1.93.33.0'; export const POLICY_KEY_INTRODUCED: Record = { bump: '1.91.73.0', queue_mode: '1.91.73.0', release_tool: '1.91.73.0', release_outputs: '1.91.73.0', stamp_paths: '1.91.73.0', @@ -245,6 +252,7 @@ export const POLICY_KEY_INTRODUCED: Record = { train: TRAIN_KEYS_VERSION, review_scope: REVIEW_SCOPE_KEY_VERSION, docs: DOCS_CAPS_KEY_VERSION, changelog_fragments: CHANGELOG_FRAGMENTS_KEY_VERSION, base_red: BASE_RED_KEY_VERSION, review_exclude_paths: REVIEW_EXCLUDE_KEY_VERSION, + stamp_placeholders: STAMP_PLACEHOLDERS_KEY_VERSION, }; const newer = (a: string, b: string): boolean => cmpVersion(parseVersion(a)!, parseVersion(b)!) > 0; @@ -274,9 +282,6 @@ const MAX_PATTERN = 256; const MAX_REGEX = 512; const MAX_COMMAND = 512; const SHA_RE = /^[0-9a-f]{7,64}$/; -const MAX_TEMPLATES = 16; -const MAX_TEMPLATE = 128; -export const VERSION_SLOT = '{version}'; function hasDotDot(p: string): boolean { return p.split(/[\\/]+/).some(seg => seg === '..'); @@ -343,17 +348,6 @@ function excludeErrors(value: unknown, out: PolicyError[]): void { }); } -function templateErrors(value: unknown, out: PolicyError[]): void { - if (!Array.isArray(value)) { out.push({ code: 'POLICY_INVALID', path: 'stamp_patterns', message: 'must be an array of literal templates' }); return; } - if (value.length > MAX_TEMPLATES) out.push({ code: 'POLICY_CONTAINMENT', path: 'stamp_patterns', message: `more than ${MAX_TEMPLATES} templates` }); - value.forEach((t, i) => { - const at = `stamp_patterns[${i}]`; - if (typeof t !== 'string') { out.push({ code: 'POLICY_INVALID', path: at, message: 'must be a string' }); return; } - if (t.split(VERSION_SLOT).length !== 2) out.push({ code: 'POLICY_INVALID', path: at, message: `${JSON.stringify(t)} must contain exactly one ${VERSION_SLOT}` }); - else if (t.length > MAX_TEMPLATE || /[\0\r\n]/.test(t)) out.push({ code: 'POLICY_CONTAINMENT', path: at, message: `must be one line of at most ${MAX_TEMPLATE} characters` }); - }); -} - function migrationErrors(value: unknown, out: PolicyError[]): void { const m = value as Record | null; if (!m || typeof m !== 'object' || Array.isArray(m)) { out.push({ code: 'POLICY_INVALID', path: 'migrations', message: 'must be { dir, id } or null' }); return; } @@ -497,6 +491,7 @@ export function validatePolicy(raw: unknown, opts: { gstackVersion?: string } = if ('release_outputs' in obj) listErrors('release_outputs', obj.release_outputs, errors, globError); if ('stamp_files' in obj) listErrors('stamp_files', obj.stamp_files, errors, globError); if ('stamp_patterns' in obj) templateErrors(obj.stamp_patterns, errors); + if ('stamp_placeholders' in obj) placeholderErrors(obj.stamp_placeholders, errors); if ('restamp_command' in obj && obj.restamp_command !== null) { const t = obj.restamp_command; if (typeof t !== 'string' || !t.trim()) errors.push({ code: 'POLICY_INVALID', path: 'restamp_command', message: 'must be a non-empty string or null' }); diff --git a/lib/ship-receipt-depth.ts b/lib/ship-receipt-depth.ts index 7cd524523..165f56b93 100644 --- a/lib/ship-receipt-depth.ts +++ b/lib/ship-receipt-depth.ts @@ -224,7 +224,8 @@ function headInputs(root: string, baseRef: string, head: string, r?: ShipReceipt /** * The queue's exclusion check: the digest the receipt recorded (none when absent) against the one `head` has under the current * base policy. Only the digest counts (the policy's patterns and the excluded list), so a base merge or a moved base that leaves - * both unchanged binds; a changed selection needs the review rerun, which receipt write alone cannot stand in for. + * both unchanged binds; a changed selection needs the review rerun (write checks the same digest and does not read the + * review ledger). */ export function excludeMismatch(x: Exclusion, r: ShipReceipt, head: string, base: string): string | null { const expected = x.state === 'active' ? x.record.digest : 'none'; @@ -232,7 +233,7 @@ export function excludeMismatch(x: Exclusion, r: ShipReceipt, head: string, base if (expected === recorded) return null; const source = x.state === 'active' ? x.record.source : r.review_exclude?.source ?? base; const h = head.slice(0, 12); - return `field=review_exclude expected=${expected} recorded=${recorded} source=${source}: the reviewers read another selection than ${base}'s review_exclude_paths gives ${h}; the receipt is unchanged; fix: rerun /ship's review on ${h} (gstack-review-dispatch pass --base ${base.replace(/^(refs\/remotes\/)?origin\//, '')}), then gstack-ship-receipt write --pr --review-exclude --body with the digest gstack-review-dispatch verify --ledger prints on REVIEW_EXCLUDE_RECORD:; a receipt write alone does not clear this`; + return `field=review_exclude expected=${expected} recorded=${recorded} source=${source}: the receipt records another selection than ${base}'s review_exclude_paths gives ${h}; the receipt is unchanged; fix: rerun /ship's review on ${h} (gstack-review-dispatch pass --base ${base.replace(/^(refs\/remotes\/)?origin\//, '')}), then gstack-ship-receipt write --pr --review-exclude --body with the digest gstack-review-dispatch verify --ledger prints on REVIEW_EXCLUDE_RECORD: (write checks the digest against the base policy and does not read the review ledger, so only the rerun shows the reviewers read this selection)`; } /** The receipt's recorded gate scan when it names this head; otherwise null (unavailable). */ const boundScan = (r: ShipReceipt, head: string): number | null => { diff --git a/lib/ship-receipt.ts b/lib/ship-receipt.ts index d8218c20b..27f911951 100644 --- a/lib/ship-receipt.ts +++ b/lib/ship-receipt.ts @@ -6,6 +6,7 @@ * computes the P7 adoption census over merged PRs (deduplicated by run and * thread id against an owner-maintained thread cohort). */ +import { spawnSync } from 'node:child_process'; import { ARTIFACT_SCHEMAS, checkSchema, type ShipReceipt, type ValidationError } from './headless-artifacts'; import { approvalSummary } from './approval-citations'; import { FRAGMENT_README } from './changelog-fragments'; @@ -177,6 +178,8 @@ export interface CarryPlanInput { rangeDiff: string; /** The base the receipt was reviewed against when it differs from the carry's (a stack parent that landed, #3570): productPatchId.from is then the recorded diff against it. */ retargetedFrom?: string; + /** A commit inside the reviewed range whose product change the base already holds (a stack parent that squash-merged under a receipt written against the base, GST-1199): productPatchId.from is then the range from it. */ + stackParent?: string; /** A clean scoped review of product paths whose only difference is base context next to unchanged hunks (lib/ship-patch.ts, GST-384): the carry reason it adds. */ scopedReview?: string; /** Stamp files the branch changed as product whose only change since the reviewed head is a stamp on the branch's own rows, the base untouched, and without which the two product patches match (GST-980). */ @@ -189,9 +192,9 @@ const CARRY_RERUNS = ['CI at head (gstack-ship-queue ready requires it); test an /** * 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. + * `package.json`) and the base policy's `stamp_patterns` templates and `stamp_placeholders`. */ -export interface CarryChange { before: string | null; after: string | null; oldVersion?: string | null; newVersion?: string | null; templates?: readonly string[]; side?: BranchSide } +export interface CarryChange { before: string | null; after: string | null; oldVersion?: string | null; newVersion?: string | null; templates?: readonly string[]; side?: BranchSide; placeholders?: readonly string[] } /** * A policy-named stamp path's change is a stamp when the new text reads as the @@ -214,7 +217,7 @@ export function stampOnlyText(change: CarryChange): boolean { const now = after.split('\n'); if (was.length !== now.length) return false; const changed = was.map((line, i) => i).filter(i => was[i] !== now[i]); - return changed.length > 0 && changed.every(i => stampOnlyLine(was[i]!, now[i]!, oldVersion, newVersion, change.templates ?? [], change.side, i + 1)); + return changed.length > 0 && changed.every(i => stampOnlyLine(was[i]!, now[i]!, oldVersion, newVersion, change.templates ?? [], change.side, i + 1, change.placeholders ?? [])); } /** @@ -253,7 +256,8 @@ export function planCarry(input: CarryPlanInput): CarryPlan { const named = input.changedProduct.slice(0, CARRY_NAMED_PATHS); return { ok: false, code: 'RECEIPT_CARRY_PRODUCT_CHANGED', message: `product diff changed between ${input.from.slice(0, 12)} and ${input.head.slice(0, 12)}: ${named.join(', ') || 'patch-id differs'}${input.changedProduct.length > named.length ? ` (+${input.changedProduct.length - named.length} more)` : ''}` }; } - const reasons: string[] = input.retargetedFrom ? [`stack parent ${input.retargetedFrom} landed`] : []; + const parent = input.retargetedFrom ?? input.stackParent?.slice(0, 12); + const reasons: string[] = parent ? [`stack parent ${parent} landed`] : []; if (input.mergeCommits > 0) reasons.push(input.mergeCommits === 1 ? 'base merge' : `${input.mergeCommits} base merges`); if (input.scopedReview && input.productPatchId.from !== input.productPatchId.head) reasons.push(input.scopedReview); if (input.stampedRows?.length && input.productPatchId.from !== input.productPatchId.head) reasons.push(`a stamp on the branch's own rows (${input.stampedRows.join(', ')})`); @@ -271,6 +275,29 @@ export function planCarry(input: CarryPlanInput): CarryPlan { }; } +/** How many commits of the reviewed range landedStackParent tries, newest first. */ +export const STACK_PARENT_WALK = 100; + +/** + * The newest commit inside the reviewed range (rangeBase..reviewed, the reviewed head itself excluded) whose product + * change the current base already holds: merging it into currentBase changes no product path. That is a stack parent + * that squash-merged (or merged) into the base after the receipt was written against the base (GST-1199). A commit with no + * product change of its own (a test-first commit) is no parent, since any base trivially holds it. Null when no + * commit within STACK_PARENT_WALK qualifies or git cannot tell; a conflicted product path is never held. + */ +export function landedStackParent(repoRoot: string, s: { rangeBase: string; reviewed: string; currentBase: string }, product: readonly string[]): string | null { + const run = (args: string[]) => spawnSync('git', args, { cwd: repoRoot, encoding: 'utf8', timeout: 120_000, maxBuffer: 64 * 1024 * 1024 }); + const commits = run(['rev-list', '--topo-order', `--max-count=${STACK_PARENT_WALK + 1}`, `${s.rangeBase}..${s.reviewed}`]); + if (commits.status !== 0) return null; + for (const commit of commits.stdout.split('\n').filter(c => c && c !== s.reviewed)) { + if (run(['diff', '--quiet', s.rangeBase, commit, '--', ...product]).status !== 1) continue; + const merged = run(['merge-tree', '--write-tree', '--no-messages', s.currentBase, commit]); + const tree = merged.status === 0 || merged.status === 1 ? merged.stdout.split('\n')[0]?.trim() : undefined; + if (tree && run(['diff', '--quiet', s.currentBase, tree, '--', ...product]).status === 0) return commit; + } + return null; +} + export interface TreeCarryInput { from: string; head: string; /** HEAD^{tree}, and every tree the receipt already covers (its head_tree plus earlier carries' trees). */ diff --git a/lib/ship-release.ts b/lib/ship-release.ts index 7c1941a5e..1cb9963fb 100644 --- a/lib/ship-release.ts +++ b/lib/ship-release.ts @@ -15,6 +15,8 @@ import { mergeChangelogs, normalizeChangelog } from './changelog-check'; import { versionAtRef } from './restamp'; import { classifyPath } from './review-depth'; import { comparePath, contextPacket, gateEditedPaths, isTestCode, PACKET_SECTION_CAP, readScopedReview, touchedPaths, type PatchSides } from './ship-patch'; +import { locate, readRecord } from './approval-record'; +import { reviewFreshness } from './review-evidence'; import { parseReviewLoopException, type ReviewLoopException } from './ship-receipt-depth'; import { parseReceiptDocs, type ReceiptDocs } from './ship-receipt-docs'; import { resolveStateRoot } from './state-root'; @@ -183,8 +185,14 @@ function harnessRepin(cwd: string, from: string, to: string, p: string): boolean || (e?.since === b.version && typeof e?.nonBehavioral === 'string' && e.nonBehavioral.trim().length >= 10)); } -/** Step 16 stage 2's route. With --reviewed-base, a path whose branch patch is the reviewed one counts as base (lib/ship-patch.ts). */ -type Routed = Record<'behavior' | 'docs' | 'metadata' | 'tests' | 'base' | 'context' | 'accepted', string[]>; +/** + * Step 16 stage 2's route. With --reviewed-base, a path whose branch patch is the reviewed one counts as base (lib/ship-patch.ts). + * A code or test path the newest completed clean review-fix-check row read (inside its range_base..commit range, where + * range_base holds the reviewed tree, and unchanged since) counts as fix_checked when an approved review_loop_grant names a stop + * head inside range_base..commit: the owner's granted fix-check after Step 11 is a review (GST-1130). Authored docs keep the + * stage 2.2 recheck. + */ +type Routed = Record<'behavior' | 'docs' | 'metadata' | 'tests' | 'base' | 'context' | 'accepted' | 'fix_checked', string[]>; function route(cwd: string, base: string, reviewed: string, reviewedBase: string | undefined, release: string[], opts: { docsRechecked: boolean; contextReviewed?: string; loop?: ReviewLoopException }, r: Report): Routed | null { const now = bin(cwd, 'gstack-wtree', [], 60_000).stdout.trim(); if (!/^[0-9a-f]{40,64}$/.test(now)) { r.stop('gstack-wtree printed no tree; the stage 2 comparison is blocked'); return null; } @@ -204,7 +212,7 @@ function route(cwd: string, base: string, reviewed: string, reviewedBase: string sides = { reviewedBase: rb, reviewed, currentBase: cb, current: now }; touched = new Set([...a, ...b]); } - const by: Routed = { behavior: [], docs: [], metadata: [], tests: [], base: [], context: [], accepted: [] }; + const by: Routed = { behavior: [], docs: [], metadata: [], tests: [], base: [], context: [], accepted: [], fix_checked: [] }; for (const p of [...changedSet].sort()) { const verdict = sides ? (touched.has(p) ? comparePath(cwd, sides, p) : 'same') : 'changed'; by[verdict === 'same' ? 'base' : release.includes(p) || harnessRepin(cwd, reviewed, now, p) ? 'metadata' : sides && isTestCode(p) ? 'tests' : classifyPath(p, cwd) === 'docs' ? 'docs' : verdict === 'context' ? 'context' : 'behavior'].push(p); @@ -219,6 +227,30 @@ function route(cwd: string, base: string, reviewed: string, reviewedBase: string by.context = by.context.filter(p => !packetBody.over.includes(p)); by.tests = by.tests.filter(p => !packetBody.over.includes(p)); } + const fixRow = bin(cwd, 'gstack-review-read', [], 60_000).stdout.split('\n---CONFIG---')[0].split('\n') + .flatMap(l => { try { const v = JSON.parse(l); return v?.skill === 'review-fix-check' ? [v] : []; } catch { return []; } }).at(-1); + const commitOf = (c: unknown) => typeof c === 'string' && /^[0-9a-f]{7,64}$/.test(c) ? git(cwd, ['rev-parse', '--verify', '--quiet', `${c}^{commit}`]).stdout.trim() : ''; + const onBranch = (c: string) => !!c && git(cwd, ['merge-base', '--is-ancestor', c, 'HEAD']).status === 0; + const fixCommit = commitOf(fixRow?.commit_full), fixFrom = commitOf(fixRow?.range_base); + const fresh = fixRow && reviewFreshness(fixRow, fixRow.wtree); + const ancestor = (a: string, d: string) => git(cwd, ['merge-base', '--is-ancestor', a, d]).status === 0; + const grants = fixRow ? (loc => 'file' in loc ? readRecord(loc).rows : [])(locate({ cwd, env: process.env })) : []; + const grant = grants.find(g => g.outcome === 'approved' && `${g.ask} ${g.scope}`.includes('review_loop_grant') + && (`${g.ask} ${g.scope}`.match(/\b[0-9a-f]{7,40}\b/g) ?? []).some(s => { const c = commitOf(s); return !!c && !!fixFrom && !!fixCommit && ancestor(fixFrom, c) && ancestor(c, fixCommit); })); + const notCounted = !fixRow ? '' : fresh?.status !== 'CURRENT' ? fresh?.reason ?? 'not a review row' + : fixRow.tree !== fixRow.wtree || !onBranch(fixCommit) ? 'the row was logged on content no branch commit holds' + : !onBranch(fixFrom) ? 'the row names no range_base commit on this branch' + : git(cwd, ['rev-parse', `${fixFrom}^{tree}`]).stdout.trim() !== reviewed ? `range_base ${fixFrom.slice(0, 12)} does not hold the reviewed tree` + : !grant ? 'no approved review_loop_grant names a stop head inside its range_base..commit' : ''; + if (notCounted) r.add(`FIX_CHECK: not counted (${notCounted})`); + const counted = !!fixRow && !notCounted; + if (counted) { + const range = touchedPaths(cwd, fixFrom, fixCommit), after = touchedPaths(cwd, fixCommit, now); + if (!range || !after) { r.stop(`git could not diff the fix-check range ${fixFrom.slice(0, 12)}..${fixCommit.slice(0, 12)} against the reviewed and current trees: stage 2 is blocked`); return null; } + by.fix_checked = [...by.behavior, ...by.tests, ...by.context].filter(p => range.has(p) && !after.has(p)).sort(); + for (const k of ['behavior', 'tests', 'context'] as const) by[k] = by[k].filter(p => !by.fix_checked.includes(p)); + r.add(`FIX_CHECK: reviewed ${by.fix_checked.length ? short(by.fix_checked) : 'none'} (fix-check at ${fixCommit.slice(0, 12)}, range from ${fixFrom.slice(0, 12)}, grant ${grant!.id})`); + } const loop = opts.loop; if (loop) { const head = loop.fix_check.head; @@ -233,7 +265,7 @@ function route(cwd: string, base: string, reviewed: string, reviewedBase: string by.behavior = by.behavior.filter(p => !by.accepted.includes(p)); by.docs = by.docs.filter(p => !by.accepted.includes(p)); } - r.add(`ROUTE: reviewed=${reviewed.slice(0, 12)} current=${now.slice(0, 12)} behavior=${by.behavior.length} docs=${by.docs.length} metadata=${by.metadata.length} tests=${by.tests.length} base=${sides ? by.base.length : 'unknown'} context=${by.context.length}${loop ? ` accepted=${by.accepted.length}` : ''}`); + r.add(`ROUTE: reviewed=${reviewed.slice(0, 12)} current=${now.slice(0, 12)} behavior=${by.behavior.length} docs=${by.docs.length} metadata=${by.metadata.length} tests=${by.tests.length} base=${sides ? by.base.length : 'unknown'} context=${by.context.length}${counted ? ` fix_checked=${by.fix_checked.length}` : ''}${loop ? ` accepted=${by.accepted.length}` : ''}`); if (loop) r.add(`REVIEW_LOOP_EXCEPTION: accepted ${by.accepted.length ? short(by.accepted) : 'none'} (changed since fix-check ${loop.fix_check.head.slice(0, 12)}; accepted by ${loop.accepted_by} ${loop.date} via ${loop.via})`); if (by.behavior.length) { r.stop(`behavior, tests or build inputs changed since review (${short(by.behavior.sort())}): insert 5–11.5 → 12–14 → 16 before Step 17 (stage 2.1)`); return null; } const scoped = [...by.context, ...by.tests].sort(); diff --git a/lib/stamp-only.ts b/lib/stamp-only.ts index bbb87dc66..53b2b5231 100644 --- a/lib/stamp-only.ts +++ b/lib/stamp-only.ts @@ -4,7 +4,8 @@ * lines the branch wrote itself. Split from restamp.ts (module-size ratchet). */ import { spawnSync } from 'node:child_process'; -import { branchAddedLines, forkVersion, rewriteStamps, stampMentions } from './restamp'; +import { branchAddedLines, escapeRegExp, forkVersion, rewriteStamps, stampMentions } from './restamp'; +import { VERSION_SLOT } from './ship-policy'; import { cmpVersion, parseVersion, versionWidth } from './version-source'; /** @@ -26,12 +27,20 @@ import { cmpVersion, parseVersion, versionWidth } from './version-source'; * stage's bound for its own lines (GST-895, GST-963): finish's stampedFiles, * the tree receipt and the receipt carry all pass the same side. When the * version does not move (a slot that keeps its claimed VERSION), only such a - * branch line may change: the stage moves nothing else then. + * branch line may change: the stage moves nothing else then. On such a branch + * line a declared placeholder (`stamp_placeholders`, GST-1026: gbrain's + * `since: 'TBD'`) may also become the new version, but only inside a + * `stamp_patterns` template with literal text around its slot, never a built-in + * form: with at least one such rewrite, the line must then be the added line, + * or one whose remaining stamp-form moves pass the fork-version bound. */ -export function stampOnlyLine(removed: string, added: string, oldV: string, newV: string, templates: readonly string[] = [], side?: BranchSide, at?: number): boolean { +export function stampOnlyLine(removed: string, added: string, oldV: string, newV: string, templates: readonly string[] = [], side?: BranchSide, at?: number, placeholders: readonly string[] = []): boolean { if (removed === added) return false; if (oldV !== newV && stampOnlyWithin(removed, added, oldV, newV, templates)) return true; - return !!side && at !== undefined && side.at.has(at) && side.added.has(removed) && stampOnlyWithin(removed, added, side.fork, newV, templates); + if (!side || at === undefined || !side.at.has(at) || !side.added.has(removed)) return false; + if (stampOnlyWithin(removed, added, side.fork, newV, templates)) return true; + const filled = fillPlaceholders(removed, newV, templates, placeholders); + return filled.count > 0 && (filled.text === added || stampOnlyWithin(filled.text, added, side.fork, newV, templates)); } /** @@ -64,6 +73,21 @@ export function branchSides(repoRoot: string, baseRef: string, head: string, fal }; } +/** Each declared placeholder rewritten to `newV` inside each template with literal text around its slot (a bare edge at a word boundary). */ +function fillPlaceholders(removed: string, newV: string, templates: readonly string[], placeholders: readonly string[]): { text: string; count: number } { + let text = removed; + let count = 0; + for (const t of templates) { + const [pre, post] = t.split(VERSION_SLOT) as [string, string]; + if (!pre && !post) continue; + for (const p of placeholders) { + const re = new RegExp(`${escapeRegExp(pre)}${pre ? '' : '(? { count++; return pre + newV + post; }); + } + } + return { text, count }; +} + function stampOnlyWithin(removed: string, added: string, oldV: string, newV: string, templates: readonly string[]): boolean { const floor = parseVersion(oldV); const width = versionWidth(oldV); diff --git a/lib/tree-receipt.ts b/lib/tree-receipt.ts index f5d46b053..0611d6074 100644 --- a/lib/tree-receipt.ts +++ b/lib/tree-receipt.ts @@ -78,15 +78,16 @@ export function classifyFiles(repoRoot: string, fileDiffs: FileDiff[], gated: st /** * 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`, bounded by the fork version on the branch's own lines). + * moved (#3512), each in a stamp form (`stampOnlyLine`, bounded by the fork version on the branch's own lines, where a + * declared placeholder may also move inside a stamp_patterns template, GST-1026). */ -function stampOnlyHunks(hunks: FileDiff['hunks'], oldV: string, newV: string, templates: readonly string[], side?: BranchSide): { why: string; line: number } | null { +function stampOnlyHunks(hunks: FileDiff['hunks'], oldV: string, newV: string, templates: readonly string[], side?: BranchSide, placeholders: 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]!; - if (!stampOnlyLine(r, a, oldV, newV, templates, side, h.removedAt[i])) 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, side, h.removedAt[i], placeholders)) return { why: `${JSON.stringify(r.trim().slice(0, 60))} -> ${JSON.stringify(a.trim().slice(0, 60))}`, line: h.at + i }; } } return null; @@ -130,7 +131,7 @@ export function classifyFile(repoRoot: string, f: FileDiff, gated: string, curre const rule = matchesAny(policy.stamp_paths, f.path) ? 'stamp_paths' : matchesAny(policy.release_outputs, f.path) ? 'release_outputs' : matchesAny(policy.stamp_files, f.path) ? 'stamp_files' : null; const own = rule ? side?.(f.path) : undefined; if (oldV && newV && (oldV !== newV || own) && rule) { - const bad = stampOnlyHunks(f.hunks, oldV, newV, policy.stamp_patterns, own); + const bad = stampOnlyHunks(f.hunks, oldV, newV, policy.stamp_patterns, own, policy.stamp_placeholders); return bad ? changed(rule, bad.why, bad.line) : stamp(rule); } return changed('other', `${f.hunks.length} hunk(s) outside every stamp rule`); diff --git a/package.json b/package.json index 3488c9cb6..64d469b2b 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "gstack", - "version": "1.94.10", + "version": "1.94.17", "description": "Garry's Stack — Claude Code skills + fast headless browser. One repo, one install, entire AI engineering workflow.", "license": "MIT", "type": "module", diff --git a/test/diff-scope.test.ts b/test/diff-scope.test.ts index 882b04af7..8d1e12650 100644 --- a/test/diff-scope.test.ts +++ b/test/diff-scope.test.ts @@ -223,6 +223,28 @@ describe('gstack-diff-scope', () => { expect(scope.SCOPE_AUTH).toBe('true'); }); + // Value: protects=auth matches as a word or path segment, so docs about authors stay out of the gate class; + // fails_when=the auth match is a bare substring again (author-spec.md, AUTHORS, authority.ts raise SCOPE_AUTH); + // why_new=GST-1065 (author-spec.md raised a docs bundle to gate class with four full readers); seam=bin/gstack-diff-scope + for (const file of ['docs/author-spec.md', 'AUTHORS', 'lib/authority.ts', 'docs/authored-by.md', 'src/coauthors.ts', 'AUTHORS.md']) { + test(`${file} is not an auth path`, () => { + expect(runScope(createRepo([file])).SCOPE_AUTH).toBe('false'); + }); + } + + // Value: protects=real auth paths still raise SCOPE_AUTH after the word-boundary fix (negative controls for GST-1065); + // fails_when=the boundary drops a segment, a separator, a camelCase name, an authn/authz/oauth spelling or a glued token (auth0, basicauth, nextauth); + // why_new=GST-1065 and its review (the first word-only matcher dropped glued tokens the old glob caught); seam=bin/gstack-diff-scope + for (const file of ['src/auth.ts', 'app/auth/login.rb', 'lib/oauth.ts', 'src/oauth2/token.ts', 'lib/authn/check.go', 'src/authz.py', 'src/authService.ts', + 'src/useAuth.ts', 'lib/OAuthClient.ts', 'lib/authentication.rb', 'lib/authorization_policy.rb', 'src/Authorize.tsx', 'config/AUTH_KEYS.md', 'pkg/auth-middleware.go', + 'lib/auth0.ts', 'src/basicauth.go', 'src/authguard.ts', 'lib/reauth.ts', 'pages/api/nextauth.ts', 'src/authtoken.ts', 'lib/preauth.rb', 'src/HTTPAuth.ts', + 'src/unauthenticated.ts', 'lib/reauthentication.ts', 'src/deauthorize.ts', 'app/oauth2client.py', 'api/HTTPAuthorization.ts', + 'src/nextauthmiddleware.ts', 'src/reauthn.ts', 'src/noauthrequired.ts']) { + test(`${file} is an auth path`, () => { + expect(runScope(createRepo([file])).SCOPE_AUTH).toBe('true'); + }); + } + test('detects config via bun.lock (Bun v1.2+ text lockfile)', () => { const dir = createRepo(['bun.lock']); const scope = runScope(dir); diff --git a/test/gstack-design-detect.test.ts b/test/gstack-design-detect.test.ts index 39388ac41..84a4e4e62 100644 --- a/test/gstack-design-detect.test.ts +++ b/test/gstack-design-detect.test.ts @@ -935,6 +935,34 @@ describe('scan', () => { expect(r.err).toContain(`${SENTINEL.DETECT_REFUSED}: main (not a repository)`); expect(r.code).toBe(1); // a base that cannot be diffed is a failed target, never a silent clean scan }); + + test.skipIf(!POSIX)('--changed main reads origin/main over a stale local main (GST-1036)', () => { + // Value: protects=design review scans only the branch's frontend files; fails_when=a bare --changed base diffs against the stale local main + // and scans a stylesheet only upstream changed; why_new=GST-1036 (ship review-army passes a bare ); seam=bin/gstack-design-detect.ts + const bare = path.join(SANDBOX, 'stale-origin.git'); + const work = path.join(SANDBOX, 'stale-work'); + const other = path.join(SANDBOX, 'stale-other'); + const log = path.join(SANDBOX, 'argv-stale.log'); + try { + git(SANDBOX, 'clone', '-q', '--bare', REPO, bare); + git(SANDBOX, 'clone', '-q', bare, work); + git(SANDBOX, 'clone', '-q', bare, other); + for (const d of [work, other]) { git(d, 'config', 'user.email', 't@example.com'); git(d, 'config', 'user.name', 't'); } + fs.writeFileSync(path.join(other, 'src', 'styles.css'), '.hero { color: blue }\n'); + git(other, 'commit', '-qam', 'upstream'); + git(other, 'push', '-q', 'origin', 'HEAD:main'); + git(work, 'fetch', '-q', 'origin'); + git(work, 'checkout', '-q', '-b', 'feature', 'origin/main'); + fs.writeFileSync(path.join(work, 'src', 'components', 'Card.tsx'), 'export const Card = () =>

;\n'); + git(work, 'commit', '-qam', 'change'); + const r = run(['scan', '--changed', 'main', '--format', 'gstack'], { env: { IMPECCABLE_BIN: FAKE, IMPECCABLE_FAKE_LOG: log }, cwd: work }); + const argv = JSON.parse(fs.readFileSync(log, 'utf-8').trim().split('\n')[0]).argv as string[]; + expect(argv.slice(2).map(a => path.relative(fs.realpathSync(work), a)).sort()).toEqual(['src/components/Card.tsx']); + expect(r.err).toContain('gstack-design-detect: local main is behind origin/main; base is origin/main'); + } finally { + for (const p of [bare, work, other, log]) fs.rmSync(p, { recursive: true, force: true }); + } + }); }); describe('design-review REPORT_DIR agrees with the allow-list', () => { diff --git a/test/gstack-ship-policy.test.ts b/test/gstack-ship-policy.test.ts index 025b1bb57..e290256a8 100644 --- a/test/gstack-ship-policy.test.ts +++ b/test/gstack-ship-policy.test.ts @@ -8,7 +8,7 @@ import * as fs from 'node:fs'; import * as path from 'node:path'; import { spawnSync } from 'node:child_process'; import { EXIT } from '../lib/headless-artifacts'; -import { BUMP_NEVER_KEY_VERSION, CHANGELOG_FRAGMENTS_KEY_VERSION, DEFAULT_POLICY, POLICY_KEYS, POLICY_KEY_INTRODUCED, REVIEW_SCOPE_KEY_VERSION, globToRegExp, matchesAny, minimumReader, shipConsequences, validatePolicy } from '../lib/ship-policy'; +import { BUMP_NEVER_KEY_VERSION, CHANGELOG_FRAGMENTS_KEY_VERSION, DEFAULT_POLICY, POLICY_KEYS, POLICY_KEY_INTRODUCED, REVIEW_SCOPE_KEY_VERSION, STAMP_PLACEHOLDERS_KEY_VERSION, globToRegExp, matchesAny, minimumReader, shipConsequences, validatePolicy } from '../lib/ship-policy'; import { fillDue } from '../lib/ship-queue-train'; import { cmpVersion, parseVersion } from '../lib/version-source'; import { BASE_POLICY, cleanup, makeFixtureRepo, runBin, type FixtureRepo } from './helpers/restamp-fixture'; @@ -63,6 +63,32 @@ describe('validatePolicy (schema + containment, pure)', () => { expect(codes({ preregistration_shas: ['not-a-sha'] })).toEqual(['POLICY_CONTAINMENT:preregistration_shas[0]']); }); + // Value: protects=stamp_placeholders defaults to none, needs its reader floor, and admits only short one-line literals + // that are neither the {version} slot nor a version (GST-1026); fails_when=a non-default value validates without + // min_gstack_version, or a version, a slot, a multi-line or an oversized entry is accepted; seam=validatePolicy + test('stamp_placeholders: empty by default, needs its floor, and admits only short literal placeholders (GST-1026)', () => { + const codes = (p: unknown) => validatePolicy(p).map(e => `${e.code}:${e.path}`); + const at = (stamp_placeholders: unknown) => codes({ stamp_placeholders, min_gstack_version: STAMP_PLACEHOLDERS_KEY_VERSION }); + expect(DEFAULT_POLICY.stamp_placeholders).toEqual([]); + expect(POLICY_KEY_INTRODUCED.stamp_placeholders).toBe(STAMP_PLACEHOLDERS_KEY_VERSION); + expect(codes({ stamp_placeholders: [] })).toEqual([]); + expect(codes({ stamp_placeholders: ['TBD'] })).toEqual(['POLICY_INVALID:stamp_placeholders']); + expect(minimumReader({ stamp_placeholders: ['TBD'] })).toBe(STAMP_PLACEHOLDERS_KEY_VERSION); + expect(at(['TBD', 'UNRELEASED'])).toEqual([]); + expect(at('TBD')).toEqual(['POLICY_INVALID:stamp_placeholders']); + expect(at([7])).toEqual(['POLICY_INVALID:stamp_placeholders[0]']); + expect(at(Array.from({ length: 17 }, (_, i) => `P${i}`))).toEqual(['POLICY_CONTAINMENT:stamp_placeholders']); + expect(at(['x'.repeat(33)])).toEqual(['POLICY_CONTAINMENT:stamp_placeholders[0]']); + expect(at(['x'.repeat(32)])).toEqual([]); + expect(at(['TB\nD'])).toEqual(['POLICY_CONTAINMENT:stamp_placeholders[0]']); + expect(at([''])).toEqual(['POLICY_CONTAINMENT:stamp_placeholders[0]']); + expect(at([' '])).toEqual(['POLICY_CONTAINMENT:stamp_placeholders[0]']); + expect(at(['{version}'])).toEqual(['POLICY_INVALID:stamp_placeholders[0]']); + expect(at(['next-{version}'])).toEqual(['POLICY_INVALID:stamp_placeholders[0]']); + expect(at(['1.2.3.0'])).toEqual(['POLICY_INVALID:stamp_placeholders[0]']); + expect(at(['0.60.553'])).toEqual(['POLICY_INVALID:stamp_placeholders[0]']); + }); + // Value: protects=bump never is refused without its floor (an older gstack names the cause) and next to any policy that stamps a version at every merge, reports its reader floor, and prints its Step 12 consequence; fails_when=never validates bare, or beside stamp-at-merge, train or changelog_fragments; // why_new=GST-380 added the value to an older key, which the per-key floor check skips; seam=validatePolicy, minimumReader, shipConsequences; no_claim=Step 12's behavior (test/ship-release.test.ts) test('bump never needs its floor and rules out every policy that stamps at merge', () => { diff --git a/test/gstack-ship-receipt-docs.test.ts b/test/gstack-ship-receipt-docs.test.ts index db89a68fb..03213266c 100644 --- a/test/gstack-ship-receipt-docs.test.ts +++ b/test/gstack-ship-receipt-docs.test.ts @@ -53,6 +53,25 @@ describe('E18: a repository without a version source or CHANGELOG', () => { expect(JSON.parse(fs.readFileSync(path.join(out, 'ship-run.json'), 'utf8')).version).toBe('none'); }); + // Value: protects=a receipt write --version named in a repository with no version source keeps that version through a + // carry (GST-1030 review); fails_when=the carry refuses because HEAD has no readable version, or drops the version; + // seam=bin/gstack-ship-receipt cmdCarry + test('a carry keeps the version write --version named when HEAD has no version source (GST-1030)', () => { + const repo = fixture(); + for (const f of ['VERSION', 'package.json', 'package-lock.json']) fs.rmSync(path.join(repo.work, f)); + repo.git('add', '-A'); repo.git('commit', '-qm', 'changelog only'); repo.git('push', '-q', 'origin', 'main'); + makePr(repo, 'pr-unversioned'); + const body = path.join(repo.root, 'body.md'); + fs.writeFileSync(body, '## Summary\n'); + expect(runBin(repo, 'gstack-ship-receipt', ['write', '--pr', '3', '--base', 'main', '--session-kind', 'x', '--version', '2.0.0', '--body', body]).status).toBe(EXIT.ok); + fs.mkdirSync(path.join(repo.work, 'test'), { recursive: true }); + fs.writeFileSync(path.join(repo.work, 'test', 'a.test.ts'), 'test a\n'); + repo.git('add', '-A'); repo.git('commit', '-qm', 'a test-only change'); + const carried = runBin(repo, 'gstack-ship-receipt', ['carry', '--body', body, '--base', 'main']); + expect(carried.out, carried.err).toContain('CARRY: carried'); + expect(extractReceipt(fs.readFileSync(body, 'utf8')).receipt?.version).toBe('2.0.0'); + }); + test('a repository with a version source keeps its version, and one with only a CHANGELOG records no version rather than "none"', () => { const repo = fixture(); const w = runBin(repo, 'gstack-ship-receipt', ['write', '--base', 'main', '--session-kind', 'x', '--json']); @@ -248,6 +267,25 @@ describe('carry follow-ups: git floor, named refusal, branch-touched walk, digit expect(stampOnlyText({ before, after: lines(key('1.2.3.0'), key('1.2.4.0')), oldVersion: '1.2.4.0', newVersion: '1.2.4.0', templates })).toBe(false); }); + // Value: protects=the carry classifier reads a declared placeholder moved to the new version on the branch's own row + // as a stamp, through the same predicate as finish and the tree receipt (GST-1026); fails_when=stampOnlyText ignores + // the policy's placeholders, or accepts an undeclared one or a base row; seam=lib/ship-receipt.ts stampOnlyText + test('stampOnlyText reads a declared placeholder moved on the branch\'s own row as a stamp, and nothing else (GST-1026)', () => { + const templates = ["since: '{version}'"]; + const row = (v: string, id: string) => ` { since: '${v}', id: '${id}' },`; + const text = (...rows: string[]) => ['export const ROWS = [', ...rows, '];', ''].join('\n'); + const before = text(row('TBD', 'base'), row('TBD', 'mine')); + const side = { fork: '0.60.552.0', added: new Set([row('TBD', 'mine')]), at: new Set([3]) }; + const change = (after: string, extra: Record = {}) => stampOnlyText({ before, after, oldVersion: '0.60.552.0', newVersion: '0.60.553.0', templates, side, placeholders: ['TBD'], ...extra }); + expect(change(text(row('TBD', 'base'), row('0.60.553.0', 'mine')))).toBe(true); + expect(change(text(row('TBD', 'base'), row('0.60.553.0', 'mine')), { oldVersion: '0.60.553.0' })).toBe(true); + expect(change(text(row('TBD', 'base'), row('0.60.553.0', 'mine')), { placeholders: [] })).toBe(false); + expect(change(text(row('TBD', 'base'), row('0.60.553.0', 'mine')), { placeholders: ['WIP'] })).toBe(false); + expect(change(text(row('TBD', 'base'), row('0.60.553.0', 'mine')), { templates: [] })).toBe(false); + expect(change(text(row('TBD', 'base'), row('0.60.553.0', 'mine')), { side: undefined })).toBe(false); + expect(change(text(row('0.60.553.0', 'base'), row('TBD', 'mine')))).toBe(false); + }); + test('planCarry names at most CARRY_NAMED_PATHS paths', () => { const many = Array.from({ length: CARRY_NAMED_PATHS + 3 }, (_, i) => `src/f${i}.ts`); const plan = planCarry({ from: 'a'.repeat(40), head: 'b'.repeat(40), changed: many.map(p => ({ path: p, class: 'product' as const })), productPatchId: { from: 'x', head: 'y' }, testPatchId: { from: '', head: '' }, changedProduct: many, mergeCommits: 0, rangeDiff: '' }); diff --git a/test/restamp-mentions.test.ts b/test/restamp-mentions.test.ts index 68a546a9c..a74641aa0 100644 --- a/test/restamp-mentions.test.ts +++ b/test/restamp-mentions.test.ts @@ -14,6 +14,8 @@ import * as fs from 'node:fs'; import * as path from 'node:path'; import { EXIT } from '../lib/headless-artifacts'; import { rewriteStamps, tentativeMentions } from '../lib/restamp'; +import { STAMP_PLACEHOLDERS_KEY_VERSION } from '../lib/ship-policy'; +import { extractReceipt } from '../lib/ship-receipt'; import type { GitFixtureTemplate } from './helpers/git-fixture-template'; import { BASE_POLICY, cleanup, cloneFixture, fixtureTemplate, makePr, mergeMain, mergeToMain, readFile, restamp, runBin, type FixtureRepo } from './helpers/restamp-fixture'; @@ -425,10 +427,165 @@ describe('a branch-added row left at the fork version after the base moved (GST- const { repo, body } = carryAfterStamp('restamp-carry-mode-', [row('1.2.3.0', 'base_row')], row('1.2.3.0', 'mine'), [row('1.2.3.0', 'base_row'), '', row('1.2.5.0', 'mine')]); fs.chmodSync(path.join(repo.work, NOTICE), 0o755); - repo.git('commit', '-qam', 'stamp 1.2.5.0, and make the file executable'); + repo.git('add', '-A'); + repo.git('update-index', '--chmod=+x', '--', NOTICE); + repo.git('commit', '-qm', 'stamp 1.2.5.0, and make the file executable'); expect(repo.git('ls-tree', 'HEAD', '--', NOTICE)).toStartWith('100755 '); const carried = runBin(repo, 'gstack-ship-receipt', ['carry', '--body', body, '--base', 'main']); expect(carried.status, carried.out).toBe(EXIT.refused); expect(carried.err).toContain('RECEIPT_CARRY_PRODUCT_CHANGED'); }); }); + +describe('declared placeholders and the receipt carry (GST-1026, GST-1030)', () => { + /** gbrain's policy (GST-1026): the since: template, and TBD declared as a placeholder when `declared`. */ + const placeholderPolicy = (declared: boolean) => ({ ...receiptPolicy(true), ...(declared ? { stamp_placeholders: ['TBD'], min_gstack_version: STAMP_PLACEHOLDERS_KEY_VERSION } : {}) }); + + /** A branch row written as since: 'TBD' at the claim 1.2.4.0, then the stamp to 1.2.5.0 with the row restamped by hand (gbrain's release:restamp). */ + function tbdStamp(prefix: string, pol: Record, ownRow: string): { repo: FixtureRepo; gated: string } { + const repo = makeFixtureRepo({ prefix, policy: pol }); repos.push(repo); + write(repo, NOTICE, notice([row('TBD', 'base_row')])); + repo.git('add', '-A'); repo.git('commit', '-qm', 'notice rows'); repo.git('push', '-q', 'origin', 'main'); + makePr(repo, 'pr-tbd'); + expect(restamp(repo, ['--version', '1.2.4.0']).status).toBe(EXIT.ok); + repo.git('commit', '-qam', 'stamp 1.2.4.0'); + write(repo, NOTICE, notice([row('TBD', 'base_row'), '', ownRow])); + repo.git('commit', '-qam', 'mine: a TBD row'); + const gated = repo.git('rev-parse', 'HEAD'); + expect(restamp(repo, ['--version', '1.2.5.0']).status).toBe(EXIT.ok); + return { repo, gated }; + } + + // Value: protects=the tree receipt (stamp, verify, gate-ahead replay) accepts gbrain's restamp of the branch's own + // since: 'TBD' row when the policy declares TBD, and refuses it undeclared, on a base row, or on a base row whose text + // the branch duplicates (GST-1026); fails_when=classifyFile reads the TBD move as a hand edit, or accepts it off the + // branch's own rows; seam=lib/tree-receipt.ts stampOnlyHunks + test('the tree receipt accepts a declared TBD row restamped on the branch, and refuses the same move on a base row (GST-1026)', () => { + const own = tbdStamp('restamp-tbd-receipt-', placeholderPolicy(true), row('TBD', 'mine')); + write(own.repo, NOTICE, readFile(own.repo, NOTICE).replace(row('TBD', 'mine'), row('1.2.5.0', 'mine'))); + own.repo.git('commit', '-qam', 'stamp 1.2.5.0: the TBD row'); + const accepted = runBin(own.repo, 'gstack-tree-receipt', ['--gated', own.gated, '--base', 'main']); + expect(accepted.out, accepted.err).toContain('TREE: same-modulo-stamps'); + write(own.repo, NOTICE, readFile(own.repo, NOTICE).replace(row('TBD', 'base_row'), row('1.2.5.0', 'base_row'))); + own.repo.git('commit', '-qam', 'move the base TBD row by hand'); + const base = runBin(own.repo, 'gstack-tree-receipt', ['--gated', own.gated, '--base', 'main']); + expect(base.out, base.err).toContain('TREE: changed'); + expect(base.out).toContain("id: 'base_row'"); + }); + + test('the tree receipt refuses an undeclared TBD row restamped on the branch (GST-1026)', () => { + const undeclared = tbdStamp('restamp-tbd-undeclared-', placeholderPolicy(false), row('TBD', 'mine')); + write(undeclared.repo, NOTICE, readFile(undeclared.repo, NOTICE).replace(row('TBD', 'mine'), row('1.2.5.0', 'mine'))); + undeclared.repo.git('commit', '-qam', 'stamp 1.2.5.0: the TBD row'); + const refused = runBin(undeclared.repo, 'gstack-tree-receipt', ['--gated', undeclared.gated, '--base', 'main']); + expect(refused.out, refused.err).toContain('TREE: changed'); + }); + + test('the tree receipt refuses a restamp of the base copy of a TBD row the branch duplicates (GST-1026)', () => { + const dup = tbdStamp('restamp-tbd-dup-', placeholderPolicy(true), row('TBD', 'base_row')); + write(dup.repo, NOTICE, notice([row('1.2.5.0', 'base_row'), '', row('TBD', 'base_row')])); + dup.repo.git('commit', '-qam', 'move the base copy of the duplicated TBD row'); + const copy = runBin(dup.repo, 'gstack-tree-receipt', ['--gated', dup.gated, '--base', 'main']); + expect(copy.out, copy.err).toContain('TREE: changed'); + }); + + /** A receipt written at the claim 1.2.4.0 after `own` changed the branch, then the stamp to 1.2.5.0, uncommitted. */ + function receiptThenStamp(prefix: string, pol: Record, own: (repo: FixtureRepo) => void): { repo: FixtureRepo; body: string } { + const repo = makeFixtureRepo({ prefix, policy: pol }); repos.push(repo); + write(repo, NOTICE, notice([row('TBD', 'base_row')])); + fs.rmSync(path.join(repo.work, 'package-lock.json')); + repo.git('add', '-A'); repo.git('commit', '-qm', 'notice rows, no lockfile'); repo.git('push', '-q', 'origin', 'main'); + makePr(repo, 'pr-carry'); + expect(restamp(repo, ['--version', '1.2.4.0']).status).toBe(EXIT.ok); + repo.git('commit', '-qam', 'stamp 1.2.4.0'); + own(repo); + repo.git('add', '-A'); repo.git('commit', '-qm', 'mine'); + const body = path.join(repo.root, 'body.md'); + fs.writeFileSync(body, '## Summary\n'); + const w = runBin(repo, 'gstack-ship-receipt', ['write', '--pr', '3', '--base', 'main', '--session-kind', 'x', '--body', body]); + expect(w.status, w.err).toBe(EXIT.ok); + expect(restamp(repo, ['--version', '1.2.5.0']).status).toBe(EXIT.ok); + return { repo, body }; + } + const carry = (repo: FixtureRepo, body: string) => runBin(repo, 'gstack-ship-receipt', ['carry', '--body', body, '--base', 'main']); + + // Value: protects=the receipt carry follows gbrain's restamp of the branch's own since: 'TBD' row when the policy + // declares TBD, and refuses it undeclared (GST-1026); fails_when=productSpec's own-row check ignores the policy's + // placeholders and refuses RECEIPT_CARRY_PRODUCT_CHANGED; seam=bin/gstack-ship-receipt productSpec ownRows + test.each([['declared', true], ['undeclared', false]])('the carry and a TBD row restamped on the branch: %s', (_, declared) => { + const { repo, body } = receiptThenStamp(`restamp-tbd-carry-${declared}-`, placeholderPolicy(declared), r => write(r, NOTICE, notice([row('TBD', 'base_row'), '', row('TBD', 'mine')]))); + write(repo, NOTICE, readFile(repo, NOTICE).replace(row('TBD', 'mine'), row('1.2.5.0', 'mine'))); + repo.git('commit', '-qam', 'stamp 1.2.5.0: the TBD row'); + const carried = carry(repo, body); + if (declared) expect(carried.out, carried.err).toContain('CARRY: carried'); + else expect([carried.status, carried.err.includes('RECEIPT_CARRY_PRODUCT_CHANGED')], carried.err).toEqual([EXIT.refused, true]); + }); + + // Value: protects=a carried receipt names the head's version, the one write would record there, after a main merge + // and a restamp (GST-1030 point 1, GGST-1); fails_when=carryReceipt copies version from the old receipt; + // seam=bin/gstack-ship-receipt cmdCarry + test('the carried receipt records the head\'s version after a main merge and a restamp (GST-1030)', () => { + const { repo, body } = receiptThenStamp('restamp-carry-version-', policy(true), r => write(r, 'src.ts', 'export const x = 0;\n')); + repo.git('reset', '-q', '--hard'); + expect(readFile(repo, 'VERSION')).toBe('1.2.4.0\n'); + makePr(repo, 'pr-other'); + expect(restamp(repo, ['--next']).status).toBe(EXIT.ok); + repo.git('commit', '-qam', 'stamp other'); + mergeToMain(repo, 'pr-other'); + repo.git('checkout', '-q', 'pr-carry'); + mergeMain(repo); + expect(restamp(repo, ['--next']).status).toBe(EXIT.ok); + repo.git('commit', '-qam', 'restamp after the lost slot'); + expect(readFile(repo, 'VERSION')).toBe('1.2.5.0\n'); + expect(extractReceipt(fs.readFileSync(body, 'utf8')).receipt?.version).toBe('1.2.4.0'); + const carried = carry(repo, body); + expect(carried.out, carried.err).toContain('CARRY: carried'); + expect(extractReceipt(fs.readFileSync(body, 'utf8')).receipt?.version).toBe('1.2.5.0'); + }); + +/** package.json with its dependency pin set to `pin`, every other field kept. */ + const pinned = (repo: FixtureRepo, pin: string) => { + const pkg = JSON.parse(readFile(repo, 'package.json')); + pkg.dependencies = { ...pkg.dependencies, 'left-pad': pin }; + write(repo, 'package.json', JSON.stringify(pkg, null, 2) + '\n'); + }; + + // Value: protects=a pin-bump PR's receipt carries across a main merge and a restamp when package.json's only change since + // review is its version field, and refuses when the pin itself moved (GST-1030, GBEV-3, gbrain-evals #306); + // fails_when=the carry counts the PR's package.json as product and refuses RECEIPT_CARRY_PRODUCT_CHANGED on a version-only + // move, or lets a changed dependency pin ride; seam=bin/gstack-ship-receipt cmdCarry walk + test.each([['the pin unchanged since review', false], ['the pin moved since review', true]])('the carry and a pin-bump PR after a main merge: %s (GST-1030)', (_, moved) => { + const { repo, body } = receiptThenStamp(`restamp-carry-pin-${moved}-`, policy(true), r => pinned(r, '1.3.0')); + repo.git('reset', '-q', '--hard'); + makePr(repo, 'pr-other'); + expect(restamp(repo, ['--next']).status).toBe(EXIT.ok); + repo.git('commit', '-qam', 'stamp other'); + mergeToMain(repo, 'pr-other'); + repo.git('checkout', '-q', 'pr-carry'); + mergeMain(repo); + expect(restamp(repo, ['--next']).status).toBe(EXIT.ok); + if (moved) pinned(repo, '1.3.1'); + repo.git('commit', '-qam', 'restamp after the lost slot'); + expect(JSON.parse(readFile(repo, 'package.json')).version).toBe('1.2.5'); + const carried = carry(repo, body); + if (moved) expect([carried.status, carried.err.includes('RECEIPT_CARRY_PRODUCT_CHANGED')], carried.err).toEqual([EXIT.refused, true]); + else expect(carried.out, carried.err).toContain('CARRY: carried'); + }); + + // Value: protects=a version-only restamp of EVIDENCE.md is release metadata to the carry when the policy declares it in + // stamp_files, and product when it does not (GST-1030 point 2, GGST-1); fails_when=the carry counts the declared + // file's stamp as a product change (RECEIPT_CARRY_PRODUCT_CHANGED), or lets an undeclared file's version edit ride; + // seam=bin/gstack-ship-receipt productSpec, lib/ship-receipt.ts stampOnlyText + test.each([['declared in stamp_files', true], ['undeclared', false]])('the carry and a version-only restamp of EVIDENCE.md: %s (GST-1030)', (_, declared) => { + const base = policy(true); + const pol = declared ? { ...base, stamp_files: [...base.stamp_files, 'EVIDENCE.md'] } : base; + const { repo, body } = receiptThenStamp(`restamp-carry-evidence-${declared}-`, pol, r => write(r, 'EVIDENCE.md', '# Evidence\n\nVerified on v1.2.4.0 with the free suite\n')); + if (!declared) write(repo, 'EVIDENCE.md', '# Evidence\n\nVerified on v1.2.5.0 with the free suite\n'); + expect(readFile(repo, 'EVIDENCE.md')).toBe('# Evidence\n\nVerified on v1.2.5.0 with the free suite\n'); + repo.git('commit', '-qam', 'stamp 1.2.5.0'); + const carried = carry(repo, body); + if (declared) expect(carried.out, carried.err).toContain('CARRY: carried'); + else expect([carried.status, carried.err.includes('RECEIPT_CARRY_PRODUCT_CHANGED')], carried.err).toEqual([EXIT.refused, true]); + }); +}); + diff --git a/test/review-exclude.test.ts b/test/review-exclude.test.ts index aefa18c08..29e750d72 100644 --- a/test/review-exclude.test.ts +++ b/test/review-exclude.test.ts @@ -11,7 +11,7 @@ import { EXIT } from '../lib/headless-artifacts'; import { KEEP_ARG_MAX_BYTES, readSelection, reviewExclude, tooLargeLine, unreadableLine } from '../lib/review-exclude'; import { collectCommit, collectLive } from '../lib/review-collect'; import { extractReceipt } from '../lib/ship-receipt'; -import { bindShippedReceipt, receiptReviewAtHead } from '../lib/ship-receipt-depth'; +import { bindShippedReceipt, excludeMismatch, receiptReviewAtHead } from '../lib/ship-receipt-depth'; import { REVIEW_EXCLUDE_KEY_VERSION, isUnavailable } from '../lib/ship-policy'; import { loadPolicyAt } from '../lib/ship-policy-at'; import { cleanup, commit, makeFixtureRepo, runBin, write, type FixtureRepo } from './helpers/pregate-fixture'; @@ -313,6 +313,31 @@ describe('the receipt binds the selection the reviewers read', () => { expect(removed.status).toBe(EXIT.fail); expect(removed.err).toContain(`field=review_exclude expected=none recorded=${digest}`); }, 240_000); + + // Value: protects=every place that describes write --review-exclude says what it checks: the digest HEAD has under the base + // policy, not that a review read that selection; fails_when=the help, troubleshooting, ship-policy or the queue's refusal + // line claims a receipt write alone cannot clear field=review_exclude while a write with the refusal's own expected= digest + // clears it with no review run; why_new=GST-972 item 2 (the #3852 cross-family trail review); seam=gstack-ship-receipt write + // and --help, excludeMismatch, the two docs; no_claim=binding the digest to the review ledger (write never reads it) + test('write binds --review-exclude to the base policy, not to a review, and no description claims more', () => { + const repo = census(); + const body = path.join(repo.root, 'body.md'); + const refused = writeReceipt(repo, body, []); + const expected = refused.err.match(/field=review_exclude expected=([0-9a-f]{64}) /)?.[1]; + expect(expected).toBeDefined(); + expect(writeReceipt(repo, body, ['--review-exclude', expected!, ...targetedAtHead(repo)]).status).toBe(EXIT.ok); + const doc = (rel: string) => fs.readFileSync(path.join(import.meta.dir, '..', rel), 'utf8').replace(/\s+/g, ' '); + const claims = { + help: runBin(repo, 'gstack-ship-receipt', ['--help']).out.replace(/\s+/g, ' '), + troubleshooting: doc('docs/troubleshooting.md'), + shipPolicy: doc('docs/ship-policy.md'), + queue: excludeMismatch({ state: 'none' }, { review_exclude: { digest: expected } } as any, repo.git('rev-parse', 'HEAD'), 'origin/main')!, + }; + for (const [where, text] of Object.entries(claims)) { + expect({ where, overClaim: /write alone (cannot|does not) clear|never a receipt write alone/i.test(text) }).toEqual({ where, overClaim: false }); + expect({ where, says: text.includes('does not read the review ledger') }).toEqual({ where, says: true }); + } + }, 240_000); }); describe('a branch cannot hide code under a base pattern', () => { @@ -348,6 +373,84 @@ describe('a branch cannot hide code under a base pattern', () => { expect(p).toMatchObject({ depth: 'full', errors: expect.arrayContaining([expect.objectContaining({ code: 'binary_code_path', input: 'assets/loader.js' })]) }); }, 120_000); + // Value: protects=a compiled binary under a base pattern stays in review, because it is code; fails_when=a .node, .so, .so.N, + // .dylib or .dll file, or an ELF, Mach-O or PE image under a content extension (.bin, .dat), is excluded as binary content; + // why_new=GST-972 item 3; seam=bin/gstack-review-plan; no_claim=what a reader can see inside a binary + test('native binaries are code: .node, .so, .dylib, .dll and executable images under a content extension stay kept', () => { + const repo = makeFixtureRepo({ prefix: 'gstack-exclude-', baseFiles: { '.gstack/ship-policy.json': policyText(['assets/**']) } }); repos.push(repo); + fs.mkdirSync(path.join(repo.work, 'assets'), { recursive: true }); + const elf = Buffer.concat([Buffer.from([0x7f, 0x45, 0x4c, 0x46, 2, 1, 1, 0]), Buffer.alloc(24)]); + const macho = Buffer.concat([Buffer.from([0xcf, 0xfa, 0xed, 0xfe]), Buffer.alloc(28)]); + const pe = Buffer.concat([Buffer.from('MZ'), Buffer.alloc(30)]); + const files = { 'assets/addon.node': elf, 'assets/libx.dylib': macho, 'assets/libx.so': elf, 'assets/libx.so.1': elf, 'assets/setup.dat': pe, 'assets/tool.bin': elf, 'assets/x.dll': pe, 'assets/real.gz': GZ }; + for (const [rel, bytes] of Object.entries(files)) fs.writeFileSync(path.join(repo.work, rel), bytes); + commit(repo, {}); + const p = JSON.parse(plan(repo, ['--json']).out); + expect(p.review_exclude.kept_files).toEqual(Object.keys(files).filter(f => f !== 'assets/real.gz').map(f => ({ path: f, reason: 'code' }))); + expect(p.review_exclude).toMatchObject({ excluded: 1, content: { binary: 1 } }); + }, 120_000); + + // Value: protects=a branch's .gitattributes cannot get an executable image excluded by making git read it as text, and a + // 64-bit universal Mach-O is an executable image; fails_when=only files git reports as binary under the branch's own + // attributes get the magic check, so an ELF named .png under '*.png diff -binary' is excluded as data, or 0xcafebabf + // is not executable magic; why_new=GST-972 item 3 follow-up (#3910, #3902 outside voice); seam=bin/gstack-review-plan and + // reviewExclude in head mode; no_claim=a NUL-free script under a content extension + test('a branch .gitattributes that makes git read an ELF as text cannot get it excluded; fat64 Mach-O is native', () => { + const repo = makeFixtureRepo({ prefix: 'gstack-exclude-', baseFiles: { '.gstack/ship-policy.json': policyText(['assets/**']) } }); repos.push(repo); + fs.mkdirSync(path.join(repo.work, 'assets'), { recursive: true }); + fs.writeFileSync(path.join(repo.work, 'assets/native.png'), Buffer.concat([Buffer.from([0x7f, 0x45, 0x4c, 0x46, 2, 1, 1, 0]), Buffer.alloc(24)])); + fs.writeFileSync(path.join(repo.work, 'assets/fat64.dat'), Buffer.concat([Buffer.from([0xca, 0xfe, 0xba, 0xbf]), Buffer.alloc(28)])); + fs.writeFileSync(path.join(repo.work, 'assets/real.gz'), GZ); + commit(repo, { '.gitattributes': '*.png diff -binary\n' }); + const p = JSON.parse(plan(repo, ['--json']).out); + const kept = [{ path: 'assets/fat64.dat', reason: 'code' as const }, { path: 'assets/native.png', reason: 'code' as const }]; + expect(p.review_exclude.kept_files).toEqual(kept); + expect(p.review_exclude).toMatchObject({ excluded: 1, content: { binary: 1 } }); + const head = repo.git('rev-parse', 'HEAD'); + const baseSha = repo.git('rev-parse', 'origin/main'); + const c = collectCommit(repo.work, baseSha, head); + const x = reviewExclude({ root: repo.work, baseSha, ref: 'origin/main', mergeBase: c.mergeBase!, head, perPath: c.perPath, added: c.added, classRoot: c.root ?? [], gateTagged: new Set(), context: 'test' }); + expect(x.state === 'active' && x.kept).toEqual(kept); + expect(x.state === 'active' && x.record.digest).toBe(p.review_exclude.digest); + }, 120_000); + + // Value: protects=a NUL byte cannot turn code or a key file into excludable content; fails_when=a NUL-bearing file with a + // source extension outside the old CODE_EXT list (.gradle, .sql, .tf, .groovy), a key store (.p12) or an extensionless key + // is excluded as binary; why_new=GST-972 item 6 (#3852 targeted gate reader at 61803d9); seam=bin/gstack-review-plan; + // no_claim=the list of binary content extensions is complete (an unknown one is kept, never excluded) + test('a NUL-bearing file is binary content only under a known content extension; code and key files stay kept', () => { + const repo = makeFixtureRepo({ prefix: 'gstack-exclude-', baseFiles: { '.gstack/ship-policy.json': policyText(['assets/**']) } }); repos.push(repo); + fs.mkdirSync(path.join(repo.work, 'assets'), { recursive: true }); + const nul = (code: string) => Buffer.from(`/* \0 */ ${code}\n`); + const kept = { 'assets/Build.groovy': nul('println "x"'), 'assets/build.gradle': nul('apply plugin: "java"'), 'assets/id_ed25519': nul('openssh-key-v1'), 'assets/main.tf': nul('resource "x" "y" {}'), 'assets/query.sql': nul('DROP TABLE users;'), 'assets/server.p12': Buffer.from([0x30, 0x82, 0, 1, 2, 3]) }; + const content = { 'assets/report.pdf': GZ, 'assets/shot.png': GZ }; + for (const [rel, bytes] of Object.entries({ ...kept, ...content })) fs.writeFileSync(path.join(repo.work, rel), bytes); + commit(repo, {}); + const p = JSON.parse(plan(repo, ['--json']).out); + expect(p.review_exclude.kept_files).toEqual(Object.keys(kept).map(f => ({ path: f, reason: 'code' }))); + expect(p.review_exclude).toMatchObject({ excluded: 2, content: { binary: 2 } }); + }, 120_000); + + // Value: protects=a branch's .gitattributes cannot get a matched file with a credential line excluded by making git call it + // binary, at the plan (live tree) and at the receipt (head blobs); fails_when=the secret-line rule reads only the + // collector's added lines, which git leaves empty for an attribute-binary file, so the file is excluded; why_new=GST-972 + // item 5 (#3852 round 3, outside-voice#2); seam=bin/gstack-review-plan and reviewExclude in head mode; no_claim=pregate's gitleaks scan, what readers' own diffs show (they still follow the attributes) + test('a branch .gitattributes marking receipts binary cannot get a credential-line file excluded: the keep rule reads the text', () => { + const repo = makeFixtureRepo({ prefix: 'gstack-exclude-', baseFiles: { '.gstack/ship-policy.json': policyText(['receipts/**']) } }); repos.push(repo); + const secret = '{"cmd": "curl -H $OPENAI_API_KEY"}\n'; + commit(repo, { '.gitattributes': 'receipts/** binary\n', 'receipts/s.json': secret, 'receipts/a.b/c.json': secret, 'receipts/a.json': '{"ok": true}\n', 'receipts/a/t.json': '{"ok": true}\n' }); + const p = JSON.parse(plan(repo, ['--json']).out); + const kept = [{ path: 'receipts/a.b/c.json', reason: 'secret-line' as const }, { path: 'receipts/s.json', reason: 'secret-line' as const }]; + expect(p.review_exclude.kept_files).toEqual(kept); + expect(p.review_exclude).toMatchObject({ excluded: 2, content: { data: 2 } }); + const head = repo.git('rev-parse', 'HEAD'); + const baseSha = repo.git('rev-parse', 'origin/main'); + const c = collectCommit(repo.work, baseSha, head); + const x = reviewExclude({ root: repo.work, baseSha, ref: 'origin/main', mergeBase: c.mergeBase!, head, perPath: c.perPath, added: c.added, classRoot: c.root ?? [], gateTagged: new Set(), context: 'test' }); + expect(x.state === 'active' && x.kept).toEqual(kept); + expect(x.state === 'active' && x.record.digest).toBe(p.review_exclude.digest); + }, 120_000); + test('an unavailable gate scan keeps every candidate instead of excluding it', () => { const repo = census(); const head = repo.git('rev-parse', 'HEAD'); diff --git a/test/review-log-findings.test.ts b/test/review-log-findings.test.ts index c463521c3..2732b09ab 100644 --- a/test/review-log-findings.test.ts +++ b/test/review-log-findings.test.ts @@ -191,6 +191,23 @@ describe('lib: deriveFromFindings, bindReview order and reviewFreshness', () => expect(reviewFreshness(bound, 'd'.repeat(64))?.status).toBe('STALE'); }); + // Value: protects=a fix-check row is written with the fields review-read verifies (GEVA-218: the row lacked them, so it read + // UNVERIFIED and finish could not count it); fails_when=the logger accepts a review-fix-check row without completed and + // converged, or consumes the start token while refusing it; seam=bindReview + test('a review-fix-check row without completed and converged is refused and keeps its start token (GST-1130)', () => { + const dir = path.join(home, 'starts', '.review-starts'); + fs.mkdirSync(dir, { recursive: true }); + const token = '77777777-2222-3333-4444-555555555555'; + const w = 'e'.repeat(64); + fs.writeFileSync(path.join(dir, `${token}.json`), JSON.stringify({ skill: 'review-fix-check', repo: '/r', branch: 'b', wtree: w, started_at: 't' })); + const env = { GSTACK_REVIEW_DIR: path.join(home, 'starts'), GSTACK_REVIEW_REPO: '/r', GSTACK_REVIEW_BRANCH: 'b', GSTACK_STAMP_WTREE: w }; + for (const row of [{}, { completed: true }, { converged: true }, { completed: 'yes', converged: true }]) { + expect(() => bindReview({ skill: 'review-fix-check', status: 'clean', ...row }, token, env)).toThrow('review-log: a review-fix-check row must carry completed and converged as true or false'); + } + expect(fs.existsSync(path.join(dir, `${token}.json`))).toBe(true); + expect(bindReview({ skill: 'review-fix-check', status: 'clean', completed: true, converged: true }, token, env).review_binding.state).toBe('verified'); + }); + test('reviewFreshness reads findings_open: resolved criticals are CURRENT, claimed issues_found stays conservative', () => { const w = 'b'.repeat(64); const base = { skill: 'review', completed: true, converged: true, wtree: w, review_binding: { state: 'verified', start_wtree: w, end_wtree: w } }; diff --git a/test/review-packets-removed.test.ts b/test/review-packets-removed.test.ts index 8275412cc..c1cf4fc10 100644 --- a/test/review-packets-removed.test.ts +++ b/test/review-packets-removed.test.ts @@ -74,6 +74,7 @@ const NO_LOSS_REPLAYS = { 'GST-967': "GST-967 dispatch-superset no-loss check (test/review-depth.test.ts, 'GST-967 dispatch-superset no-loss check'): over 1,440 planReview cases the branch plans exactly what main planned at 1022f4f except the 71 gate-class stops before Step 11, which add only Step 11's adversarial, outside and targeted voices, so no reviewer main dispatches is dropped or re-efforted; the brief bodies lib/review-brief.ts cuts from the ship sections hash to main's; only the lead's repeat-decision and Step 11 instructions changed; reviewer text is unchanged", 'GST-508+GST-464+GST-967': "wave review-53cfb5bfe011 merge of main 68410828f into #3852: ship/sections/review-army.md and adversarial.md are main's GST-967 bytes plus exactly the GST-508+GST-464 lines (the merged file minus main equals the wave minus fc292e22f, line for line), so GST-967's dispatch-superset no-loss check and GST-508's no-loss replay both hold; merged-head checks in GStack Minder drive handoffs/GSTA-202/merge-6841-checks.md", 'GST-508+R51+GST-911+GST-967': "wave review-53cfb5bfe011 merge of main 68410828f into #3852: lib/review-depth.ts is main's GST-967 bytes plus exactly the GST-508+R51+GST-911 lines (the merged file minus main equals the wave minus fc292e22f, line for line); GST-967's dispatch-superset no-loss check (test/review-depth.test.ts) passes on the merged file, and the GST-508+R51+GST-911 checks are rerun at the merged head (GStack Minder drive handoffs/GSTA-202/merge-6841-checks.md)", + 'GST-940': "GST-940 verdict no-loss check: svgDrawing reads a style element's opening tag as quoted pairs with ASCII whitespace, walks its body over the raw text, refuses any & in it, reads comments as CSS and refuses a non-ASCII element or attribute name; on 62,805 unique SVG files (21,658 with a style element) it gives the same verdict as main at 4bbad2422 on every file, and the planner over the last 100 first-parent main commits to 4bbad2422 changes no commit's depth, class or risk classes; the only verdicts it changes are the GST-940 cases in test/review-planner-misreads.test.ts; rerun on the merge with main at 8f08cb76a (GST-967 merged), with the same results; rerun on the merge with main at 5f705f367 (#3852 and #3864 merged), with the same results", } as const; type Replay = keyof typeof NO_LOSS_REPLAYS; const REVIEWER_BRIEFS: Record = { @@ -95,7 +96,7 @@ const REVIEWER_BRIEFS: Record = { 'review/specialists/testing.md': { sha: 'd6cc2bef7be562387d0c0a22fefe646dff9c295f1d34d8453a6cb496584150e4', replay: '#3638' }, 'review/specialists/targeted.md': { sha: '7f92a0d0a8319c3c0e076fb3b1a4689c45120b2be2c14dfc7d4cb03aafa9d6a5', replay: 'GST-257' }, 'scripts/resolvers/fix-check.ts': { sha: '712bf5b0c439e96bac32f19cbe25394dabb4c94aa66600100bfde9b100e15a08', replay: '#3638' }, - 'lib/review-depth.ts': { sha: 'd76a9afcfadfe0a95b15428c04e6f7a88c6b0fa75591201c45f023fec2bb90db', replay: 'GST-508+R51+GST-911+GST-967' }, + 'lib/review-depth.ts': { sha: '25050a7a61f81df649d1df0d0237dbf961671356395317c6a527741f5bb56877', replay: 'GST-940' }, 'lib/review-risk.ts': { sha: '08464b78a723ee463ca3a28d0abbc4eb8f4f512192061c703743707b66b5a311', replay: 'GST-942' }, }; diff --git a/test/review-planner-misreads.test.ts b/test/review-planner-misreads.test.ts index 0c34036a0..2bbaea298 100644 --- a/test/review-planner-misreads.test.ts +++ b/test/review-planner-misreads.test.ts @@ -125,7 +125,9 @@ describe('GST-911 misread 1: a chart SVG is data, an SVG that can run code is se // Value: protects=only an SVG made of drawing elements is data: anything that loads, links or declares outside that set stays code; // fails_when=an external image or stylesheet, an entity declaration, an external DTD, an animation or a CSS url() to another document reads as docs, // including through a CSS escape or a zero-padded character reference (review pass 1 findings) or behind a stray quote that - // pairs with a later one (review pass 2 finding), or any CSS beyond declarations and url(#id) (Step 11 finding: image-set()); + // pairs with a later one (review pass 2 finding), or any CSS beyond declarations and url(#id) (Step 11 finding: image-set()), + // including CSS behind a character-reference or comment, an HTML-only comment end, a quoted > on the style tag, a + // non-ASCII space before > in , or a tag or attribute under a non-ASCII namespace prefix (GST-940); // seam=collectCommit + depthOfCommitInput test.each([ ['an external image', ' '], @@ -153,11 +155,29 @@ describe('GST-911 misread 1: a chart SVG is data, an SVG that can run code is se ['a style attribute cross-fade()', ' '], ['a style element whose hides in a comment', ' -->image-set("https://x.test/p.png" 1x)}'], ['a style element whose hides in CDATA', ' */ svg{fill:image-set("https://x.test/p.png" 1x)} ]]>'], + ['a character-reference in a style element', ' '], + ['a hex character-reference in a style element', ' '], + ['a character-reference comment in a style element', ' '], + ['CSS after an HTML empty comment in a style element', ' '], + ['CSS after an HTML --!> comment end in a style element', ' '], + ['a inside a quoted style attribute', ' '], + ['an escaped ampersand in a style element', ' '], + ['a no-break space before > in ', ' '], + ['a vertical tab before > in ', ' '], + ['a script under a non-ASCII prefix', ' fetch(1)'], + ['a style under a non-ASCII prefix', ' rect{fill:image-set("https://x.test/p.png" 1x)}'], + ['an external href under a non-ASCII prefix', ' '], ])('%s keeps the SVG code', (_name, line) => { const fx = fixture(); expect(plan(fx, fx.commit({ 'results/chart.svg': chart(5, line) })).class).toBe('code'); }, 60_000); + // Value: protects=the GST-940 style walker still reads a plain chart style as a drawing: a comment, CDATA and a quoted attribute + // without markup; fails_when=the walker over-refuses them, so chart SVGs lose the light review; seam=svgDrawing + test('a style comment, CDATA and quoted attribute keep a chart a drawing (GST-940)', () => { + expect(svgDrawing(chart(5, ' '))).toBe(true); + }); + // Value: protects=a passive edit to an SVG that already carries a script is still code, because the whole file decides, not the // changed lines; and an SVG code names stays code like any consumed data file; fails_when=either plans as docs; seam=collectCommit test('an edit inside a scripted SVG, and an SVG code names, stay code', () => { diff --git a/test/ship-receipt-carry-squash.test.ts b/test/ship-receipt-carry-squash.test.ts index 2c763e7b3..a47e025c1 100644 --- a/test/ship-receipt-carry-squash.test.ts +++ b/test/ship-receipt-carry-squash.test.ts @@ -8,7 +8,7 @@ import { afterEach, describe, expect, test } from 'bun:test'; import * as fs from 'node:fs'; import * as path from 'node:path'; import { EXIT } from '../lib/headless-artifacts'; -import { extractReceipt, renderReceiptBlock } from '../lib/ship-receipt'; +import { extractReceipt, landedStackParent, renderReceiptBlock } from '../lib/ship-receipt'; import { bindShippedReceipt } from '../lib/ship-receipt-depth'; import { cleanup, commit, makeFixtureRepo, runBin, write, type FixtureRepo } from './helpers/pregate-fixture'; @@ -21,7 +21,7 @@ const SUB = "exports.add = (a, b) => a + b;\nexports.sub = (a, b) => a - b;\n"; const RETRY4 = "exports.MAX_RETRIES = 4;\nexports.TIMEOUT_MS = 5000;\nexports.retry = (fn) => fn();\n"; /** main <- parent (lib/util.js) <- feature (lib/retry.js, its /ship receipt written against the parent branch). */ -function stack(repo: FixtureRepo, parentFiles: Record = { 'lib/util.js': SUB }, childFiles: Record = { 'lib/retry.js': RETRY4 }): { parent: string; child: string; body: string } { +function stack(repo: FixtureRepo, parentFiles: Record = { 'lib/util.js': SUB }, childFiles: Record = { 'lib/retry.js': RETRY4 }, base = 'parent'): { parent: string; child: string; body: string } { repo.git('checkout', '-q', '-b', 'parent'); const parent = commit(repo, parentFiles, 'parent: sub'); repo.git('push', '-q', 'origin', 'parent'); @@ -29,7 +29,7 @@ function stack(repo: FixtureRepo, parentFiles: Record = { 'lib/u repo.git('reset', '-q', '--hard', parent); const child = commit(repo, childFiles, 'child: four retries'); const body = path.join(repo.root, 'body.md'); - const w = runBin(repo, 'gstack-ship-receipt', ['write', '--pr', '8', '--base', 'parent', '--version', '1.0.0', '--session-kind', 'unattended', '--body', body]); + const w = runBin(repo, 'gstack-ship-receipt', ['write', '--pr', '8', '--base', base, '--version', '1.0.0', '--session-kind', 'unattended', '--body', body]); expect(w.status).toBe(EXIT.ok); return { parent, child, body }; } @@ -166,3 +166,77 @@ describe('carry after a stack parent squash-merges (#3570)', () => { expect(receiptIn(body).head).toBe(child); }, 90_000); }); + +const CONFIG = ['a = 1', 'b = 2', 'c = 3', 'd = 4', 'e = 5', 'f = 6', 'g = 7', 'h = 8', ''].join('\n'); +const configWith = (edits: Record) => CONFIG.split('\n').map(l => edits[l] ?? l).join('\n'); + +describe('carry after a stack parent squash-merges, receipt written against main (GST-1199)', () => { + // Value: protects=a child PR stacked on its parent but targeting main keeps its receipt once the parent squash-merges and + // main is merged in (gstack-evals#178 faced a 45-60M-token gate re-review for it); fails_when=carry compares the child's + // whole range from the old fork point, parent hunks included, with HEAD's diff against a base that now holds them; + // seam=none; no_claim=gh pr edit (--body stands in) + test('the child carries when its range from the landed parent is HEAD\'s whole product diff against main', () => { + const repo = makeFixtureRepo(); repos.push(repo); + const { parent, child, body } = stack(repo, undefined, undefined, 'main'); + landOnMain(repo, { 'lib/util.js': SUB }); + const head = repo.git('rev-parse', 'HEAD'); + const out = carry(repo, body); + expect(out.err).not.toContain('RECEIPT_CARRY_PRODUCT_CHANGED'); + expect(out.status).toBe(EXIT.ok); + expect(out.out).toContain(`CARRY: carried from ${child.slice(0, 12)} to ${head.slice(0, 12)}`); + const r = receiptIn(body); + expect(r.head).toBe(head); + expect(r.base).toBe('main'); + expect(r.carry!.reason).toBe(`stack parent ${parent.slice(0, 12)} landed + base merge`); + expect(bindShippedReceipt(repo.work, 'origin/main', fs.readFileSync(body, 'utf8'), '8', [head]).ok).toBe(true); + }, 90_000); + + // Value: protects=fail closed: a product edit after the receipt, or a parent that never landed and whose change the + // child's merge dropped, takes a fresh /ship; fails_when=any reviewed ancestor counts as a landed parent, or the edit + // hides behind the shorter range; seam=none; no_claim=- + test('a product edit after the receipt, or a dropped parent that never landed, refuses with RECEIPT_CARRY_PRODUCT_CHANGED', () => { + const repo = makeFixtureRepo(); repos.push(repo); + const { child, body } = stack(repo, undefined, undefined, 'main'); + landOnMain(repo, { 'lib/util.js': SUB }); + commit(repo, { 'lib/retry.js': RETRY4.replace('4', '5') }, 'child: five'); + const edited = carry(repo, body); + expect(edited.status).toBe(EXIT.refused); + expect(edited.out).toContain('CARRY: refused (RECEIPT_CARRY_PRODUCT_CHANGED)'); + expect(receiptIn(body).head).toBe(child); + + const other = makeFixtureRepo(); repos.push(other); + const s = stack(other, undefined, undefined, 'main'); + landOnMain(other, {}); + commit(other, { 'lib/util.js': other.git('show', 'origin/main:lib/util.js') + '\n' }, 'drop the parent change'); + const dropped = carry(other, s.body); + expect(dropped.status).toBe(EXIT.refused); + expect(dropped.out).toContain('CARRY: refused (RECEIPT_CARRY_PRODUCT_CHANGED)'); + expect(receiptIn(s.body).head).toBe(s.child); + }, 120_000); + + // Value: protects=a base change next to the child's reviewed hunks gets one fix-check reviewer, not a carry and not a full + // /ship; fails_when=the landed-parent comparison ignores base context or refuses it outright; seam=none; no_claim=- + test('a base change next to the child\'s reviewed hunks routes to RECEIPT_CARRY_SCOPED_REVIEW', () => { + const repo = makeFixtureRepo({ baseFiles: { 'lib/config.js': CONFIG } }); repos.push(repo); + const { child, body } = stack(repo, undefined, { 'lib/config.js': configWith({ 'd = 4': 'd = 40' }) }, 'main'); + landOnMain(repo, { 'lib/util.js': SUB, 'lib/config.js': configWith({ 'f = 6': 'f = 60' }) }); + const out = carry(repo, body); + expect(out.status).toBe(EXIT.refused); + expect(out.out).toContain('CARRY: refused (RECEIPT_CARRY_SCOPED_REVIEW)'); + expect(out.err).toContain('lib/config.js'); + expect(receiptIn(body).head).toBe(child); + }, 90_000); + + // Value: protects=the receipt never names a stack parent that does not exist; fails_when=a test-first commit with no + // product change counts as landed (any base holds it), so the reason claims a parent and the test range shrinks; seam=none; + // no_claim=- + test('a test-only commit in a branch that was never stacked is no landed parent', () => { + const repo = makeFixtureRepo({ baseFiles: { 'lib/config.js': CONFIG } }); repos.push(repo); + const fork = repo.git('rev-parse', 'HEAD'); + commit(repo, { 'test/config.test.js': "const test = require('node:test');\ntest('d', () => {});\n" }, 'test first'); + const reviewed = commit(repo, { 'lib/config.js': configWith({ 'd = 4': 'd = 40' }) }, 'fix d'); + repo.git('checkout', '-q', 'main'); + const mainTip = commit(repo, { 'docs/README.md': '# main moves on\n' }, 'main moves on'); + expect(landedStackParent(repo.work, { rangeBase: fork, reviewed, currentBase: mainTip }, ['.', ':(exclude,glob)test/**'])).toBeNull(); + }, 60_000); +}); diff --git a/test/ship-rereview-triggers.test.ts b/test/ship-rereview-triggers.test.ts index 63bc87850..3854c7808 100644 --- a/test/ship-rereview-triggers.test.ts +++ b/test/ship-rereview-triggers.test.ts @@ -9,6 +9,7 @@ import * as fs from 'node:fs'; import * as path from 'node:path'; import { EXIT } from '../lib/headless-artifacts'; import { comparePath } from '../lib/ship-patch'; +import { STAMP_PLACEHOLDERS_KEY_VERSION } from '../lib/ship-policy'; import { extractReceipt, placeReceiptFirst } from '../lib/ship-receipt'; import { cleanup, makeFixtureRepo, runBin, write, type FixtureRepo } from './helpers/pregate-fixture'; import { lineOf, PREFLIGHT, RELEASE, shipFixture, type ShipFx } from './helpers/ship-bin-fixture'; @@ -266,6 +267,37 @@ describe('a restamp after a lost slot (GST-895, GBRA-218, gbrain #8134)', () => const base = kept([['0.9.0.0', 'old'], ['1.0.1.0', 'base'], ['1.0.0.0', 'mine']]); expect([base.status, lineOf(base.stdout, 'STOP')]).toEqual([3, behaviorStop]); }); + + // Value: protects=gbrain's restamp of the PR's own `since: 'TBD'` row to the new version routes as metadata when the + // policy declares TBD in stamp_placeholders (GST-1026, gbrain #8444); an undeclared placeholder, a base row moved the + // same way, a duplicated base copy, or a line no stamp_pattern matches still stops; fails_when=stampedFiles reads the + // move as behavior (gbrain #8444) or accepts it off the branch's own rows; seam=lib/ship-bin-common.ts stampedFiles + test('a declared placeholder on the branch row is metadata; undeclared, a base row or no template stops (GST-1026)', () => { + const declared = (placeholders: string[] | undefined, templates = ["since: '{version}'"]) => JSON.stringify({ stamp_files: ['VERSION', 'src/notice.ts'], stamp_patterns: templates, + ...(placeholders ? { stamp_placeholders: placeholders, min_gstack_version: STAMP_PLACEHOLDERS_KEY_VERSION } : {}) }); + const restamped = (pol: string, baseRows: [string, string][], ownRows: [string, string][], rows: [string, string][]) => { + const f = fx({ '.gstack/ship-policy.json': pol, VERSION: '1.0.0.0\n', 'src/notice.ts': notice(...baseRows) }, + { VERSION: '1.0.1.0\n', 'src/notice.ts': notice(...baseRows, ...ownRows) }); + const reviewed = wtree(f); + const post = snapshot(f); + f.write({ VERSION: '1.0.2.0\n', 'src/notice.ts': notice(...rows) }); f.commit('restamp: TBD rows to 1.0.2.0'); + return finish(f, reviewed, post, ['--reviewed-base', mergeBase(f)]); + }; + const base: [string, string][] = [['0.9.0.0', 'old'], ['TBD', 'base']]; + const metadata = / behavior=0 docs=0 metadata=2 tests=0 /; + const own = restamped(declared(['TBD']), base, [['TBD', 'mine']], [...base, ['1.0.2.0', 'mine']]); + expect(lineOf(own.stdout, 'ROUTE'), own.stdout).toEqual(expect.stringMatching(metadata)); + const undeclared = restamped(declared(undefined), base, [['TBD', 'mine']], [...base, ['1.0.2.0', 'mine']]); + expect([undeclared.status, lineOf(undeclared.stdout, 'STOP')]).toEqual([3, behaviorStop]); + const other = restamped(declared(['WIP']), base, [['TBD', 'mine']], [...base, ['1.0.2.0', 'mine']]); + expect([other.status, lineOf(other.stdout, 'STOP')]).toEqual([3, behaviorStop]); + const baseRow = restamped(declared(['TBD']), base, [['TBD', 'mine']], [['0.9.0.0', 'old'], ['1.0.2.0', 'base'], ['1.0.2.0', 'mine']]); + expect([baseRow.status, lineOf(baseRow.stdout, 'STOP')]).toEqual([3, behaviorStop]); + const dup = restamped(declared(['TBD']), base, [['TBD', 'base']], [['0.9.0.0', 'old'], ['1.0.2.0', 'base'], ['TBD', 'base']]); + expect([dup.status, lineOf(dup.stdout, 'STOP')]).toEqual([3, behaviorStop]); + const noTemplate = restamped(declared(['TBD'], ['version: {version}']), base, [['TBD', 'mine']], [...base, ['1.0.2.0', 'mine']]); + expect([noTemplate.status, lineOf(noTemplate.stdout, 'STOP')]).toEqual([3, behaviorStop]); + }); }); describe('adjacent base edits (GST-384, #3631, founderstar#441)', () => { @@ -757,3 +789,83 @@ describe('finish with an owner-accepted edit after a fix-check (GST-965, GBRA-27 }); }); + +// Value: protects=a granted fix-check's clean read clears the paths it reviewed at Step 16, a gate-edited test included, so +// finish does not send them back through Steps 5–11.5 (GEVA-218, gstack-evals#168); fails_when=finish routes a path inside +// the fix-check's range as behavior, counts a path edited after the fix-check, or counts a row that is not a completed clean +// fix-check with a range base; seam=finish's route reading the review log; no_claim=Step 11.5's binding +describe('finish after a completed fix-check (GST-1130, GEVA-218)', () => { + const TEST_V1 = "import { expect, test } from 'bun:test';\ntest('a', () => { expect(2).toBe(2); });\n"; + const TEST_V2 = "import { expect, test } from 'bun:test';\ntest('a', () => { expect(2).toBe(2); }, 1_000);\ntest('b finishes fast', () => { expect(1 + 1).toBe(2); }, 1_000);\n"; + const logFixCheck = (f: ShipFx, row: Record) => { + const start = f.run(path.join(BIN, 'gstack-review-log'), ['--start', 'review-fix-check']); + expect(start.status).toBe(0); + return f.run(path.join(BIN, 'gstack-review-log'), [JSON.stringify({ skill: 'review-fix-check', timestamp: '2026-11-02T00:00:00Z', via: 'ship', review_scope: 'frozen', status: 'clean', issues_found: 0, critical: 0, ...row }), '--finish', start.stdout.trim()]); + }; + const grant = (f: ShipFx, stopHead: string) => { + const a = f.run(path.join(BIN, 'gstack-approvals'), ['add', '--ask', `Owner grant: one scoped fix-check after round 2 (review_loop_grant ${stopHead.slice(0, 12)})`, '--words', 'one scoped fix-check after round 2', '--by', 'GStack Minder', '--scope', 'this /ship invocation', '--json'], { GSTACK_APPROVALS_MIRROR: '0' }); + expect(a.status).toBe(0); + return JSON.parse(a.stdout).added[0] as string; + }; + const afterFixCheck = () => { + const f = fx({ 'src/x.ts': 'export const x = 1;\n' }, { 'src/x.ts': 'export const x = 2;\n', 'test/x.test.ts': TEST_V1 }); + const passHead = f.git('rev-parse', 'HEAD').stdout.trim(); + const reviewed = wtree(f); + f.write({ 'src/x.ts': 'export const x = 3;\n', 'test/x.test.ts': TEST_V2 }); f.commit('fix the round-2 blocker and bound its case'); + return { f, passHead, fixHead: f.git('rev-parse', 'HEAD').stdout.trim(), reviewed, post: snapshot(f) }; + }; + + test('paths inside a completed clean fix-check range count as reviewed, a gate-edited test included; a later edit still stops', () => { + const { f, passHead, fixHead, reviewed, post } = afterFixCheck(); + const plain = finish(f, reviewed, post); + expect([plain.status, plain.stdout.includes('STOP: behavior, tests or build inputs changed since review (src/x.ts,test/x.test.ts)')]).toEqual([3, true]); + const id = grant(f, passHead); + expect(logFixCheck(f, { completed: true, converged: true, range_base: passHead }).status).toBe(0); + const r = finish(f, reviewed, post); + expect(lineOf(r.stdout, 'FIX_CHECK')).toBe(`FIX_CHECK: reviewed src/x.ts,test/x.test.ts (fix-check at ${fixHead.slice(0, 12)}, range from ${passHead.slice(0, 12)}, grant ${id})`); + expect(lineOf(r.stdout, 'ROUTE')).toMatch(/ behavior=0 docs=0 metadata=0 tests=0 base=unknown context=0 fix_checked=2$/); + expect([r.status, lineOf(r.stdout, 'EVIDENCE')]).toEqual([0, expect.stringMatching(/^EVIDENCE: FRESH label=tests /)]); + f.write({ 'src/x.ts': 'export const x = 4;\n' }); f.commit('an edit after the fix-check'); + const late = finish(f, reviewed, post); + expect(lineOf(late.stdout, 'FIX_CHECK')).toStartWith('FIX_CHECK: reviewed test/x.test.ts ('); + expect([late.status, late.stdout.includes('STOP: behavior, tests or build inputs changed since review (src/x.ts)')]).toEqual([3, true]); + }); + + test('a clean fix-check row with no recorded review_loop_grant for a stop head in its range is not counted', () => { + const { f, passHead, reviewed, post } = afterFixCheck(); + expect(logFixCheck(f, { completed: true, converged: true, range_base: passHead }).status).toBe(0); + const ungranted = finish(f, reviewed, post); + expect(lineOf(ungranted.stdout, 'FIX_CHECK')).toBe('FIX_CHECK: not counted (no approved review_loop_grant names a stop head inside its range_base..commit)'); + expect([ungranted.status, ungranted.stdout.includes('STOP: behavior, tests or build inputs changed since review (src/x.ts,test/x.test.ts)')]).toEqual([3, true]); + grant(f, mergeBase(f)); + const elsewhere = finish(f, reviewed, post); + expect([elsewhere.status, lineOf(elsewhere.stdout, 'FIX_CHECK')]).toEqual([3, 'FIX_CHECK: not counted (no approved review_loop_grant names a stop head inside its range_base..commit)']); + }); + + test('a fix-check row that did not finish clean, or names no range base, is not counted', () => { + const { f, passHead, reviewed, post } = afterFixCheck(); + expect(logFixCheck(f, { completed: false, converged: false, range_base: passHead }).status).toBe(0); + const incomplete = finish(f, reviewed, post); + expect(lineOf(incomplete.stdout, 'FIX_CHECK')).toBe('FIX_CHECK: not counted (missing start capture or incomplete/nonconverged pass)'); + expect([incomplete.status, incomplete.stdout.includes('STOP: behavior, tests or build inputs changed since review (src/x.ts,test/x.test.ts)')]).toEqual([3, true]); + expect(logFixCheck(f, { completed: true, converged: true }).status).toBe(0); + const unranged = finish(f, reviewed, post); + expect(lineOf(unranged.stdout, 'FIX_CHECK')).toBe('FIX_CHECK: not counted (the row names no range_base commit on this branch)'); + expect(unranged.status).toBe(3); + const fork = mergeBase(f); + expect(logFixCheck(f, { completed: true, converged: true, range_base: fork }).status).toBe(0); + const early = finish(f, reviewed, post); + expect(lineOf(early.stdout, 'FIX_CHECK')).toBe(`FIX_CHECK: not counted (range_base ${fork.slice(0, 12)} does not hold the reviewed tree)`); + expect(early.status).toBe(3); + }); + + test('a fix-check logged on uncommitted content is not counted', () => { + const { f, passHead, reviewed, post } = afterFixCheck(); + f.write({ 'src/x.ts': 'export const x = 9;\n' }); + expect(logFixCheck(f, { completed: true, converged: true, range_base: passHead }).status).toBe(0); + f.git('checkout', '--', 'src/x.ts'); + const r = finish(f, reviewed, post); + expect(lineOf(r.stdout, 'FIX_CHECK')).toBe('FIX_CHECK: not counted (the row was logged on content no branch commit holds)'); + expect(r.status).toBe(3); + }); +}); diff --git a/test/stale-local-base.test.ts b/test/stale-local-base.test.ts new file mode 100644 index 000000000..5c2c749d6 --- /dev/null +++ b/test/stale-local-base.test.ts @@ -0,0 +1,112 @@ +/** + * A bare --base reads origin's tracking branch, not a stale local branch (GST-1036). Capy clones and linked + * worktrees never move the local base branch, so `--base main` there merged against an old local main: review and ship + * bins then counted upstream commits the branch only inherited (a docs diff planned at gate depth, banned terms and + * prose findings in lines the branch never wrote). Every such bin resolves the base through resolveBase + * (lib/gate-diff/git.ts) and names the ref it used. + */ +import { afterEach, describe, expect, test } from 'bun:test'; +import { spawnSync } from 'node:child_process'; +import * as fs from 'node:fs'; +import * as os from 'node:os'; +import * as path from 'node:path'; +import { BANNED_TERMS_SCHEMA } from '../lib/banned-terms'; +import { resolveCommit } from '../lib/pr-shots'; + +const BIN = path.join(import.meta.dir, '..', 'bin'); +const dirs: string[] = []; +afterEach(() => { while (dirs.length) fs.rmSync(dirs.pop()!, { recursive: true, force: true }); }); + +function fixture() { + const root = fs.realpathSync(fs.mkdtempSync(path.join(os.tmpdir(), 'stale-base-'))); + dirs.push(root); + const work = path.join(root, 'work'); + const env = { ...process.env, GSTACK_STATE_ROOT: path.join(root, 'state'), GIT_AUTHOR_NAME: 't', GIT_AUTHOR_EMAIL: 't@t', GIT_COMMITTER_NAME: 't', GIT_COMMITTER_EMAIL: 't@t', GIT_CONFIG_GLOBAL: path.join(root, 'gitconfig') }; + fs.writeFileSync(path.join(root, 'gitconfig'), '[commit]\n\tgpgsign = false\n'); + const sh = (cwd: string, c: string) => { const r = spawnSync('bash', ['-c', c], { cwd, env, encoding: 'utf8', timeout: 60_000 }); if (r.status !== 0) throw new Error(`${c}: ${r.stderr}`); return r.stdout.trim(); }; + sh(root, 'git init -q --bare -b main origin.git && git clone -q origin.git work 2>/dev/null'); + fs.mkdirSync(path.join(work, '.gstack')); + fs.writeFileSync(path.join(work, '.gstack', 'banned-terms.json'), JSON.stringify({ $schema: BANNED_TERMS_SCHEMA, terms: ['frobnicate'] }) + '\n'); + fs.writeFileSync(path.join(work, 'README.md'), '# t\n'); + sh(work, 'git add -A && git commit -qm base && git push -q origin main'); + sh(root, "git clone -q origin.git other && cd other && mkdir docs && printf 'Upstream notes frobnicate the parser.\\n\\nShowcasing vibrant features.\\n' > docs/up.md && git add -A && git commit -qm upstream && git push -q origin main"); + sh(work, "git fetch -q origin && git checkout -qb feat origin/main && mkdir src && echo 'export const v = 1;' > src/v.ts && git add -A && git commit -qm feat"); + const origin = sh(work, 'git rev-parse refs/remotes/origin/main'); + expect(sh(work, 'git rev-parse refs/heads/main')).not.toBe(origin); + const run = (bin: string, args: string[]) => { const r = spawnSync(path.join(BIN, bin), args, { cwd: work, env, encoding: 'utf8', timeout: 180_000 }); return { status: r.status, out: r.stdout ?? '', err: r.stderr ?? '' }; }; + return { root, work, sh, run, origin }; +} + +const NOTE = 'local main is behind origin/main; base is origin/main'; + +describe('a bare --base reads origin/ over a stale local branch (GST-1036)', () => { + // Value: protects=review-plan names the base ref its plan was computed against, so a stale-base plan shows in its output; + // fails_when=review-plan prints no REVIEW_BASE line, or names the local main while origin/main is ahead, or re-qualifies origin/main; + // why_new=GST-1036 (#3882 planned a docs diff at full depth against local main 319ef500); seam=bin/gstack-review-plan, lib/review-collect.ts collectLive + test('gstack-review-plan prints the ref it used and why', () => { + const { run, origin } = fixture(); + const bare = run('gstack-review-plan', ['--base', 'main']); + expect(bare.status, bare.out + bare.err).toBe(0); + expect(bare.out).toContain(`REVIEW_BASE: origin/main ${origin.slice(0, 12)}\n`); + expect(bare.err).toContain(`gstack-review-plan: ${NOTE}`); + const json = run('gstack-review-plan', ['--base', 'main', '--json']); + expect(JSON.parse(json.out)).toMatchObject({ paths: ['src/v.ts'], base: { ref: 'origin/main', sha: origin, local: 'behind' } }); + const qualified = run('gstack-review-plan', ['--base', 'origin/main']); + expect(qualified.out).toContain(`REVIEW_BASE: origin/main ${origin.slice(0, 12)}\n`); + expect(qualified.err).not.toContain('base is'); + }, 240_000); + + // Value: protects=banned-terms scans only the lines the branch adds; fails_when=it merges against the stale local main and reports a term an upstream commit added; + // why_new=GST-1036 sibling bin (/review passes a bare ); seam=bin/gstack-banned-terms + test('gstack-banned-terms check ignores inherited upstream lines', () => { + const { run } = fixture(); + for (const base of ['main', 'origin/main']) { + const r = run('gstack-banned-terms', ['check', '--base', base]); + expect(r.status, r.out + r.err).toBe(0); + expect(r.out).toContain('BANNED_TERMS: ok'); + expect(r.err).toContain(`gstack-banned-terms: ${NOTE}`); + } + }, 120_000); + + // Value: protects=without a remote-tracking twin the local base still stands, so the origin preference never hides lines; + // fails_when=the fix drops the local fallback; why_new=negative control for GST-1036; seam=bin/gstack-banned-terms + test('no origin: gstack-banned-terms keeps the local main and reports what the branch carries over it', () => { + const { sh, work, run } = fixture(); + sh(work, 'git remote remove origin'); + const r = run('gstack-banned-terms', ['check', '--base', 'main']); + expect(r.status, r.out + r.err).toBe(1); + expect(r.out).toContain('frobnicate'); + expect(r.err).not.toContain('base is'); + }, 120_000); + + // Value: protects=prose-lint --base lints only the lines the branch adds; fails_when=a bare base merges against the stale local main; + // why_new=GST-1036 sibling bin; seam=lib/prose-lint.ts addedOnly + test('gstack-prose-lint --base main skips a file only upstream changed; origin/main is read as given', () => { + const { run } = fixture(); + for (const base of ['main', 'origin/main']) { + const r = run('gstack-prose-lint', ['docs/up.md', '--base', base, '--gate']); + expect(r.status, r.out + r.err).toBe(0); + expect(r.out).not.toContain('PROSE:'); + } + }, 120_000); + + // Value: protects=the docs-audit snapshot selects only the branch's files; fails_when=--base main resolves to the stale local main and selects upstream docs; + // why_new=GST-1036 sibling bin; seam=bin/gstack-docs-candidate + test('gstack-docs-candidate snapshot --base main records origin/main and selects only the branch', () => { + const { root, run, origin } = fixture(); + const r = run('gstack-docs-candidate', ['snapshot', '--out', path.join(root, 'c.json'), '--audit-id', 'a1', '--mode', 'read-only', '--base', 'main']); + expect(r.status, r.out + r.err).toBe(0); + expect(r.out).toContain(`base=${origin} `); + expect(r.out).toContain('committed: "src/v.ts"\n'); + expect(r.err).toContain(`gstack-docs-candidate: ${NOTE}`); + }, 120_000); + + // Value: protects=the PR screenshot's before side is origin's base, not a stale local copy; fails_when=resolveCommit reads refs/heads/main first; + // why_new=GST-1036 sibling bin; seam=lib/pr-shots.ts resolveCommit + test('gstack-pr-shots --base main reads origin/main; a sha resolves as given', () => { + const { work, sh, origin } = fixture(); + expect(resolveCommit(work, 'main')).toBe(origin); + const local = sh(work, 'git rev-parse refs/heads/main'); + expect(resolveCommit(work, local)).toBe(local); + }, 120_000); +}); diff --git a/test/tree-receipt-stamp-sensitive.test.ts b/test/tree-receipt-stamp-sensitive.test.ts index e9c56e660..033f4ea5b 100644 --- a/test/tree-receipt-stamp-sensitive.test.ts +++ b/test/tree-receipt-stamp-sensitive.test.ts @@ -121,6 +121,36 @@ describe('tree receipt: stamps, stamp-sensitive gates and negative fixtures', () expect(stampOnlyLine("mine since: '1.2.3.0'", "mine since: '1.2.4.0'", '1.2.4.0', '1.2.4.0', tpl, mine, 5)).toBe(true); }); + // Value: protects=a declared placeholder on the branch's own row moves to the new version as a stamp, only inside a + // stamp_patterns template (gbrain's `since: 'TBD'` row, GST-1026); fails_when=stampOnlyLine reads the move as a hand + // edit, or accepts an undeclared placeholder, a base row, a duplicated base line, a built-in form or a line no + // template matches; seam=lib/stamp-only.ts stampOnlyLine placeholders + test('a declared placeholder on a branch row moves to the new version inside a stamp_patterns template, and nowhere else (GST-1026)', () => { + const tpl = ["since: '{version}'"]; + const row = " { since: 'TBD', id: 'mine' },"; + const moved = " { since: '0.60.553.0', id: 'mine' },"; + const side = (r: string, at = 10) => ({ fork: '0.60.552.0', added: new Set([r]), at: new Set([at]) }); + const line = (r: string, a: string, opts: { templates?: string[]; at?: number; placeholders?: string[]; oldV?: string } = {}) => + stampOnlyLine(r, a, opts.oldV ?? '0.60.552.0', '0.60.553.0', opts.templates ?? tpl, side(r), opts.at ?? 10, opts.placeholders ?? ['TBD']); + expect(line(row, moved)).toBe(true); + expect(line(row, moved, { oldV: '0.60.553.0' })).toBe(true); + expect(line(row, moved, { placeholders: [] })).toBe(false); + expect(line(row, moved, { placeholders: ['WIP'] })).toBe(false); + expect(line(" { since: 'WIP', id: 'mine' },", moved)).toBe(false); + expect(line(row, moved, { at: 2 })).toBe(false); + expect(stampOnlyLine(row, moved, '0.60.552.0', '0.60.553.0', tpl, undefined, undefined, ['TBD'])).toBe(false); + expect(stampOnlyLine(row, moved, '0.60.552.0', '0.60.553.0', tpl, side(" { since: 'TBD', id: 'other' },"), 10, ['TBD'])).toBe(false); + expect(line(row, moved, { templates: [] })).toBe(false); + expect(line(" { release: 'TBD', id: 'mine' },", " { release: '0.60.553.0', id: 'mine' },")).toBe(false); + expect(line(' { since: TBD, id: mine },', ' { since: 0.60.553.0, id: mine },')).toBe(false); + expect(line(row, " { since: '0.60.553.0', id: 'yours' },")).toBe(false); + expect(line(row, " { since: '0.60.554.0', id: 'mine' },")).toBe(false); + const both = " { since: 'TBD', note: 'Added in v0.60.552.0' },"; + expect(line(both, " { since: '0.60.553.0', note: 'Added in v0.60.553.0' },")).toBe(true); + expect(line(both, " { since: '0.60.553.0', note: 'Added in v0.60.551.0' },")).toBe(false); + expect(line(' // TBD: tidy', ' // 0.60.553.0: tidy', { templates: ['{version}'] })).toBe(false); + }); + test('negative fixture: a dependency pin equal to the old version is `changed`, not a stamp, even inside stamp_paths', () => { const repo = fixture(); const g = gated(repo);