Skip to content

fix(github): treat completed third-party action_required checks as failing/manual-hold - #4414

Merged
JSONbored merged 1 commit into
mainfrom
codex/fix-incorrect-handling-of-action_required-checks
Jul 9, 2026
Merged

fix(github): treat completed third-party action_required checks as failing/manual-hold#4414
JSONbored merged 1 commit into
mainfrom
codex/fix-incorrect-handling-of-action_required-checks

Conversation

@JSONbored

Copy link
Copy Markdown
Owner

Motivation

  • A recent change classified a completed third-party check-run with conclusion === "action_required" as non-blocking/passing, allowing the agent to auto-approve and auto-merge PRs even when an external app requested human action.
  • The goal is to preserve the fork-approval behavior for github-actions while treating completed third-party action_required verdicts as adverse/manual-hold signals so they cannot be bypassed by auto-maintain logic.

Description

  • Change in src/github/backfill.ts: add isThirdPartyActionRequiredFailure and treat a completed non-github-actions action_required check-run as a failing/manual-hold result by pushing it into failingDetails instead of letting it count as passing or pending.
  • Preserve the existing github-actions special-case so an Actions workflow awaiting "Approve and run" remains pending.
  • Update the explanatory comment near CI_FAILING_CONCLUSIONS to reflect the new semantics for third-party tools.
  • Update the regression in test/unit/backfill.test.ts to expect ciState === "failed" and a corresponding failingDetails entry for a completed third-party action_required check (the test previously asserted passed).

Testing

  • Ran npx vitest run test/unit/backfill.test.ts -t "action_required" --reporter=dot, which executed the targeted regression tests for the changed behavior and they passed.
  • Ran npm run typecheck which completed successfully.
  • Ran git diff --check which reported no whitespace/conflict issues.
  • A full run of npx vitest run test/unit/backfill.test.ts --reporter=dot in this environment hit unrelated timing failures (two existing tests timed out) that are not related to the change; those environment timeouts should be re-run in CI or locally with increased test timeout if needed.

Codex Task

@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.96%. Comparing base (64fb1a3) to head (c499f4f).
⚠️ Report is 6 commits behind head on main.
✅ All tests successful. No failed tests found.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #4414   +/-   ##
=======================================
  Coverage   93.96%   93.96%           
=======================================
  Files         401      401           
  Lines       36892    36892           
  Branches    13490    13490           
=======================================
  Hits        34667    34667           
  Misses       1569     1569           
  Partials      656      656           
Files with missing lines Coverage Δ
src/github/backfill.ts 97.05% <100.00%> (ø)
🚀 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 faf5125 into main Jul 9, 2026
10 checks passed
@JSONbored
JSONbored deleted the codex/fix-incorrect-handling-of-action_required-checks branch July 9, 2026 11:27
andriypolanski pushed a commit to andriypolanski/gittensory that referenced this pull request Jul 9, 2026
…tead of auto-closing (JSONbored#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.
JSONbored added a commit that referenced this pull request Jul 10, 2026
…ng PRs on repos with no branch protection (#4735) (#4736)

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