Skip to content

ci: ensure required checks always report status - #34674

Merged
kshitijk4poor merged 2 commits into
mainfrom
ethie/ci-required-checks-always-report
May 29, 2026
Merged

ci: ensure required checks always report status#34674
kshitijk4poor merged 2 commits into
mainfrom
ethie/ci-required-checks-always-report

Conversation

@ethernet8023

Copy link
Copy Markdown
Collaborator

What does this PR do?

Removes paths: filters from the contributor-check and supply-chain-audit workflows so that their required checks always report a status. Currently, when no matching files change (e.g. a nix-only PR), those workflows never run and the checks stay "pending" forever, which blocks merge under the branch protection ruleset.

The actual scanning/attribution work still skips when no relevant files changed — gate jobs with the same check name report success in the skip case.

Related Issue

Unblocks PRs like #33773 that only touch nix files and get stuck with pending required checks.

Fixes #

Type of Change

  • 🐛 Bug fix (non-breaking change that fixes an issue)
  • ✨ New feature (non-breaking change that adds functionality)
  • 🔒 Security fix
  • 📝 Documentation update
  • ✅ Tests (adding or improving test coverage)
  • ♻️ Refactor (no behavior change)
  • 🎯 New skill (bundled or hub)

Changes Made

  • .github/workflows/contributor-check.yml — Removed paths: from on: trigger. Added a Check if relevant files changed step that sets run=true/false. The attribution check step is now gated on steps.filter.outputs.run == 'true'. When skipped, the job still reports success (satisfying the required check).
  • .github/workflows/supply-chain-audit.yml — Removed paths: from on: trigger. Added a changes job that computes scan and deps path-filter outputs. scan and dep-bounds are gated on their respective outputs. Added scan-gate and dep-bounds-gate jobs (with matching name: values) that report success when the real job was skipped. Also fixed dep-bounds — its old condition if: contains(..., 'pyproject.toml') || true was always true; now uses the proper needs.changes.outputs.deps output.

How to Test

  1. Open a PR that only changes nix files (like fix(nix): drop stale "vercel" group from #full variant #33773) — check-attribution, Scan PR for critical supply chain risks, and Check PyPI dependency upper bounds should all report green (either from the real job or the gate job), not stay "pending"
  2. Open a PR that changes *.py files — attribution check should run as before
  3. Open a PR that changes pyproject.toml — both scan and dep-bounds should run as before

Checklist

Code

  • I've read the Contributing Guide
  • My commit messages follow Conventional Commits (fix(scope):, feat(scope):, etc.)
  • I searched for existing PRs to make sure this isn't a duplicate
  • My PR contains only changes related to this fix/feature (no unrelated commits)
  • I've run pytest tests/ -q and all tests pass — or N/A (CI workflow changes only)
  • I've added tests for my changes — or N/A (the gate jobs ARE the test)
  • I've tested on my platform: NixOS

Documentation & Housekeeping

  • I've updated relevant documentation — or N/A
  • I've updated cli-config.yaml.example — or N/A
  • I've updated CONTRIBUTING.md or AGENTS.md — or N/A
  • I've considered cross-platform impact — N/A (GitHub Actions only)
  • I've updated tool descriptions/schemas — or N/A

Screenshots / Logs

PR #33773 status before (3 required checks stuck pending):

check-attribution                           — (pending forever)
Scan PR for critical supply chain risks     — (pending forever)
Check PyPI dependency upper bounds          — (pending forever)

After this change, the gate jobs report success for those same check names when no relevant files changed.

Remove paths filters from contributor-check and supply-chain-audit
workflows. When no matching files changed, the workflows never ran and
the required checks (check-attribution, supply chain scan, dep bounds)
stayed "pending" forever, blocking merge.

Now both workflows always trigger. A path-check step/job determines
whether the real work should run; gate jobs with matching names report
success when the real job was skipped, so branch protection always
gets a check status.

Also fixes dep-bounds: the old condition
  if: contains(github.event.pull_request.changed_files_url, 'pyproject.toml') || true
was always true (the || true made it unconditional). Now uses the
proper changes.deps output from the shared filter job.
@github-actions

github-actions Bot commented May 29, 2026

Copy link
Copy Markdown
Contributor

🔎 Lint report: ethie/ci-required-checks-always-report vs origin/main

ruff

Total: 0 on HEAD, 0 on base (➖ 0)

🆕 New issues: none

✅ Fixed issues: none

Unchanged: 0 pre-existing issues carried over.

ty (type checker)

Total: 9447 on HEAD, 9447 on base (➖ 0)

🆕 New issues: none

✅ Fixed issues: none

Unchanged: 4905 pre-existing issues carried over.

Diagnostics are surfaced as warnings — this check never fails the build.

@alt-glitch alt-glitch added type/refactor Code restructuring, no behavior change P2 Medium — degraded but workaround exists labels May 29, 2026
@ethernet8023
ethernet8023 requested a review from alt-glitch May 29, 2026 15:15
@kshitijk4poor

Copy link
Copy Markdown
Collaborator

Reviewed — this is correct and well-scoped. Thanks for catching this.

The bug is real and documented. Per GitHub's docs: a workflow skipped by a paths: filter leaves its required check "Pending" forever, blocking merge. Confirmed against the live branch ruleset — the only required checks that currently carry paths: filters are exactly the three this PR touches (check-attribution, Scan PR for critical supply chain risks, Check PyPI dependency upper bounds). No other required check has the same latent issue, so the scope is precise.

The dep-bounds fix is a genuine catch. The old if: contains(github.event.pull_request.changed_files_url, 'pyproject.toml') || true is always-true because of the trailing || true, so dep-bounds ran on every PR regardless. Replacing it with the proper needs.changes.outputs.deps output is the right fix.

Both halves are mechanically sound per GitHub's own semantics:

  • contributor-check.yml keeps a single always-running job with a job-internal if: step gate — the textbook recommended pattern; the required context always reports SUCCESS.
  • supply-chain-audit.yml uses the duplicate-name: gate pattern. Per the docs, a job skipped via conditional reports Success, and checks sharing a required name are aggregated — so in both the scan=true and scan=false cases the named check always reports a status.

One minor, non-blocking note: the scan / dep-bounds / gate jobs all needs: changes. If the changes job itself fails (checkout/git diff error), its dependents are skipped due to a failed dependency — which GitHub treats differently from a conditional skip, and the required checks could land unreported rather than Success. The changes job is trivial so this is low-probability, but adding if: always() && needs.changes.outputs.scan == 'false' (and the deps equivalent) on the gate jobs would make it bulletproof. Not required to merge.

Note that #33773 has since merged on its own, so the immediate motivation is resolved — but the underlying mechanism bug remains and will block the next nix/docs-only PR, so this is still worth landing.

The scan-gate / dep-bounds-gate jobs use needs.changes; if the changes
job itself fails, its dependents would be skipped via a failed dependency
(not a conditional skip), leaving the required check unreported — the same
"pending forever" failure this PR fixes. Add always() and switch the gate
condition from == 'false' to != 'true' so the gate still fires (and reports
SUCCESS) when changes fails and its output is empty.
@kshitijk4poor
kshitijk4poor enabled auto-merge (rebase) May 29, 2026 16:15
@kshitijk4poor
kshitijk4poor merged commit 5cd0673 into main May 29, 2026
20 checks passed
@kshitijk4poor
kshitijk4poor deleted the ethie/ci-required-checks-always-report branch May 29, 2026 16:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

P2 Medium — degraded but workaround exists type/refactor Code restructuring, no behavior change

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants