fix(ci): restore the composite-reached lock pins #28 silently dropped - #34
Merged
Merged
Conversation
`Gate controls` has been dead since 3b4afaf (#28), failing in `Set up job` with one step and no gate ever executed: ##[error]lockfile missing pin for hyperpolymath/deed-ecosystem@f9d999b Mechanism. `uses: $/actions/manifest-check` is a same-repo ref, inherently pinned at the running commit, and needs no lockfile entry of its own. But the runner also validates that composite's INTERNALS against the lock, and it does so against the calling workflow's entry in the `workflows:` map. manifest-check reaches deed-ecosystem and k9-ecosystem at full SHA, so both must be listed under `code-hygiene-self-test.yml` — being present in `dependencies:` is not enough. This restores the exact shape that was green at d6bc01a, keeping the two `[]` keys #28 correctly added. Why it was invisible for a day, and why a comment would not have helped: ecd0240 (#16) added both transitive SHA refs ... survived #17, #18, #25 — `Gate controls` green throughout 3b4afaf (#28) regenerated the lock and dropped both `gh actions-lock`'s extractor reads step-level `uses:` only, so it cannot derive a ref reached through a composite and a regenerate deletes any added by hand. `--verify` then returns rc=0 on the result: #28's message says it "pins the SHA-form transitive deps reached via called reusables" while its diff removes exactly those, and the rc=0 was its cited proof. `--no-fix` still prints "Scanning 4 workflows", rc=0, against the broken file today. The tool cannot see the defect it creates. Three fixes, because the defect had three parts: 1. Restore the two pins. 2. `tests/lock-transitive-closure.sh` — walks each onboarded workflow's `$/` composites to a fixpoint and asserts every remote ref they reach is declared for that workflow, plus that every declared ref resolves in `dependencies:`. Read with yq, never grep (Y-1). Non-vacuity is asserted, not assumed: the checked population must be non-empty (that assertion caught a jq-vs-mikefarah dialect bug on this file's first run, where `--arg` failed as a lexer error and a stray 2>/dev/null turned it into a clean empty pass). Mutants: the real broken lock at fa71ac2 goes red naming both refs, the green-era lock at d6bc01a passes, and a declared-but-undefined ref is caught. 3. `actions.lock` added to both `paths:` filters. #28 was a lock-only commit, so no filter matched and this workflow never ran on the commit that broke it. The guard runs in `Composite shell contract`, not `Gate controls`: a missing composite-reached pin kills `Gate controls` in `Set up job`, so a check living there could never report the defect it tests for. Closes #33. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01WRvDivYwLSeVCJUrfjic3f
Contributor
|
Warning Review limit reachedNext included review available in 1 minute. View limit detailsLimit details: You’ve used the included review currently available. You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Review configuration: ⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Advanced Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (2)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
hyperpolymath
added a commit
that referenced
this pull request
Sep 22, 2026
…nsumer (#35) ## The cure was green here and inert everywhere else `fa71ac2` (#32) cured the composite `-e` kill. `0c1bc9f` (#34) restored the lockfile pins #28 dropped. Both merged green. **Neither reached a single consumer**, and nothing was red to say so. pons-asinorum was repointed at `0c1bc9f` (pons#26) and its estate audit *still* died at `Required Files Gate` — 82 ms, silent, exit 1, 25 downstream gates skipped. Identical to before the cure. ### Why The reusable invokes its gates by **branch** ref: ```yaml uses: hyperpolymath/cicd-suite/actions/required-files-check@main ``` A branch ref is *trusted from the lockfile*, and the runner executes **the commit the lockfile names** — not the branch tip. This lock pinned `cicd-suite@main` at `9adb3908`, four commits back: | commit | PR | carried | |---|---|---| | `0c1bc9f` | #34 | lock pins restored | | `fa71ac2` | #32 | the `-e` cure (3 + 1 guards) | | `6ff6057` | #31 | linguist-check | | `3b4afaf` | #28 | (the regression) | `9adb3908` has **0** of those guards. So no SHA a consumer picks for the *reusable* can reach a cured *composite*. The lock's `@main` entry is the real gate, and re-pinning it is the actual delivery step — the one #31, #32 and #34 all skipped. ## What this does 1. Bumps `dependencies['hyperpolymath/cicd-suite@main'].commit` to `0c1bc9f`. Ancestor-clean fast-forward (`9adb3908` is an ancestor of `0c1bc9f`); the transitive `uses:` list is unchanged, so `dependencies:` needs no other churn. 2. **Makes it impossible to forget again.** `tests/lock-transitive-closure.sh` gains a third assertion: for any self-referencing branch pin, the `actions/` tree at the locked commit must equal `HEAD`'s. On drift it names the files. 3. `shell-contract` gains `fetch-depth: 0` so the locked commit is present to compare against. The house pattern for this is a follow-up *"pin cicd-suite lock at `<sha>`"* commit — `f8c8f4a`, `4f9a7a4`, `373714a` all do exactly this. It has been carried by memory, and memory dropped it three times running. Now it is a test. ## Non-vacuity — twice **The assertion was written before the bump and caught the live defect**, naming all three drifted composites: ``` FAIL hyperpolymath/cicd-suite@main pins 9adb390..., whose actions/ tree differs from HEAD actions/linguist-check/action.yml actions/required-files-check/action.yml actions/spdx-license-check/action.yml ``` And its first draft printed its header while checking *nothing* — an ERE `sed` has no lazy quantifiers, so `.git` stayed on the repo name and no dependency key ever matched. A header is not a check, so the block now carries its own non-vacuity counter, separate from block 1's. ## Local ``` tests/lock-transitive-closure.sh PASS=9 FAIL=0 tests/composite-shell-contract.sh PASS=7 FAIL=0 (mutant dies silently) ``` ## Acceptance `Composite shell contract` green with the new assertion passing — and then pons#26's audit re-measured. Per the standing owner ruling, the gates that have been *skipped rather than passing* may now surface new findings; those become issues with acceptance criteria, not blockers on this PR. 🤖 Generated with [Claude Code](https://claude.com/claude-code) https://claude.ai/code/session_01WRvDivYwLSeVCJUrfjic3f Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #33.
What was broken
Gate controlshas been dead since3b4afaf(#28), dying inSet up jobwith 1 step and no gate ever executed. Measured on run35786094563(push, main,fa71ac2):Mechanism
uses: $/actions/manifest-checkis a same-repo ref — inherently pinned at the running commit, no lockfile entry of its own required. But the runner also validates that composite's internals against the lock, and it does so against the calling workflow's entry in theworkflows:map.manifest-checkreachesdeed-ecosystemandk9-ecosystemat full SHA, so both must appear undercode-hygiene-self-test.yml. Being present independencies:is not sufficient — the map is keyed per workflow.Why it stayed invisible
ecd0240(#16)Gate controlsgreen throughout3b4afaf(#28)gh actions-lock's extractor reads step-leveluses:only, so it cannot derive a ref reached through a composite — a regenerate deletes any added by hand.--verifythen returns rc=0 on the result. #28's commit message states it "pins the SHA-form transitive deps reached via called reusables" while its diff removes exactly those, and that rc=0 was its cited proof.gh actions-lock --no-fixstill printsScanning 4 workflows, rc=0, against the broken file today.The tool cannot see the defect it creates. So the lockfile needs a gate that is not the tool.
Three fixes, because the defect had three parts
Restore the two pins — byte-for-byte the shape that was green at
d6bc01a, keeping the two[]keys fix(ci): reconcile actions.lock so the lockfile validates #28 correctly added.tests/lock-transitive-closure.sh— walks each onboarded workflow's$/composites to a fixpoint and asserts every remote ref reached is declared for that workflow, and that every declared ref resolves independencies:. Reads YAML withyq, nevergrep(Rule Y-1).actions.lockadded to bothpaths:filters. fix(ci): reconcile actions.lock so the lockfile validates #28 was a lock-only commit; no filter matched, so this workflow never ran on the commit that broke it.The guard runs in
Composite shell contract, notGate controls: a missing composite-reached pin killsGate controlsinSet up job, so a check living there could never report the very defect it tests for.Non-vacuity
Asserted, not assumed. The checked population must be non-empty — and that assertion earned its place on this file's first run, catching a jq-vs-mikefarah dialect bug where
--argfailed as a lexer error and a stray2>/dev/nullturned it into a clean empty pass.fa71ac2PASS=3 FAIL=2d6bc01arc=0dependencies:entryLocal:
lock-transitive-closure.shPASS=7 FAIL=0;composite-shell-contract.shPASS=7 FAIL=0 with its mutant killed.The witness
This PR's own run is the evidence.
Gate controlsmust spawn more than 1 step and actually execute the gate controls — 1 step means it died in prepare again.🤖 Generated with Claude Code
https://claude.ai/code/session_01WRvDivYwLSeVCJUrfjic3f