Skip to content

fix(ci): governance-enforce gate can pass having scanned nothing - #39

Open
yakimoto wants to merge 2 commits into
mainfrom
fix/1747-enforce-diff-base-fail-open
Open

yakimoto wants to merge 2 commits into
mainfrom
fix/1747-enforce-diff-base-fail-open

Conversation

@yakimoto

@yakimoto yakimoto commented Aug 6, 2026

Copy link
Copy Markdown
Contributor

Defect

.github/workflows/governance-enforce.yml resolved its diff base to HEAD~1, which scans only
ONE 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~1 fails to resolve and falls back to
BASE=HEAD, diffing HEAD against itself — an empty diff, zero lines scanned, job green. A gate
that passes without reading any line is worse than no gate.

Changes (targeted, .github/workflows/governance-enforce.yml only)

  1. Add a merge_group: trigger alongside pull_request: / push: — also clears a latent
    merge-queue deadlock (the gate previously never ran in the merge queue).

  2. Add MERGE_BASE_SHA: ${{ github.event.merge_group.base_sha }} to the env of the step that
    computes BASE.

  3. Replace the base resolution: BASE="${PR_BASE_SHA:-$PUSH_BEFORE_SHA}" becomes
    BASE="${PR_BASE_SHA:-${MERGE_BASE_SHA:-$PUSH_BEFORE_SHA}}", and the indeterminate-base
    fallback (previously git rev-parse HEAD~1 2>/dev/null || git rev-parse HEAD) now diffs against
    git'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.

  4. Raise the enforcer floor to @wave-av/governance@^0.4.6: 0.4.4/0.4.5 diffed <base>...HEAD
    (three-dot requires a commit base) inside a fail-open catch, so the empty-tree fallback would
    have scanned 0 files and passed; 0.4.6 diffs <base> HEAD (accepts a tree) and fails closed.

wave-av/wave-conferencing-bridge, wave-monitor, wave-desktop, and wave-multiviewer already
carry 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 range
on the @wave-av/governance dependency.

Refs claude-workstation#1747.


View with [code]smith
Need help on this PR? Tag @codesmith-bot with what you need. Autofix is enabled.


Note

Cursor Bugbot is generating a summary for commit e5e1636. Configure here.

Review in cubic

Note

Fix governance-enforce gate to scan full tree when diff base is indeterminate

  • Adds merge_group as a workflow trigger in governance-enforce.yml so the gate runs on merge queue events
  • When no valid diff base is found, replaces the HEAD~1 fallback with a diff against the empty-tree object (git hash-object -t tree /dev/null) and emits a workflow warning, ensuring nothing is silently skipped
  • Adds MERGE_BASE_SHA from github.event.merge_group.base_sha with precedence after PR_BASE_SHA but before PUSH_BEFORE_SHA
  • Bumps @wave-av/governance from ^0.4.4 to ^0.4.6

Macroscope summarized 5e2d08e.

…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.
@cursor

cursor Bot commented Aug 6, 2026

Copy link
Copy Markdown

Bugbot couldn't run - usage limit reached

Bugbot 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_d1c16997-2e82-4411-992b-1c79ddf1dd47)

@coderabbitai

coderabbitai Bot commented Aug 6, 2026

Copy link
Copy Markdown

Warning

Review limit reached

You’ve reached a temporary PR review limit under our Fair Usage Limits Policy.

Your recent review volume is higher than typical usage, so adaptive limits are currently applied.

Next review available in: 48 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 @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

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 configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 1e47a5ca-7943-4e08-ad3c-9c87774859cc

📥 Commits

Reviewing files that changed from the base of the PR and between 7107247 and 5e2d08e.

📒 Files selected for processing (1)
  • .github/workflows/governance-enforce.yml

Comment @coderabbitai help to get the list of available commands.

@qodo-code-review

Copy link
Copy Markdown

PR Summary by Qodo

Fix governance-enforce to never pass without scanning (merge_group + empty-tree base)

🐞 Bug fix ⚙️ Configuration changes 🕐 10-20 Minutes

Grey Divider

AI Description

• Ensure governance-enforce runs in merge queue by adding merge_group trigger.
• Fix diff-base selection to cover multi-commit pushes and merge_group base_sha.
• Prevent silent no-op scans by falling back to empty-tree diff with a warning.
Diagram

