Skip to content

Commit cad4e91

Browse files
julianknutsenclaude
andcommitted
Add wl approve and wl request-changes for PR review workflow
Submit GitHub PR reviews (APPROVE / REQUEST_CHANGES) on PR shells created by wl review --gh-pr. Also adds best-effort approval status warnings to wl merge (advisory only, never blocks). Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
1 parent f6e15d2 commit cad4e91

10 files changed

Lines changed: 344 additions & 1 deletion

cmd/wl/cmd_approve.go

Lines changed: 59 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,59 @@
1+
package main
2+
3+
import (
4+
"fmt"
5+
"io"
6+
"os/exec"
7+
8+
"github.com/spf13/cobra"
9+
"github.com/steveyegge/wasteland/internal/style"
10+
)
11+
12+
func newApproveCmd(stdout, stderr io.Writer) *cobra.Command {
13+
var comment string
14+
15+
cmd := &cobra.Command{
16+
Use: "approve <branch>",
17+
Short: "Approve a PR-mode branch",
18+
Long: `Submit an approving review on the GitHub PR for a wl/* branch.
19+
20+
Requires a GitHub PR to exist (created via 'wl review --gh-pr').
21+
22+
Examples:
23+
wl approve wl/my-rig/w-abc123
24+
wl approve wl/my-rig/w-abc123 --comment "LGTM"`,
25+
Args: cobra.ExactArgs(1),
26+
RunE: func(cmd *cobra.Command, args []string) error {
27+
return runApprove(cmd, stdout, stderr, args[0], comment)
28+
},
29+
}
30+
31+
cmd.Flags().StringVar(&comment, "comment", "", "Review comment")
32+
33+
return cmd
34+
}
35+
36+
func runApprove(cmd *cobra.Command, stdout, _ io.Writer, branch, comment string) error {
37+
cfg, err := resolveWasteland(cmd)
38+
if err != nil {
39+
return fmt.Errorf("loading wasteland config: %w", err)
40+
}
41+
42+
if cfg.GitHubRepo == "" {
43+
return fmt.Errorf("github-repo not configured (run 'wl config set github-repo owner/repo')")
44+
}
45+
46+
ghPath, err := exec.LookPath("gh")
47+
if err != nil {
48+
return fmt.Errorf("gh not found in PATH — install from https://cli.github.com")
49+
}
50+
51+
prURL, err := submitPRReview(ghPath, cfg.GitHubRepo, cfg.ForkOrg, branch, "APPROVE", comment)
52+
if err != nil {
53+
return err
54+
}
55+
56+
fmt.Fprintf(stdout, "%s Approved %s\n", style.Bold.Render("✓"), branch)
57+
fmt.Fprintf(stdout, " %s\n", prURL)
58+
return nil
59+
}

cmd/wl/cmd_approve_test.go

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
1+
package main
2+
3+
import (
4+
"bytes"
5+
"testing"
6+
)
7+
8+
func TestApproveRequiresArg(t *testing.T) {
9+
var stdout, stderr bytes.Buffer
10+
root := newRootCmd(&stdout, &stderr)
11+
12+
for _, c := range root.Commands() {
13+
if c.Name() == "approve" {
14+
if err := c.Args(c, []string{}); err == nil {
15+
t.Error("approve should require exactly 1 argument")
16+
}
17+
if err := c.Args(c, []string{"wl/rig/w-abc"}); err != nil {
18+
t.Errorf("approve should accept 1 argument: %v", err)
19+
}
20+
if err := c.Args(c, []string{"a", "b"}); err == nil {
21+
t.Error("approve should reject 2 arguments")
22+
}
23+
return
24+
}
25+
}
26+
t.Fatal("approve command not found")
27+
}

cmd/wl/cmd_merge.go

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -54,6 +54,18 @@ func runMerge(cmd *cobra.Command, stdout, _ io.Writer, branch string, noPush, ke
5454
return fmt.Errorf("branch %q does not exist", branch)
5555
}
5656

