Repository navigation
style: make rust-ci green on main (fmt, needless lifetime, dead fn) - #116
Merged
Merged
Conversation
`rust-ci / Cargo check + clippy + fmt` is red on main (b12657f, #113): - cargo fmt diffs in squabble-fight (lib.rs, workflows.rs), plus squabble-cli/fetch.rs and squabble-core/moves.rs; - clippy::needless_lifetimes on workflows.rs `uses_target`; - `fetch::run_with_greens` is never called: `run_bundle` already returns `.greens`, which is what `fight` consumes. Because the fmt step fails, clippy, test, coverage and audit are all skipped on every PR, so this also restores the rest of the pipeline. Locally: check --locked, fmt --check and clippy -D warnings all rc=0; workspace tests green. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0136eszqrQ53Kj7aBH1D4rXK
Contributor
|
Warning Review limit reachedNext included review available in 35 minutes. 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 selected for processing (4)
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 |
This was referenced Sep 30, 2026
hyperpolymath
added a commit
that referenced
this pull request
Oct 1, 2026
…it 5 if not) (#119) Adds `squabble verify-satisfied <owner>/<repo> <pr>`, one implementation of "is this PR actually done?" for the D2 Stop hook and for humans. JSON goes to stdout and a readable list to stderr. **Exit 5** means agent work remains; 0/2/3/4 keep their existing meanings. > ⚠ **Stacked on #114** (base `fix/rollup-neutral-skipped-pass`). When #114 merges, retarget this PR to `main` before merging. Otherwise GitHub closes it when the base branch is deleted. ## What counts as done | # | Condition | Agent item when unmet | |---|---|---| | 1 | Not `CONFLICTING`; mergeability computed; not draft; not `BEHIND` | resolve in-file / re-run / mark ready / update | | 2 | No required context `Failed` or `Missing`. `Pending` is fine; skipped/neutral satisfy but are listed as **evidence-free** | fix the check or the ruleset | | 2b | No **red non-required** check, unless a PR-body line names it **with an issue link** (the 2026-09-15 ruling: a finding becomes an issue, not a blocker, but someone must have looked). An acknowledged red moves to notes and is still shown | fix it, or file an issue and name it in the body | | 3 | Zero unresolved review threads (outdated ones count); no review-bot `CHANGES_REQUESTED`; no review bot still pending | act on or resolve; wait for the bot | | 4 | Code-scanning alerts the PR introduces | **not evaluated yet** and said so in `notes` → #117 | | 5+6 | Automerge armed with SQUASH wherever GitHub allows arming | arm / re-arm with squash | | 7 | Every `[.rules[].type]` on the base, not just required checks. `merge_queue` refuses rather than passing vacuously; unknown types go to notes | — | **Held by a human** (listed, not agent work): human approval, a required deployment, and a CLEAN PR, which GitHub refuses to arm automerge on, so it waits for a human merge. Change requests made only by review bots are agent work, not human-held. ## Shared rules, so `fetch` and `verify-satisfied` can't disagree - `CheckRun::from_github` maps rollup vocabulary. An unknown completed conclusion is `Failed`. - `CheckRun::for_context` decides a required context from **all** same-named runs, and the worst run wins. Both readers used first-match before, so a passing twin could mask a failing one depending on rollup order. ## Evidence - Tests: core 103, cli 44, forge 16, plus the rest, all green. Mutants killed: first-match ordering (2 red), the non-required loop disabled (1), the link requirement dropped (1), the context match dropped (1), digit-after-`#` dropped (1), `ctx_done = true` in pagination (2). - **Live control 1, both arms** (#114): with no acknowledgement it exits **5** naming `rust-ci / Cargo check + clippy + fmt`. After a body line citing #116 it exits **0**, and the acknowledged red is still shown in notes. - **Live control 3** (standards#1076): exit 5, correctly (Debt ratchet red on main; `SonarCloud Code Analysis` required but never emitted → standards#1081). - **Live control 4** (hypatia#883, another session's PR, read only): exit 5 with the same 5 unresolved threads an independent GraphQL count finds. - The "armed + required pending ⇒ exit 0" arm is **unit-tested only**; I found no live PR in that state to exercise it. ## Not in this PR - No SPARK mirror for D1. `done.rs` is a policy table over forge facts with no state machine or invariant; the proof obligations live in `gate_machine`, which #114 mirrors. (#115 tracks getting gnatprove into CI at all.) - Check 4 and inherited-vs-new red checks → #117. The positive control is launch-scaffolder#46's `hardcoded_tmp` alert. - `~/.claude/hooks/apps-ack-gate.sh` is a second implementation of this question → #118. ## Inherited red - `rust-ci / Cargo check + clippy + fmt` is red on `main` (b12657f): unformatted `squabble-fight`, a `needless_lifetimes` in `workflows.rs:628`, and a dead `run_with_greens`. Cured by #116; merge order **#116 → #114 → this**. Checked locally with #116's fix applied temporarily: this branch's own code passes `clippy -D warnings` and `rustfmt --check`. 🤖 Generated with [Claude Code](https://claude.com/claude-code) https://claude.ai/code/session_0136eszqrQ53Kj7aBH1D4rXK --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
hyperpolymath
added a commit
that referenced
this pull request
Oct 1, 2026
## What
`parse_rollup` mapped `SKIPPED` and `NEUTRAL` check conclusions to
`CheckRun::Failed`. GitHub's ruleset treats both as a **met**
requirement, so squabbler reported a failing gate on PRs GitHub would
merge. This is a prerequisite for `verify-satisfied` (D1), which must
agree with GitHub on what "satisfied" means.
**Owner ruling (2026-09-30):** add a new variant, `CheckRun::Skipped`,
rather than folding these into `Passed`. It satisfies the ruleset but
carries no evidence.
## Changes
- `squabble-core/src/gate.rs`
- new `Skipped` variant;
- `is_satisfied` = `Passed | Skipped`;
- new `Gate::evidence_free()` lists checks that were satisfied without
evidence, so consumers can report them separately;
- also moves a pre-existing misplaced doc comment and `#[inline]`, which
had been attached to `with_cause`.
- `squabble-cli/src/fetch.rs`: `SKIPPED | NEUTRAL => Skipped`, plus a
test.
- `spark/src/gate_machine.{ads,adb}`: the mirror, via `Satisfies()`. The
`Evaluate` postcondition and loop invariant are restated over it.
## Evidence
- **SPARK:** `gnatprove -P squabble_gate.gpr -j0 --level=2` proves
**10/10** checks.
- Mutant (body reverted to `/= Passed`): the loop invariant fails, so
the mutant is killed.
- **Rust:** `cargo test --workspace`, all green.
- Mutant (`is_satisfied` = `Passed` only):
`a_skipped_check_satisfies_but_is_listed_as_evidence_free` fails, so the
mutant is killed.
- **JSON output:** serde emits `"skipped"`. The CLI has no
human-readable gate table, so there is no render site to change.
## Known gap
No CI workflow runs `gnatprove`, so the SPARK proof is local evidence
only. I'll file a follow-up issue for it.
## Inherited red
- `rust-ci / Cargo check + clippy + fmt` is red on `main` itself
(b12657f): unformatted `squabble-fight` + a clippy `needless_lifetimes`
+ a dead `run_with_greens`, none introduced here. Cured by #116, which
should merge before this PR.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
https://claude.ai/code/session_0136eszqrQ53Kj7aBH1D4rXK
---------
Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.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.
Why
rust-ci / Cargo check + clippy + fmtis red onmain(b12657f, #113). The fmt step fails first, so clippy never runs, andCargo test,llvm-cov line coverageandCargo auditare all skipped on every PR, #114 included. Nothing in CI has been testing this workspace.What
cargo fmt --all(squabble-fightlib.rs+workflows.rs, squabble-clifetch.rs, squabble-coremoves.rs). Formatting only.clippy::needless_lifetimes:workflows.rsuses_targetnow elides its lifetimes.fetch::run_with_greens. It has no caller:run_bundlealready returns.greens, and that is whatfightconsumes.Evidence (local, same commands as the workflow)
cargo check --locked --all-targetscargo fmt --all -- --checkcargo clippy --locked --all-targets -- -D warningscargo test --workspaceFound by
squabble verify-satisfied(stacked on #114), which initially reported #114 as done while this check was red. That was a gap in the evaluator, and it is fixed in that PR.Merge before #114, so #114's CI is the first to actually run tests.
🤖 Generated with Claude Code
https://claude.ai/code/session_0136eszqrQ53Kj7aBH1D4rXK