Skip to content

Respect required CI checks#16

Open
SeanMcTex wants to merge 6 commits into
mainfrom
respect-required-ci-checks
Open

Respect required CI checks#16
SeanMcTex wants to merge 6 commits into
mainfrom
respect-required-ci-checks

Conversation

@SeanMcTex
Copy link
Copy Markdown
Owner

Summary

This PR is based on #10 by @lukelabonte (Luke LaBonte), rebased cleanly onto main and extended with a few additional fixes. All four of Luke's commits are preserved with his authorship intact.

What Luke's work does

  • Fetches required check metadata from GitHub's GraphQL API (isRequired, branchProtectionRule).
  • Treats missing required contexts as pending, so a required check that hasn't reported yet doesn't show as green.
  • Introduces a notStarted build status (gray play.slash icon) for PRs where required CI hasn't kicked off yet — previously these showed a misleading green checkmark.
  • Keeps non-required checks out of the main merge-blocking status.
  • Shows approval-gated non-blocking checks as a secondary summary row ("Non-blocking: 1 waiting for approval").
  • Unifies the single-PR fetch path to use the same GraphQL query as the batch path, removing the GHPRViewResponse struct.
  • Adds tests for required-check handling, missing required contexts, and the non-blocking summary.

What was added on top

  • Replace name-based approval gate detection with GitHub's WAITING status. The original used name heuristics ("approve"/"approval" in check names) to identify approval gates. In GitHub Actions, CheckRun.status == WAITING is the authoritative signal — set exclusively when a job is blocked on environment protection approval. The heuristic functions (isApprovalGate, approvalGateName, parentWorkflowNameForApprovalGate) are removed; WAITING status drives all approval gate logic. Test fixtures updated to use WAITING CheckRuns instead of approval-named StatusContexts.
  • Deterministic required context ordering. Array(Set(...)) now has .sorted() to give stable ordering across calls.
  • Simpler ForEach in non-blocking summary. Replaced ForEach(Array(summary.segments.enumerated()), id: \.element.id) with ForEach(summary.segments) using Identifiable directly.

Test plan

  • Build and run on a repo that uses required CI checks — PR status should reflect required checks only
  • Open a PR where required CI hasn't started yet — should show gray play.slash, not green checkmark
  • Verify non-blocking approval-gated checks appear in the secondary summary row
  • Run test suite: xcodebuild -project MonitorLizard/MonitorLizard.xcodeproj -scheme MonitorLizard -destination 'platform=macOS' -configuration Debug CODE_SIGNING_ALLOWED=NO test

lukelabonte and others added 6 commits May 11, 2026 16:10
Show approval-gated non-blocking checks separately while keeping merge status based on required GitHub checks.
Use statusCheckRollup.state when required metadata and individual checks are unavailable, including EXPECTED as pending. Expand required-CI and parameterized test coverage around the fallback paths.
Treat not-started required CI as needing attention and avoid counting waiting approval parent checks as active CI work.
Replace name-based approval gate detection (checking for "approve"/"approval"
in check names) with GitHub's authoritative signal: CheckRun status WAITING,
which is set exclusively when a job is blocked on environment protection approval.

- Remove isApprovalGate(), approvalGateName(), parentWorkflowNameForApprovalGate()
- approvalParentWorkflowNames() now identifies parents via WAITING CheckRuns
- hasActiveNonApprovalWork() guards on status != WAITING instead of name check
- StatusContext PENDING/EXPECTED always maps to .pending (no name override)
- isRequiredAggregateWork excludes WAITING checks, not name-matched ones
- Update fixtures to use WAITING CheckRuns in place of approval-named StatusContexts
- Sort requiredStatusCheckContexts after Set deduplication so ordering
  is stable across calls
- Simplify non-blocking summary ForEach to use Identifiable directly,
  replacing enumerated index with last-element id comparison
Copy link
Copy Markdown

@lukelabonte lukelabonte left a comment

Choose a reason for hiding this comment

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

I left two comments on the WAITING handling. I think using WAITING is the right direction, but I’m not sure we want to make it the only approval-gate signal or treat every WAITING check as approval-related.

}

private func parseStatusChecks(from checks: [GHPRDetailResponse.StatusCheck]?) -> [StatusCheck] {
private func hasActiveNonApprovalWork(
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

I think this now treats every WAITING check as approval-related work.

That might be right for GitHub Actions environment approvals, but the code doesn’t check that the waiting check is actually an approval gate. If GitHub or another provider returns a non-approval check in WAITING, we could show the PR as Not started even though CI is active.

Could we either scope this more tightly, or add a test that proves a non-approval WAITING check doesn’t get hidden?

}

let hasRequiredMetadata = requiredContextSet != nil || checks.contains { $0.isRequired != nil }
return hasRequiredMetadata ? [] : checks
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

I think this drops the fallback for approval gates that come through as legacy StatusContext entries.

The old path handled contexts like ci/circleci: dead_code_cleanup/approve_dead_code_cleanup by treating them as waiting for approval. This now only works if we get a CheckRun with status: WAITING.

Are we sure external providers won’t still send approval gates as StatusContext + PENDING? If not, I think we should keep the WAITING path but leave the old name-based fallback for legacy contexts.

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.

2 participants