57+
// Best-effort: check PR approval status before merging.
58+
if cfg.GitHubRepo != "" {
59+
if ghPath, err := exec.LookPath("gh"); err == nil {
60+
hasApproval, hasChangesRequested := prApprovalStatus(ghPath, cfg.GitHubRepo, cfg.ForkOrg, branch)
61+
if hasChangesRequested {
62+
fmt.Fprintf(stdout, " %s PR has outstanding change requests\n", style.Warning.Render("⚠"))
63+
} else if !hasApproval {
64+
fmt.Fprintf(stdout, " %s PR has no approvals\n", style.Warning.Render("⚠"))
65+
}
66+
}
67+
}
68+
5769
if err := commons.MergeBranch(cfg.LocalDir, branch); err != nil {
5870
return err
5971
}

cmd/wl/cmd_request_changes.go

Lines changed: 60 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,60 @@
1+
package main
2+
3+
import (
4+
"fmt"
5+
"io"
6+
"os/exec"
7+
8+
"github.com/spf13/cobra"
9+
"github.com/steveyegge/wasteland/internal/style"
10+
)
11+
12+
func newRequestChangesCmd(stdout, stderr io.Writer) *cobra.Command {
13+
var comment string
14+
15+
cmd := &cobra.Command{
16+
Use: "request-changes <branch>",
17+
Short: "Request changes on a PR-mode branch",
18+
Long: `Submit a "request changes" review on the GitHub PR for a wl/* branch.
19+
20+
Requires a GitHub PR to exist (created via 'wl review --gh-pr').
21+
The --comment flag is required to explain what needs to change.
22+
23+
Examples:
24+
wl request-changes wl/my-rig/w-abc123 --comment "needs tests"`,
25+
Args: cobra.ExactArgs(1),
26+
RunE: func(cmd *cobra.Command, args []string) error {
27+
return runRequestChanges(cmd, stdout, stderr, args[0], comment)
28+
},
29+
}
30+
31+
cmd.Flags().StringVar(&comment, "comment", "", "Review comment (required)")
32+
_ = cmd.MarkFlagRequired("comment")
33+
34+
return cmd
35+
}
36+
37+
func runRequestChanges(cmd *cobra.Command, stdout, _ io.Writer, branch, comment string) error {
38+
cfg, err := resolveWasteland(cmd)
39+
if err != nil {
40+
return fmt.Errorf("loading wasteland config: %w", err)
41+
}
42+
43+
if cfg.GitHubRepo == "" {
44+
return fmt.Errorf("github-repo not configured (run 'wl config set github-repo owner/repo')")
45+
}
46+
47+
ghPath, err := exec.LookPath("gh")
48+
if err != nil {
49+
return fmt.Errorf("gh not found in PATH — install from https://cli.github.com")
50+
}
51+
52+
prURL, err := submitPRReview(ghPath, cfg.GitHubRepo, cfg.ForkOrg, branch, "REQUEST_CHANGES", comment)
53+
if err != nil {
54+
return err
55+
}
56+
57+
fmt.Fprintf(stdout, "%s Requested changes on %s\n", style.Bold.Render("✓"), branch)
58+
fmt.Fprintf(stdout, " %s\n", prURL)
59+
return nil
60+
}

cmd/wl/cmd_request_changes_test.go

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
1+
package main
2+
3+
import (
4+
"bytes"
5+
"testing"
6+
)
7+
8+
func TestRequestChangesRequiresArg(t *testing.T) {
9+
var stdout, stderr bytes.Buffer
10+
root := newRootCmd(&stdout, &stderr)
11+
12+
for _, c := range root.Commands() {
13+
if c.Name() == "request-changes" {
14+
if err := c.Args(c, []string{}); err == nil {
15+
t.Error("request-changes should require exactly 1 argument")
16+
}
17+
if err := c.Args(c, []string{"wl/rig/w-abc"}); err != nil {
18+
t.Errorf("request-changes should accept 1 argument: %v", err)
19+
}
20+
if err := c.Args(c, []string{"a", "b"}); err == nil {
21+
t.Error("request-changes should reject 2 arguments")
22+
}
23+
return
24+
}
25+
}
26+
t.Fatal("request-changes command not found")
27+
}

cmd/wl/cmd_review.go

