Conversation
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
required_state to gh aw checks --json to isolate check-run CI verdict from optional commit statuses
There was a problem hiding this comment.
Pull request overview
Adds a separate JSON-reported CI verdict for gh aw checks --json that reflects GitHub Actions check runs only, avoiding false failures caused by optional third-party commit statuses (e.g., Vercel/Netlify).
Changes:
- Added
required_statetoChecksResultJSON output, computed from check runs only. - Updated
checkscommand help text to documentstatevsrequired_state. - Added unit tests covering the intended
required_statesemantics vs aggregatestate.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| pkg/cli/checks_command.go | Adds required_state to JSON output and documents it in command help; computes it during result assembly. |
| pkg/cli/checks_command_test.go | Updates ChecksResult shape test and adds new tests validating required_state behavior. |
Comments suppressed due to low confidence (1)
pkg/cli/checks_command_test.go:259
- The section header for the
classifyGHAPIErrortests was replaced by the newrequired_statesection, but theclassifyGHAPIErrortests still follow immediately after. Consider reintroducing a header comment beforeTestClassifyGHAPIError_*to keep the test file structure clear.
// ---------------------------------------------------------------------------
// required_state — commit status failures do not affect check-runs-only state
// ---------------------------------------------------------------------------
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| @@ -245,16 +246,88 @@ 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") | |||
| assert.Equal(t, "build", result.CheckRuns[0].Name, "check run name should be preserved") | |||
| } | |||
There was a problem hiding this comment.
TestChecksResultJSONShape doesn't actually marshal ChecksResult to JSON, so it won't catch a wrong JSON tag/name (e.g., required_state vs requiredState) or missing fields. Since this PR adds a new JSON field, please marshal/unmarshal and assert that the encoded JSON contains the required_state key with the expected value.
There was a problem hiding this comment.
Fixed in 7f35c19. TestChecksResultJSONShape now marshals the struct to JSON, unmarshals into map[string]json.RawMessage, and asserts that all expected keys (state, required_state, pr_number, head_sha, check_runs, statuses, total_count) are present with correct values.
|
@copilot review and apply comment. Surface policy_blocked as a field. |
…rshal assertions Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
required_state to gh aw checks --json to isolate check-run CI verdict from optional commit statusesrequired_state to gh aw checks --json to isolate CI verdict from optional third-party commit statuses
gh aw checks --jsoncollapsed any failing third-party commit status (Vercel, Netlify deployment previews, etc.) intostate: "failed", even when all GitHub Actions check runs passed — causing falseREQUEST_CHANGESfrom the review agent.Changes
required_statefield inChecksResultJSON output — computed from check runs plus policy/account-gate commit statuses only (classifyCheckState(checkRuns, policyStatuses(statuses))). Optional third-party deployment statuses (Vercel, Netlify, etc.) are excluded, but branch-protection and account-gate commit statuses are still included and can yieldpolicy_blocked. The existingstatefield is unchanged.policyStatuses()helper filters commit statuses to only those matchingisPolicyCheck(), reusing the existing policy-pattern logic.required_stateas the authoritative CI verdict in repos with optional deployment integrations.JSON output shape
{ "state": "failed", // aggregate: check runs + all commit statuses "required_state": "success", // check runs + policy commit statuses only — Vercel/Netlify excluded ... }required_statecan return any of the normalized states includingpolicy_blockedwhen branch-protection or account-gate commit statuses fail. Downstream automation (e.g. review agent) should gate onrequired_staterather thanstatewhen optional deployment integrations are present.Original prompt
gh aw checks --jsoncollapses optional third-party failures into top-level state #19158💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.