Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 4 additions & 3 deletions internal/git/hook_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -833,9 +833,10 @@ func TestIsolateHooksPath_SkipsIsolationWhenWorktreeConfigUnsupported(t *testing
// from this directory, and gh resolves the repository by running git there.
// This guards the invariant that such a worktree is a real work tree with no
// leaked core.bare and a resolvable origin - i.e. gh can determine the repo
// from cwd alone. See issue #255: when this breaks, `gh pr checks` fails every
// poll and the CI step hangs until ci_timeout. No real gh or network is
// needed; the failure is purely in git's repo resolution, which gh depends on.
// from cwd alone. See issue #255: when this breaks, the `gh pr view` status
// rollup query fails every poll and the CI step hangs until ci_timeout. No real
// gh or network is needed; the failure is purely in git's repo resolution,
// which gh depends on.
func TestIsolateHooksPath_LinkedWorktreeResolvesRepoForCLI(t *testing.T) {
ctx := context.Background()
base := t.TempDir()
Expand Down
3 changes: 2 additions & 1 deletion internal/pipeline/steps/helpers_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -421,7 +421,8 @@ func newTestContextWithDBRecords(t *testing.T, ag agent.Agent, workDir, baseSHA,
}

// fakeCIGH creates a fake gh binary that responds to CI-related
// commands (pr view --json state, pr checks --json, pr view --json comments).
// commands (pr view --json state, pr view --json statusCheckRollup,
// pr view --json comments).
func fakeCIGH(t *testing.T, state, checksJSON string) []string {
t.Helper()
binDir := fakeCLIBinDir(t)
Expand Down
54 changes: 44 additions & 10 deletions internal/pipeline/steps/steps_test.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package steps

import (
"encoding/json"
"errors"
"fmt"
"io"
Expand Down Expand Up @@ -308,14 +309,47 @@ func fakeCIGHReconcileHandler(args []string) {
fmt.Println("MERGEABLE")
os.Exit(0)
}
if strings.Contains(joined, "pr checks") {
fmt.Println(`[{"name":"build","state":"SUCCESS","bucket":"pass"}]`)
if strings.Contains(joined, "pr view") && strings.Contains(joined, "--json statusCheckRollup") {
fmt.Println(`{"statusCheckRollup":[{"name":"build","state":"SUCCESS"}]}`)
os.Exit(0)
}
fmt.Fprintln(os.Stderr, "unsupported reconcile gh argv:", joined)
os.Exit(1)
}

func fakeGitHubStatusRollup(checksJSON string) string {
var checks []map[string]any
if err := json.Unmarshal([]byte(checksJSON), &checks); err == nil {
for _, check := range checks {
if _, ok := check["detailsUrl"]; !ok {
if link, ok := check["link"]; ok {
check["detailsUrl"] = link
}
}
if _, hasState := check["state"]; !hasState {
if bucket, ok := check["bucket"].(string); ok {
switch bucket {
case "pass":
check["conclusion"] = "SUCCESS"
case "fail":
check["conclusion"] = "FAILURE"
case "pending":
check["status"] = "IN_PROGRESS"
case "cancel":
check["conclusion"] = "CANCELLED"
case "skipping":
check["conclusion"] = "SKIPPED"
}
}
}
}
if encoded, err := json.Marshal(checks); err == nil {
checksJSON = string(encoded)
}
}
return fmt.Sprintf(`{"statusCheckRollup":%s}`, checksJSON)
}

func fakeCIGHHandler(args []string) {
state := os.Getenv("FAKE_CLI_STATE")
stateErr := os.Getenv("FAKE_CLI_STATE_ERR")
Expand Down Expand Up @@ -347,12 +381,12 @@ func fakeCIGHHandler(args []string) {
fmt.Println(state)
os.Exit(0)
}
if strings.Contains(joined, "pr checks") {
if strings.Contains(joined, "pr view") && strings.Contains(joined, "--json statusCheckRollup") {
if checksErr != "" {
fmt.Fprintln(os.Stderr, checksErr)
os.Exit(1)
}
fmt.Println(checksJSON)
fmt.Println(fakeGitHubStatusRollup(checksJSON))
os.Exit(0)
}
if strings.Contains(joined, "run rerun") {
Expand Down Expand Up @@ -401,15 +435,15 @@ func fakeCIGHSequenceHandler(args []string) {
fmt.Println(state)
os.Exit(0)
}
if strings.Contains(joined, "pr checks") {
if strings.Contains(joined, "pr view") && strings.Contains(joined, "--json statusCheckRollup") {
data, err := os.ReadFile(checksPath)
if err != nil {
fmt.Fprintln(os.Stderr, err)
os.Exit(1)
}
entries := strings.Split(strings.TrimSpace(string(data)), "\n")
if len(entries) == 0 || entries[0] == "" {
fmt.Println("[]")
fmt.Println(`{"statusCheckRollup":[]}`)
os.Exit(0)
}

Expand All @@ -426,7 +460,7 @@ func fakeCIGHSequenceHandler(args []string) {
fmt.Fprintln(os.Stderr, err)
os.Exit(1)
}
fmt.Println(entries[index])
fmt.Println(fakeGitHubStatusRollup(entries[index]))
os.Exit(0)
}
if strings.Contains(joined, "run rerun") {
Expand Down Expand Up @@ -559,9 +593,9 @@ func fakeCIGHNoChecksHandler(args []string) {
if len(args) >= 2 && args[0] == "auth" && args[1] == "status" {
os.Exit(0)
}
if strings.Contains(joined, "pr checks") {
fmt.Fprintln(os.Stderr, "no checks reported on the 'feature/e2e' branch")
os.Exit(1)
if strings.Contains(joined, "pr view") && strings.Contains(joined, "--json statusCheckRollup") {
fmt.Println(`{"statusCheckRollup":[]}`)
os.Exit(0)
}
if strings.Contains(joined, "pr view") && strings.Contains(joined, "--json state") {
fmt.Println("OPEN")
Expand Down
105 changes: 75 additions & 30 deletions internal/scm/github/github.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import (
"strings"
"time"

"github.com/kunchenguid/no-mistakes/internal/safeurl"
"github.com/kunchenguid/no-mistakes/internal/scm"
)

Expand Down Expand Up @@ -297,53 +298,97 @@ func (h *Host) GetChecks(ctx context.Context, pr *scm.PR) ([]scm.Check, error) {
if err != nil {
return nil, err
}
args := append([]string{"pr", "checks", selector}, h.repoArgs()...)
args = append(args, "--json", "name,state,bucket,completedAt,link")
args := append([]string{"pr", "view", selector}, h.repoArgs()...)
args = append(args, "--json", "statusCheckRollup")
cmd := h.cmd(ctx, "gh", args...)
out, err := cmd.CombinedOutput()
if err != nil {
if strings.Contains(string(out), "no checks reported") {
return nil, nil
}
return nil, fmt.Errorf("gh pr checks: %w", err)
return nil, fmt.Errorf("gh pr view statusCheckRollup: %s: %w", boundedCommandOutput(out), err)
}
var raw []struct {
Name string `json:"name"`
State string `json:"state"`
Bucket string `json:"bucket"`
CompletedAt string `json:"completedAt"`
Link string `json:"link"`
var raw struct {
StatusCheckRollup []githubStatusCheck `json:"statusCheckRollup"`
}
if err := json.Unmarshal(out, &raw); err != nil {
return nil, fmt.Errorf("parse CI checks: %w", err)
}
checks := make([]scm.Check, 0, len(raw))
for _, r := range raw {
var completedAt time.Time
if r.CompletedAt != "" {
if parsed, parseErr := time.Parse(time.RFC3339, r.CompletedAt); parseErr == nil {
completedAt = parsed
}
checks := make([]scm.Check, 0, len(raw.StatusCheckRollup))
for _, r := range raw.StatusCheckRollup {
state := strings.ToUpper(strings.TrimSpace(r.Conclusion))
if state == "" {
state = strings.ToUpper(strings.TrimSpace(r.State))
}
checks = append(checks, scm.Check{
Name: r.Name,
Bucket: normalizeCheckBucket(r.Bucket, r.State),
State: strings.ToUpper(strings.TrimSpace(r.State)),
CompletedAt: completedAt,
Link: strings.TrimSpace(r.Link),
Name: r.checkName(),
Bucket: githubStatusCheckBucket(r),
State: state,
CompletedAt: parseGitHubTime(r.CompletedAt),
Link: strings.TrimSpace(r.detailsLink()),
})
}
return checks, nil
}

// githubStatusCheck is the common subset returned by GitHub's CheckRun and
// StatusContext union members in statusCheckRollup.
type githubStatusCheck struct {
Type string `json:"__typename"`
Name string `json:"name"`
Context string `json:"context"`
State string `json:"state"`
Status string `json:"status"`
Conclusion string `json:"conclusion"`
CompletedAt string `json:"completedAt"`
DetailsURL string `json:"detailsUrl"`
TargetURL string `json:"targetUrl"`
}

func (r githubStatusCheck) checkName() string {
if strings.TrimSpace(r.Name) != "" {
return strings.TrimSpace(r.Name)
}
return strings.TrimSpace(r.Context)
}

func (r githubStatusCheck) detailsLink() string {
if strings.TrimSpace(r.DetailsURL) != "" {
return r.DetailsURL
}
return r.TargetURL
}

func parseGitHubTime(raw string) time.Time {
parsed, _ := time.Parse(time.RFC3339, strings.TrimSpace(raw))
return parsed
}

func githubStatusCheckBucket(r githubStatusCheck) scm.CheckBucket {
if outcome := strings.TrimSpace(r.Conclusion); outcome != "" {
return normalizeCheckBucket("", outcome)
}
if state := strings.TrimSpace(r.State); state != "" {
return normalizeCheckBucket("", state)
}
return normalizeCheckBucket("", r.Status)
}

const maxGitHubCommandOutput = 4096

func boundedCommandOutput(out []byte) string {
text := strings.TrimSpace(safeurl.RedactText(string(out)))
if len(text) > maxGitHubCommandOutput {
return text[:maxGitHubCommandOutput] + "...[truncated]"
}
return text
}

// RerunCheck re-runs the Actions job behind check for the same commit, so a
// check the provider cancelled rather than failed can be retried without a new
// push. The job is identified from the check's details link, which is the only
// run/job identity `gh pr checks` reports: a link naming a job re-runs just that
// job (and its dependencies), and a link naming only a run re-runs that run's
// failed jobs. Anything else - a third-party status pointing at an external
// dashboard, or a run path this backend cannot read - names no re-runnable job,
// and the error says so rather than falling back to a wider rerun.
// push. The job is identified from the CheckRun details URL in the PR status
// rollup: a link naming a job re-runs just that job (and its dependencies), and
// a link naming only a run re-runs that run's failed jobs. Anything else - a
// third-party status pointing at an external dashboard, or a run path this
// backend cannot read - names no re-runnable job, and the error says so rather
// than falling back to a wider rerun.
func (h *Host) RerunCheck(ctx context.Context, _ *scm.PR, check scm.Check) error {
rerunArgs, ok := rerunTargetArgs(check.Link)
if !ok {
Expand Down
Loading
Loading