fix(ci): governance-enforce gate can pass having scanned nothing - #35
fix(ci): governance-enforce gate can pass having scanned nothing#35yakimoto wants to merge 1 commit into
Conversation
…ead nothing BASE=HEAD~1 scans one commit of a multi-commit push and reports the rest as passing; when HEAD~1 does not resolve it degrades to BASE=HEAD, an empty diff and a green job. Resolve to the empty-tree object so the full tree is scanned, and add merge_group so the gate runs in the merge queue. Refs #1747.
|
Warning Review limit reachedYou’ve reached a temporary PR review limit under our Fair Usage Limits Policy. Next review available in: 53 minutes Your organization has reached its usage spending cap. Adjust your spending cap in the billing tab. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
Comment |
Bugbot couldn't run - usage limit reachedBugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit. A user or team admin can review and increase usage limits in the Cursor dashboard. (requestId: serverGenReqId_c20672c7-2700-4b68-954f-74ad0a4928b6) |
PR Summary by QodoFix governance-enforce diff base to never pass without scanning
AI Description
Diagram
High-Level Assessment
Files changed (1)
|
|
Running ultrareview automatically — This is a security-critical CI gate change: the base-SHA resolution and new merge_group trigger directly control whether the governance scan sees the full diff, so a subtle shell or event-payload bug could let the gate pass without scanning — exactly the failure this PR aims to fix.. I'll post findings when complete. |
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
| # Indeterminate base (first push / force-push). Do NOT silently scan a partial range — | ||
| # HEAD~1 would skip earlier commits in a multi-commit push and let a violation through | ||
| # (a config-no-silent-noop hole). Diff the full tree against git's empty-tree object so | ||
| # every introduced file is scanned; loud, never a silent empty/partial pass. | ||
| BASE=$(git hash-object -t tree /dev/null) | ||
| echo "::warning::indeterminate diff base; scanning full tree (empty-tree base) so no commit is skipped" |
There was a problem hiding this comment.
🟡 Force-pushes and first pushes now fail the required check on pre-existing issues
When the starting point for the comparison cannot be determined, the check now compares against an empty starting point (git hash-object -t tree /dev/null at .github/workflows/governance-enforce.yml:52) instead of a recent commit, so every file in the repository is inspected and any long-standing issue fails the required gate.
Impact: A first push to a branch or a force-push can block merges because of old, unrelated issues that the gate was explicitly designed to tolerate.
Fallback base switches the gate from diff-scoped to whole-repository scope
The workflow header (.github/workflows/governance-enforce.yml:3-6) states the gate is "Diff-scoped: blocks NEW violations without failing on legacy debt", and the org ruleset requires this job's enforce check. Previously the indeterminate-base fallback used HEAD~1 (or HEAD), keeping the scan small. With the empty-tree base, --changed "$BASE" now yields every tracked file, so any legacy violation in capabilities.json, the profile YAMLs, or scripts/ will fail the job. This path triggers on push when github.event.before is all-zeros (new branch / first push) and after force-pushes.
A less disruptive option is to keep a loud warning but resolve a real commit base (e.g. git merge-base origin/main HEAD), or make the full-tree scan report-only / warn-only while still failing on genuinely undeterminable state.
Was this helpful? React with 👍 or 👎 to provide feedback.
| # HEAD~1 would skip earlier commits in a multi-commit push and let a violation through | ||
| # (a config-no-silent-noop hole). Diff the full tree against git's empty-tree object so | ||
| # every introduced file is scanned; loud, never a silent empty/partial pass. | ||
| BASE=$(git hash-object -t tree /dev/null) |
There was a problem hiding this comment.
🔍 Enforcer may not accept a bare tree object as the diff base
--changed "$BASE" is passed to the external @wave-av/governance enforcer (.github/workflows/governance-enforce.yml:56), which is not vendored in this repo, so its handling of the base argument can't be verified here. Git's empty-tree id works with git diff <tree> HEAD, but not with commit-only forms commonly used by such scripts (git diff BASE...HEAD, git merge-base, git log BASE..HEAD, git rev-parse BASE^{commit}), which would abort with a fatal error. Worth confirming against enforce.mjs v0.4.4 that a tree-ish base is supported before relying on this fallback in a required gate.
Was this helpful? React with 👍 or 👎 to provide feedback.
|
|
||
| on: | ||
| pull_request: | ||
| merge_group: |
There was a problem hiding this comment.
🔍 merge_group only added to this workflow
This is the only workflow in .github/workflows/ with a merge_group trigger (the others — _checks.yml, foundation-gate.yml, public-repo-guard.yml — are PR/push only). If a merge queue is enabled and those checks are also required, they will never report on queue entries and could stall the queue. Confirm the queue's required-check set matches the workflows that actually trigger on merge_group.
Was this helpful? React with 👍 or 👎 to provide feedback.
![macroscopeapp[bot]](https://avatars.githubusercontent.com/in/900172?s=60&v=4){kind=small}
ApprovabilityVerdict: Approved e7004e5 Minor CI workflow fix that makes governance enforcement stricter by ensuring the gate cannot pass with partial/empty scans. Author owns this workflow file. No application runtime impact. You can customize Macroscope's approvability policy. Learn more. |
Code Review by Qodo🐞 Bugs (0) 📘 Rule violations (0) 📎 Requirement gaps (0)
Great, no issues found!Qodo reviewed your code and found no material issues that require reviewTo customize comments, go to the Qodo configuration screen, or learn more in the docs. |
Qodo FixerNo findings are available for this PR yet. Findings appear here once Qodo has reviewed the PR. |
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
1 participant
Defect
.github/workflows/governance-enforce.ymlresolved its diff base toHEAD~1, which scans onlyONE commit of a multi-commit push and reports every earlier commit as passing. On an initial push,
force-push, or shallow clone this degrades further:
HEAD~1fails to resolve and falls back toBASE=HEAD, diffing HEAD against itself — an empty diff, zero lines scanned, job green. A gatethat passes without reading any line is worse than no gate.
Changes (targeted,
.github/workflows/governance-enforce.ymlonly)merge_group:trigger alongsidepull_request:/push:— also clears a latentmerge-queue deadlock (the gate previously never ran in the merge queue).
MERGE_BASE_SHA: ${{ github.event.merge_group.base_sha }}to the env of the step thatcomputes
BASE.BASE="${PR_BASE_SHA:-$PUSH_BEFORE_SHA}"becomesBASE="${PR_BASE_SHA:-${MERGE_BASE_SHA:-$PUSH_BEFORE_SHA}}", and the indeterminate-basefallback (previously
git rev-parse HEAD~1 2>/dev/null || git rev-parse HEAD) now diffs againstgit's empty-tree object (
git hash-object -t tree /dev/null) so the full tree is scanned —loud (
::warning::) and never a silent empty/partial pass.wave-av/wave-conferencing-bridge,wave-monitor,wave-desktop, andwave-multivieweralreadycarry the correct form of this file; this brings the same fix to this repo.
Out of scope (tracked separately, claude-workstation#1747)
This repo's gate still has three OTHER open defects tracked in #1747 which are deliberately NOT
addressed here: fail-open token scope, unpinned install scripts, and a caret (
^) version rangeon the
@wave-av/governancedependency.Refs claude-workstation#1747.
Need help on this PR? Tag
@codesmith-botwith what you need. Autofix is enabled.Note
Cursor Bugbot is generating a summary for commit e7004e5. Configure here.
Note
Fix governance-enforce gate to scan full tree when diff base is indeterminate
merge_groupas a workflow trigger in governance-enforce.yml and usesmerge_group.base_shaas the diff base when present.HEAD~1orHEAD.Macroscope summarized e7004e5.