Lines changed: 66 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -453,6 +453,72 @@ func wantedTitleFromBranch(doltPath, dbDir, branch string) string {
453453
return strings.TrimSpace(lines[1])
454454
}
455455

456+
// submitPRReview submits a review on the GitHub PR for the given branch.
457+
// event must be "APPROVE" or "REQUEST_CHANGES".
458+
func submitPRReview(ghPath, upstreamRepo, forkOrg, branch, event, comment string) (string, error) {
459+
head := forkOrg + ":" + branch
460+
prURL, number := findExistingPR(ghPath, upstreamRepo, head)
461+
if number == "" {
462+
return "", fmt.Errorf("no open PR found for branch %s", branch)
463+
}
464+
465+
reviewBody, _ := json.Marshal(map[string]string{
466+
"event": event,
467+
"body": comment,
468+
})
469+
_, err := ghAPICall(ghPath, "POST", fmt.Sprintf("repos/%s/pulls/%s/reviews", upstreamRepo, number), string(reviewBody))
470+
if err != nil {
471+
return "", fmt.Errorf("submitting review: %w", err)
472+
}
473+
return prURL, nil
474+
}
475+
476+
// parseReviewStatus parses GitHub review list JSON into approval state.
477+
// It tracks the latest review state per user and returns two independent bools.
478+
func parseReviewStatus(data []byte) (hasApproval, hasChangesRequested bool) {
479+
var reviews []struct {
480+
User struct{ Login string } `json:"user"`
481+
State string `json:"state"`
482+
}
483+
if err := json.Unmarshal(data, &reviews); err != nil {
484+
return false, false
485+
}
486+
487+
latest := map[string]string{}
488+
for _, r := range reviews {
489+
switch r.State {
490+
case "APPROVED", "CHANGES_REQUESTED":
491+
latest[r.User.Login] = r.State
492+
}
493+
}
494+
495+
for _, state := range latest {
496+
switch state {
497+
case "APPROVED":
498+
hasApproval = true
499+
case "CHANGES_REQUESTED":
500+
hasChangesRequested = true
501+
}
502+
}
503+
return hasApproval, hasChangesRequested
504+
}
505+
506+
// prApprovalStatus checks the review status of a GitHub PR. Best-effort.
507+
// Silently returns (false, false) on any error.
508+
func prApprovalStatus(ghPath, upstreamRepo, forkOrg, branch string) (hasApproval, hasChangesRequested bool) {
509+
head := forkOrg + ":" + branch
510+
_, number := findExistingPR(ghPath, upstreamRepo, head)
511+
if number == "" {
512+
return false, false
513+
}
514+
515+
data, err := ghAPICall(ghPath, "GET", fmt.Sprintf("repos/%s/pulls/%s/reviews", upstreamRepo, number), "")
516+
if err != nil {
517+
return false, false
518+
}
519+
return parseReviewStatus(data)
520+
}
521+
456522
// closeGitHubPR finds and closes an open GitHub PR for the given branch.
457523
// Best-effort: failures print warnings but don't block the merge.
458524
func closeGitHubPR(ghPath, upstreamRepo, forkOrg, forkDB, branch string, stdout io.Writer) {

cmd/wl/cmd_review_test.go

Lines changed: 70 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -72,6 +72,76 @@ func TestReviewGhPRRequiresBranch(t *testing.T) {
7272
}
7373
}
7474

