Resolve the Solidity verification toolchain from the repo's own CI - #195
Conversation
The producer verified Solidity through a hardcoded `github:rainlanguage/rainix#sol-shell` while the same clause named "the repo's" toolchain for Rust and TS. That flake tracks rainix HEAD and no repo's CI runs it, so a green local `forge fmt` was never a claim about CI: cyclofinance/cyclo.sol#42 spent its one permitted back-off attempt producing a diff the repo's own pinned `forge fmt --check` could not accept. `sol-toolchain <dir>` computes the answer per checkout instead, because there is no org-wide one. Measured across the 45 foundry repos in the pipeline's org scope: 33 gate Solidity on the rainix reusables and run at the `RAINIX_SHA` the REUSABLE pins (their own flake.lock is never read by those checks); 4 run a bare `nix develop -c rainix-sol-<task>` against their own flake; 1 gates on two toolchains that can disagree; 7 gate no Solidity check on a push at all. Exit 3 covers the last two and the no-flake case — a fact for the run record rather than a silent fallback, which is how the skew got reintroduced invisibly. Steps 4 and 3d now take their prefix from it. Step 1's health check keeps the unpinned URL: "can this box realise a rainix shell" is a different question from "will this satisfy that repo's CI". Closes #116 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The extra `return false` differed from the reassignment below it only for a document with a second column-zero `on:` key, which is not YAML — an equivalent mutant, and one a mutation pass reports as a coverage gap it is not. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
WalkthroughThe PR adds ChangesSolidity toolchain verification
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant CampaignWorkflow
participant sol-toolchain
participant RepositoryWorkflows
participant GitHub
CampaignWorkflow->>sol-toolchain: resolve checkout toolchain
sol-toolchain->>RepositoryWorkflows: inspect push and PR workflows
RepositoryWorkflows->>GitHub: resolve reusable RAINIX_SHA pins
GitHub-->>sol-toolchain: return pin information
sol-toolchain-->>CampaignWorkflow: return selected or unresolved verdict
CampaignWorkflow->>CampaignWorkflow: run verification or record the unresolved state
Possibly related issues
Suggested reviewers: 🚥 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 |
…sol-toolchain-from-repo-ci # Conflicts: # CLAUDE.md
SolToolchain modelled three nix shapes, so a repo whose CI installs Foundry WITHOUT nix matched none of them and fell into `absent` — "no CI toolchain to match". That is false, and false in the worst place: rain.will-overflow gates `forge fmt --check` on foundry NIGHTLY, which is further from any shell the producer can open than the widest rainix pin gap. The classifier was blind exactly where the skew is largest. `Foreign(String)` carries the acquisition verbatim and reports `mode: foreign` with its own text — there IS a toolchain here and it is one you cannot enter, so name it and expect the mismatch. Recount over all 45: 33 rainix-pin, 4 repo-flake, 4 foreign (rain.will-overflow, rain.erc1820, view-quoter-v3, dvin.poc), 1 conflict, 3 absent. Detection reads the ACQUISITION, not the invocation. A bare `forge` in a `run:` proves nothing — st0x.deploy opens `nix develop --command bash -c` and calls forge on the next line, so an invocation-based reading would report a nix repo as foreign. A third way of getting forge is still a blind spot; a test names that bound rather than leaving it implicit. Also: the trigger block is recognised in all four spellings GitHub accepts (YAML 1.1 reads bare `on` as true, so formatters emit `"on":` and a 1.1 round trip can emit `true:`) — the failure was a silent misclassification into the same `absent` bucket. And a comment inside that block is stripped, so `# runs on push` is prose rather than a gate. Refs #116 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@campaign-prompt.txt`:
- Line 72: Update the Solidity verification guidance around the sol-toolchain
exit-3 modes to define an explicit handoff and stop condition before mutation
testing or PR creation: require a non-Nix verification command from the
repository workflow for foreign toolchains, and direct conflict or missing
repo-flake cases to pr-review-report flag-design rather than permitting
fallback. Ensure absent also records the mode and stops appropriately, while
preserving the existing toolchain-selection rules.
In `@pr-review-report-rs/src/main.rs`:
- Around line 4817-4823: Strip YAML comments before parsing reusable and
toolchain declarations: update the loop around
sol_toolchains/sol_toolchain_lines to pass a strip_yaml_comment-processed line
to rainix_sol_reusable_ref, nix_develop_step, and foreign_sol_toolchain, or
apply it directly inside both rainix_sol_reusable_ref and nix_develop_step.
Ensure commented-out uses lines are ignored and do not create duplicate
toolchains or conflict mode.
- Around line 5097-5112: Update the workflow discovery logic around the read_dir
and read_to_string calls to report I/O failures instead of silently treating
them as absent workflows. Emit a distinct error message on the existing
error-reporting path for both unreadable workflow directories and individual
workflow files, while preserving the exit code of 3 and normal behavior for
successfully read entries.
- Around line 4800-4815: Update workflow_gates_a_push to strip YAML comments
before evaluating block boundaries, and skip comment-only or empty lines before
assigning in_on. Ensure column-zero comments do not close an active on: block,
while real column-zero keys still establish the correct boundary and trigger
detection remains unchanged.
- Line 5037: Update the SolToolchain::Explicit verdict handling to return exit 0
and an authoritative verify command only for references containing a pinned
revision; unpinned explicit flakes must not produce that successful verdict. Add
a regression test covering the single-Explicit HARDCODED_WF case, and document
explicit in the sol-toolchain CLI table in CLAUDE.md.
- Around line 24669-24672: Update sol_toolchain_mode to canonicalize the
provided dir into an absolute path before constructing SolToolchain and passing
it to sol_toolchain_lines. Preserve the existing command-reporting flow, and
emit a clear error if canonicalization fails.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: f17ae671-19e8-4923-a743-ecc47a3e73c1
📒 Files selected for processing (3)
CLAUDE.mdcampaign-prompt.txtpr-review-report-rs/src/main.rs
All five are CodeRabbit findings on this PR and all five are real. None changes any of the 45 org repos' classification — verified by re-running the binary over every one and diffing the result, which is identical — so they are latent, and four of them land in the same `absent` bucket this PR exists to stop being a dumping ground. A column-zero comment closed the `on:` block. The boundary test read the RAW line, so `on:` / `# gate every branch` / ` push:` reported `mode: absent` on a workflow that gates every branch. Strip first: a comment is not a key at any indentation. A commented-out step registered as a toolchain. Only `foreign_sol_toolchain` stripped comments, so a `# uses: …rainix-sol` above a live `nix develop -c` made a one-toolchain repo report `mode: conflict` — a disagreement invented out of a comment, refused with exit 3. Stripping moves into the loop, once, for all parsers. An unreadable checkout reported `absent`. A directory that cannot be read yields the same empty list a repo with no Solidity CI does, and knowing nothing is not knowing that nothing gates it. `mode: unreadable` says which, and a single unreadable FILE counts — that one could drop the only workflow that gates the repo. An unpinned explicit flake was handed over as authoritative. It is still the right command (it is what CI runs), so it is still printed; it now carries a `warn:` that it floats with a default branch and that matching text is not matching toolchain. A relative `dir` reached the printed command verbatim. That command is the whole deliverable and only reproduced CI from the directory it was typed in, while the doc comment claimed absolute. Canonicalized, with the failure reported rather than swallowed. Refs #116 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The per-FILE read failure lived inside the impure directory walk, so a mutant that swallowed it SURVIVED — the one gap in this PR's mutation pass. `collect_workflows` takes already-taken reads and returns the workflows plus the first failure, which is the same impure-edge / pure-decision split the rest of this module already uses. Refs #116 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
Reviewed 7ee1761: ready Verified before merge: 20 SUCCESS / 1 skipped / 0 failures, MERGEABLE/CLEAN, 0 unresolved threads. 930 tests, 18/18 mutations killed. I reviewed the first head and found a false negative: The recount found FOUR, not the three I named — The concern I had about Three decisions I would keep. A fourth VARIANT rather than a fourth exit code, because 3 already means "no single CodeRabbit reviewed this head properly: five findings accepted, four of them landing in It also flagged the one place its first pass would have shipped a false all-killed: a mutant survived inside an impure directory walk, so it extracted Remaining gap, unchanged from round one and correctly not smuggled in: nothing VERIFIES the producer used the shell it was told to. This is a prompt rule with the same enforceability as the one it replaces; a |
1 participant
The producer verified Solidity through a hardcoded
github:rainlanguage/rainix#sol-shellwhile the same sentence said "the repo's" for Rust and TS. That flake tracks rainix HEAD and no repo's CI runs it, so "it passes locally" was never a claim about CI — cyclofinance/cyclo.sol#42 is parked having spent its one permitted back-off attempt producing aforge fmtdiff the repo's own pinnedforge fmt --checkcould not accept.What the survey found, and why "use the repo's flake" is not the fix
Every non-archived repo with a root
foundry.tomlacross the pipeline's org scope (ORGS="rainlanguage cyclofinance S01-Issuer"), 45 repos, classified by what its push/PR-gating workflows actually invoke — the tally below is the built binary's own output over all 45 real workflow sets:rainix-pinuses: rainlanguage/rainix/.github/workflows/rainix-sol.yaml@main→ every step isnix develop github:rainlanguage/rainix/${RAINIX_SHA}#sol-shell -c …, andRAINIX_SHAis declared in the reusable, currently53e96a7d(2026-07-10)repo-flakenix develop -c rainix-sol-<task>→ the calling repo's flake, at its ownflake.lockpin. cyclo.sol, flow, rain.tier.interface, st0x.atomic-bridgeforeignfoundry-rs/foundry-toolchain@v1, three of them atnightly. rain.will-overflow, rain.erc1820, view-quoter-v3, dvin.pocconflictRAINIX_SHA, andgit-clean.yaml(on: [push]) runningforge fmtat unpinned rainix HEADabsentSo the issue's ask — verify with the repo's own flake, as Rust and TS already do — is exactly right for 4 repos and wrong for 33. Those 33 never read their own
flake.lockin a Solidity check;nix develop <dir> -cthere would be the same bug mirrored (rain.math.float pins rainixf22d4dcaca61, 2026-06-01; its CI runs53e96a7d, 2026-07-10).There is therefore no rule to memorise, which is why this is computed per checkout rather than written into the prompt.
The change
pr-review-report sol-toolchain <dir>reads the checkout's own push/PR workflows and prints thenix developprefix its CI will judge with:Exit 3 is the issue's "legible record rather than a silent fallback", and the
mode:line says which of four distinct facts it is — never one undifferentiated "no answer":Steps 4 and 3d take their prefix from it and route exit 3 to the run record, naming
foreignspecifically. Step 1's health check keeps the unpinned URL and a test pins that it does: "can this box realise a rainix shell and run forge at all" is a different question from "will this satisfy that repo's CI", and pinning it would prove a shell no clone enters.Two facts the issue asked to settle
rainprotocol/rainixandrainlanguage/rainixare one repo.gh api repos/rainprotocol/rainixanswersfull_name: rainlanguage/rainix, same head SHA, samepushed_at— an org-rename redirect. Several repos'flake.nixsayrainprotocoland theirflake.lockrecords ownerrainlanguage. The owner path in the issue is cosmetic; the whole skew is the rev.sol-shellanddefaultare the same Solidity toolchain at a given rev. rainix defines onesol-build-inputs(foundry-bin,slither-analyzer,solc_0_8_25) and both shells consume it;sol-shellis only the slimmer closure. So arepo-flakerepo whose CI entersdefaultis correctly verified throughnix develop <dir>, and the reusables'#sol-shellis not a third thing.Dev-shell availability: 40 of the 45 have a
flake.nix, and all 40 re-export rainix's shells wholesale (devShells = rainix.devShells.${system}), sonix develop <dir>resolves everywhere it is asked to. The 5 without one — rain.erc1820, rain.will-overflow, sushixswap-v2, view-quoter-v3, dvin.poc — are the interesting case rather than a harmless one: four of them still gate Solidity, on a forge installed by ACTION.rain.will-overflowrunsforge fmt --checkon foundry nightly as a push gate. There is nonix developthat enters that, so the producer cannot match it — a fact it must state, not one it may read as freedom to pick a shell.mode: foreignsays exactly that and names the toolchain; folding it intoabsentwould leave #116's widest instance reported as "no toolchain here".Known bound, stated rather than implied:
foreignis detected from the ACQUISITION (foundry-rs/foundry-toolchain,foundryup, the paradigm installer), never from a bareforgein arun:— st0x.deploy opensnix develop --command bash -c 'and calls forge on the NEXT line, so an invocation-based reading would report a nix repo as foreign. A repo that gets forge a third way (a container image, a hand-rolled curl) still reads asabsent;a_third_way_of_getting_forge_is_a_known_blind_spotputs that limit on the record.The trigger block is also read in all four spellings GitHub accepts — YAML 1.1 reads a bare
onas the boolean true, so formatters emit"on":and a 1.1 round trip can emittrue:. No org repo writes those today, but the failure mode was a silent misclassification intoabsent, so it is handled rather than assumed. Comments inside that block are stripped, so# runs on pushis prose rather than a gate.Latent paths back into a wrong verdict, closed
Five CodeRabbit findings on this PR, all real, all fixed. None changes any of the 45 org repos' classification — verified by re-running the binary over every one and diffing, which comes back identical — so they were latent, and four of them landed in the same
absentbucket this PR exists to stop being a dumping ground:on:block (on:/# gate every branch/push:read asabsenton a workflow that gates every branch);mode: conflict— a disagreement invented out of a comment;absent, so knowing nothing looked like knowing nothing gates it. There is nowmode: unreadable, and a single unreadable FILE counts;warn:that it floats;dirreached the printed command verbatim while the doc comment claimed absolute. Canonicalized.The sixth — route every exit-3 mode to
flag-design, and giveforeigna non-nix verify command — is rejected on the record: exit-3 modes are properties of the repo's CI, not of the PR, so flagging them re-files one standing fact per PR (the shape #108 retiredflag-blocked-infrafor), and acquiring foundry outside nix is refused by this repo's owntools come from the flakegate (#76).Closes #116
QA
the_verify_steps_take_the_solidity_toolchain_from_the_repos_own_ci— fails on base (mutation 6 restores the base prompt clause byte-for-byte and it goes red).a_non_nix_gating_toolchain_is_never_reported_as_no_toolchainandabsent_and_foreign_say_different_thingsare the follow-up's discriminators: both fail on this PR's own first round, where afoundry-toolchainrepo classified asabsent. The remaining tests cover code that does not exist on base, so their proof is the mutation table.in_on = line.starts_with("on:")→in_on || …→a_push_after_the_on_block_does_not_count;is_sol_check'sforge soldeerexclusion deleted →a_soldeer_install_is_not_a_solidity_check;None | Some(".")→None→a_nix_develop_step_is_read_for_its_flake_and_its_command;Some(Err(why)) => Err(…)→Ok("…rainix#sol-shell -c")→an_unreadable_pin_offers_no_command_at_all;agreed_rainix_sha's disagreement branch deleted →the_leaves_must_agree_on_the_pin; step 4's VERIFY clause reverted to the base hardcoded flake → the prompt test;foreign_sol_toolchainrecognises nothing, folding a non-nix gating toolchain back intoabsent→ killed bya_non_nix_gating_toolchain_is_never_reported_as_no_toolchain+absent_and_foreign_say_different_things+a_conflict_that_includes_a_foreign_toolchain_says_one_side_is_unreachable; the foreign verdict text replaced by the absent text →absent_and_foreign_say_different_things;WORKFLOW_TRIGGER_KEYScut to["on:"]→a_quoted_or_boolean_on_key_is_still_the_trigger_block;strip_yaml_commentreturns the line unchanged →a_comment_inside_the_trigger_block_is_not_a_trigger;is_nix()→true→a_conflict_that_includes_a_foreign_toolchain_says_one_side_is_unreachable;foundry_toolchain_versionnever reads a version →a_non_nix_gating_toolchain_is_never_reported_as_no_toolchain; step 4 stops teachingforeign→ the prompt test. One earlier candidate (workflow_gates_a_push's redundantreturn false) proved EQUIVALENT — the assignment below it already carried the boundary — so the branch was deleted rather than reported as a coverage gap. The five CodeRabbit fixes are mutation-covered too: the block boundary reading the RAW line →a_column_zero_comment_does_not_close_the_trigger_block; the loop not stripping comments →a_commented_out_step_registers_no_toolchain;flake_ref_is_pinnedalways true →a_flake_ref_is_pinned_only_when_it_names_a_revision+an_unpinned_explicit_flake_still_gets_its_command_but_is_flagged_as_floating; the unreadable verdict falling through to the ordinary one →an_unreadable_checkout_is_not_absent; and a per-file read failure swallowed →a_single_unreadable_workflow_file_is_reported_not_dropped. That last mutant SURVIVED at first, because the branch sat in the impure directory walk where no test could reach it;collect_workflowsmoves the decision out and the re-run kills it.rainix.yaml, a reusable consumer'srainix-sol.yaml, st0x.deploy'sgit-clean.yamland rain.will-overflow'stest.yml. The end-to-end check runs the built binary over all 45 real workflow sets;RAINIX_SHA(53e96a7d…) is the live value inrainix-sol-static.yaml@main, read independently of the subcommand.rainprotocol/rainixandrainlanguage/rainixare the same thing. Covered a, b, d, and c in its full shape:absent,foreign,conflictand a flake-lessrepo-flakeare four distinguished outcomes, none of them a silent fallback. Not covered, deliberately: whether a toolchain-mismatch CI failure should stop consuming the back-off attempt — the mismatch is removed at source here and a "doesn't count" carve-out needs a typed discriminant the producer does not have.