graph TD
  A["GitHub Events"] --> B["governance-enforce.yml"] --> C["Resolve diff BASE"] --> D["governance enforce.mjs"]
  C --> E["git empty-tree"]
  subgraph Legend
    direction LR
    _evt["Event"] ~~~ _wf["Workflow"] ~~~ _step["Step"] ~~~ _git["Git object"]
  end
Loading
High-Level Assessment

The following are alternative approaches to this PR:

1. Compute base via git merge-base (with fetch-depth: 0)
  • ➕ More semantically accurate base for PR/merge scenarios than event-provided SHAs in some edge cases
  • ➕ Works even if event payload base is missing/malformed when history is available
  • ➖ Requires full history (or sufficient depth), increasing checkout time and complexity
  • ➖ Does not fully solve first-push/force-push indeterminate-base cases without additional fallback logic
2. Fail closed on indeterminate base (hard error)
  • ➕ Maximizes safety: never produces a potentially incomplete scan result
  • ➕ Simple to reason about for compliance gates
  • ➖ Higher operational friction (blocks first push/force-push/shallow scenarios)
  • ➖ May cause noisy false blocks if GitHub event metadata is temporarily unavailable

Recommendation: The PR’s approach is a good balance for a gating workflow: it prefers the best available event-specific base SHA (PR, then merge_group, then push-before), and when that’s indeterminate it forces a full-tree scan via the empty-tree object and emits a warning. This avoids both partial-range scans (e.g., multi-commit push) and silent no-op passes, while keeping the workflow non-blocking for legitimate first-push/force-push cases.

Files changed (1) +9 / -2

Bug fix (1) +9 / -2
governance-enforce.ymlFix diff-base selection and enable merge queue execution +9/-2

Fix diff-base selection and enable merge queue execution

• Adds a merge_group trigger so the governance gate runs for merge queue entries. Expands diff-base resolution to prefer PR base SHA, then merge-group base_sha, then push 'before' SHA. Replaces the unsafe HEAD~1/HEAD fallback with an empty-tree base (plus a warning) to guarantee a full-tree scan when the base is indeterminate.

.github/workflows/governance-enforce.yml

devin-ai-integration[bot]

This comment was marked as resolved.

macroscopeapp[bot]
macroscopeapp Bot previously approved these changes Aug 6, 2026
@macroscopeapp

macroscopeapp Bot commented Aug 6, 2026

Copy link
Copy Markdown

Approvability

Verdict: Approved 5e2d08e

CI workflow change to fix governance gate that could silently pass without scanning files. Author owns this workflow file. The unresolved comment about empty-tree compatibility is addressed by the explicit version bump to ^0.4.6, which uses two-arg diff form accepting tree objects.

You can customize Macroscope's approvability policy. Learn more.

@cubic-dev-ai

cubic-dev-ai Bot commented Aug 6, 2026

Copy link
Copy Markdown

Running ultrareview automatically — This edits the governance-enforce CI gate's diff-base logic and adds a merge-queue trigger, so a subtle error could silently skip scanning and let secrets or hardcoded paths merge.. I'll post findings when complete.

@qodo-code-review

Copy link
Copy Markdown

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 📎 Requirement gaps (0)

Grey Divider

Great, no issues found!

Qodo reviewed your code and found no material issues that require review

Grey Divider

To customize comments, go to the Qodo configuration screen, or learn more in the docs.

Qodo Logo

@qodo-code-review

Copy link
Copy Markdown

Qodo Fixer

No findings are available for this PR yet. Findings appear here once Qodo has reviewed the PR.

… fail open

Co-authored-by: Codesmith <codesmith-bot@users.noreply.github.com>

@devin-ai-integration devin-ai-integration Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Devin Review found 1 new potential issue.

Open in Devin Review

Comment on lines +46 to +49
MERGE_BASE_SHA: ${{ github.event.merge_group.base_sha }}
PUSH_BEFORE_SHA: ${{ github.event.before }}
run: |
BASE="${PR_BASE_SHA:-$PUSH_BEFORE_SHA}"
BASE="${PR_BASE_SHA:-${MERGE_BASE_SHA:-$PUSH_BEFORE_SHA}}"

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔍 merge_group base_sha resolvability under the checked-out ref

For merge_group events, checkout runs against the gh-readonly-queue/... ref with fetch-depth: 0, so github.event.merge_group.base_sha should be an ancestor and resolvable by git diff. Worth confirming once in a real merge-queue run: with 0.4.6 now failing closed on git errors, an unresolvable base would hard-fail the required check and stall the queue rather than silently passing.

Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant