From 280c0a5284bc3c526edb485301b21d8d58d50306 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Mon, 2 Mar 2026 04:01:50 +0000 Subject: [PATCH 1/3] Initial plan From a978c29bd3c4809801214300b1983267d6fbd5b2 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Mon, 2 Mar 2026 04:13:35 +0000 Subject: [PATCH 2/3] Add required_state field to checks command JSON output Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> --- pkg/cli/checks_command.go | 35 ++++++++++----- pkg/cli/checks_command_test.go | 81 ++++++++++++++++++++++++++++++++-- 2 files changed, 100 insertions(+), 16 deletions(-) diff --git a/pkg/cli/checks_command.go b/pkg/cli/checks_command.go index 2f02846a70c..e15af8254e4 100644 --- a/pkg/cli/checks_command.go +++ b/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,14 @@ 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 only, ignoring optional + third-party commit statuses (e.g. Vercel, Netlify deployments) + +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 +161,16 @@ func FetchChecksResult(repoOverride string, prNumber string) (*ChecksResult, err } state := classifyCheckState(checkRuns, statuses) + requiredState := classifyCheckState(checkRuns, nil) 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 } diff --git a/pkg/cli/checks_command_test.go b/pkg/cli/checks_command_test.go index ac2a1eaa453..96b87463c82 100644 --- a/pkg/cli/checks_command_test.go +++ b/pkg/cli/checks_command_test.go @@ -234,9 +234,10 @@ 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"}, }, @@ -245,6 +246,7 @@ func TestChecksResultJSONShape(t *testing.T) { } 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") @@ -252,9 +254,80 @@ func TestChecksResultJSONShape(t *testing.T) { } // --------------------------------------------------------------------------- -// classifyGHAPIError – error classification tests +// required_state — commit status failures do not affect check-runs-only state // --------------------------------------------------------------------------- +// TestRequiredStateIgnoresCommitStatusFailures validates the core fix: a failing +// third-party commit status (e.g. Vercel, Netlify) must not pollute the +// required_state field, which is computed from check runs only. Check runs are +// typically posted by GitHub Actions; commit statuses are posted by third-party +// integrations and are often optional deployment previews. +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 (check runs only) must not be affected. + required := classifyCheckState(runs, nil) + assert.Equal(t, CheckStateSuccess, required, "required_state should be success when check runs all pass") +} + +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, nil) + 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, nil) + 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 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, nil) + assert.Equal(t, CheckStateNoChecks, required, "required_state should be no_checks when there are no check runs") +} + func TestClassifyGHAPIError_NotFound(t *testing.T) { err := classifyGHAPIError(1, "HTTP 404: Not Found", "42", "") require.Error(t, err, "should return an error") From 7f35c193a380d4e474111b82c4f7df6debf4b747 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Mon, 2 Mar 2026 09:22:01 +0000 Subject: [PATCH 3/3] Address review: surface policy_blocked in required_state, add JSON marshal assertions Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> --- pkg/cli/checks_command.go | 24 ++++++++-- pkg/cli/checks_command_test.go | 84 ++++++++++++++++++++++++++++------ 2 files changed, 92 insertions(+), 16 deletions(-) diff --git a/pkg/cli/checks_command.go b/pkg/cli/checks_command.go index e15af8254e4..2974a2d77f7 100644 --- a/pkg/cli/checks_command.go +++ b/pkg/cli/checks_command.go @@ -85,8 +85,10 @@ Maps PR check rollups to one of the following normalized states: JSON output includes two state fields: state - aggregate state across all check runs and commit statuses - required_state - state derived from check runs only, ignoring optional - third-party commit statuses (e.g. Vercel, Netlify deployments) + 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. @@ -161,7 +163,7 @@ func FetchChecksResult(repoOverride string, prNumber string) (*ChecksResult, err } state := classifyCheckState(checkRuns, statuses) - requiredState := classifyCheckState(checkRuns, nil) + requiredState := classifyCheckState(checkRuns, policyStatuses(statuses)) return &ChecksResult{ State: state, @@ -316,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 { diff --git a/pkg/cli/checks_command_test.go b/pkg/cli/checks_command_test.go index 96b87463c82..6f890629afc 100644 --- a/pkg/cli/checks_command_test.go +++ b/pkg/cli/checks_command_test.go @@ -3,6 +3,7 @@ package cli import ( + "encoding/json" "testing" "github.com/stretchr/testify/assert" @@ -245,23 +246,44 @@ func TestChecksResultJSONShape(t *testing.T) { 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'") } // --------------------------------------------------------------------------- -// required_state — commit status failures do not affect check-runs-only state +// 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, which is computed from check runs only. Check runs are -// typically posted by GitHub Actions; commit statuses are posted by third-party -// integrations and are often optional deployment previews. +// 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{ @@ -276,9 +298,9 @@ func TestRequiredStateIgnoresCommitStatusFailures(t *testing.T) { aggregate := classifyCheckState(runs, statuses) assert.Equal(t, CheckStateFailed, aggregate, "aggregate state should be failed when commit status fails") - // required_state (check runs only) must not be affected. - required := classifyCheckState(runs, nil) - assert.Equal(t, CheckStateSuccess, required, "required_state should be success when check runs all pass") + // 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) { @@ -292,7 +314,7 @@ func TestRequiredStateNetlifyDeployFailure(t *testing.T) { aggregate := classifyCheckState(runs, statuses) assert.Equal(t, CheckStateFailed, aggregate, "aggregate state should be failed for Netlify failure") - required := classifyCheckState(runs, nil) + required := classifyCheckState(runs, policyStatuses(statuses)) assert.Equal(t, CheckStateSuccess, required, "required_state should be success when only Netlify fails") } @@ -309,13 +331,13 @@ func TestRequiredStateCheckRunFailureStillFails(t *testing.T) { aggregate := classifyCheckState(runs, statuses) assert.Equal(t, CheckStateFailed, aggregate, "aggregate state should be failed when check run fails") - required := classifyCheckState(runs, nil) + 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 commit status passes, required_state returns - // no_checks while aggregate state is success — this documents the intentional + // 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"}, @@ -324,8 +346,44 @@ func TestRequiredStateNoCheckRunsOnlyCommitStatus(t *testing.T) { aggregate := classifyCheckState(nil, statuses) assert.Equal(t, CheckStateSuccess, aggregate, "aggregate state should be success") - required := classifyCheckState(nil, nil) - assert.Equal(t, CheckStateNoChecks, required, "required_state should be no_checks when there are no check runs") + 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) {