Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
yakimoto wants to merge 1 commit into
base: main
Choose a base branch
from
Open

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

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 9 additions & 2 deletions .github/workflows/governance-enforce.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ name: governance-enforce

on:
pull_request:
merge_group:

@devin-ai-integration devin-ai-integration Bot Aug 6, 2026

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 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.

Open in Devin Review

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

push:
branches: [main, master]

Expand Down Expand Up @@ -39,11 +40,17 @@ jobs:
- name: A_BLOCK enforce (secrets + hardcoded paths on the diff)
env:
PR_BASE_SHA: ${{ github.event.pull_request.base.sha }}
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}}"
if [ -z "$BASE" ] || [ "$BASE" = "0000000000000000000000000000000000000000" ]; then
BASE=$(git rev-parse HEAD~1 2>/dev/null || git rev-parse HEAD)
# 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)

@devin-ai-integration devin-ai-integration Bot Aug 6, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🔍 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.

Open in Devin Review

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

echo "::warning::indeterminate diff base; scanning full tree (empty-tree base) so no commit is skipped"
Comment on lines +48 to +53

@devin-ai-integration devin-ai-integration Bot Aug 6, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🟡 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.

Open in Devin Review

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

fi
echo "diffing against $BASE"
node "$RUNNER_TEMP/gov/node_modules/@wave-av/governance/bin/enforce.mjs" --changed "$BASE"
Loading