fix(github): stop a third-party action_required check from auto-closing PRs on repos with no branch protection - #4736
Conversation
…ng PRs on repos with no branch protection (#4735) reduceLiveCiAggregate gated a third-party app's own action_required verdict (e.g. Superagent's advisory-only "Contributor trust" check) on isRequired(name), which defaults to true for every check name whenever a repo has NO branch-protection required-status-checks configured at all (enforceRequiredOnly false). That silently reopened #4414 for any such repo: confirmed live, JSONbored/metagraphed's required_status_checks .contexts is [], and PR #4812 there was auto-closed this morning with the literal reason "CI is failing (Contributor trust)" despite every real CI check green and the review panel's own gate result correctly reading "Advisory; not blocking". Adds isConfirmedRequired (enforceRequiredOnly && requiredContexts.has), the deliberate opposite default from isRequired, and uses it only for this one branch. isRequired() itself is untouched -- a genuine CI failure should still fail closed when required-ness is unconfirmed; a third-party app's advisory opinion should not.
|
Superagent didn't find any vulnerabilities or security issues in this PR. |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #4736 +/- ##
=======================================
Coverage 94.22% 94.22%
=======================================
Files 439 439
Lines 38704 38705 +1
Branches 14100 14100
=======================================
+ Hits 36468 36469 +1
Misses 1576 1576
Partials 660 660
🚀 New features to boost your workflow:
|
|
Warning 🟨🟨🟨🟨🟨🟨🟨🟨🟨🟨🟨🟨 ⏸️ Gittensory review result - manual review recommendedReview updated: 2026-07-10 22:03:06 UTC
⏸️ Suggested Action - Manual Review
Review summary Nits — 6 non-blocking
Linked issue satisfactionAddressed Review context
Contributor next steps
Signal definitions
🟩 Safe / merged · 🟦 Advisory · 🟨 Held for review · 🟥 Blocked / closed 💰 Earn for open-source contributions like this. Gittensor lets GitHub contributors earn for the work they already do — register to start earning →. Checked by Gittensory, a quiet PR intelligence layer for OSS maintainers.
|
Summary
JSONbored/metagraphed, contributorjimcody1995) was auto-closed this morning with the reasonCI is failing (Contributor trust)— despite every real CI check green (tests, ui, codecov/patch, codecov/project) and despite fix(github): treat completed third-partyaction_requiredchecks as failing/manual-hold #4414 having previously fixed exactly this class of bug.reduceLiveCiAggregategates a third-party app's ownaction_requiredverdict (Superagent's advisory-only "Contributor trust" check) onisRequired(name), which is!enforceRequiredOnly || requiredContexts.has(name)— defaults totruefor every check name whenever a repo has no branch-protection required-status-checks configured at all. Confirmed live:JSONbored/metagraphed'srequired_status_checks.contextsis[]. fix(github): treat completed third-partyaction_requiredchecks as failing/manual-hold #4414's fix only actually protects a repo that has some required contexts configured; a repo with none falls through the "assume required when unknown" fail-safe and reopens the exact bug.isConfirmedRequired(enforceRequiredOnly && requiredContexts.has(name)) used only for this one branch — requires POSITIVE confirmation instead of "unknown → assume yes".isRequired()itself is untouched and still governs genuine CI-failure detection (where "unknown → assume required" is the correct fail-safe).Closes #4735.
Scope
type(scope): short summaryConventional Commit format.CONTRIBUTING.mdand does not reintroduce GitHub Pages, VitePress,site/, orCNAME.Validation
git diff --checknpm run actionlint— skipped, no.github/workflows/**changes.npm run typechecknpm run test:coverage(targeted, not the full unsharded suite — see Notes) locally; every changed line insrc/github/backfill.tsconfirmed hit via lcov (both the true and false branch of the newisConfirmedRequiredgate).npm run test:workers— skipped, this function has no Worker-runtime-specific behavior beyond what the unit suite covers.npm run build:mcp/npm run test:mcp-pack— skipped, no MCP package changes.npm run ui:openapi:check/ui:lint/ui:typecheck/ui:test/ui:build— skipped, noapps/gittensory-ui/**or API-shape changes.npm audit --audit-level=moderateSafety
Notes
Test coverage: extended
test/unit/backfill.test.tswith two new regression tests, both reproducing PR #4812's exact real-world scenario:requiredContextsSet (new Set(), matchingmetagraphed's live branch-protection response exactly) — the third-partyaction_requiredcheck-run now correctly lands innonRequiredFailingDetails,ciStatestays"passed".requiredContexts: null(a distinct origin — the required-status-checks fetch outright failed/403'd, rather than confirming an empty list) — same expected outcome, since "unconfirmed" should behave identically regardless of why it's unconfirmed.Both new tests were verified to actually fail against the pre-fix
isRequired()gate (temporarily reverted the one-line change, confirmed both tests fail withciState: "failed"as PR #4812 actually did, then restored the fix) before being finalized — not just passing vacuously. The full existingbackfill.test.tssuite (236 tests, including all 4 pre-existing #4414 tests) plus the broader consumer suites (queue.test.ts,unified-comment.test.ts, 892 tests) pass unchanged, confirming no regression for repos that DO configure required contexts.Not retroactive: this fix prevents the bug going forward; it does not reopen or re-review PR #4812 itself (closed PRs are never automatically reopened, per the bot's own close message) — the contributor will need to open a fresh PR once this deploys.