From aa19e3478c9f9cad03c14841322fa763ed2b0553 Mon Sep 17 00:00:00 2001 From: Tom Boucher Date: Thu, 30 Jul 2026 11:19:45 -0400 Subject: [PATCH] fix(#2854): pin the emitted gate to the base the tree was merged with (#2859) * test(#2854): failing-first coverage for CI baseline export provenance Extracts the export decision out of main() behind injected IO so it is unit-testable, preserving today's export-whenever-present semantics, and adds the matrix that proves those semantics are wrong. The PR lane restores the emitted baseline keyed on the PR's recorded base sha while the gate resolves the base ref live, so the two drift whenever next advances mid-flight. The restore was published straight to GSD_EMITTED_BASELINE, where a mismatch is fatal, turning a recoverable cache into a hard failure on diffs that touched nothing related. Also renames the stale-env fixture from 'from-cache-restore.json' to an operator-pin name: that fixture asserted the exact conflation this bug is, documenting the defect as intended behavior. Refs #2854 * fix(#2854): validate a restored baseline before publishing it as an operator pin GSD_EMITTED_BASELINE is an operator pin: resolveBaseline() treats a mismatch there as a hard stop, because the operator said "use this one". CI published its cache restore to that same variable whenever the file merely existed, so a restore keyed on the PR's recorded base sha - while the gate resolves the base ref live - turned a recoverable cache into a fatal error whenever next advanced mid-flight. Required tests went red on diffs that touched nothing related, and named a test file the contributor never opened. The export step is the boundary, so it is the boundary that validates. It now publishes only a baseline already valid for the sha under test, judged by validateBaseline so the staleness rule keeps one definition. Anything refused is left to be found via the cache path, where a mismatch degrades to the in-job build exactly as ADR-2719 SS5 specifies. The operator hard stop is untouched, and the fast path still hits on a current cache. Also reports the sources actually reached rather than asserting all three ran: the failure message claimed an in-job build it had returned before calling, sending contributors after a rebuild that never happened. Fixes #2854 * fix(#2854): pin the emitted gate to the base the tree was actually merged with The differential compared a tree built on one commit against a baseline at a different one. "Rebase check" merges pull_request.base.sha, pinned by #2472 so all 12 matrix jobs agree on one tree, but resolveBase() fell through to origin/next, which fetch-depth 0 leaves at the live tip. Nothing set GSD_EMITTED_BASE, so whenever next advanced mid-flight the two disagreed. The cached baseline, keyed on base.sha, was correct for that tree and was rejected as STALE by a target that was not. Required tests went red on diffs that touched nothing related, naming a test file the contributor never opened. The near miss is the worse half: had resolution gotten past the baseline step, a baseline at the live tip would have attributed commits merged to next in between to the PR under test. The hard stop was shielding us from a wrong answer, so making it fall through would have made this worse. Pins GSD_EMITTED_BASE to the same expression as CI_REBASE_BASE_SHA in every rebase-merged job, with a parity test asserting the two cannot diverge. That parity check immediately caught a third lane, test-inert, that merges a pinned base and had been missed. Fixes #2854 * fix(#2854): keep the export step self-contained across the package boundary scripts/ ships in the npm tarball and tests/ does not, so requiring the validator across that boundary is MODULE_NOT_FOUND in a published install. The export step now reads the same GSD_EMITTED_BASE pin the gate resolves through, and applies a cheap self-contained precondition; validateBaseline remains the sole authority and still runs downstream on whatever is published, so there is no second opinion to drift. Reading the pin rather than re-deriving a base is the point: a second, divergent base lookup is exactly what caused this bug. The same hazard pre-exists in scripts/gen-emitted-baseline.cjs, which ships and requires three tests/ modules. Filed as #2858 rather than folded in: fixing it means relocating the shared helpers out of tests/ and updating every consumer, which would bury this change. Refs #2854 * fix(#2854): stop announcing the restored cache through the operator-pin door Two independent reviewers found the same blocker in the previous approach. Validating before publishing to GSD_EMITTED_BASELINE only narrowed the hole: the precondition gated on sha equality alone, so a document with a MATCHING sha but a wrong schema version or malformed manifests was still announced as an operator pin and still hard-stopped downstream. That reproduces this bug's own class, triggered by malformation instead of staleness. The step was never load-bearing. The cache restores to .gsd-cache/emitted-baseline.json, which is resolveBaseline's DEFAULT_CACHE_PATH and is read whether or not anything announces it. Publishing the same file to the pin door could only ever convert recoverable into fatal, so the step and its script are deleted rather than made cleverer. Every failure mode now degrades to the in-job build by construction, and validateBaseline is once again the only thing that judges a baseline. Coverage moves to where the behaviour lives: stale sha, wrong schema version, manifests array/absent, non-object documents, unreadable file, and the 39/40/41 hex boundary all assert degradation via the cache path. Adds the empty-pin case a reviewer flagged as untested - the pin is job-level env, so on push events it renders as an empty string, and only baseRefCandidates' truthy check keeps it out of the candidate list. Fixes #2854 --------- Co-authored-by: Test --- .github/workflows/test.yml | 48 +++-- scripts/ci-export-emitted-baseline-env.cjs | 44 ----- tests/emitted-attribution.test.cjs | 194 ++++++++++++++++++++- tests/helpers/emitted-baseline.cjs | 62 +++++-- 4 files changed, 271 insertions(+), 77 deletions(-) delete mode 100644 scripts/ci-export-emitted-baseline-env.cjs diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index a04aa2e00..f3a64bd89 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -119,6 +119,15 @@ jobs: timeout-minutes: 15 env: GSD_PLUGIN_ROOT: .ci-gsd-plugin-root-disabled + # #2854: pin the emitted gate's baseline to the SAME commit the tree was merged + # with. "Rebase check" merges `pull_request.base.sha` (pinned by #2472 so all 12 + # matrix jobs agree on one tree), but `resolveBase()` otherwise falls through to + # `origin/next`, which `fetch-depth: 0` leaves at the LIVE tip. Whenever `next` + # advanced mid-flight the gate compared a tree built on base.sha against a + # baseline at a newer commit — so the correctly-keyed cache was rejected as + # "stale" and the run hard-failed on diffs that touched nothing related. + # This must stay equal to CI_REBASE_BASE_SHA; a test asserts that parity. + GSD_EMITTED_BASE: ${{ github.event.pull_request.base.sha }} strategy: fail-fast: false matrix: @@ -188,8 +197,16 @@ jobs: # #2724 (ADR-2719 §5): the differential attribution check's baseline is cached, # not committed. This restores what the push-to-next job published, keyed on - # the PR's base sha — the "next sha the PR was merged with" the ADR requires. - # A miss is NOT a failure here: resolveBaseline() degrades to an in-job build. + # the PR's base sha — the "next sha the PR was merged with" the ADR requires, + # and the same commit `ci-rebase-check` merged into this tree. + # + # #2854: the restore lands on resolveBaseline()'s DEFAULT_CACHE_PATH, so it is + # found without being announced. There used to be a step that published this + # path to GSD_EMITTED_BASELINE — an OPERATOR PIN, where any rejection is a hard + # stop rather than a fall-through. That converted every recoverable condition + # (stale, malformed, wrong schema version) into a fatal CI failure on diffs that + # touched nothing related. Restoring to the default path and saying nothing lets + # a bad artifact degrade to the in-job build, which is what ADR-2719 §5 specifies. - name: Restore emitted-baseline cache if: github.event_name == 'pull_request' uses: actions/cache/restore@0057852bfaa89a56745cba8c7296529d2fc39830 # v4.3.0 @@ -197,13 +214,6 @@ jobs: path: .gsd-cache/emitted-baseline.json key: emitted-baseline-${{ github.event.pull_request.base.sha }} - - name: Export GSD_EMITTED_BASELINE when the cache restored - if: github.event_name == 'pull_request' - # No `shell:` override — H1 shell policy requires each job's native shell - # (pwsh on Windows, zsh on macOS test-full, bash on Linux). A bare `node` - # invocation has no shell-specific syntax, so it is valid under all three. - run: node scripts/ci-export-emitted-baseline-env.cjs - - name: Prepare scoped test list if: matrix.scope != 'full' env: @@ -275,6 +285,10 @@ jobs: timeout-minutes: 15 env: GSD_PLUGIN_ROOT: .ci-gsd-plugin-root-disabled + # #2854: same pin as the other rebase-merged lanes. This lane runs a targeted + # list that can include the emitted gate, and the invariant is easier to keep + # with no exceptions: if a job merges a pinned base, the gate uses that base. + GSD_EMITTED_BASE: ${{ github.event.pull_request.base.sha }} steps: - uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2 with: @@ -342,6 +356,15 @@ jobs: timeout-minutes: 20 env: GSD_PLUGIN_ROOT: .ci-gsd-plugin-root-disabled + # #2854: pin the emitted gate's baseline to the SAME commit the tree was merged + # with. "Rebase check" merges `pull_request.base.sha` (pinned by #2472 so all 12 + # matrix jobs agree on one tree), but `resolveBase()` otherwise falls through to + # `origin/next`, which `fetch-depth: 0` leaves at the LIVE tip. Whenever `next` + # advanced mid-flight the gate compared a tree built on base.sha against a + # baseline at a newer commit — so the correctly-keyed cache was rejected as + # "stale" and the run hard-failed on diffs that touched nothing related. + # This must stay equal to CI_REBASE_BASE_SHA; a test asserts that parity. + GSD_EMITTED_BASE: ${{ github.event.pull_request.base.sha }} strategy: fail-fast: false matrix: @@ -439,13 +462,6 @@ jobs: path: .gsd-cache/emitted-baseline.json key: emitted-baseline-${{ github.event.pull_request.base.sha }} - - name: Export GSD_EMITTED_BASELINE when the cache restored - if: github.event_name == 'pull_request' - # No `shell:` override — H1 shell policy requires each job's native shell - # (pwsh on Windows, zsh on macOS test-full, bash on Linux). A bare `node` - # invocation has no shell-specific syntax, so it is valid under all three. - run: node scripts/ci-export-emitted-baseline-env.cjs - # The heavy unit suite is split across the 3 shards — each runs a # deterministic cost-balanced third of the sorted unit-file list (#2472). The # union of shards 1/3 + 2/3 + 3/3 is the full unit suite, so coverage is diff --git a/scripts/ci-export-emitted-baseline-env.cjs b/scripts/ci-export-emitted-baseline-env.cjs deleted file mode 100644 index b6e95b0f4..000000000 --- a/scripts/ci-export-emitted-baseline-env.cjs +++ /dev/null @@ -1,44 +0,0 @@ -#!/usr/bin/env node -'use strict'; - -/** - * ci-export-emitted-baseline-env.cjs — exports GSD_EMITTED_BASELINE for the - * differential attribution check when its baseline cache was restored (#2724, - * ADR-2719 §5). - * - * A plain `node