diff --git a/CHANGELOG.md b/CHANGELOG.md
index 88621ab19..701625087 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -7,6 +7,20 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
## [Unreleased]
+### Added
+
+- **The pull request's description is editable in the app, and its reviewers are
+ picked there too.** The Overview tab was a read-only rendering of the body: a
+ description an agent wrote and you wanted to trim, and choosing who reviews,
+ were the last two things about a pull request that sent you to github.com in
+ the middle of the work. Overview now says who opened it, lists the review
+ roster — who was asked, who approved, who requested changes — with **Request
+ review** beside it, ticking anyone the repository allows on a review, and
+ carries an **Edit** for the description that saves straight to GitHub. A
+ reviewer who already answered comes back unticked: ticking them again is the
+ re-request. Merged and closed pull requests keep the roster and lose the
+ picker, which GitHub would refuse anyway.
+
### Changed
- **A review comment's box grows with the comment.** Every field review prose is
diff --git a/frontend/src/components/pulls/PullRequestView.tsx b/frontend/src/components/pulls/PullRequestView.tsx
index 9f07b8d77..a95053e93 100644
--- a/frontend/src/components/pulls/PullRequestView.tsx
+++ b/frontend/src/components/pulls/PullRequestView.tsx
@@ -33,7 +33,6 @@ import {
} from "@/lib/pulls/merge-gate"
import { useBranchRules } from "@/lib/pulls/use-branch-rules"
import { cn, errorText } from "@/lib/utils"
-import { Markdown } from "@/components/Markdown"
import { Button } from "@/components/ui/button"
import {
DropdownMenu,
@@ -47,6 +46,7 @@ import { PullsChecks } from "./PullsChecks"
import { PullsCommits } from "./PullsCommits"
import { PullsConversation } from "./PullsConversation"
import { PullsFiles } from "./PullsFiles"
+import { PullsOverview } from "./PullsOverview"
import { ChecksStat, MergeableStat, ReviewStat, StateStat } from "./PullsStats"
import type { ThreadActions } from "./ReviewThread"
import { SubmitReviewDialog } from "./SubmitReviewDialog"
@@ -442,13 +442,7 @@ export function PullRequestView({
/>
)}
{tab === "overview" && (
-
+
)}
)}
diff --git a/frontend/src/components/pulls/PullsOverview.tsx b/frontend/src/components/pulls/PullsOverview.tsx
new file mode 100644
index 000000000..d60458c10
--- /dev/null
+++ b/frontend/src/components/pulls/PullsOverview.tsx
@@ -0,0 +1,238 @@
+import { useState } from "react"
+import { toast } from "sonner"
+import {
+ Check,
+ ChevronDown,
+ CircleDashed,
+ MessageSquare,
+ Pencil,
+ UserPlus,
+ Users,
+ X,
+ type LucideIcon,
+} from "lucide-react"
+import type { PullRequestDetail, PullRequestReviewer, ReviewCandidate } from "@/lib/api-types"
+import { pendingReviewerLogins, pickableReviewers } from "@/lib/pulls/reviewers"
+import { ProjectService } from "@/lib/rpc"
+import { cn, errorText } from "@/lib/utils"
+import { Markdown } from "@/components/Markdown"
+import { Button } from "@/components/ui/button"
+import {
+ DropdownMenu,
+ DropdownMenuCheckboxItem,
+ DropdownMenuContent,
+ DropdownMenuItem,
+ DropdownMenuTrigger,
+} from "@/components/ui/dropdown-menu"
+import { commentFieldClass } from "./CommentBox"
+
+interface PullsOverviewProps {
+ path: string
+ detail: PullRequestDetail
+ /** Re-read the pull request: an edit here lands on GitHub, and GitHub's
+ * answer — not a local copy of it — is what the screen then shows. */
+ onRefresh: () => void
+}
+
+// PullsOverview is the "Overview" tab: who opened the pull request, who is
+// reviewing it, and what it says. The description and the reviewers are both
+// editable here — they were the last two things about a pull request that sent
+// the reader out to github.com.
+export function PullsOverview({ path, detail, onRefresh }: PullsOverviewProps) {
+ // null is "not editing"; an empty string is a description being cleared, which
+ // is a legitimate edit and must not read as the same thing.
+ const [draft, setDraft] = useState(null)
+ const [saving, setSaving] = useState(false)
+
+ const save = async () => {
+ if (draft === null) {
+ return
+ }
+ setSaving(true)
+ try {
+ await ProjectService.EditPullRequestBody(path, detail.number, draft)
+ setDraft(null)
+ onRefresh()
+ } catch (err: unknown) {
+ toast.error(`Couldn’t save the description: ${errorText(err)}`)
+ } finally {
+ setSaving(false)
+ }
+ }
+
+ return (
+
+
+ {detail.author !== "" && (
+
+ Opened by {detail.author}
+
+ )}
+
+
+ )
+}
+
+// The roster, and the one control that changes it. The picker is the only way a
+// reviewer is added or dropped — a second affordance on the chips would be two
+// ways to say the same thing, and the tick already reads as "asked".
+function Reviewers({ path, detail, onRefresh }: PullsOverviewProps) {
+ // Read on first open rather than with the pull request: most readings of a
+ // pull request never touch the roster, and this is a gh round-trip.
+ const [candidates, setCandidates] = useState(null)
+ const [busy, setBusy] = useState(false)
+ const roster = detail.reviewers ?? []
+ const pending = pendingReviewerLogins(detail.reviewers)
+ const pickable = pickableReviewers(candidates, detail.author)
+
+ const load = async () => {
+ if (candidates !== null) {
+ return
+ }
+ try {
+ setCandidates((await ProjectService.AssignableReviewers(path)) ?? [])
+ } catch (err: unknown) {
+ setCandidates([])
+ toast.error(`Couldn’t read who can review: ${errorText(err)}`)
+ }
+ }
+
+ const toggle = async (login: string, requested: boolean) => {
+ setBusy(true)
+ try {
+ await ProjectService.RequestReview(path, detail.number, login, requested)
+ onRefresh()
+ } catch (err: unknown) {
+ const asked = requested ? "request a review from" : "withdraw the review request for"
+ toast.error(`Couldn’t ${asked} ${login}: ${errorText(err)}`)
+ } finally {
+ setBusy(false)
+ }
+ }
+
+ return (
+
+ Reviewers
+ {roster.length === 0 && Nobody yet}
+ {roster.map((reviewer) => (
+
+ ))}
+ {/* A closed or merged pull request takes no new reviewer: GitHub refuses
+ the request, so the control goes rather than failing on click. */}
+ {detail.state === "OPEN" && (
+ {
+ if (open) {
+ void load()
+ }
+ }}
+ >
+
+
+ Request review
+
+
+ }
+ />
+
+ {candidates === null ? (
+ Reading who can review…
+ ) : pickable.length === 0 ? (
+ Nobody else can review this
+ ) : (
+ pickable.map((candidate) => (
+ void toggle(candidate.login, next)}
+ >
+ {candidate.login}
+ {candidate.name !== "" && (
+ {candidate.name}
+ )}
+
+ ))
+ )}
+
+
+ )}
+
+ )
+}
+
+// What each verdict reads as on a chip. A reviewer with no verdict yet is the
+// default below: the review is still owed.
+const VERDICTS: Record = {
+ APPROVED: { icon: Check, tone: "text-emerald-500", label: "approved" },
+ CHANGES_REQUESTED: { icon: X, tone: "text-destructive", label: "requested changes" },
+ COMMENTED: { icon: MessageSquare, tone: "text-muted-foreground", label: "commented" },
+ DISMISSED: { icon: CircleDashed, tone: "text-muted-foreground", label: "review dismissed" },
+}
+
+const AWAITING = { icon: CircleDashed, tone: "text-muted-foreground", label: "review requested" }
+
+function ReviewerChip({ reviewer }: { reviewer: PullRequestReviewer }) {
+ const verdict = VERDICTS[reviewer.state] ?? AWAITING
+ // A team only ever arrives on the roster as a pending request — a verdict
+ // belongs to the person who left it — so its own glyph hides nothing.
+ const Icon = reviewer.isTeam ? Users : verdict.icon
+ return (
+
+
+ {reviewer.login}
+
+ )
+}
diff --git a/frontend/src/lib/api-types.ts b/frontend/src/lib/api-types.ts
index b1bad2fb9..4068c1907 100644
--- a/frontend/src/lib/api-types.ts
+++ b/frontend/src/lib/api-types.ts
@@ -120,6 +120,8 @@ export interface PullRequestDetail {
url: string
title: string
body: string
+ /** Who opened it: a login, or a display name when gh reports no login. */
+ author: string
/** gh: OPEN | CLOSED | MERGED. Only a number-addressed lookup returns a
* non-OPEN one; the branch lookup still hides them. */
state: string
@@ -132,6 +134,9 @@ export interface PullRequestDetail {
/** gh's aggregate verdict: APPROVED | CHANGES_REQUESTED | REVIEW_REQUIRED;
* "" where the repository requires no review. */
reviewDecision: string
+ /** Who was asked for a review and who answered, in one list; null when
+ * nobody is on it. */
+ reviewers: PullRequestReviewer[] | null
baseRefName: string
headRefName: string
changedFiles: number
@@ -148,6 +153,25 @@ export interface PullRequestDetail {
commits: PullRequestCommit[] | null
}
+/** internal/project.PRReviewer — one entry of the review roster: somebody asked
+ * and still owing a review, or somebody who has answered. */
+export interface PullRequestReviewer {
+ /** A login, or a team's slug. */
+ login: string
+ /** A team, which gh addresses as org/slug — so it is shown, never edited. */
+ isTeam: boolean
+ /** "" while the review is still owed, else gh's APPROVED | CHANGES_REQUESTED
+ * | COMMENTED | DISMISSED. */
+ state: string
+}
+
+/** internal/project.PRCandidate — one account the reviewer picker may offer. */
+export interface ReviewCandidate {
+ login: string
+ /** The display name; "" when the account has none. */
+ name: string
+}
+
/** internal/project.PRReview — one submitted review and its verdict. */
export interface PullRequestReview {
author: string
diff --git a/frontend/src/lib/pulls/merge-gate.test.ts b/frontend/src/lib/pulls/merge-gate.test.ts
index fb9619724..51ba7c89e 100644
--- a/frontend/src/lib/pulls/merge-gate.test.ts
+++ b/frontend/src/lib/pulls/merge-gate.test.ts
@@ -21,11 +21,13 @@ const detail = (over: Partial = {}): PullRequestDetail => ({
url: "https://github.com/o/l/pull/130",
title: "feat: something",
body: "",
+ author: "omartelo",
state: "OPEN",
isDraft: false,
mergeable: "MERGEABLE",
mergeStateStatus: "CLEAN",
reviewDecision: "",
+ reviewers: null,
baseRefName: "main",
headRefName: "quiet-willow",
changedFiles: 1,
diff --git a/frontend/src/lib/pulls/reviewers.test.ts b/frontend/src/lib/pulls/reviewers.test.ts
new file mode 100644
index 000000000..1ed066c76
--- /dev/null
+++ b/frontend/src/lib/pulls/reviewers.test.ts
@@ -0,0 +1,43 @@
+import { describe, expect, it } from "vitest"
+import type { PullRequestReviewer, ReviewCandidate } from "@/lib/api-types"
+import { pendingReviewerLogins, pickableReviewers } from "./reviewers"
+
+const candidate = (login: string, name = ""): ReviewCandidate => ({ login, name })
+const reviewer = (login: string, state = "", isTeam = false): PullRequestReviewer => ({
+ login,
+ state,
+ isTeam,
+})
+
+describe("pickableReviewers", () => {
+ it("sorts by login", () => {
+ const picked = pickableReviewers([candidate("cai"), candidate("ana"), candidate("bo")], "zed")
+ expect(picked.map((c) => c.login)).toEqual(["ana", "bo", "cai"])
+ })
+
+ // GitHub refuses a review request addressed to the author, so offering them
+ // is offering a click that can only fail.
+ it("drops the pull request's own author", () => {
+ const picked = pickableReviewers([candidate("ana"), candidate("bo")], "ana")
+ expect(picked.map((c) => c.login)).toEqual(["bo"])
+ })
+
+ it("has nothing to offer before the list is read", () => {
+ expect(pickableReviewers(null, "ana")).toEqual([])
+ })
+})
+
+describe("pendingReviewerLogins", () => {
+ it("ticks only who still owes a review", () => {
+ const pending = pendingReviewerLogins([
+ reviewer("ana", "APPROVED"),
+ reviewer("bo"),
+ reviewer("platform", "", true),
+ ])
+ expect([...pending].sort()).toEqual(["bo", "platform"])
+ })
+
+ it("is empty with no roster", () => {
+ expect(pendingReviewerLogins(null).size).toBe(0)
+ })
+})
diff --git a/frontend/src/lib/pulls/reviewers.ts b/frontend/src/lib/pulls/reviewers.ts
new file mode 100644
index 000000000..4e07c599e
--- /dev/null
+++ b/frontend/src/lib/pulls/reviewers.ts
@@ -0,0 +1,22 @@
+import type { PullRequestReviewer, ReviewCandidate } from "@/lib/api-types"
+
+// Who the picker may offer: everyone the repository allows on a review, minus
+// the pull request's own author — GitHub refuses a request addressed to them,
+// and an item that can only fail is not an item. Sorted by login so the menu
+// keeps one order between reads, whatever order GitHub answered in.
+export function pickableReviewers(
+ candidates: ReviewCandidate[] | null,
+ author: string,
+): ReviewCandidate[] {
+ return (candidates ?? [])
+ .filter((c) => c.login !== "" && c.login !== author)
+ .sort((a, b) => a.login.localeCompare(b.login))
+}
+
+// The logins the picker shows ticked: those still owing an answer. Somebody who
+// already reviewed is not ticked — GitHub cleared their request when the review
+// landed, so the tick would offer to withdraw nothing, and the empty box is what
+// asks them again.
+export function pendingReviewerLogins(reviewers: PullRequestReviewer[] | null): Set {
+ return new Set((reviewers ?? []).filter((r) => r.state === "").map((r) => r.login))
+}
diff --git a/frontend/src/lib/rpc.ts b/frontend/src/lib/rpc.ts
index d50a0ff0b..01be0518e 100644
--- a/frontend/src/lib/rpc.ts
+++ b/frontend/src/lib/rpc.ts
@@ -24,6 +24,7 @@ import type {
PullRequestDetail,
PullRequestSummary,
RecentProject,
+ ReviewCandidate,
ReviewEvent,
StoredProject,
StoredSession,
@@ -205,6 +206,16 @@ export const ProjectService = {
/** Comment on the pull request itself — no file, no line. */
CommentOnPullRequest: (path: string, number: number, body: string) =>
call("project.CommentOnPullRequest", [path, number, body]),
+ /** Replace the PR's description. An empty body clears it. */
+ EditPullRequestBody: (path: string, number: number, body: string) =>
+ call("project.EditPullRequestBody", [path, number, body]),
+ /** Ask a login for a review, or withdraw the request. GitHub refuses one
+ * addressed to the PR's own author. */
+ RequestReview: (path: string, number: number, login: string, requested: boolean) =>
+ call("project.RequestReview", [path, number, login, requested]),
+ /** Who this repository allows on a review — the picker's list; capped at 100. */
+ AssignableReviewers: (path: string) =>
+ call("project.AssignableReviewers", [path]),
/** What a repository's rulesets say about merging into one branch. Answers
* per branch, not per pull request — every PR onto the same base shares it. */
BranchRules: (path: string, branch: string) =>
diff --git a/internal/project/pr.go b/internal/project/pr.go
index 32580e52e..00dd02cbe 100644
--- a/internal/project/pr.go
+++ b/internal/project/pr.go
@@ -23,6 +23,9 @@ const (
prMergeTimeout = 30 * time.Second
prCreateTimeout = 20 * time.Second
prReviewTimeout = 20 * time.Second
+ // An edit (the description, a reviewer) is one mutating round-trip, the same
+ // budget a review write gets.
+ prEditTimeout = 20 * time.Second
// A checkout fetches the head ref, so it is bounded by the size of the
// branch rather than by a round-trip.
prCheckoutTimeout = 90 * time.Second
@@ -65,7 +68,7 @@ func runGH(timeout time.Duration, dir, token string, args ...string) ([]byte, er
}
// prViewFields is the gh `pr view --json` selection backing the Pulls panel.
-const prViewFields = "number,url,state,title,body,isDraft,mergeable,mergeStateStatus,reviewDecision,baseRefName,headRefName,isCrossRepository,maintainerCanModify,statusCheckRollup,changedFiles,commits"
+const prViewFields = "number,url,state,title,body,author,isDraft,mergeable,mergeStateStatus,reviewDecision,reviewRequests,latestReviews,baseRefName,headRefName,isCrossRepository,maintainerCanModify,statusCheckRollup,changedFiles,commits"
// prSelector renders the pull request argument gh's subcommands take ahead of
// their flags. Zero means "the branch checked out at path" — gh's own default,
@@ -96,6 +99,10 @@ type PRDetail struct {
URL string `json:"url"`
Title string `json:"title"`
Body string `json:"body"`
+ // Author is the login of whoever opened it, or their display name when gh
+ // reports no login. GitHub refuses a review request addressed to them, which
+ // is what keeps them out of the picker.
+ Author string `json:"author"`
// State is gh's OPEN | CLOSED | MERGED. Only a number-addressed lookup can
// return a non-OPEN one; the branch lookup still gates them out.
State string `json:"state"`
@@ -110,13 +117,15 @@ type PRDetail struct {
// ReviewDecision is gh's aggregate verdict — APPROVED | CHANGES_REQUESTED |
// REVIEW_REQUIRED — and "" where the repository requires no review. Who
// reviewed is a different field (latestReviews) this does not carry.
- ReviewDecision string `json:"reviewDecision"`
- BaseRefName string `json:"baseRefName"`
- HeadRefName string `json:"headRefName"`
- ChangedFiles int `json:"changedFiles"`
- Checks ChecksRollup `json:"checks"`
- CheckRuns []CheckItem `json:"checkRuns"`
- Commits []PRCommit `json:"commits"`
+ ReviewDecision string `json:"reviewDecision"`
+ // Reviewers is who was asked and who answered, in one list (prreviewers.go).
+ Reviewers []PRReviewer `json:"reviewers"`
+ BaseRefName string `json:"baseRefName"`
+ HeadRefName string `json:"headRefName"`
+ ChangedFiles int `json:"changedFiles"`
+ Checks ChecksRollup `json:"checks"`
+ CheckRuns []CheckItem `json:"checkRuns"`
+ Commits []PRCommit `json:"commits"`
// IsCrossRepository marks a head branch that lives on a fork.
IsCrossRepository bool `json:"isCrossRepository"`
// MaintainerCanModify is GitHub's "allow edits by maintainers" on a fork PR,
@@ -140,22 +149,25 @@ type PRCommit struct {
// ghPRView mirrors the requested gh payload; statusCheckRollup is reduced to
// ChecksRollup inside parsePRDetail and never leaves this package raw.
type ghPRView struct {
- Number int `json:"number"`
- URL string `json:"url"`
- State string `json:"state"`
- Title string `json:"title"`
- Body string `json:"body"`
- IsDraft bool `json:"isDraft"`
- Mergeable string `json:"mergeable"`
- MergeStateStatus string `json:"mergeStateStatus"`
- ReviewDecision string `json:"reviewDecision"`
- BaseRefName string `json:"baseRefName"`
- HeadRefName string `json:"headRefName"`
- ChangedFiles int `json:"changedFiles"`
- IsCrossRepository bool `json:"isCrossRepository"`
- MaintainerCanModify bool `json:"maintainerCanModify"`
- StatusCheckRollup []checkItem `json:"statusCheckRollup"`
- Commits []ghCommit `json:"commits"`
+ Number int `json:"number"`
+ URL string `json:"url"`
+ State string `json:"state"`
+ Title string `json:"title"`
+ Body string `json:"body"`
+ Author ghPRAuthor `json:"author"`
+ IsDraft bool `json:"isDraft"`
+ Mergeable string `json:"mergeable"`
+ MergeStateStatus string `json:"mergeStateStatus"`
+ ReviewDecision string `json:"reviewDecision"`
+ ReviewRequests []ghReviewRequest `json:"reviewRequests"`
+ LatestReviews []ghLatestReview `json:"latestReviews"`
+ BaseRefName string `json:"baseRefName"`
+ HeadRefName string `json:"headRefName"`
+ ChangedFiles int `json:"changedFiles"`
+ IsCrossRepository bool `json:"isCrossRepository"`
+ MaintainerCanModify bool `json:"maintainerCanModify"`
+ StatusCheckRollup []checkItem `json:"statusCheckRollup"`
+ Commits []ghCommit `json:"commits"`
}
// ghCommit is one entry of gh's commits array. Authors is a list because of
@@ -210,11 +222,13 @@ func parsePRDetail(out []byte, openOnly bool) (*PRDetail, error) {
URL: v.URL,
Title: v.Title,
Body: v.Body,
+ Author: firstNonEmpty(v.Author.Login, v.Author.Name),
State: v.State,
IsDraft: v.IsDraft,
Mergeable: v.Mergeable,
MergeStateStatus: v.MergeStateStatus,
ReviewDecision: v.ReviewDecision,
+ Reviewers: toReviewers(v.LatestReviews, v.ReviewRequests),
BaseRefName: v.BaseRefName,
HeadRefName: v.HeadRefName,
ChangedFiles: v.ChangedFiles,
@@ -323,6 +337,15 @@ func (s *Service) CreatePullRequest(path string) error {
return nil
}
+// EditPullRequestBody replaces the pull request's description — the one thing
+// about a pull request that could only be changed on github.com, and the one an
+// agent's first draft usually needs. An empty body is a real edit (it clears the
+// description), so nothing is rejected here.
+func (s *Service) EditPullRequestBody(path string, number int, body string) error {
+ _, err := s.gh(prEditTimeout, path, prArgs("edit", number, "--body", body)...)
+ return err
+}
+
// PullRequestDiff returns the unified diff of the path's branch pull request —
// every change the PR would merge into its base, as GitHub computes it — for the
// Pulls screen's "Files changed" tab. --color never keeps the output plain so
diff --git a/internal/project/prreviewers.go b/internal/project/prreviewers.go
new file mode 100644
index 000000000..5ff398836
--- /dev/null
+++ b/internal/project/prreviewers.go
@@ -0,0 +1,133 @@
+package project
+
+import (
+ "encoding/json"
+ "fmt"
+)
+
+// prAssignableQuery lists the accounts that may be asked for a review — GitHub's
+// assignableUsers, the same list gh's own `pr edit` prompt offers, and one a
+// read-only account can still see (the collaborators endpoint needs push).
+//
+// Ceiling: the first 100, unpaged. A larger organisation keeps a tail the picker
+// cannot reach, and github.com is the way to those until somebody hits it.
+const prAssignableQuery = `query($owner:String!,$repo:String!){
+ repository(owner:$owner,name:$repo){
+ assignableUsers(first:100){nodes{login name}}
+ }
+}`
+
+// PRReviewer is one entry of a pull request's review roster: somebody who was
+// asked and has not answered yet, or somebody who has.
+type PRReviewer struct {
+ // Login is the account's login, or a team's slug — which is not what gh takes
+ // to add or remove one (that is org/slug), so a team is shown and not edited.
+ Login string `json:"login"`
+ IsTeam bool `json:"isTeam"`
+ // State is "" while the review is still owed, else gh's APPROVED |
+ // CHANGES_REQUESTED | COMMENTED | DISMISSED.
+ State string `json:"state"`
+}
+
+// PRCandidate is one account the reviewer picker may offer.
+type PRCandidate struct {
+ Login string `json:"login"`
+ Name string `json:"name"`
+}
+
+// ghReviewRequest is one entry of gh's reviewRequests: a User carries a login, a
+// Team a slug, and __typename is the only thing that says which.
+type ghReviewRequest struct {
+ TypeName string `json:"__typename"`
+ Login string `json:"login"`
+ Slug string `json:"slug"`
+}
+
+// ghLatestReview is one entry of gh's latestReviews — the last review each
+// person left.
+type ghLatestReview struct {
+ Author ghPRAuthor `json:"author"`
+ State string `json:"state"`
+}
+
+// toReviewers merges gh's two halves of the roster into one list. They have to
+// be read together: a submitted review clears its request, so the pending half
+// alone reads as "nobody is reviewing this" the moment the first review lands,
+// and the answered half alone hides everyone still owing one.
+func toReviewers(reviews []ghLatestReview, requests []ghReviewRequest) []PRReviewer {
+ out := make([]PRReviewer, 0, len(reviews)+len(requests))
+ at := make(map[string]int, len(reviews))
+ for _, r := range reviews {
+ login := firstNonEmpty(r.Author.Login, r.Author.Name)
+ if login == "" {
+ continue
+ }
+ at[login] = len(out)
+ out = append(out, PRReviewer{Login: login, State: r.State})
+ }
+ for _, req := range requests {
+ login := firstNonEmpty(req.Login, req.Slug)
+ if login == "" {
+ continue
+ }
+ // Asked again after reviewing: GitHub keeps the old verdict in
+ // latestReviews and re-files the request, and what stands now is the
+ // answer owed, not the one already given.
+ if i, ok := at[login]; ok {
+ out[i].State = ""
+ continue
+ }
+ out = append(out, PRReviewer{Login: login, IsTeam: req.TypeName == "Team"})
+ }
+ if len(out) == 0 {
+ return nil
+ }
+ return out
+}
+
+// AssignableReviewers lists who this repository allows on a review, so the
+// picker offers accounts GitHub will accept rather than a typed login it would
+// refuse. Returns nil for a repository that reports none.
+func (s *Service) AssignableReviewers(path string) ([]PRCandidate, error) {
+ out, err := s.gh(prReadTimeout, path, "api", "graphql",
+ "-F", "owner={owner}", "-F", "repo={repo}", "-f", "query="+prAssignableQuery)
+ if err != nil {
+ return nil, err
+ }
+ return parseAssignable(out)
+}
+
+func parseAssignable(out []byte) ([]PRCandidate, error) {
+ var payload struct {
+ Data struct {
+ Repository struct {
+ AssignableUsers struct {
+ Nodes []PRCandidate `json:"nodes"`
+ } `json:"assignableUsers"`
+ } `json:"repository"`
+ } `json:"data"`
+ }
+ if err := json.Unmarshal(out, &payload); err != nil {
+ return nil, errUnreadableAnswer(err)
+ }
+ nodes := payload.Data.Repository.AssignableUsers.Nodes
+ if len(nodes) == 0 {
+ return nil, nil
+ }
+ return nodes, nil
+}
+
+// RequestReview asks login for a review on the pull request, or withdraws the
+// request. GitHub refuses one addressed to the pull request's own author, which
+// is why the picker never offers them (the detail carries the author).
+func (s *Service) RequestReview(path string, number int, login string, requested bool) error {
+ if login == "" {
+ return fmt.Errorf("a reviewer is required")
+ }
+ flag := "--remove-reviewer"
+ if requested {
+ flag = "--add-reviewer"
+ }
+ _, err := s.gh(prEditTimeout, path, prArgs("edit", number, flag, login)...)
+ return err
+}
diff --git a/internal/project/prreviewers_test.go b/internal/project/prreviewers_test.go
new file mode 100644
index 000000000..0b0da08cd
--- /dev/null
+++ b/internal/project/prreviewers_test.go
@@ -0,0 +1,194 @@
+package project
+
+import (
+ "errors"
+ "slices"
+ "testing"
+)
+
+func TestToReviewers(t *testing.T) {
+ t.Run("the two halves merge into one roster", func(t *testing.T) {
+ reviewers := toReviewers(
+ []ghLatestReview{{Author: ghPRAuthor{Login: "ana"}, State: "APPROVED"}},
+ []ghReviewRequest{{TypeName: "User", Login: "bo"}},
+ )
+ want := []PRReviewer{
+ {Login: "ana", State: "APPROVED"},
+ {Login: "bo"},
+ }
+ if !slices.Equal(reviewers, want) {
+ t.Errorf("wrong roster: %+v", reviewers)
+ }
+ })
+
+ // The one case the merge exists for: GitHub keeps the old verdict in
+ // latestReviews when a reviewer is asked again, and what stands is the answer
+ // now owed — a roster still reading "approved" would hide the re-request.
+ t.Run("a re-request outranks the verdict it follows", func(t *testing.T) {
+ reviewers := toReviewers(
+ []ghLatestReview{{Author: ghPRAuthor{Login: "ana"}, State: "CHANGES_REQUESTED"}},
+ []ghReviewRequest{{TypeName: "User", Login: "ana"}},
+ )
+ if len(reviewers) != 1 {
+ t.Fatalf("expected one entry, got %+v", reviewers)
+ }
+ if reviewers[0].Login != "ana" || reviewers[0].State != "" {
+ t.Errorf("expected ana pending, got %+v", reviewers[0])
+ }
+ })
+
+ t.Run("a team rides its slug and is marked as one", func(t *testing.T) {
+ reviewers := toReviewers(nil, []ghReviewRequest{{TypeName: "Team", Slug: "platform"}})
+ if len(reviewers) != 1 || reviewers[0].Login != "platform" || !reviewers[0].IsTeam {
+ t.Errorf("wrong team entry: %+v", reviewers)
+ }
+ })
+
+ t.Run("a deleted account leaves no nameless entry", func(t *testing.T) {
+ reviewers := toReviewers(
+ []ghLatestReview{{State: "APPROVED"}},
+ []ghReviewRequest{{TypeName: "User"}},
+ )
+ if reviewers != nil {
+ t.Errorf("expected no roster, got %+v", reviewers)
+ }
+ })
+
+ t.Run("nothing on either side is no roster", func(t *testing.T) {
+ if reviewers := toReviewers(nil, nil); reviewers != nil {
+ t.Errorf("expected nil, got %+v", reviewers)
+ }
+ })
+}
+
+func TestParsePRDetailReviewers(t *testing.T) {
+ out := []byte(`{
+ "number": 7,
+ "state": "OPEN",
+ "author": {"login": "ana"},
+ "latestReviews": [{"author": {"login": "bo"}, "state": "APPROVED"}],
+ "reviewRequests": [{"__typename": "User", "login": "cai"}]
+ }`)
+ pr, err := parsePRDetail(out, true)
+ if err != nil {
+ t.Fatalf("unexpected error: %v", err)
+ }
+ if pr == nil {
+ t.Fatal("expected a detail, got nil")
+ }
+ if pr.Author != "ana" {
+ t.Errorf("wrong author: %q", pr.Author)
+ }
+ want := []PRReviewer{{Login: "bo", State: "APPROVED"}, {Login: "cai"}}
+ if !slices.Equal(pr.Reviewers, want) {
+ t.Errorf("wrong roster: %+v", pr.Reviewers)
+ }
+}
+
+func TestAssignableReviewers(t *testing.T) {
+ t.Run("the nodes are the picker's list", func(t *testing.T) {
+ gh := &fakeGH{out: []byte(`{"data":{"repository":{"assignableUsers":{"nodes":[
+ {"login":"ana","name":"Ana"},{"login":"bo","name":""}]}}}}`)}
+ people, err := withGH(gh).AssignableReviewers("/repo")
+ if err != nil {
+ t.Fatalf("unexpected error: %v", err)
+ }
+ want := []PRCandidate{{Login: "ana", Name: "Ana"}, {Login: "bo"}}
+ if !slices.Equal(people, want) {
+ t.Errorf("wrong list: %+v", people)
+ }
+ if gh.dir != "/repo" {
+ t.Errorf("wrong directory: %q", gh.dir)
+ }
+ })
+
+ t.Run("a repository with nobody answers nothing", func(t *testing.T) {
+ gh := &fakeGH{out: []byte(`{"data":{"repository":{"assignableUsers":{"nodes":[]}}}}`)}
+ people, err := withGH(gh).AssignableReviewers("/repo")
+ if err != nil || people != nil {
+ t.Errorf("expected an empty answer, got %+v (%v)", people, err)
+ }
+ })
+
+ t.Run("gh's failure travels", func(t *testing.T) {
+ gh := &fakeGH{err: errors.New("gh is not signed in to GitHub.")}
+ if _, err := withGH(gh).AssignableReviewers("/repo"); err == nil {
+ t.Error("expected an error")
+ }
+ })
+
+ t.Run("an unreadable answer is an error, not an empty list", func(t *testing.T) {
+ if _, err := withGH(&fakeGH{out: []byte(`{not json`)}).AssignableReviewers("/repo"); err == nil {
+ t.Error("expected an error")
+ }
+ })
+}
+
+func TestRequestReview(t *testing.T) {
+ t.Run("asking adds the reviewer to the numbered pull request", func(t *testing.T) {
+ gh := &fakeGH{}
+ if err := withGH(gh).RequestReview("/repo", 12, "ana", true); err != nil {
+ t.Fatalf("unexpected error: %v", err)
+ }
+ want := []string{"pr", "edit", "12", "--add-reviewer", "ana"}
+ if !slices.Equal(gh.args, want) {
+ t.Errorf("wrong args: %v", gh.args)
+ }
+ })
+
+ t.Run("withdrawing removes them", func(t *testing.T) {
+ gh := &fakeGH{}
+ if err := withGH(gh).RequestReview("/repo", 12, "ana", false); err != nil {
+ t.Fatalf("unexpected error: %v", err)
+ }
+ want := []string{"pr", "edit", "12", "--remove-reviewer", "ana"}
+ if !slices.Equal(gh.args, want) {
+ t.Errorf("wrong args: %v", gh.args)
+ }
+ })
+
+ // Nothing reaches gh without a name: an empty one would leave the flag
+ // holding an argument nobody meant to send.
+ t.Run("no reviewer, no call", func(t *testing.T) {
+ gh := &fakeGH{}
+ if err := withGH(gh).RequestReview("/repo", 12, "", true); err == nil {
+ t.Error("expected an error")
+ }
+ if gh.calls != 0 {
+ t.Errorf("expected no gh call, got %d", gh.calls)
+ }
+ })
+}
+
+func TestEditPullRequestBody(t *testing.T) {
+ t.Run("the body replaces the description of the numbered PR", func(t *testing.T) {
+ gh := &fakeGH{}
+ if err := withGH(gh).EditPullRequestBody("/repo", 12, "what it does"); err != nil {
+ t.Fatalf("unexpected error: %v", err)
+ }
+ want := []string{"pr", "edit", "12", "--body", "what it does"}
+ if !slices.Equal(gh.args, want) {
+ t.Errorf("wrong args: %v", gh.args)
+ }
+ })
+
+ // Clearing the description is an edit like any other, and gh applies the
+ // flag it was given rather than skipping an empty one.
+ t.Run("an empty body still goes out", func(t *testing.T) {
+ gh := &fakeGH{}
+ if err := withGH(gh).EditPullRequestBody("/repo", 12, ""); err != nil {
+ t.Fatalf("unexpected error: %v", err)
+ }
+ want := []string{"pr", "edit", "12", "--body", ""}
+ if !slices.Equal(gh.args, want) {
+ t.Errorf("wrong args: %v", gh.args)
+ }
+ })
+
+ t.Run("gh's refusal travels", func(t *testing.T) {
+ gh := &fakeGH{err: errors.New("could not resolve to a Repository")}
+ if err := withGH(gh).EditPullRequestBody("/repo", 12, "x"); err == nil {
+ t.Error("expected an error")
+ }
+ })
+}