core/lint
8
0
Fork 0
Code Issues 1 Pull requests 1 Projects Releases Packages Wiki Activity Actions

fix(ax): preserve partial review results

Browse source
This commit is contained in:
Virgil 2026-03-30 10:14:03 +00:00
parent 29a2722eda
commit 95c32c21ca
2 changed files with 222 additions and 26 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

86
cmd/qa/cmd_review.go
View file

@ -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 {

162
cmd/qa/cmd_review_test.go Normal file
View file

@ -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
})
}