diff --git a/cmd/qa/cmd_review.go b/cmd/qa/cmd_review.go index e4269f6..cb1b5ea 100644 --- a/cmd/qa/cmd_review.go +++ b/cmd/qa/cmd_review.go @@ -83,13 +83,22 @@ type Review struct { State string `json:"state"` } +// ReviewFetchError captures a partial fetch failure while preserving any +// successfully fetched PRs in the same review run. +type ReviewFetchError struct { + Repo string `json:"repo"` + Scope string `json:"scope"` + Error string `json:"error"` +} + type reviewOutput struct { - Mine []PullRequest `json:"mine"` - Requested []PullRequest `json:"requested"` - TotalMine int `json:"total_mine"` - TotalRequested int `json:"total_requested"` - ShowingMine bool `json:"showing_mine"` - ShowingRequested bool `json:"showing_requested"` + Mine []PullRequest `json:"mine"` + Requested []PullRequest `json:"requested"` + TotalMine int `json:"total_mine"` + TotalRequested int `json:"total_requested"` + ShowingMine bool `json:"showing_mine"` + ShowingRequested bool `json:"showing_requested"` + FetchErrors []ReviewFetchError `json:"fetch_errors"` } // addReviewCommand adds the 'review' subcommand to the qa command. @@ -133,34 +142,56 @@ func runReview() error { // Default: show both mine and requested if neither flag is set showMine := reviewMine || (!reviewMine && !reviewRequested) showRequested := reviewRequested || (!reviewMine && !reviewRequested) - var minePRs, requestedPRs []PullRequest + minePRs := []PullRequest{} + requestedPRs := []PullRequest{} + fetchErrors := make([]ReviewFetchError, 0) + mineFetched := false + requestedFetched := false if showMine { prs, err := fetchPRs(ctx, repoFullName, "author:@me") if err != nil { - return err - } - sort.Slice(prs, func(i, j int) bool { - if prs[i].Number == prs[j].Number { - return strings.Compare(prs[i].Title, prs[j].Title) < 0 + fetchErrors = append(fetchErrors, ReviewFetchError{ + Repo: repoFullName, + Scope: "mine", + Error: strings.TrimSpace(err.Error()), + }) + if !reviewJSON { + cli.Warnf("failed to fetch your PRs for %s: %s", repoFullName, strings.TrimSpace(err.Error())) } - return prs[i].Number < prs[j].Number - }) - minePRs = prs + } else { + sort.Slice(prs, func(i, j int) bool { + if prs[i].Number == prs[j].Number { + return strings.Compare(prs[i].Title, prs[j].Title) < 0 + } + return prs[i].Number < prs[j].Number + }) + minePRs = prs + mineFetched = true + } } if showRequested { prs, err := fetchPRs(ctx, repoFullName, "review-requested:@me") if err != nil { - return err - } - sort.Slice(prs, func(i, j int) bool { - if prs[i].Number == prs[j].Number { - return strings.Compare(prs[i].Title, prs[j].Title) < 0 + fetchErrors = append(fetchErrors, ReviewFetchError{ + Repo: repoFullName, + Scope: "requested", + Error: strings.TrimSpace(err.Error()), + }) + if !reviewJSON { + cli.Warnf("failed to fetch review requested PRs for %s: %s", repoFullName, strings.TrimSpace(err.Error())) } - return prs[i].Number < prs[j].Number - }) - requestedPRs = prs + } else { + sort.Slice(prs, func(i, j int) bool { + if prs[i].Number == prs[j].Number { + return strings.Compare(prs[i].Title, prs[j].Title) < 0 + } + return prs[i].Number < prs[j].Number + }) + requestedPRs = prs + requestedFetched = true + } } if reviewJSON { @@ -171,6 +202,7 @@ func runReview() error { TotalRequested: len(requestedPRs), ShowingMine: showMine, ShowingRequested: showRequested, + FetchErrors: fetchErrors, }, "", " ") if err != nil { return err @@ -179,14 +211,14 @@ func runReview() error { return nil } - if showMine { + if showMine && mineFetched { if err := printMyPRs(minePRs); err != nil { return err } } - if showRequested { - if showMine { + if showRequested && requestedFetched { + if showMine && mineFetched { cli.Blank() } if err := printRequestedPRs(requestedPRs); err != nil { @@ -196,6 +228,7 @@ func runReview() error { return nil } + // printMyPRs shows the user's open PRs with status func printMyPRs(prs []PullRequest) error { if len(prs) == 0 { @@ -211,6 +244,7 @@ func printMyPRs(prs []PullRequest) error { return nil } + // printRequestedPRs shows PRs where user's review is requested func printRequestedPRs(prs []PullRequest) error { if len(prs) == 0 { diff --git a/cmd/qa/cmd_review_test.go b/cmd/qa/cmd_review_test.go new file mode 100644 index 0000000..cd12341 --- /dev/null +++ b/cmd/qa/cmd_review_test.go @@ -0,0 +1,162 @@ +package qa + +import ( + "encoding/json" + "path/filepath" + "testing" + + "forge.lthn.ai/core/cli/pkg/cli" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestRunReviewJSONOutput_PreservesPartialResultsAndFetchErrors(t *testing.T) { + dir := t.TempDir() + writeExecutable(t, filepath.Join(dir, "gh"), `#!/bin/sh +case "$*" in + *"author:@me"*) + printf '%s\n' 'simulated author query failure' >&2 + exit 1 + ;; + *"review-requested:@me"*) + cat <<'JSON' +[ + { + "number": 42, + "title": "Refine agent output", + "author": {"login": "alice"}, + "state": "OPEN", + "isDraft": false, + "mergeable": "MERGEABLE", + "reviewDecision": "", + "url": "https://example.com/pull/42", + "headRefName": "feature/agent-output", + "createdAt": "2026-03-30T00:00:00Z", + "updatedAt": "2026-03-30T00:00:00Z", + "additions": 12, + "deletions": 3, + "changedFiles": 2, + "reviewRequests": {"nodes": []}, + "reviews": [] + } +] +JSON + ;; + *) + printf '%s\n' "unexpected gh invocation: $*" >&2 + exit 1 + ;; +esac +`) + + restoreWorkingDir(t, dir) + prependPath(t, dir) + resetReviewFlags(t) + t.Cleanup(func() { + reviewRepo = "" + }) + + parent := &cli.Command{Use: "qa"} + addReviewCommand(parent) + command := findSubcommand(t, parent, "review") + require.NoError(t, command.Flags().Set("repo", "forge/example")) + require.NoError(t, command.Flags().Set("json", "true")) + + output := captureStdout(t, func() { + require.NoError(t, command.RunE(command, nil)) + }) + + var payload reviewOutput + require.NoError(t, json.Unmarshal([]byte(output), &payload)) + assert.True(t, payload.ShowingMine) + assert.True(t, payload.ShowingRequested) + require.Len(t, payload.Mine, 0) + require.Len(t, payload.Requested, 1) + assert.Equal(t, 42, payload.Requested[0].Number) + assert.Equal(t, "Refine agent output", payload.Requested[0].Title) + require.Len(t, payload.FetchErrors, 1) + assert.Equal(t, "forge/example", payload.FetchErrors[0].Repo) + assert.Equal(t, "mine", payload.FetchErrors[0].Scope) + assert.Contains(t, payload.FetchErrors[0].Error, "simulated author query failure") +} + +func TestRunReviewHumanOutput_PreservesSuccessfulSectionWhenOneFetchFails(t *testing.T) { + dir := t.TempDir() + writeExecutable(t, filepath.Join(dir, "gh"), `#!/bin/sh +case "$*" in + *"author:@me"*) + printf '%s\n' 'simulated author query failure' >&2 + exit 1 + ;; + *"review-requested:@me"*) + cat <<'JSON' +[ + { + "number": 42, + "title": "Refine agent output", + "author": {"login": "alice"}, + "state": "OPEN", + "isDraft": false, + "mergeable": "MERGEABLE", + "reviewDecision": "", + "url": "https://example.com/pull/42", + "headRefName": "feature/agent-output", + "createdAt": "2026-03-30T00:00:00Z", + "updatedAt": "2026-03-30T00:00:00Z", + "additions": 12, + "deletions": 3, + "changedFiles": 2, + "reviewRequests": {"nodes": []}, + "reviews": [] + } +] +JSON + ;; + *) + printf '%s\n' "unexpected gh invocation: $*" >&2 + exit 1 + ;; +esac +`) + + restoreWorkingDir(t, dir) + prependPath(t, dir) + resetReviewFlags(t) + t.Cleanup(func() { + reviewRepo = "" + }) + + parent := &cli.Command{Use: "qa"} + addReviewCommand(parent) + command := findSubcommand(t, parent, "review") + require.NoError(t, command.Flags().Set("repo", "forge/example")) + + output := captureStdout(t, func() { + require.NoError(t, command.RunE(command, nil)) + }) + + assert.Contains(t, output, "#42 Refine agent output") + assert.Contains(t, output, "gh pr checkout 42") + assert.NotContains(t, output, "Your pull requests") + assert.NotContains(t, output, "cmd.qa.review.no_prs") +} + +func resetReviewFlags(t *testing.T) { + t.Helper() + oldMine := reviewMine + oldRequested := reviewRequested + oldRepo := reviewRepo + oldJSON := reviewJSON + + reviewMine = false + reviewRequested = false + reviewRepo = "" + reviewJSON = false + + t.Cleanup(func() { + reviewMine = oldMine + reviewRequested = oldRequested + reviewRepo = oldRepo + reviewJSON = oldJSON + }) +}