ci: currency check sees files regeneration wrote but nothing committed - #315
ci: currency check sees files regeneration wrote but nothing committed#315thedavidmeister wants to merge 1 commit into
Conversation
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (3)
Included review availability: Your plan includes up to 1 review per rolling hour; 0 remain after this review. 📝 WalkthroughWalkthroughThe workflow now stages non-ignored changes before comparing generated artifacts. New Bats tests cover modified, added, deleted, unchanged, and ignored artifacts. The test runs in the default shell test suite. ChangesArtifact currency verification
Estimated code review effort: 3 (Moderate) | ~20 minutes Merge Risk: ⚪ Minimal · up to The change makes artifact currency checks include added and deleted generated files while preserving ignored build output, with focused tests covering the behavior. No actionable merge-blocking risk remains beyond normal checks and review. 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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 |
1 participant
What the check missed
The last step of
rainix-copy-artifacts.yamlasserted artifact currency withgit diff --exit-code.git diffcompares tracked working-tree content againstthe index; untracked paths are outside what it reads. So the step caught "a
committed artifact's content drifted" and never caught "regeneration now writes
somewhere else, and the artifact it replaced is still committed".
Measured in a
rain.sol.codegencheckout at7aa85a4— this comes from thewhole-repo audit at that sha, not from a filed issue. Renaming the generated
contract in
script/Build.soland replaying the copy-artifacts step sequence:src/generated/CodeGennable.solstays committed, untouched, and frozen atwhatever bytecode it last held. In a consumer that file is where
src/**imports
OPCODE_FUNCTION_POINTERS,INTEGRITY_FUNCTION_POINTERSandBYTECODE_HASHfrom, so the currency check's whole purpose is defeated inexactly the case it exists for.
What the step does now
Staging first makes the comparison whole-tree.
git add --allrespects.gitignore, soout/,cache/anddependencies/still contribute nothing,while additions, deletions and edits all reach the diff. The step is still the
last one in the job, so mutating the index affects nothing downstream.
Deviation from the proposed fix
The audit proposed
if [ -n "$(git status --porcelain)" ]. Same detection set,but staging and diffing is better on two counts, and I took it instead:
body, where
--porcelainprints one path and leaves the reader guessing;command output, which is the shape
CLAUDE.mdasks for ("bash mayorchestrate… the moment it branches over data, that logic is Rust").
Test
New:
test/bats/workflow/copy-artifacts-currency.test.bats, registered indefault-shell-testsocheck-shell.ymlruns it. It extracts the step'srunbody straight out of the workflow YAML with
yq— the same techniquetest/bats/action/rpc-preflight.test.batsalready uses — and executes it againstthrowaway git repos that stand in for a consumer checkout after regeneration.
Before the fix (the untracked case is the defect; the other four already held):
After:
Mutation matrix
Each mutant edits the step in the workflow YAML, runs the bats file, and is
reverted. Every run printed the
1..5plan line and five result lines, so thesuite demonstrably ran rather than erroring out into a false "survived".
git add --allgit diff --cached->git diffgit add --all->git add --all --force(stop honouring.gitignore)No mutant survived.
Consumer verification
The stricter check only earns its place if it does not red existing consumers
for unrelated reasons. Every repo in the org that calls this reusable was
enumerated by sweeping
.github/workflows/**across 229 non-archived repos inrainlanguage,cyclofinance,S01-Issuer,ST0x-Technology,h20liquidity,gildlabandraincommercial— ten consumers, listed below.All ten were cloned fresh and driven through the job's step sequence locally in
nix develop github:rainlanguage/rainix/53e96a7…#sol-shell, then both the oldand the new check were run on the resulting tree. Nine reach the check; the
tenth fails an earlier gate.
git status --porcelainafter regenerationrain.sol.codegen@935c725rain.flare@b634aadrain.erc4626.words@e02d60erain.metadata@382d44erainlang@a6b7ad7raindex@16049a3rain.dia@35cf7a5?? .pre-commit-config.yamlrain.merkle@ca6d324?? .pre-commit-config.yamlrain.pyth@9f9cd8c?? .pre-commit-config.yamlrain.math.float@d3fb611raindexis the strongest of the clean results: 76 committed files undersrc/generated, plus both thescript/build-meta.shandscript/build.shhooks, and the tree comes out with nothing untracked at all.
The three that go red, and why the file is ours
The three failures are one file, and it is rainix's own. Entering any rainix devshell
runs the git-hooks installation script, which writes
.pre-commit-config.yamlinto whatever repo is the working directory. Proven directly, in a
rain.merklecheckout that had never seen the file:
The job's very first step enters that shell, so every consumer's checkout gets
the file. Seven of the ten consumers list it in
.gitignoreand are unaffected.Three do not —
rain.dia,rain.merkle,rain.pyth— and those are exactlythe three that go red in the table, with nothing else in their staged diff.
This is not new behaviour and this PR does not cause it — the pollution has
happened on every copy-artifacts run all along, and
git diffsimply could notsee it. Making the check honest is what surfaces it.
I have deliberately not weakened the check to step around this, and have not
touched the consumer repos. The fix belongs at the source, and there are two
places it could sit — one line of
.gitignorein each of the three repos, whichis what the other seven already do, or rainix's devshells not installing hooks
into a tree they were invoked in non-interactively. That call, and the ordering
against this PR, is a human's.
rain.math.floatNot a regression from this PR either: its job never reaches the currency check.
It commits
src/generated/LogTables.pointers.solwithscript/BuildPointers.solrather than
script/Build.sol, so it fails the existingscript/Build.solgate.Its last green copy-artifacts run was 2026-07-15T13:17:50Z; the gate reached
rainix
mainat 2026-07-15T13:51Z, 34 minutes later. That green predates thegate and will not survive the repo's next push, with or without this change.
Enumerated consumers
raindex,rain.dia,rain.erc4626.words,rain.flare,rainlang,rain.math.float,rain.merkle,rain.metadata,rain.pyth,rain.sol.codegen.QA
test/bats/workflow/copy-artifacts-currency.test.bats— "an artifact regenerated under a new name, leaving the old one committed, fails" fails on base (run against the unmodified workflow vianix develop -c bats …:not ok 3 … [ "$status" -eq 1 ]' failed, the other fourok); it passes after the change,1..5allok. The other four tests pin the behaviour the step already had — drift, deletion, clean tree, gitignored build output — so the widening cannot quietly cost any of it.git add --alldeleted -> killed by tests 2, 3, 4;git diff --cached --exit-code->git diff --exit-code-> killed by tests 2, 3, 4;git add --all->git add --all --force-> killed by test 5. Three mutants, zero survivors, each applied to the workflow YAML and reverted. Every mutant run printed the1..5plan and five result lines, so no run was a harness error masquerading as a survivor.git diffreads tracked content against the index and does not consider untracked paths;git add --allstages additions, deletions and edits while honouring.gitignore. Each test asserts the outcome derived from that, and the expectations were confirmed independently against scratch repositories before the workflow was touched. The orphaned-artifact scenario itself is the one measured inrain.sol.codegenat7aa85a4, wheregit diff --exit-codereturned 0 on an untracked regenerated file..gitignoreis still respected, and that the stricter check be verified against real consumers before pushing. Covered: additions (test 3), deletions (test 4), edits (test 2),.gitignorerespected (test 5), and all ten consumers enumerated and replayed end to end (table above). The four consumers that do not come out clean are diagnosed above and none of it is caused by this change. The finding's adjacent note that rainix#313 covers hardcodedsrc/generatedin this same file is out of scope here and untouched.sol-shell, recording the old check,git status --porcelainand the new check for each — six clean, three carrying only rainix's own.pre-commit-config.yaml, one failing an earlier pre-existing gate.nix develop -c pre-commit run --all-files— nixfmt, no-consumer-prettier, prettier-rainix, shellcheck, statix, taplo and yamlfmt all pass, and no hook rewrote a file (git status --porcelainempty afterwards).rustfmt-conditionalfails withcould not find Cargo.toml in …/rainix; it fails identically on a cleanorigin/mainat7f223b4, so it is pre-existing and unrelated to this branch.