Add required_state to gh aw checks --json to isolate CI verdict from optional third-party commit statuses

pkg/cli/checks_command.go

@@ -58,12 +58,13 @@ type PRCommitStatus struct {
 
 // ChecksResult is the normalized output for the checks command.
 type ChecksResult struct {
-	State      CheckState       `json:"state"`
-	PRNumber   string           `json:"pr_number"`
-	HeadSHA    string           `json:"head_sha"`
-	CheckRuns  []PRCheckRun     `json:"check_runs"`
-	Statuses   []PRCommitStatus `json:"statuses"`
-	TotalCount int              `json:"total_count"`
+	State         CheckState       `json:"state"`
+	RequiredState CheckState       `json:"required_state"`
+	PRNumber      string           `json:"pr_number"`
+	HeadSHA       string           `json:"head_sha"`
+	CheckRuns     []PRCheckRun     `json:"check_runs"`
+	Statuses      []PRCommitStatus `json:"statuses"`
+	TotalCount    int              `json:"total_count"`
 }
 
 // NewChecksCommand creates the checks command.
@@ -82,6 +83,16 @@ Maps PR check rollups to one of the following normalized states:
 
 ` + "Raw check run and commit status signals are included in JSON output." + `
 
+JSON output includes two state fields:
+  state          - aggregate state across all check runs and commit statuses
+  required_state - state derived from check runs and policy commit statuses only;
+                   ignores optional third-party commit statuses (e.g. Vercel,
+                   Netlify deployments) but still surfaces policy_blocked when
+                   branch-protection or account-gate statuses fail
+
+Use required_state as the authoritative CI verdict in repos that have optional
+deployment integrations posting commit statuses alongside required CI checks.
+
 Examples:
   ` + string(constants.CLIExtensionPrefix) + ` checks 42                    # Classify checks for PR #42
   ` + string(constants.CLIExtensionPrefix) + ` checks 42 --repo owner/repo  # Specify repository
@@ -152,14 +163,16 @@ func FetchChecksResult(repoOverride string, prNumber string) (*ChecksResult, err
 	}
 
 	state := classifyCheckState(checkRuns, statuses)
+	requiredState := classifyCheckState(checkRuns, policyStatuses(statuses))
 
 	return &ChecksResult{
-		State:      state,
-		PRNumber:   prNumber,
-		HeadSHA:    headSHA,
-		CheckRuns:  checkRuns,
-		Statuses:   statuses,
-		TotalCount: len(checkRuns) + len(statuses),
+		State:         state,
+		RequiredState: requiredState,
+		PRNumber:      prNumber,
+		HeadSHA:       headSHA,
+		CheckRuns:     checkRuns,
+		Statuses:      statuses,
+		TotalCount:    len(checkRuns) + len(statuses),
 	}, nil
 }
 
@@ -305,6 +318,22 @@ func isPolicyCheck(name string) bool {
 	return false
 }
 