75+
func TestParseReviewStatus(t *testing.T) {
76+
tests := []struct {
77+
name string
78+
json string
79+
wantApproval, wantChangesReq bool
80+
}{
81+
{
82+
name: "empty reviews",
83+
json: `[]`,
84+
wantApproval: false, wantChangesReq: false,
85+
},
86+
{
87+
name: "single approval",
88+
json: `[{"user":{"login":"alice"},"state":"APPROVED"}]`,
89+
wantApproval: true, wantChangesReq: false,
90+
},
91+
{
92+
name: "single changes requested",
93+
json: `[{"user":{"login":"alice"},"state":"CHANGES_REQUESTED"}]`,
94+
wantApproval: false, wantChangesReq: true,
95+
},
96+
{
97+
name: "changes then approval same user",
98+
json: `[
99+
{"user":{"login":"alice"},"state":"CHANGES_REQUESTED"},
100+
{"user":{"login":"alice"},"state":"APPROVED"}
101+
]`,
102+
wantApproval: true, wantChangesReq: false,
103+
},
104+
{
105+
name: "approval then changes same user",
106+
json: `[
107+
{"user":{"login":"alice"},"state":"APPROVED"},
108+
{"user":{"login":"alice"},"state":"CHANGES_REQUESTED"}
109+
]`,
110+
wantApproval: false, wantChangesReq: true,
111+
},
112+
{
113+
name: "mixed users",
114+
json: `[
115+
{"user":{"login":"alice"},"state":"APPROVED"},
116+
{"user":{"login":"bob"},"state":"CHANGES_REQUESTED"}
117+
]`,
118+
wantApproval: true, wantChangesReq: true,
119+
},
120+
{
121+
name: "comment only ignored",
122+
json: `[{"user":{"login":"alice"},"state":"COMMENTED"}]`,
123+
wantApproval: false, wantChangesReq: false,
124+
},
125+
{
126+
name: "invalid JSON",
127+
json: `not json`,
128+
wantApproval: false, wantChangesReq: false,
129+
},
130+
}
131+
132+
for _, tc := range tests {
133+
t.Run(tc.name, func(t *testing.T) {
134+
gotApproval, gotChangesReq := parseReviewStatus([]byte(tc.json))
135+
if gotApproval != tc.wantApproval {
136+
t.Errorf("hasApproval = %v, want %v", gotApproval, tc.wantApproval)
137+
}
138+
if gotChangesReq != tc.wantChangesReq {
139+
t.Errorf("hasChangesRequested = %v, want %v", gotChangesReq, tc.wantChangesReq)
140+
}
141+
})
142+
}
143+
}
144+
75145
func TestExtractWantedID(t *testing.T) {
76146
tests := []struct {
77147
branch, want string

cmd/wl/main.go

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -79,6 +79,8 @@ func newRootCmd(stdout, stderr io.Writer) *cobra.Command {
7979
newListCmd(stdout, stderr),
8080
newConfigCmd(stdout, stderr),
8181
newReviewCmd(stdout, stderr),
82+
newApproveCmd(stdout, stderr),
83+
newRequestChangesCmd(stdout, stderr),
8284
newMergeCmd(stdout, stderr),
8385
newVersionCmd(stdout),
8486
)

cmd/wl/main_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,7 @@ func TestSubcommandRegistration(t *testing.T) {
2828
var stdout, stderr bytes.Buffer
2929
root := newRootCmd(&stdout, &stderr)
3030

31-
expected := []string{"join", "post", "claim", "unclaim", "done", "accept", "reject", "update", "delete", "browse", "status", "sync", "leave", "list", "config", "review", "merge", "version"}
31+
expected := []string{"join", "post", "claim", "unclaim", "done", "accept", "reject", "update", "delete", "browse", "status", "sync", "leave", "list", "config", "review", "approve", "request-changes", "merge", "version"}
3232
for _, name := range expected {
3333
found := false
3434
for _, c := range root.Commands() {

cmd/wl/testdata/errors.txtar

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -138,6 +138,26 @@ stderr 'accepts 1 arg'
138138
! exec wl status w-abc
139139
stderr 'not joined'
140140

141+
# approve with no args.
142+
! exec wl approve
143+
stderr 'accepts 1 arg'
144+
145+
# approve not joined.
146+
! exec wl approve wl/some/branch
147+
stderr 'not joined'
148+
149+
# request-changes with no args.
150+
! exec wl request-changes
151+
stderr 'accepts 1 arg'
152+
153+
# request-changes missing --comment.
154+
! exec wl request-changes wl/some/branch
155+
stderr 'required flag.*"comment"'
156+
157+
# request-changes not joined.
158+
! exec wl request-changes wl/some/branch --comment needs-work
159+
stderr 'not joined'
160+
141161
# review not joined.
142162
! exec wl review
143163
stderr 'not joined'

0 commit comments

Comments
 (0)