Skip to content
Merged
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
2 changes: 1 addition & 1 deletion cmd/worktree.go
Original file line number Diff line number Diff line change
Expand Up @@ -554,7 +554,7 @@ func loadWorktreeReviews(wtClient *worktree.Client, worktrees []worktree.Info) w
if !wtShowPR || len(worktrees) == 0 {
return worktree.ReviewLookup{}
}
return wtClient.ReviewStates()
return wtClient.ReviewStates(worktrees)
}

func printReviewWarning(w io.Writer, reviews worktree.ReviewLookup) {
Expand Down
254 changes: 222 additions & 32 deletions internal/worktree/review.go
Original file line number Diff line number Diff line change
@@ -1,18 +1,23 @@
package worktree

import (
"crypto/sha256"
"encoding/json"
"errors"
"fmt"
"net/url"
"os"
"os/exec"
"path/filepath"
"strings"
"time"
)

const (
reviewProviderGitHub = "github"
reviewProviderGitLab = "gitlab"
reviewCacheTTL = 5 * time.Minute
reviewCacheVersion = "v1"
)

type ReviewInfo struct {
Expand All @@ -34,6 +39,20 @@ type reviewRemote struct {
provider string
}

type reviewTarget struct {
Branch string
Commit string
}

type reviewCandidate struct {
Provider string
Number int
State string
HeadBranch string
HeadCommit string
URL string
}

type missingReviewToolError struct {
tool string
}
Expand All @@ -43,6 +62,7 @@ func (e missingReviewToolError) Error() string {
}

var reviewRunFunc = reviewRunDefault
var reviewCacheDirFunc = os.UserCacheDir

func reviewRunDefault(repoDir string, tool string, args ...string) ([]byte, error) {
if _, err := exec.LookPath(tool); err != nil {
Expand All @@ -61,13 +81,18 @@ func reviewRunDefault(repoDir string, tool string, args ...string) ([]byte, erro
return out, nil
}

func (c *Client) ReviewStates() ReviewLookup {
func (c *Client) ReviewStates(worktrees []Info) ReviewLookup {
result := ReviewLookup{Reviews: map[string]ReviewInfo{}}
if err := c.ensureInit(); err != nil {
result.Warning = "review lookup skipped: failed to initialize repository: " + err.Error()
return result
}

targets := c.reviewTargets(worktrees)
if len(targets) == 0 {
return result
}

remote, err := c.detectReviewRemote()
if err != nil {
result.Warning = "review lookup skipped: " + err.Error()
Expand All @@ -77,9 +102,9 @@ func (c *Client) ReviewStates() ReviewLookup {
var reviews map[string]ReviewInfo
switch remote.provider {
case reviewProviderGitHub:
reviews, err = githubReviewStates(c.repoDir, remote.url)
reviews, err = githubReviewStates(c.repoDir, remote.url, targets)
case reviewProviderGitLab:
reviews, err = gitlabReviewStates(c.repoDir, remote.url)
reviews, err = gitlabReviewStates(c.repoDir, remote.url, targets)
default:
err = fmt.Errorf("unsupported review provider for remote %q", remote.name)
}
Expand Down Expand Up @@ -120,6 +145,35 @@ func (c *Client) detectReviewRemote() (reviewRemote, error) {
return reviewRemote{}, errors.New("no usable git remote found")
}

func (c *Client) reviewTargets(worktrees []Info) map[string]reviewTarget {
targets := make(map[string]reviewTarget)
mainBranch := ""
if branch, err := c.resolvedMainBranch(); err == nil {
mainBranch = branch
}
for _, wt := range worktrees {
branch := strings.TrimSpace(wt.Branch)
if branch == "" || branch == "(detached)" || branch == mainBranch {
continue
}
if !c.branchPushedToOrigin(branch) {
continue
}
if _, exists := targets[branch]; !exists {
targets[branch] = reviewTarget{Branch: branch, Commit: wt.Commit}
}
}
return targets
}

func (c *Client) branchPushedToOrigin(branch string) bool {
if branch == "" {
return false
}
_, err := c.runner.Run("-C", c.repoDir, "show-ref", "--verify", "--quiet", "refs/remotes/origin/"+branch)
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Respect non-origin remotes when filtering pushed branches

In repositories whose review remote is not named origin—for example a single GitHub/GitLab remote named upstream or github—this hard-coded refs/remotes/origin/<branch> check makes every worktree look unpushed, so ReviewStates returns before calling detectReviewRemote/the CLI and gmc wt --pr silently shows no PR/MR data. Since reviewRemoteCandidates already supports upstream and a single arbitrary remote, the pushed-branch check should use the selected review remote (or otherwise consider non-origin remotes) rather than only origin.

Useful? React with 👍 / 👎.

return err == nil
}

func (c *Client) remoteURL(remote string) (string, error) {
result, err := c.runner.Run("-C", c.repoDir, "remote", "get-url", remote)
if err != nil {
Expand Down Expand Up @@ -188,17 +242,30 @@ type githubReviewInfo struct {
Number int `json:"number"`
State string `json:"state"`
HeadRefName string `json:"headRefName"`
HeadRefOid string `json:"headRefOid"`
URL string `json:"url"`
}

func githubReviewStates(repoDir string, repoURL string) (map[string]ReviewInfo, error) {
out, err := reviewRunFunc(repoDir,
"gh",
"pr", "list",
"-R", repoURL,
"--state", "all",
"--json", "number,state,headRefName,url",
"--limit", "300",
func githubReviewStates(
repoDir string,
repoURL string,
targets map[string]reviewTarget,
) (map[string]ReviewInfo, error) {
out, err := cachedReviewOutput(
reviewProviderGitHub,
repoURL,
"me",
func() ([]byte, error) {
return reviewRunFunc(repoDir,
"gh",
"pr", "list",
"-R", repoURL,
"--author", "@me",
"--state", "all",
"--json", "number,state,headRefName,headRefOid,url",
"--limit", "1000",
)
},
)
if err != nil {
return nil, err
Expand All @@ -209,37 +276,56 @@ func githubReviewStates(repoDir string, repoURL string) (map[string]ReviewInfo,
return nil, err
}

reviews := make(map[string]ReviewInfo, len(prs))
candidates := make([]reviewCandidate, 0, len(prs))
for _, pr := range prs {
if _, exists := reviews[pr.HeadRefName]; exists {
continue
}
reviews[pr.HeadRefName] = ReviewInfo{
candidates = append(candidates, reviewCandidate{
Provider: reviewProviderGitHub,
Number: pr.Number,
State: normalizeReviewState(pr.State),
HeadBranch: pr.HeadRefName,
HeadCommit: pr.HeadRefOid,
URL: pr.URL,
}
})
}
return reviews, nil
return selectReviewCandidates(candidates, targets), nil
}

type gitlabReviewInfo struct {
IID int `json:"iid"`
State string `json:"state"`
SourceBranch string `json:"source_branch"`
SHA string `json:"sha"`
WebURL string `json:"web_url"`
}

func gitlabReviewStates(repoDir string, repoURL string) (map[string]ReviewInfo, error) {
out, err := reviewRunFunc(repoDir,
"glab",
"mr", "list",
"-R", repoURL,
"--all",
"--output", "json",
"--per-page", "100",
type gitlabUserInfo struct {
Username string `json:"username"`
}

func gitlabReviewStates(
repoDir string,
repoURL string,
targets map[string]reviewTarget,
) (map[string]ReviewInfo, error) {
out, err := cachedReviewOutput(
reviewProviderGitLab,
repoURL,
"me",
func() ([]byte, error) {
username, err := gitlabCurrentUsername(repoDir)
if err != nil {
return nil, err
}
return reviewRunFunc(repoDir,
"glab",
"mr", "list",
"-R", repoURL,
"--all",
"--author", username,
"--output", "json",
"--per-page", "100",
)
},
)
if err != nil {
return nil, err
Expand All @@ -250,20 +336,124 @@ func gitlabReviewStates(repoDir string, repoURL string) (map[string]ReviewInfo,
return nil, err
}

reviews := make(map[string]ReviewInfo, len(mrs))
candidates := make([]reviewCandidate, 0, len(mrs))
for _, mr := range mrs {
if _, exists := reviews[mr.SourceBranch]; exists {
continue
}
reviews[mr.SourceBranch] = ReviewInfo{
candidates = append(candidates, reviewCandidate{
Provider: reviewProviderGitLab,
Number: mr.IID,
State: normalizeReviewState(mr.State),
HeadBranch: mr.SourceBranch,
HeadCommit: mr.SHA,
URL: mr.WebURL,
})
}
return selectReviewCandidates(candidates, targets), nil
}

func gitlabCurrentUsername(repoDir string) (string, error) {
out, err := reviewRunFunc(repoDir, "glab", "api", "user")
if err != nil {
return "", err
}
var user gitlabUserInfo
if err := decodeReviewJSON(out, &user); err != nil {
return "", err
}
if user.Username == "" {
return "", errors.New("failed to determine GitLab username")
}
return user.Username, nil
}

func selectReviewCandidates(
candidates []reviewCandidate,
targets map[string]reviewTarget,
) map[string]ReviewInfo {
reviews := make(map[string]ReviewInfo, len(targets))
exact := make(map[string]bool, len(targets))
for _, candidate := range candidates {
target, ok := targets[candidate.HeadBranch]
if !ok {
continue
}
if exact[candidate.HeadBranch] {
continue
}
info := ReviewInfo{
Provider: candidate.Provider,
Number: candidate.Number,
State: candidate.State,
HeadBranch: candidate.HeadBranch,
URL: candidate.URL,
}
if target.Commit != "" && candidate.HeadCommit != "" && target.Commit == candidate.HeadCommit {
reviews[candidate.HeadBranch] = info
exact[candidate.HeadBranch] = true
continue
}
if _, exists := reviews[candidate.HeadBranch]; !exists {
reviews[candidate.HeadBranch] = info
}
}
return reviews
}

func cachedReviewOutput(
provider string,
repoURL string,
author string,
load func() ([]byte, error),
) ([]byte, error) {
if out, ok := readReviewCache(provider, repoURL, author); ok {
return out, nil
}
out, err := load()
if err != nil {
return nil, err
}
writeReviewCache(provider, repoURL, author, out)
return out, nil
}

func readReviewCache(provider string, repoURL string, author string) ([]byte, bool) {
path, ok := reviewCachePath(provider, repoURL, author)
if !ok {
return nil, false
}
info, err := os.Stat(path)
if err != nil || time.Since(info.ModTime()) > reviewCacheTTL {
return nil, false
}
out, err := os.ReadFile(path)
if err != nil {
return nil, false
}
return out, true
}

func writeReviewCache(provider string, repoURL string, author string, out []byte) {
path, ok := reviewCachePath(provider, repoURL, author)
if !ok {
return
}
if err := os.MkdirAll(filepath.Dir(path), 0o755); err != nil {
return
}
tmp := path + ".tmp"
if err := os.WriteFile(tmp, out, 0o644); err != nil {
return
}
_ = os.Rename(tmp, path)
}

func reviewCachePath(provider string, repoURL string, author string) (string, bool) {
dir, err := reviewCacheDirFunc()
if err != nil || dir == "" {
return "", false
}
return reviews, nil
key := strings.Join([]string{reviewCacheVersion, provider, repoURL, author}, "\x00")
sum := sha256.Sum256([]byte(key))
return filepath.Join(dir, "gmc", "reviews", fmt.Sprintf("%x.json", sum)), true
}

func decodeReviewJSON(out []byte, target any) error {
Expand Down
Loading
Loading