Repository navigation
fix(deonboard): compare owner/repo case-insensitively when pruning - #52
Merged
Merged
Conversation
scripts/deonboard-estate-audit.sh pruned a dependency that was still in use. gh actions-lock writes the nested `uses:` of a dependency with owner/repo in lower case and keeps the record key's original case. hyperpolymath/januskey's lock (2026-10-14) holds the edge 'swatinem/rust-cache@6323deb…' under its hyperpolymath/standards record, and the record 'Swatinem/rust-cache@6323deb…'. The byte-for-byte `grep -vxF` judged that record orphaned and deleted it, leaving a dangling edge. Standards scripts/close-lock.sh measured that shape killing runs at startup with zero jobs. On januskey the script would have broken rust-ci and governance to de-onboard the audit. The script now lower-cases owner/repo on both sides before comparing, the way GitHub resolves them. Refs keep their case, because git refs are case-sensitive. When unsure it keeps a record, which is the safe direction: an orphan record is harmless, a dangling edge is fatal. On januskey's lock it now prunes exactly cicd-suite@main, deed-ecosystem@f9d999b and k9-ecosystem@2155aa2, and keeps Swatinem/rust-cache. tests/deonboard-estate-audit.sh (new, run by the "Composite shell contract" job; the script joins both path filters) uses a fixture lock in gh-actions-lock layout with the mixed-case edge. It checks: - the audit is de-onboarded; - the true orphans are pruned; - the still-reached record is kept; - no edge dangles; - a second run changes nothing; - a mutant with the byte-for-byte compare restored is killed. Against main's script the same suite reports FAIL=3, against this one FAIL=0. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Kjdf4Mf4Z41bNQh2TrsEp9
Contributor
|
Warning Review limit reachedYou've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Next included review available in 53 minutes. View limit details
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
marked this pull request as ready for review
October 14, 2026 13:42
hyperpolymath
added a commit
that referenced
this pull request
Oct 19, 2026
… the schema guard and the case fix (#56) ## What was wrong `scripts/deonboard-workflow.sh` (#54) is a copy of the estate-audit de-onboard script, taken before #52 and #53 landed. It has neither fix: 1. **No schema guard.** It accepts any `actions.lock` schema. On a `v9.9.9` lock it exits 0 and edits the lock. #53's fail-closed guard (exit 3) lives only in `deonboard-estate-audit.sh`. 2. **Case-sensitive prune.** It compares dependency keys byte for byte. Take a lock where the `hyperpolymath/standards` record reaches `swatinem/rust-cache@6323deb…` but the record key is `Swatinem/rust-cache@6323deb…`. The script prunes that record and leaves a dangling edge, which startup-fails the run. This is the januskey shape #52 fixed, but only in the estate-audit copy. **Who was exposed:** `rollout_language_gates.sh` de-onboards every Rust CI, Creusot, SPARK and miniKanren caller through `deonboard-workflow.sh`. Rust CI callers are exactly the repositories whose locks carry the `standards` → `rust-cache` edge. As far as this session knows, no rollout has run since #54 merged, so this is a latent defect rather than a broken estate. That is ASSUMED: I have no rollout logs. The `deonboard-workflow.sh` header and README line 273 on main already said the estate-audit script "now delegates" to it. It did not; it was a second copy. ## The change - **`scripts/deonboard-workflow.sh`** is now the one implementation. It carries #53's `known_versions` guard and #52's `lower_repo` prune, with their comments carried over verbatim. - **`scripts/deonboard-estate-audit.sh`** is now just a usage check and one `exec …/deonboard-workflow.sh "$repo" .github/workflows/main-estate-audit.yml`. `rollout_estate.sh`, the templates and the README keep working through it unchanged. - **`tests/deonboard-lock-schemas.sh`**: - It reads `known_versions` from the generic script. - Every fixture and the v9.9.9 control run through both entry points. - The prune-skipping mutant is applied to the generic script and must be killed through the wrapper as well, which fails if the wrapper ever stops delegating. - The optional argument is now a scripts directory. - **`tests/deonboard-estate-audit.sh`** (#52): - Both entry points run the same checks. - The two entry points must produce byte-identical locks. - The `| lower_repo` mutant is applied to the generic script and must be killed through both. - **CI** (`code-hygiene-self-test.yml`): - `tests/deonboard-estate-audit.sh` was never run in CI. It now runs in the Composite shell contract job. - The workflow also triggers on `scripts/deonboard-workflow.sh`. - No `uses:` added. - **Docs:** README and ARCHITECTURE now describe the delegation as it is. - **Ported from #55:** `3325927` is #55's repin, cherry-picked with `-x`; the diff is byte-identical. - **Why:** main's `lock-transitive-closure` failure would otherwise stop the Composite shell contract job before the de-onboard steps run. - **If #55 merges first:** this commit becomes a no-op. ## Evidence (TESTED) I copied the new suites into a tree of `origin/main` (`9a947fa`) and ran them against main's scripts. `tests/deonboard-estate-audit.sh`: PASS=14 FAIL=4 - `FAIL pruned Swatinem/rust-cache, which the standards record still reaches` - `FAIL dangling edge(s): Swatinem/rust-cache@6323deb` - `FAIL the entry points disagree: …` - `FAIL mutant is identical to the script: the case-folding the test exists for is gone` `tests/deonboard-lock-schemas.sh`: PASS=57 FAIL=4 - `FAIL script declares no known_versions` - `FAIL exit 0 on schema v9.9.9 (want 3)` - `FAIL lock modified despite refusal` - `FAIL orphan check passed a mutant that does not prune` (main's wrapper does not delegate) On this branch: | Check | Result | | --- | --- | | `tests/deonboard-estate-audit.sh` | PASS=19 FAIL=0 | | `tests/deonboard-lock-schemas.sh` | PASS=63 FAIL=0 | | `tests/lock-transitive-closure.sh` | PASS=31 FAIL=0 | | `tests/composite-shell-contract.sh` | PASS=28 FAIL=0 | | `bash -n` | clean | | `shellcheck -S warning` | clean (default severity: same SC2015/SC2016 counts as main) | | `actionlint` | no new findings (the 8 `"$/actions/…"` findings in the hygiene self-test are also on main) | 🤖 Generated with [Claude Code](https://claude.com/claude-code) https://claude.ai/code/session_01VTeVWDEJgdYKsUNr4uhnXX --------- Co-authored-by: Claude <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
2 participants
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.
Why
scripts/deonboard-estate-audit.sh(from #50) prunes a dependency that is still in use.gh actions-lockwrites a dependency's nesteduses:with owner/repo in lower case, but keeps the record key's original case. hyperpolymath/januskey's lock (2026-10-14) shows this:hyperpolymath/standards@7d2041arecord listsswatinem/rust-cache@6323deb…;Swatinem/rust-cache@6323deb….The script's byte-for-byte
grep -vxFjudges that record orphaned and deletes it. That leaves a dangling edge, which standardsscripts/close-lock.shmeasured killing runs at startup with zero jobs. Dry-running the script on januskey would have broken its rust-ci and governance runs to de-onboard the audit.Fix
Lower-case owner/repo on both sides before comparing, the way GitHub resolves names. Refs keep their case, because git refs are case-sensitive.
When in doubt the script now keeps a record, which is the safe direction: an extra record is harmless, a dangling edge is fatal.
On januskey's lock it now prunes exactly
cicd-suite@main,deed-ecosystem@f9d999bandk9-ecosystem@2155aa2, and keepsSwatinem/rust-cache.Test
tests/deonboard-estate-audit.shis new. It runs in the "Composite shell contract" job, and the script is added to both path filters. It builds a fixture lock in gh-actions-lock's layout that contains the mixed-case edge, and asserts:tests/lock-transitive-closure.shPASS=8 FAIL=0 andtests/composite-shell-contract.shPASS=28 FAIL=0. This PR does not touchactions/**, so no repin is needed. shellcheck is clean. actionlint reports only the five$/actions/…findings already on main.Independent of #51; either can merge first.
🤖 Generated with Claude Code
https://claude.ai/code/session_01Kjdf4Mf4Z41bNQh2TrsEp9
Generated by Claude Code