Skip to content

fix(github): hold non-required third-party action_required checks instead of auto-closing - #4423

Merged
JSONbored merged 1 commit into
mainfrom
claude/fix-third-party-check-manual-hold
Jul 9, 2026
Merged

fix(github): hold non-required third-party action_required checks instead of auto-closing#4423
JSONbored merged 1 commit into
mainfrom
claude/fix-third-party-check-manual-hold

Conversation

@JSONbored

Copy link
Copy Markdown
Owner

Summary

  • A completed action_required check-run from a third-party GitHub App was treated as a settled CI failure regardless of whether it was an actual branch-protection required context. Superagent posts a "Contributor trust" check-run alongside its own, separately-required "Superagent Security Scan" check -- but "Contributor trust" itself was never a required context. Real contributor PRs were getting auto-closed citing "CI is failing (Contributor trust)" even though branch protection never required that check.
  • reduceLiveCiAggregate (src/github/backfill.ts) now only hard-fails a third-party action_required check when isRequired(name) confirms it actually is a required context. A non-required one is routed to the existing (previously always-empty) nonRequiredFailingDetails bucket -- it never flips ciState or blocks merge, but it's also never silently dropped: it renders under its own "Flagged checks (non-blocking)" collapsible section in the PR comment (src/review/unified-comment.ts), wired through src/queue/processors.ts's existing MergeReadiness construction.
  • Relates to feat(gate): configurable advisory check-runs so a non-resolving external status never blocks or stalls the gate forever #4372 (a non-resolving external status should never silently block or silently pass). This is a narrower fix scoped to the specific regression: it does not build the full config-driven allowlist feat(gate): configurable advisory check-runs so a non-resolving external status never blocks or stalls the gate forever #4372 describes, but it stops the concrete harm (false auto-close) using data the aggregate already collects, with no new settings surface or migration.

Scope

  • The PR title follows type(scope): short summary Conventional Commit format, for example fix(api): restore profile access checks.
  • This PR is focused and does not mix unrelated backend, UI, MCP, docs, dependency, and deploy changes.
  • This follows CONTRIBUTING.md and does not reintroduce GitHub Pages, VitePress, site/, or CNAME.
  • I linked a currently open issue this PR resolves (e.g. Closes #123) -- owner-authored maintenance/incident fix, relates to feat(gate): configurable advisory check-runs so a non-resolving external status never blocks or stalls the gate forever #4372 without closing it (the full config-driven system it describes isn't built here).

Validation

  • git diff --check
  • npm run typecheck
  • npm run test:coverage locally (full unsharded run, 12,507 tests passed); 100% branch coverage confirmed on every changed line across the 3 touched src/** files
  • npm audit --audit-level=moderate (0 vulnerabilities)
  • npm run test:ci (full local gate, exit 0)
  • New or changed behavior has unit/integration tests for new branches, fallback paths, and sanitizer boundaries -- added regression tests in test/unit/backfill.test.ts (required vs. non-required third-party action_required, with/without summary+detailsUrl), test/unit/unified-comment.test.ts (render + verbosity + angle-escaping + blank-name filtering), and test/unit/queue.test.ts (two full webhook-processing end-to-end tests: a flagged-with-detail case and a bare no-output case)

Safety

  • No secrets, wallet details, hotkeys, coldkeys, user PATs, private keys, raw trust scores, private rankings, or private maintainer evidence are exposed.
  • Public GitHub text stays sanitized, low-noise, and does not imply compensation guarantees or optimization tactics.
  • No auth/cookie/CORS/GitHub App/Cloudflare/session changes in this PR.
  • No API/OpenAPI/MCP surface changed.
  • No UI changes (this is a backend + PR-comment-rendering fix).
  • No visible product UI changed -- no UI Evidence section needed.
  • No changelog/public docs changes needed for this fix.

Notes

  • Verified live via gh api before this PR: JSONbored/gittensory's branch protection required contexts are ["Superagent Security Scan", "validate"] -- "Contributor trust" is a separate, never-required check-run posted by the same app, confirming this is the exact real-world shape the fix targets.

…tead of auto-closing (#4414)

A completed action_required check-run from a third-party app was treated as a hard CI failure
regardless of whether it was an actual branch-protection required context. Superagent posts
"Contributor trust" alongside its own required "Superagent Security Scan" check, but
"Contributor trust" itself is never required -- so real contributor PRs were auto-closed on a
signal branch protection never asked for.

reduceLiveCiAggregate now only hard-fails a third-party action_required check when isRequired()
confirms it's an actual required context. A non-required one is routed to the existing
nonRequiredFailingDetails bucket instead -- never flipping ciState/blocking merge, but still
rendered under its own "Flagged checks (non-blocking)" section in the PR comment so it's never
silently dropped either.
@superagent-security

Copy link
Copy Markdown
Contributor

Superagent didn't find any vulnerabilities or security issues in this PR.

@codecov

codecov Bot commented Jul 9, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 93.98%. Comparing base (6f40ce1) to head (708cbd8).
⚠️ Report is 2 commits behind head on main.
✅ All tests successful. No failed tests found.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #4423   +/-   ##
=======================================
  Coverage   93.98%   93.98%           
=======================================
  Files         406      406           
  Lines       36993    37017   +24     
  Branches    13521    13532   +11     
=======================================
+ Hits        34767    34791   +24     
  Misses       1570     1570           
  Partials      656      656           
Files with missing lines Coverage Δ
src/github/backfill.ts 97.07% <100.00%> (+0.01%) ⬆️
src/queue/processors.ts 95.38% <100.00%> (+<0.01%) ⬆️
src/review/unified-comment.ts 99.57% <100.00%> (+0.02%) ⬆️
🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@JSONbored
JSONbored merged commit 7967e78 into main Jul 9, 2026
10 checks passed
@JSONbored
JSONbored deleted the claude/fix-third-party-check-manual-hold branch July 9, 2026 13:43
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