If two workflows are triggered on a branch and one is successful and one is skipped, mp doesn't allow merge

[brettjenkins]

Hey,

Just trying MP, and noticed a bug, if two workflows are configured on a branch, and one is successful and one is skipped (because it doesn't apply in this case), when trying to merge you get this error:

merge error: Build status was not 'success', instead was 'pending'. Use --ignore-build-status to override this check.

Presumably it's treating the skipped check as pending and so I have to ignore the build status to continue the merge.

Thanks

[nathanleiby]

@brettjenkins thanks for the reporting! Agreed, seems like unexpected UX.

Did this happen for a build in a private or public repo? If public please link! Else can try to reproduce.

---

For reference, the relevant code is here:

microplane/merge/merge.go

Lines 71 to 83 in c7e05e2

	// (2) Check commit status
	<-repoLimiter.C
	status, _, err := client.Repositories.GetCombinedStatus(ctx, input.Repo.Owner, input.Repo.Name, input.CommitSHA, &github.ListOptions{})
	if err != nil {
	return Output{Success: false}, err
	}
	if input.RequireBuildSuccess {
	state := status.GetState()
	if state != "success" {
	return Output{Success: false}, fmt.Errorf("Build status was not 'success', instead was '%s'. Use --ignore-build-status to override this check.", state)
	}
	}

We are calling GetCombinedStatus. Docs:

• https://pkg.go.dev/github.com/google/go-github/github#RepositoriesService.GetCombinedStatus
• https://docs.github.com/en/rest/reference/repos#get-the-combined-status-for-a-specific-reference

The most recent status for each context is returned, up to 100. This field paginates if there are over 100 contexts.
Additionally, a combined state is returned. The state is one of:

• failure if any of the contexts report as error or failure
• pending if there are no statuses or a context is pending
• success if the latest status for all contexts is success

We may need to dig into the individual statuses to handle this, rather than just accept the combined status.

[robherley]

👋 @nathanleiby The /repos/{owner}/{repo}/commits/{ref}/status route used by GetCombinedStatus unfortunately only accounts for states from the Commit Status API.

If you want to account for all of the states from the Commit Status API, Checks API and Actions, you'll need to use the GraphQL to get the StatusCheckRollup.

For example, a PR from this repository: #159

That PR has both Checks (from the dependabot Actions) and a Commit Status (from Circle CI).

If you query with just the REST API for Statuses, you'll see it only has a single status:

Status API REST:

$ curl -H "Accept: application/vnd.github.v3+json" \
  https://api.github.com/repos/Clever/microplane/commits/dd872cc3820a726e242f9f20680f2e35aad9950b/status

📬 Status API REST Result

{
  "state": "success",
  "statuses": [
    {
      "url": "https://api.github.com/repos/Clever/microplane/statuses/dd872cc3820a726e242f9f20680f2e35aad9950b",
      "avatar_url": "https://avatars.githubusercontent.com/oa/4808?v=4",
      "id": 17692110259,
      "node_id": "SC_kwDOBmm9eM8AAAAEHogtsw",
      "state": "success",
      "description": "Your tests passed on CircleCI!",
      "target_url": "https://circleci.com/gh/Clever/microplane/374?utm_campaign=vcs-integration-link&utm_medium=referral&utm_source=github-build-link",
      "context": "ci/circleci: build",
      "created_at": "2022-06-06T17:16:42Z",
      "updated_at": "2022-06-06T17:16:42Z"
    }
  ],
  "sha": "dd872cc3820a726e242f9f20680f2e35aad9950b",
  "total_count": 1,
  // ...
}

⚠️ Note the total_count of 1, it's only looking at the single Status.

Instead, you should be using the StatusCheckRollup present in the GraphQL API:

query {
  repository(owner: "Clever", name: "microplane") {
    object(oid: "dd872cc3820a726e242f9f20680f2e35aad9950b") {
      ... on Commit {
        statusCheckRollup {
          contexts {
            totalCount
          }
          state
        }
      }
    }
  }
}

📬 StatusCheckRollup GraphQL Result

{
  "data": {
    "repository": {
      "object": {
        "statusCheckRollup": {
          "contexts": {
            "totalCount": 2
          },
          "state": "SUCCESS"
        }
      }
    }
  }
}

This'll factor in all the runs (checks + actions) plus the commit statuses. I apologize for the confusing nature of these endpoints 😅

I'd also suggest you look into handling cases for branch protection rules as well: https://docs.github.com/en/repositories/configuring-branches-and-merges-in-your-repository/defining-the-mergeability-of-pull-requests/managing-a-branch-protection-rule

/cc @benwaffle