+// policyStatuses returns only the commit statuses whose context matches a policy/account-gate
+// pattern. Used to compute required_state, which excludes optional third-party commit statuses
+// (e.g. Vercel, Netlify deployments) but still surfaces policy_blocked when policy gates fail.
+func policyStatuses(statuses []PRCommitStatus) []PRCommitStatus {
+	if len(statuses) == 0 {
+		return nil
+	}
+	var filtered []PRCommitStatus
+	for _, s := range statuses {
+		if isPolicyCheck(s.Context) {
+			filtered = append(filtered, s)
+		}
+	}
+	return filtered
+}
+
 // classifyCheckState derives a normalized CheckState from raw check runs and commit statuses.
 func classifyCheckState(checkRuns []PRCheckRun, statuses []PRCommitStatus) CheckState {
 	if len(checkRuns) == 0 && len(statuses) == 0 {

pkg/cli/checks_command_test.go

@@ -3,6 +3,7 @@
 package cli
 
 import (
+	"encoding/json"
 	"testing"
 
 	"github.com/stretchr/testify/assert"
@@ -234,27 +235,157 @@ func TestChecksCommand_RejectsMultipleArgs(t *testing.T) {
 
 func TestChecksResultJSONShape(t *testing.T) {
 	result := &ChecksResult{
-		State:    CheckStateFailed,
-		PRNumber: "42",
-		HeadSHA:  "abc123",
+		State:         CheckStateFailed,
+		RequiredState: CheckStateSuccess,
+		PRNumber:      "42",
+		HeadSHA:       "abc123",
 		CheckRuns: []PRCheckRun{
 			{Name: "build", Status: "completed", Conclusion: "failure", HTMLURL: "https://example.com"},
 		},
 		Statuses:   []PRCommitStatus{},
 		TotalCount: 1,
 	}
 
+	// Verify struct fields directly.
 	require.Equal(t, CheckStateFailed, result.State, "state should be failed")
+	require.Equal(t, CheckStateSuccess, result.RequiredState, "required_state should be success")
 	require.Equal(t, "42", result.PRNumber, "PR number should be preserved")
 	require.Equal(t, "abc123", result.HeadSHA, "head SHA should be preserved")
 	require.Len(t, result.CheckRuns, 1, "should have one check run")
 	assert.Equal(t, "build", result.CheckRuns[0].Name, "check run name should be preserved")
+
+	// Marshal to JSON and verify key names match the json struct tags.
+	data, err := json.Marshal(result)
+	require.NoError(t, err, "should marshal to JSON without error")
+
+	var decoded map[string]json.RawMessage
+	require.NoError(t, json.Unmarshal(data, &decoded), "should unmarshal JSON without error")
+
+	assert.Contains(t, decoded, "state", "JSON should contain 'state' key")
+	assert.Contains(t, decoded, "required_state", "JSON should contain 'required_state' key")
+	assert.Contains(t, decoded, "pr_number", "JSON should contain 'pr_number' key")
+	assert.Contains(t, decoded, "head_sha", "JSON should contain 'head_sha' key")
+	assert.Contains(t, decoded, "check_runs", "JSON should contain 'check_runs' key")
+	assert.Contains(t, decoded, "statuses", "JSON should contain 'statuses' key")
+	assert.Contains(t, decoded, "total_count", "JSON should contain 'total_count' key")
+
+	assert.JSONEq(t, `"failed"`, string(decoded["state"]), "state JSON value should be 'failed'")
+	assert.JSONEq(t, `"success"`, string(decoded["required_state"]), "required_state JSON value should be 'success'")
+	assert.JSONEq(t, `"42"`, string(decoded["pr_number"]), "pr_number JSON value should be '42'")
+	assert.JSONEq(t, `"abc123"`, string(decoded["head_sha"]), "head_sha JSON value should be 'abc123'")
 }
 
 // ---------------------------------------------------------------------------
-// classifyGHAPIError – error classification tests
+// required_state — optional third-party commit status failures are excluded,
+// but policy commit statuses (branch protection, etc.) are still included
 // ---------------------------------------------------------------------------
 
+// TestRequiredStateIgnoresCommitStatusFailures validates the core fix: a failing
+// third-party commit status (e.g. Vercel, Netlify) must not pollute the
+// required_state field. Check runs are posted by GitHub Actions; optional
+// deployment commit statuses are posted by third-party integrations.
+func TestRequiredStateIgnoresCommitStatusFailures(t *testing.T) {
+	// All check runs (GitHub Actions) pass; Vercel posts a failure commit status.
+	runs := []PRCheckRun{
+		{Name: "build", Status: "completed", Conclusion: "success"},
+		{Name: "test", Status: "completed", Conclusion: "success"},
+	}
+	statuses := []PRCommitStatus{
+		{Context: "vercel", State: "failure"},
+	}
+
+	// Aggregate state includes the commit status failure.
+	aggregate := classifyCheckState(runs, statuses)
+	assert.Equal(t, CheckStateFailed, aggregate, "aggregate state should be failed when commit status fails")
+
+	// required_state excludes non-policy commit statuses.
+	required := classifyCheckState(runs, policyStatuses(statuses))
+	assert.Equal(t, CheckStateSuccess, required, "required_state should be success when check runs all pass and only Vercel fails")
+}
+
+func TestRequiredStateNetlifyDeployFailure(t *testing.T) {
+	runs := []PRCheckRun{
+		{Name: "ci", Status: "completed", Conclusion: "success"},
+	}
+	statuses := []PRCommitStatus{
+		{Context: "netlify/my-site/deploy-preview", State: "failure"},
+	}
+
+	aggregate := classifyCheckState(runs, statuses)
+	assert.Equal(t, CheckStateFailed, aggregate, "aggregate state should be failed for Netlify failure")
+
+	required := classifyCheckState(runs, policyStatuses(statuses))
+	assert.Equal(t, CheckStateSuccess, required, "required_state should be success when only Netlify fails")
+}
+
+func TestRequiredStateCheckRunFailureStillFails(t *testing.T) {
+	// A real check run failure must still propagate to required_state.
+	runs := []PRCheckRun{
+		{Name: "build", Status: "completed", Conclusion: "success"},
+		{Name: "tests", Status: "completed", Conclusion: "failure"},
+	}
+	statuses := []PRCommitStatus{
+		{Context: "vercel", State: "success"},
+	}
+
+	aggregate := classifyCheckState(runs, statuses)
+	assert.Equal(t, CheckStateFailed, aggregate, "aggregate state should be failed when check run fails")
+
+	required := classifyCheckState(runs, policyStatuses(statuses))
+	assert.Equal(t, CheckStateFailed, required, "required_state should be failed when a check run fails")
+}
+
+func TestRequiredStateNoCheckRunsOnlyCommitStatus(t *testing.T) {
+	// When there are no check runs but a non-policy commit status passes, required_state
+	// returns no_checks while aggregate state is success — this documents the intentional
+	// difference between the two fields.
+	statuses := []PRCommitStatus{
+		{Context: "ci/circleci", State: "success"},
+	}
+
+	aggregate := classifyCheckState(nil, statuses)
+	assert.Equal(t, CheckStateSuccess, aggregate, "aggregate state should be success")
+
+	required := classifyCheckState(nil, policyStatuses(statuses))
+	assert.Equal(t, CheckStateNoChecks, required, "required_state should be no_checks when there are no check runs and no policy statuses")
+}
+
+func TestRequiredStatePolicyCommitStatusStillSurfaced(t *testing.T) {
+	// A failing policy/account-gate commit status must still surface as policy_blocked
+	// in required_state, even though non-policy commit statuses are excluded.
+	runs := []PRCheckRun{
+		{Name: "build", Status: "completed", Conclusion: "success"},
+	}
+	statuses := []PRCommitStatus{
+		{Context: "branch protection rule check", State: "failure"},
+		{Context: "vercel", State: "failure"},
+	}
+
+	// required_state should be policy_blocked (not success), because the policy gate failed.
+	required := classifyCheckState(runs, policyStatuses(statuses))
+	assert.Equal(t, CheckStatePolicyBlocked, required, "required_state should be policy_blocked when a policy commit status fails")
+}
+
+// ---------------------------------------------------------------------------
+// policyStatuses – filter helper tests
+// ---------------------------------------------------------------------------
+
+func TestPolicyStatuses_FiltersNonPolicy(t *testing.T) {
+	statuses := []PRCommitStatus{
+		{Context: "vercel", State: "failure"},
+		{Context: "netlify/deploy", State: "failure"},
+		{Context: "branch protection rule check", State: "failure"},
+	}
+	filtered := policyStatuses(statuses)
+	require.Len(t, filtered, 1, "should retain only policy statuses")
+	assert.Equal(t, "branch protection rule check", filtered[0].Context)
+}
+
+func TestPolicyStatuses_EmptyInput(t *testing.T) {
+	assert.Nil(t, policyStatuses(nil), "nil input should return nil")
+	assert.Nil(t, policyStatuses([]PRCommitStatus{}), "empty input should return nil")
+}
+
 func TestClassifyGHAPIError_NotFound(t *testing.T) {
 	err := classifyGHAPIError(1, "HTTP 404: Not Found", "42", "")
 	require.Error(t, err, "should return an error")