fix(ci): run the full clippy matrix in the merge queue + fix libsql-only dead code - #5840
Conversation
🔎 IronLoop Review StatusHead: Current reviewers:
Reviewer summaries
Recent activity
Available commands
Run metadataAdmission: webhook accepted the request and IronLoop persisted review state before this projection. |
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
📝 WalkthroughSummary by CodeRabbit
WalkthroughThis PR tightens the CI clippy matrix for pull requests, enforces required lanes on merge queue and push runs, and gates two Rust enum variants plus their matching branches behind feature flags. ChangesCI Matrix and Feature Gating
Estimated code review effort: 2 (Simple) | ~10 minutes 🚥 Pre-merge checks | ✅ 3 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (3 passed)
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 |
There was a problem hiding this comment.
Code Review
This pull request introduces conditional compilation attributes (#[cfg(...)]) to gate specific enum variants and their corresponding match arms behind feature flags. Specifically, the Prebuilt variant of FilesystemProductionEventStoresInput is now conditionally compiled under the postgres feature, and the RequestedOnly variant of CallbackScopeResolution is gated under the slack-v2-host-beta feature. There are no review comments, and I have no feedback to provide.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
There was a problem hiding this comment.
✅ IronLoop Review: reviewer
Review at a glance
| Verdict | Blocking | Notes | Inline | Head |
|---|---|---|---|---|
| ✅ Approved | 0 | 0 | 0 | f29230e278f6 |
Head: f29230e278f64b4d5d6c5f040acd5d4a95b25b4a
Next: No reviewer action needed.
Run details
Status: Current
Needs human: no
Needs validation: no
Summary
No concrete blocking issues found in the CI matrix change or feature-gated dead-code fixes.
Findings
None.
Developer follow-up
After fixing this feedback:
- Push the fix to this PR branch.
- Re-run this reviewer with
@ironloopai review --agent reviewerif you only changed this reviewer's findings. - Re-run all reviewers with
@ironloopai reviewwhen the fix may affect multiple areas. - Use
@ironloopai statusto check queued/running/completed/stale/stalled state while reviewers run.
Coverage ratchetReborn integration-tier coverageLine coverage (Reborn crates): 85.16% — 282278 / 331449 lines Per-crate breakdown (65 crates, lowest-covered first)
This table itself is informational and never gates the PR on its own — not the percentage, not the per-crate holes, not the 0-coverage callout. A separate coverage ratchet (dry-run until enforce=true; see tests/integration/coverage-floor.toml) can fail the build on specific configured floors. Exemptions (4 entry/entries excluded from the accounting above)
|
|
🚅 Deployed to the ironclaw-pr-5840 environment in ironclaw-ci-preview
|
…nly dead code The merge queue (the production gate) ran only the slim all-features clippy lane while pushes to main ran the full matrix, so feature-gated dead code passed the queue and broke main post-merge: #5726 landed FilesystemProductionEventStoresInput::Prebuilt, which is constructed only under the postgres feature, and behind it CallbackScopeResolution::RequestedOnly is constructed only under slack-v2-host-beta. Both are dead code in the libsql-only lane, which before this change only ran on push to main. - cfg-gate Prebuilt behind postgres and RequestedOnly behind slack-v2-host-beta (variant + match arm), matching construction reality; #[allow(dead_code)] would hide the signal instead - merge_group now gets the FULL clippy matrix in code_style.yml; PRs keep the slim lane for fast feedback - the Code Style roll-up asserts all three lanes actually ran on merge-queue/push events, so a slim-gate regression cannot silently return (the roll-up is a required check) Regression coverage: the libsql-only clippy lane itself — after this change it runs in the merge queue, and this PR's own queue run exercises it on the merged state. Verified locally with the exact failing lane: cargo clippy --all --tests --examples --no-default-features --features libsql -- -D warnings. Regression coverage is the libsql-only clippy lane itself, which this change adds to the merge queue — there is no unit-testable surface for a cfg attribute + matrix condition. [skip-regression-check] Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
f29230e to
d8d9f78
Compare
…6018) (#6022) * ci: add static pre-push checks — include_str/Docker-COPY + hermetic env (#6018) Categorized main-branch CI history showed the deterministic (non-flaky) breakages share one trait: a cheap static check would catch them, but the pre-push gate ran none. Adds three checks mapped to the failure classes. 1. include_str! path + Docker-COPY coverage (scripts/ci/check-include-str-paths.sh) Every include_str!("…") target must exist AND be present in the build context of each Dockerfile that compiles the referencing crate. Guards the #5603 Docker outage class (host build passes, Docker build fails because a repo-root prompts/ dir was never COPYd). Excludes #[cfg(test)] includes and COPY . . images; attributes per-Dockerfile so Dockerfile.reborn is never blamed for a src/ prompt it doesn't compile. This surfaced a real latent bug: Dockerfile.test builds --bin ironclaw but omitted COPY prompts/ / profiles/ / providers.json (all required by prod consts) — fixed here. 2. Hermetic env guard (scripts/ci/check-hermetic-env.sh) Delta + function-scoped (git diff -W): flags only newly-added raw std::env::set_var/remove_var whose enclosing function lacks an env lock guard (lock_env/lock_runtime_env/ENV_MUTEX/EnvGuard). Targets the #6015 coverage-flake class and Rust 1.82 set_var UB. Quiet on the ~700 existing sites; honors // env-hermetic: for genuine single-threaded cases. 3. libsql-only clippy leg Added to the pre-push strict branch and quality_gate_strict.sh. Catches the cfg/dead_code class (#5840 Prebuilt) that default-feature clippy misses. Wiring: checks 1 & 2 run on every pre-push (no compile); check 3 under IRONCLAW_STRICT_LINT. New required code_style.yml `static-checks` job runs check 1 + both self-test suites (14 cases) server-side. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * ci: harden static pre-push checks per PR #6022 review Addresses review findings on the include_str/Docker-COPY and hermetic-env static checks. Each fix ships with a regression case in the self-tests. check-include-str-paths.sh: - cfg_test_spans: stop a braceless `#[cfg(test)]` item (e.g. `use x;`) at the first `;` instead of running forward to the next unrelated `{`, which swallowed real code and hid its include_str! calls (false negative). - Track full normalized COPY paths and add an `is_covered` nested-path check so a narrowed `COPY crates/foo/` covers crates/foo but not sibling crates (previously top-segment matching treated all of crates/ as copied). - Scan the repo-root build.rs (cargo build compiles it too) and flag include_str! targets that resolve outside the repo — they exist on the host but no Docker COPY can ever include them. check-hermetic-env.sh: - Prefer GITHUB_BASE_REF for base-ref resolution so the check works on a shallow CI checkout that lacks origin/main. - Drop `|| true` on the git diff so a diff failure aborts instead of silently yielding an empty diff that bypasses the guard. - Strip `//` comments before the guard-token test so a bare `// EnvGuard` comment no longer exempts an adjacent raw set_var. code_style.yml: - Broaden the has_code predicate to cover scripts/ci/, .githooks/, and all Dockerfile* so changes to the guardrails themselves run static-checks instead of being skipped (and accepted by the rollup). - Run check-hermetic-env.sh against the actual PR diff, not just the synthetic self-tests, fetching the PR base tip first. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Why
Green merges keep breaking main. The merge queue is the enforced production gate (ruleset "Main": required checks + merge queue), but
code_style.ymlgavemerge_groupthe slim clippy matrix (all-features only) while push-to-main ran the full matrix (all-features, default, libsql-only). Any feature-gated lint failure therefore surfaced only after merge.That's what happened today: #5726 introduced
FilesystemProductionEventStoresInput::Prebuilt, which is constructed only under thepostgresfeature. The queue linted all-features only (wherepostgresis on), and every push run since has been red onClippy (libsql-only):variant Prebuilt is never constructed. Hidden behind it was a second instance the CI log never reached:CallbackScopeResolution::RequestedOnlyis constructed only underslack-v2-host-beta.What
PrebuiltbehindpostgresandRequestedOnlybehindslack-v2-host-beta(variant + match arm), completing the pattern the event-store enum already uses for itsConfigvariant.#[allow(dead_code)]would suppress exactly the signal that caught a real enum-shape/feature mismatch.merge_groupnow runs the full clippy matrix; PRs keep the slim lane for fast feedback; push stays full (post-merge confirmation + cache warming).Code Style (fmt + clippy)roll-up now asserts all three lanes were actually in the matrix on merge_group/push runs, so a slim-gate regression fails loudly instead of passing green-but-hollow.Verification
cargo clippy --all --tests --examples --no-default-features --features libsql -- -D warnings— the exact failing CI lane — passes locally with these changes (it fails on both dead-code errors without them).cargo fmt -p ironclaw_reborn_composition -- --checkclean; actionlint clean on the workflow (pre-existing style nits only).Regression coverage: the libsql-only clippy lane itself — after this change it runs in the merge queue on every merge group.
Pre-existing and deliberately not addressed here: building
ironclaw_reborn_compositionstandalone with bare--features libsql --testshits unrelated dead test doubles (SlackIdentityProviderClientand friends inproduct_auth/serve/oauth.rstests); no CI lane builds that shape today.🤖 Generated with Claude Code