feat(rules): BP009 — phantom required context in a ruleset - #658
Conversation
BP008 already detects phantom required contexts, but it has two structural blind spots that let the estate's worst phantom through. Both measured 2026-07-29 against live data. 1. RULESETS ARE INVISIBLE. BP008 reads only classic branches/main/protection. There is no reference to 'ruleset' anywhere in this module. Measured across 418 estate repos: 236 required-context rows live in rulesets. BP008 sees almost none of them. 2. IT SAMPLES THE WRONG BRANCH. BP008 walks default-branch commits; merges are gated on PR heads. For 'Dependabot' the two disagree decisively: 10 check-runs on main (GitHub's managed dynamic/dependabot/dependabot-updates runner, fired on a schedule and never on a PR) versus 0 on any PR head. BP008 would have scored it HEALTHY while it made every PR in 25 repos unmergeable -- a false negative, not merely a gap. The inverse error is equally real: a pull_request-only workflow never appears on main, so sampling main invents phantoms that do not exist. BP009 therefore reads ACTIVE branch-target rulesets and samples PR heads, checking both check-runs and commit statuses since a context may be satisfied by either. Severity :critical rather than :high -- this is a proven merge-stopper. The suggested fix carries the two API traps found while remediating it: emptying required_status_checks returns HTTP 422 so the whole rule must be dropped, and a ruleset PUT replaces the object so bypass_actors must be preserved from a live GET. Verified: elixirc compiles clean (exit 0, .beam produced); the only warning is the pre-existing Jason-outside-mix one. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
![gitar-bot[bot]](https://avatars.githubusercontent.com/in/827041?s=60&v=4){kind=small}
|
Note Automatic reviews are paused because your trial's included automatic processing has been used for this period. Upgrade now, or comment "Gitar review" to run a review anytime. Code Review ✅ Approved 2 resolved / 2 findingsIntroduces BP009 to detect phantom required contexts in rulesets using PR head sampling, but flags every context critical when no PRs are sampled and fails to paginate ruleset lists. ✅ 2 resolved✅ Edge Case: Ruleset list not paginated; contexts beyond page 1 missed
✅ Edge Case: BP009 flags every context critical when no PRs are sampled
OptionsDisplay: compact → Showing less information. Comment with these commands to change the behavior for this request:
Was this helpful? React with 👍 / 👎 | Gitar |
…sers (#659) The rule is named "in hot path" but had no concept of one — `pattern: ~r/\.expect\(/`, a bare substring match over the whole file. So it fires on: ```rust static RE: LazyLock<Regex> = LazyLock::new(|| Regex::new(..).expect(..)); ``` which compiles **exactly once** on first access. **Measured on gitbot-fleet, 2026-07-29:** of 28 `expect_in_hot_path` findings, **11 were this shape**. The remediation those findings invite — *"cache it with once_cell"* — describes code that **already exists**, so the finding could never be cleared by following its own advice. That is worse than a miss: it spends reviewer trust, and it is the kind of noise that drives people to baseline a whole gate. Adds an opt-in `context: :runtime_path` field. Rules carrying it match against a copy with lazy-initialiser bodies elided; every other rule is untouched. Follows the existing `strip_inline_test_blocks/2` precedent rather than inventing a second mechanism. **Verified against the real files, both directions:** | file | before | after | |---|---|---| | `deps.rs` — every `expect` inside `LazyLock` | flagged | **`[]`** | | `contrast.rs` — genuine per-call `Regex::new` in a fn | flagged | **6 occurrences still detected** | The rule keeps its teeth and loses its false alarms. Follow-on from #658. Two sibling imprecisions found in the same triage and **not** fixed here: `actions_expression_injection` fires when untrusted context is correctly bound via `env:` and never interpolated into `run:`; `secret_action_without_presence_gate` fires when a presence gate is already present. 🤖 Generated with [Claude Code](https://claude.com/claude-code)
1 participant
BP008 detects phantom required contexts but has two structural blind spots. Both measured 2026-07-29 against live estate data.
1. Rulesets are invisible. BP008 reads only classic
branches/main/protection— the word "ruleset" does not appear in the module. Measured across 418 repos: 236 required-context rows live in rulesets.2. It samples the wrong branch. BP008 walks default-branch commits; merges are gated on PR heads. For
Dependabotthese disagree decisively:DependabotonmainDependaboton a PR headSo BP008 would have scored it healthy while it made every PR in 25 repos unmergeable. That is a false negative, not a gap. The inverse error is also real: a
pull_request-only workflow never appears on main, so sampling main invents phantoms.BP009 reads active branch-target rulesets and samples PR heads, checking check-runs and commit statuses (a context may be satisfied by either). Severity
:critical— a proven merge-stopper.Verified:
elixirccompiles clean (exit 0, .beam produced); only the pre-existingJason-outside-mix warning.🤖 Generated with Claude Code