Skip to content

SK-16: Fix gh errors and own-activity detection #17

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
mrdon merged 1 commit into from
Apr 9, 2026
Merged

SK-16: Fix gh errors and own-activity detection #17

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
71 changes: 23 additions & 48 deletions internal/github/fetch.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,12 +14,12 @@ import (
// ListOpenPRsMeta returns lightweight PR metadata (no diffs) — fast single API call.
func ListOpenPRsMeta(repo string) ([]map[string]any, error) {
logger.Info("listing open PRs for %s", repo)
out, err := exec.Command("gh", "pr", "list",
out, err := ghOutput(exec.Command("gh", "pr", "list",
"--repo", repo,
"--state", "open",
"--json", "number,title,author,url,createdAt,additions,deletions,files,body,reviewRequests,headRefOid,headRefName,mergeStateStatus,commits",
"--json", "number,title,author,url,createdAt,additions,deletions,files,body,reviewRequests,headRefOid,headRefName,mergeStateStatus",
"--limit", "50",
).Output()
))
if err != nil {
return nil, fmt.Errorf("gh pr list: %w", err)
}
Expand Down Expand Up @@ -133,8 +133,7 @@ func FetchPRDetails(repo string, raw map[string]any) (*PR, error) {
Body: body,
HeadSHA: fmt.Sprintf("%v", raw["headRefOid"]),
HeadRefName: fmt.Sprintf("%v", raw["headRefName"]),
LatestCommitAuthor: latestCommitAuthorFromRaw(raw),
State: state,
State: state,
MergeStateStatus: mergeStateStatus,
Checks: checks,
Reviews: reviews,
Expand Down Expand Up @@ -163,8 +162,8 @@ func FetchPRActivity(repo string, number int) (*PRActivity, error) {
var state, mergeStateStatus string
go func() {
defer wg.Done()
out, err := exec.Command("gh", "pr", "view", fmt.Sprintf("%d", number),
"--repo", repo, "--json", "title,body,headRefOid,headRefName,state,mergeStateStatus").Output()
out, err := ghOutput(exec.Command("gh", "pr", "view", fmt.Sprintf("%d", number),
"--repo", repo, "--json", "title,body,headRefOid,headRefName,state,mergeStateStatus"))
if err != nil {
logger.Error("PR #%d: pr view meta: %v", number, err)
return
Expand Down Expand Up @@ -304,13 +303,13 @@ func FetchPRFull(repo string, existing *PR) (*PR, error) {
func ListMergedPRsMeta(repo, currentUser string, since time.Time) ([]map[string]any, error) {
sinceStr := since.Format("2006-01-02")
logger.Info("listing merged PRs for %s since %s", repo, sinceStr)
out, err := exec.Command("gh", "pr", "list",
out, err := ghOutput(exec.Command("gh", "pr", "list",
"--repo", repo,
"--state", "merged",
"--search", fmt.Sprintf("merged:>%s", sinceStr),
"--json", "number,title,author,url,createdAt,additions,deletions,files,body,reviewRequests,headRefOid,headRefName,mergeStateStatus,state,commits",
"--search", fmt.Sprintf("repo:%s merged:>%s", repo, sinceStr),
"--json", "number,title,author,url,createdAt,additions,deletions,files,body,reviewRequests,headRefOid,headRefName,mergeStateStatus,state",
"--limit", "50",
).Output()
))
if err != nil {
return nil, fmt.Errorf("gh pr list (merged): %w", err)
}
Expand All @@ -333,11 +332,11 @@ func ListMergedPRsMeta(repo, currentUser string, since time.Time) ([]map[string]

// FetchPRMeta returns lightweight metadata for a single PR by number.
func FetchPRMeta(repo string, number int) (map[string]any, error) {
out, err := exec.Command("gh", "pr", "view",
out, err := ghOutput(exec.Command("gh", "pr", "view",
fmt.Sprintf("%d", number),
"--repo", repo,
"--json", "number,title,author,url,createdAt,additions,deletions,files,body,reviewRequests,headRefOid,headRefName,mergeStateStatus,state,commits",
).Output()
"--json", "number,title,author,url,createdAt,additions,deletions,files,body,reviewRequests,headRefOid,headRefName,mergeStateStatus,state",
))
if err != nil {
return nil, fmt.Errorf("gh pr view %d: %w", number, err)
}
Expand All @@ -348,43 +347,19 @@ func FetchPRMeta(repo string, number int) (map[string]any, error) {
return raw, nil
}

// latestCommitAuthorFromRaw extracts the login of the most recent commit author
// from a raw PR JSON object containing a "commits" field. Returns "" if absent.
// gh returns commits ordered oldest-first, so the last entry is the most recent.
func latestCommitAuthorFromRaw(raw map[string]any) string {
commits, ok := raw["commits"].([]any)
if !ok || len(commits) == 0 {
return ""
}
last, ok := commits[len(commits)-1].(map[string]any)
if !ok {
return ""
}
authors, ok := last["authors"].([]any)
if !ok || len(authors) == 0 {
return ""
}
first, ok := authors[0].(map[string]any)
if !ok {
return ""
}
if login, ok := first["login"].(string); ok {
return login
}
return ""
}


func getDiff(repo string, number int) (string, error) {
out, err := exec.Command("gh", "pr", "diff", fmt.Sprintf("%d", number), "--repo", repo).Output()
out, err := ghOutput(exec.Command("gh", "pr", "diff", fmt.Sprintf("%d", number), "--repo", repo))
if err != nil {
return "", fmt.Errorf("gh pr diff: %w", err)
}
return string(out), nil
}

func getChecks(repo string, number int) ([]CheckStatus, error) {
out, err := exec.Command("gh", "pr", "checks", fmt.Sprintf("%d", number),
"--repo", repo, "--json", "name,state").Output()
out, err := ghOutput(exec.Command("gh", "pr", "checks", fmt.Sprintf("%d", number),
"--repo", repo, "--json", "name,state"))
if err != nil || len(strings.TrimSpace(string(out))) == 0 {
return nil, nil
}
Expand All @@ -404,10 +379,10 @@ func getChecks(repo string, number int) ([]CheckStatus, error) {
}

func getReviews(repo string, number int) ([]ReviewComment, error) {
out, err := exec.Command("gh", "api",
out, err := ghOutput(exec.Command("gh", "api",
fmt.Sprintf("repos/%s/pulls/%d/reviews", repo, number),
"--jq", `[.[] | {author: .user.login, body: .body, state: .state, submitted_at: .submitted_at}]`,
).Output()
))
if err != nil || len(strings.TrimSpace(string(out))) == 0 {
return nil, nil
}
Expand Down Expand Up @@ -438,11 +413,11 @@ func getReviews(repo string, number int) ([]ReviewComment, error) {
}

func getComments(repo string, number int) ([]ReviewComment, error) {
out, err := exec.Command("gh", "api",
out, err := ghOutput(exec.Command("gh", "api",
fmt.Sprintf("repos/%s/issues/%d/comments", repo, number),
"--paginate",
"--jq", `.[] | {id: .id, author: .user.login, body: .body, submitted_at: .created_at}`,
).Output()
))
if err != nil || len(strings.TrimSpace(string(out))) == 0 {
return nil, nil
}
Expand Down Expand Up @@ -475,11 +450,11 @@ func getComments(repo string, number int) ([]ReviewComment, error) {
}

func getInlineComments(repo string, number int) ([]ReviewComment, error) {
out, err := exec.Command("gh", "api",
out, err := ghOutput(exec.Command("gh", "api",
fmt.Sprintf("repos/%s/pulls/%d/comments", repo, number),
"--paginate",
"--jq", `.[] | {id: .id, in_reply_to_id: (.in_reply_to_id // 0), author: .user.login, body: .body, path: .path, line: (.line // .original_line // 0), submitted_at: .created_at}`,
).Output()
))
if err != nil || len(strings.TrimSpace(string(out))) == 0 {
return nil, nil
}
Expand Down
22 changes: 18 additions & 4 deletions internal/github/github.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,22 @@ import (
"sync"
)

// ghOutput runs an exec.Cmd and returns its stdout. On failure, it extracts
// stderr from the ExitError so callers get a useful message instead of just
// "exit status 1".
func ghOutput(cmd *exec.Cmd) ([]byte, error) {
out, err := cmd.Output()
if err != nil {
if ee, ok := err.(*exec.ExitError); ok && len(ee.Stderr) > 0 {
return nil, fmt.Errorf("%s: %s", cmd.Args[0], strings.TrimSpace(string(ee.Stderr)))
}
return nil, err
}
return out, nil
}

func CurrentUser() (string, error) {
out, err := exec.Command("gh", "api", "user", "--jq", ".login").Output()
out, err := ghOutput(exec.Command("gh", "api", "user", "--jq", ".login"))
if err != nil {
return "", fmt.Errorf("gh api user: %w", err)
}
Expand All @@ -26,7 +40,7 @@ func DetectRepo(dir string) (string, error) {

cmd := exec.Command("git", "remote", "get-url", "origin")
cmd.Dir = dir
out, err := cmd.Output()
out, err := ghOutput(cmd)
if err != nil {
return "", fmt.Errorf("no 'origin' remote found — prx requires a GitHub remote")
}
Expand Down Expand Up @@ -101,10 +115,10 @@ func RequestChanges(repo string, number int, body string) error {

// GetReactions returns all reactions on a PR (GitHub treats PRs as issues for reactions).
func GetReactions(repo string, number int) ([]Reaction, error) {
out, err := exec.Command("gh", "api",
out, err := ghOutput(exec.Command("gh", "api",
fmt.Sprintf("repos/%s/issues/%d/reactions", repo, number),
"--jq", `[.[] | {id: .id, user: .user.login, content: .content}]`,
).Output()
))
if err != nil || len(strings.TrimSpace(string(out))) == 0 {
return nil, nil
}
Expand Down
3 changes: 1 addition & 2 deletions internal/github/types.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,8 +41,7 @@ type PR struct {
Body string
HeadSHA string
HeadRefName string
LatestCommitAuthor string // login of the author of the most recent commit on the head ref
State string // "OPEN", "MERGED", or "CLOSED"
State string // "OPEN", "MERGED", or "CLOSED"
MergeStateStatus string // "CLEAN", "BLOCKED", "DIRTY", "BEHIND", "UNSTABLE", "UNKNOWN", etc.
Checks []CheckStatus
Reviews []ReviewComment // PR-level review submissions (APPROVED, CHANGES_REQUESTED, etc.)
Expand Down
11 changes: 5 additions & 6 deletions internal/tui/model_helpers.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -240,9 +240,8 @@ func (m *Model) computeHasNewContent(card *PRCard) {
commentDigests := diff.CommentDigestsFromPR(card.PR, m.app.CurrentUser)
inc := reviewstate.ComputeIncremental(fileNames, fileHunks, commentDigests, state)
hasChanges := inc.HasChanges
if hasChanges && card.PR.LatestCommitAuthor == m.app.CurrentUser {
// The diff changed since last snapshot but the most recent commit is
// ours — don't resurface PRs because of our own pushes.
if hasChanges && card.PR.Author == m.app.CurrentUser {
// It's our PR — our own diff changes should never resurface the card.
hasChanges = false
}
card.HasNewContent = hasChanges || inc.HasNewComments
Expand All @@ -267,9 +266,9 @@ func (m *Model) updateIncrementalState(card *PRCard) {
fileNames, fileHunks := diff.FileHunkInfo(card.parsedFiles)
commentDigests := diff.CommentDigestsFromPR(card.PR, m.app.CurrentUser)
state := reviewstate.ComputeIncremental(fileNames, fileHunks, commentDigests, card.ReviewState)
if state.HasChanges && card.PR.LatestCommitAuthor == m.app.CurrentUser {
// Latest commit is ours — treat all new hunks as already-seen so we
// don't get nagged about our own pushes (visibility & diff badges).
if state.HasChanges && card.PR.Author == m.app.CurrentUser {
// It's our PR — suppress our own diff changes in both the badge and
// the per-hunk highlights so only other people's comments surface.
for _, fileHunks := range state.HunkStatus {
for hi := range fileHunks {
fileHunks[hi] = reviewstate.StatusSeen
Expand Down
Loading