diff --git a/docs/2-handbook/swarm/ui.md b/docs/2-handbook/swarm/ui.md index 0e57bd4f..1fdd74d5 100644 --- a/docs/2-handbook/swarm/ui.md +++ b/docs/2-handbook/swarm/ui.md @@ -587,6 +587,37 @@ fail on their own: a repo list that fails to load shows its error beside the report, and a report that fails to load clears the table instead of leaving the previous repo's issues under the error. +### Pull requests + +The **pull requests** card, below the issue report on the **reports** tab, lists the open +pull requests of every repo that has one, from `GET /api/pr-report`, or of one +repo from `GET /api/repos/{org}/{repo}/pr-report`. It starts shut and reads +nothing until you open it. The **repo** select lists the repos from +`GET /api/pulls/repos`, and picking one fetches that repo's report. No filter +applies at first. + +Each row shows the repo, the PR, its title, author, labels, requested reviewers +and base branch, and what a reviewer or merger filters on: + +- **WIP**: the forge's draft flag. Giving the title a `WIP:` prefix sets it. +- **approvals**: how many approvals count against how many the base branch's protection + rule asks for (`1/2`). An approval counts when it stands as the reviewer's latest + decision, on the PR's current head, neither stale nor dismissed, and from a + reviewer whose approval the rule accepts. A reviewer whose latest review asks for changes shows **changes + requested**. +- **has all approvals**: the approvals meet the rule. A PR into a branch with no + protection rule has nothing to approve, so it has them all. +- **conflicts**: the forge can't merge the PR as it stands. +- **protected base**: the base branch has a protection rule. A stacked PR targets a + feature branch, so filtering this to **no** lists them. +- **CI**: the commit statuses on the head, as one state (`success`, `pending`, + `failure`, or a dash when it has none). + +Every column sorts, and the columns with a small set of values (repo, author, +labels, reviewers, base, the flags and CI) filter by picking values. The browser +keeps the filters you set and the repo you picked. The card fetches the report once per +repo choice, with no refresh timer. + ### Builds The builds sit in a card under the job graph on the **jobs** tab. While the graph diff --git a/frontend/packages/swarm-ui/src/pages/PrReportPage.tsx b/frontend/packages/swarm-ui/src/pages/PrReportPage.tsx new file mode 100644 index 00000000..9994e32b --- /dev/null +++ b/frontend/packages/swarm-ui/src/pages/PrReportPage.tsx @@ -0,0 +1,287 @@ +// — a card of the reports tab: the open pull requests of every +// repo that has one, the way the issue report lists issues. Backed by +// swarm-controller's `GET /api/pulls/repos` (the repo dropdown), +// `GET /api/pr-report` (every repo) and `GET /api/repos/{org}/{repo}/pr-report` +// (one repo): the flags a reviewer or merger filters on — WIP, has all required +// approvals, has conflicts, targets a protected branch — arrive resolved per +// row, so nothing here walks reviews or branch rules. +// +// The table's own column filters and sort do the narrowing, over what is +// loaded; only the repo selection fetches again. No default filter is applied. +// The filters the user sets are kept in the browser (the table's storage key, +// and the repo choice). +import { useEffect, useState } from "preact/hooks"; +import { ApiErrorPanel } from "@hive/shared/api-error-panel.js"; +import { readApiError, type ProblemDetails } from "@hive/shared/api-error.js"; +import { Badge, type BadgeTone } from "@hive/shared/badge.js"; +import { useLocalSetting } from "@hive/shared/settings-storage.js"; +import { LazyDisclosure } from "../ui/disclosure/LazyDisclosure.js"; +import { ExternalLink } from "../ui/external-link/ExternalLink.js"; +import { RelativeTime } from "../ui/relative-time/RelativeTime.js"; +import { SelectField } from "../ui/select-field/SelectField.js"; +import { Table, type TableColumn } from "../ui/table/Table.js"; +import { TableSkeleton } from "../ui/table/TableSkeleton.js"; +import { ALL_REPOS, repoOptions } from "./issue-report-repo.js"; +import { + approvalsText, + hasAllApprovals, + prReportUrl, + yesNo, + type PrReportRow, +} from "./pr-report.js"; + +const REPO_FILTER_KEY = "swarm-ui:pr-report:repo-filter"; +const TABLE_FILTERS_KEY = "swarm-ui:pr-report:table-filters"; + +const CI_TONE: Record, BadgeTone> = { + success: "positive", + pending: "warning", + failure: "negative", +}; + +const COLUMNS: TableColumn[] = [ + { + key: "repo", + header: "repo", + render: (r) => r.repo, + sortBy: (r) => r.repo, + filterValues: (r) => [r.repo], + filterMode: "multiselect", + }, + { + key: "pr", + header: "PR", + render: (r) => + r.html_url ? ( + #{r.number} + ) : ( + `#${r.number}` + ), + sortBy: (r) => r.number, + }, + { + key: "title", + header: "title", + render: (r) => r.title, + sortBy: (r) => r.title, + filterValue: (r) => r.title, + cellClass: "ui-table-prose", + }, + { + key: "author", + header: "author", + render: (r) => r.author ?? "—", + sortBy: (r) => r.author ?? "", + filterValues: (r) => (r.author ? [r.author] : []), + filterMode: "multiselect", + }, + { + key: "labels", + header: "labels", + render: (r) => (r.labels.length ? r.labels.join(", ") : "—"), + filterValues: (r) => r.labels, + filterMode: "multiselect", + }, + { + key: "reviewers", + header: "reviewers", + render: (r) => + r.requested_reviewers.length ? r.requested_reviewers.join(", ") : "—", + filterValues: (r) => r.requested_reviewers, + filterMode: "multiselect", + }, + { + key: "base", + header: "into", + render: (r) => r.base_branch, + sortBy: (r) => r.base_branch, + filterValues: (r) => [r.base_branch], + filterMode: "multiselect", + }, + { + key: "wip", + header: "WIP", + render: (r) => (r.wip ? : "—"), + sortBy: (r) => Number(r.wip), + filterValues: (r) => [yesNo(r.wip)], + filterMode: "multiselect", + }, + { + key: "approvals", + header: "approvals", + render: (r) => ( + <> + {approvalsText(r)} + {r.changes_requested > 0 ? ( + <> + {" "} + + + ) : null} + + ), + sortBy: (r) => r.approvals, + }, + { + key: "approved", + header: "has all approvals", + render: (r) => + hasAllApprovals(r) ? : "no", + sortBy: (r) => Number(hasAllApprovals(r)), + filterValues: (r) => [yesNo(hasAllApprovals(r))], + filterMode: "multiselect", + }, + { + key: "conflicts", + header: "conflicts", + render: (r) => + r.has_conflicts ? : "—", + sortBy: (r) => Number(r.has_conflicts), + filterValues: (r) => [yesNo(r.has_conflicts)], + filterMode: "multiselect", + }, + { + key: "protected", + header: "protected base", + render: (r) => yesNo(r.targets_protected_branch), + sortBy: (r) => Number(r.targets_protected_branch), + filterValues: (r) => [yesNo(r.targets_protected_branch)], + filterMode: "multiselect", + }, + { + key: "ci", + header: "CI", + render: (r) => (r.ci ? : "—"), + sortBy: (r) => r.ci ?? "", + filterValues: (r) => [r.ci ?? "none"], + filterMode: "multiselect", + }, + { + key: "updated", + header: "updated", + render: (r) => + r.updated_unix !== null ? ( + + ) : ( + "—" + ), + sortBy: (r) => r.updated_unix ?? 0, + }, +]; + +function PrReportBody() { + const [repos, setRepos] = useState(null); + const [repoFilter, setRepoFilter] = useLocalSetting( + REPO_FILTER_KEY, + ALL_REPOS, + ); + const [rows, setRows] = useState(null); + // One slot per request, so a report that loads does not clear a repo list + // that failed. + const [reposError, setReposError] = useState(null); + const [error, setError] = useState(null); + const [loading, setLoading] = useState(false); + + // The dropdown's source, read once: a repo gaining or losing its first or + // last open PR between visits is rare enough that a reload covers it. + useEffect(() => { + let cancelled = false; + (async () => { + const r = await fetch("/api/pulls/repos"); + if (!r.ok) { + if (!cancelled) setReposError(await readApiError(r)); + return; + } + const data = (await r.json()) as string[]; + if (!cancelled) setRepos(data); + })().catch((e: unknown) => { + if (!cancelled) setReposError({ detail: String(e) }); + }); + return () => { + cancelled = true; + }; + }, []); + + // The one fetch that re-runs on a state change: the repo choice. + useEffect(() => { + let cancelled = false; + setLoading(true); + // A failed report drops the rows, so the table never shows another + // repo's PRs under this repo's error. + const fail = (problem: ProblemDetails) => { + setRows(null); + setError(problem); + }; + (async () => { + const r = await fetch(prReportUrl(repoFilter)); + if (!r.ok) { + const problem = await readApiError(r); + if (!cancelled) fail(problem); + return; + } + const data = (await r.json()) as PrReportRow[]; + if (cancelled) return; + setRows(data); + setError(null); + })() + .catch((e: unknown) => { + if (!cancelled) fail({ detail: String(e) }); + }) + .finally(() => { + if (!cancelled) setLoading(false); + }); + return () => { + cancelled = true; + }; + }, [repoFilter]); + + return ( + <> + + {reposError ? ( + + ) : null} + {error ? ( + + ) : null} + {!error && loading ? ( + + ) : null} + {!loading && rows ? ( + `${r.repo}#${r.number}`} + emptyMessage="no open pull requests" + storageKey={TABLE_FILTERS_KEY} + /> + ) : null} + + ); +} + +// A folded card on the reports tab: the repo list and the report are read only +// once it is opened. +export function PrReportCard() { + return ( + + + + ); +} diff --git a/frontend/packages/swarm-ui/src/pages/issue-report-repo.ts b/frontend/packages/swarm-ui/src/pages/issue-report-repo.ts index 52a2827b..2c6760c4 100644 --- a/frontend/packages/swarm-ui/src/pages/issue-report-repo.ts +++ b/frontend/packages/swarm-ui/src/pages/issue-report-repo.ts @@ -27,6 +27,8 @@ export function reportUrl(repoFilter: string): string { export function repoOptions( repos: string[] | null, selected: string, + /** What the missing repo has none of: "issues", or "pull requests". */ + what = "issues", ): RepoOption[] { const options: RepoOption[] = [ { value: ALL_REPOS, label: "all repos" }, @@ -35,7 +37,7 @@ export function repoOptions( if (selected !== ALL_REPOS && !(repos ?? []).includes(selected)) { options.push({ value: selected, - label: repos === null ? selected : `${selected} (no open issues)`, + label: repos === null ? selected : `${selected} (no open ${what})`, }); } return options; diff --git a/frontend/packages/swarm-ui/src/pages/pr-report.ts b/frontend/packages/swarm-ui/src/pages/pr-report.ts new file mode 100644 index 00000000..6c11415f --- /dev/null +++ b/frontend/packages/swarm-ui/src/pages/pr-report.ts @@ -0,0 +1,62 @@ +// Pure pieces of the PR report: the row the controller returns, the report +// URL for a repo filter, and the words its flag columns filter on. Kept out of +// the page so the cases are unit tested. + +/** One open pull request, as `GET /api/pr-report` returns it. */ +export interface PrReportRow { + /** `owner/name`. */ + repo: string; + number: number; + title: string; + html_url: string | null; + author: string | null; + labels: string[]; + assignees: string[]; + requested_reviewers: string[]; + base_branch: string; + head_branch: string; + created_unix: number | null; + updated_unix: number | null; + /** The forge's draft flag. */ + wip: boolean; + has_conflicts: boolean; + /** The base branch has a branch protection rule. */ + targets_protected_branch: boolean; + required_approvals: number; + approvals: number; + /** Reviewers whose latest review asks for changes. */ + changes_requested: number; + /** `null` when the head has no commit status. */ + ci: "success" | "pending" | "failure" | null; +} + +/** The report URL for a repo filter. The path parts are encoded, so a name + * with `?`, `#` or `%` stays one path segment. A filter without a slash is + * not a repo, so it reads as all repos. */ +export function prReportUrl(repoFilter: string): string { + const i = repoFilter.indexOf("/"); + if (i < 0) return "/api/pr-report"; + const org = encodeURIComponent(repoFilter.slice(0, i)); + const name = encodeURIComponent(repoFilter.slice(i + 1)); + return `/api/repos/${org}/${name}/pr-report`; +} + +/** What a flag column offers as filter values. */ +export function yesNo(flag: boolean): string { + return flag ? "yes" : "no"; +} + +/** The approvals meet the base branch's rule. An unprotected base asks for + * none, so its PRs have them all. */ +export function hasAllApprovals(row: PrReportRow): boolean { + return row.approvals >= row.required_approvals; +} + +/** The approvals cell: how many of the rule's approvals the PR has, or a dash + * when its base branch asks for none. */ +export function approvalsText(row: PrReportRow): string { + if (!row.targets_protected_branch || row.required_approvals === 0) { + return row.approvals > 0 ? `${row.approvals}` : "—"; + } + return `${row.approvals}/${row.required_approvals}`; +} diff --git a/frontend/packages/swarm-ui/src/pages/reports/ReportsPage.tsx b/frontend/packages/swarm-ui/src/pages/reports/ReportsPage.tsx index cb8a9f8a..ea83301b 100644 --- a/frontend/packages/swarm-ui/src/pages/reports/ReportsPage.tsx +++ b/frontend/packages/swarm-ui/src/pages/reports/ReportsPage.tsx @@ -1,8 +1,9 @@ -// — `/reports`: what the swarm reports about itself, as two -// folded cards: the alerts Grafana holds now, and mara's issue report. Neither -// reads anything until it is opened. +// — `/reports`: what the swarm reports about itself, as folded +// cards: the alerts Grafana holds now, mara's issue report and the PR report. +// None reads anything until it is opened. import { AlertsCard } from "../AlertsPage.js"; import { IssueReportCard } from "../IssueReportPage.js"; +import { PrReportCard } from "../PrReportPage.js"; import "./ReportsPage.css"; export function ReportsPage() { @@ -10,6 +11,7 @@ export function ReportsPage() {
+
); } diff --git a/frontend/packages/swarm-ui/src/ui/table/Table.css b/frontend/packages/swarm-ui/src/ui/table/Table.css index 06277b49..277712ba 100644 --- a/frontend/packages/swarm-ui/src/ui/table/Table.css +++ b/frontend/packages/swarm-ui/src/ui/table/Table.css @@ -31,6 +31,9 @@ not a short discrete label) rather than open-ended growth that drags the whole table wider — caps the column and wraps within it instead. */ .ui-table-prose { + /* A table with many columns squeezes free text to a word a line; + the table scrolls sideways instead. */ + min-width: 14em; max-width: 32em; white-space: normal; word-break: break-word; diff --git a/frontend/packages/swarm-ui/test/pr-report.test.mjs b/frontend/packages/swarm-ui/test/pr-report.test.mjs new file mode 100644 index 00000000..64fe22c6 --- /dev/null +++ b/frontend/packages/swarm-ui/test/pr-report.test.mjs @@ -0,0 +1,69 @@ +// The PR report's URL and the words its columns show and filter on. +import assert from "node:assert/strict"; +import { test } from "node:test"; +import { + approvalsText, + hasAllApprovals, + prReportUrl, + yesNo, +} from "../src/pages/pr-report.ts"; + +test("no repo, or a filter that is not owner/name, reads as all repos", () => { + assert.equal(prReportUrl(""), "/api/pr-report"); + assert.equal(prReportUrl("nothing"), "/api/pr-report"); +}); + +test("a repo has its own URL, with each part encoded", () => { + assert.equal( + prReportUrl("hyperhive/hyperhive"), + "/api/repos/hyperhive/hyperhive/pr-report", + ); + assert.equal( + prReportUrl("o/a?b#c%d"), + "/api/repos/o/a%3Fb%23c%25d/pr-report", + ); +}); + +test("a flag filters on yes and no", () => { + assert.equal(yesNo(true), "yes"); + assert.equal(yesNo(false), "no"); +}); + +const row = (over) => ({ + targets_protected_branch: true, + required_approvals: 2, + approvals: 1, + ...over, +}); + +test("approvals read as have/required against a protected base", () => { + assert.equal(approvalsText(row({})), "1/2"); + assert.equal(approvalsText(row({ approvals: 0 })), "0/2"); +}); + +test("an unprotected base, or a rule asking for none, shows only what it has", () => { + assert.equal( + approvalsText(row({ targets_protected_branch: false, approvals: 0 })), + "—", + ); + assert.equal( + approvalsText(row({ required_approvals: 0, approvals: 2 })), + "2", + ); +}); + +test("a PR has all approvals when it meets the rule, and an unprotected base always does", () => { + assert.equal(hasAllApprovals(row({})), false); + assert.equal(hasAllApprovals(row({ approvals: 2 })), true); + assert.equal(hasAllApprovals(row({ approvals: 3 })), true); + assert.equal( + hasAllApprovals( + row({ + targets_protected_branch: false, + required_approvals: 0, + approvals: 0, + }), + ), + true, + ); +}); diff --git a/swarm-controller/src/forge.rs b/swarm-controller/src/forge.rs index df101afb..82122095 100644 --- a/swarm-controller/src/forge.rs +++ b/swarm-controller/src/forge.rs @@ -56,6 +56,7 @@ mod meta_repo; pub mod objects; pub mod operator_profiles; pub(crate) mod pr_comment; +pub mod pr_report; pub mod pull_diff; pub mod review_request; pub mod runner_token; diff --git a/swarm-controller/src/forge/pr_report.rs b/swarm-controller/src/forge/pr_report.rs new file mode 100644 index 00000000..801572a8 --- /dev/null +++ b/swarm-controller/src/forge/pr_report.rs @@ -0,0 +1,515 @@ +//! The open pull requests of a repo as the swarm UI's PR report reads them: +//! each with what a reviewer or merger wants to filter on already resolved, so +//! the page does no calls of its own. Read-only, over the controller's forge +//! token. + +use anyhow::{Context, Result}; +use forgejo_api::structs::{ + BranchProtection, CombinedStatus, CommitStatusState, PullRequest, PullReview, + RepoListPullRequestsQuery, RepoListPullRequestsQueryState, +}; +use futures_util::{StreamExt as _, TryStreamExt as _}; +use serde::Serialize; +use std::collections::HashMap; +use utoipa::ToSchema; + +use super::Client; + +/// What the commit statuses on a PR's head say together. +#[derive(Clone, Copy, Debug, PartialEq, Eq, Serialize, ToSchema)] +#[serde(rename_all = "snake_case")] +pub enum CiState { + Success, + Pending, + Failure, +} + +/// One open pull request, resolved for the PR report. +#[derive(Clone, Debug, PartialEq, Serialize, ToSchema)] +pub struct PrReportRow { + /// `owner/name`, on every row so the all-repos and the one-repo report + /// share one row type. + pub repo: String, + pub number: i64, + pub title: String, + pub html_url: Option, + pub author: Option, + pub labels: Vec, + pub assignees: Vec, + pub requested_reviewers: Vec, + pub base_branch: String, + pub head_branch: String, + pub created_unix: Option, + pub updated_unix: Option, + /// The forge's draft flag (setting a `WIP:` title is what sets it). + pub wip: bool, + /// The forge cannot merge the PR as it is: its head conflicts with the + /// base. + pub has_conflicts: bool, + /// The base branch has a branch protection rule. A stacked PR targets a + /// feature branch, so `false` filters those out. + pub targets_protected_branch: bool, + /// Approvals the protection rule asks for; 0 when the base is not + /// protected. + pub required_approvals: u32, + /// Approvals that count now: the reviewer's latest review is an approval + /// of the current head, not dismissed, not stale, and counts toward the + /// rule (the forge's `official`). + pub approvals: u32, + /// Reviewers whose latest review asks for changes and is not dismissed. + pub changes_requested: u32, + /// `None` when the head has no commit status at all. + pub ci: Option, +} + +/// Whether `branch` is named by a branch protection pattern: a name, or a glob +/// where `*` stops at a `/` and `**` does not. +fn glob_matches(pattern: &str, branch: &str) -> bool { + fn go(p: &[u8], s: &[u8]) -> bool { + match p.split_first() { + None => s.is_empty(), + Some((b'*', rest)) => { + if let Some(rest) = rest.strip_prefix(b"*") { + // `**`: any run, slashes included. + (0..=s.len()).any(|i| go(rest, &s[i..])) + } else { + // `*`: any run up to the next slash. + let reach = s.iter().position(|&c| c == b'/').unwrap_or(s.len()); + (0..=reach).any(|i| go(rest, &s[i..])) + } + } + Some((&c, rest)) => s + .split_first() + .is_some_and(|(&d, tail)| c == d && go(rest, tail)), + } + } + go(pattern.as_bytes(), branch.as_bytes()) +} + +/// The protection rule for `base`, as the approvals it asks for: `None` when +/// no rule names the branch. The first matching rule wins, which is how the +/// forge orders them. +fn required_approvals_for(rules: &[BranchProtection], base: &str) -> Option { + rules + .iter() + .find(|r| { + r.rule_name + .as_deref() + .or(r.branch_name.as_deref()) + .is_some_and(|p| glob_matches(p, base)) + }) + .map(|r| u32::try_from(r.required_approvals.unwrap_or(0)).unwrap_or(0)) +} + +/// How a PR's reviews stand against its current head. +#[derive(Debug, PartialEq, Eq)] +struct Tally { + approvals: u32, + changes_requested: u32, +} + +/// Each reviewer's latest review decides: an approval counts when it is of +/// `head_sha`, not dismissed, not stale and official; a review asking for +/// changes stands until it is dismissed or the reviewer reviews again. A +/// review that only comments does not replace an earlier decision. +fn tally_reviews(reviews: &[PullReview], head_sha: &str) -> Tally { + let mut latest: HashMap<&str, &PullReview> = HashMap::new(); + for review in reviews { + let Some(login) = review.user.as_ref().and_then(|u| u.login.as_deref()) else { + continue; + }; + let decides = matches!( + review.state.as_deref(), + Some("APPROVED" | "REQUEST_CHANGES") + ); + if decides && review.dismissed != Some(true) { + latest.insert(login, review); + } else if decides { + // A dismissed decision no longer stands, and neither does the + // one it replaced. + latest.remove(login); + } + } + let mut tally = Tally { + approvals: 0, + changes_requested: 0, + }; + for review in latest.values() { + match review.state.as_deref() { + Some("APPROVED") => { + if review.commit_id.as_deref() == Some(head_sha) + && review.stale != Some(true) + && review.official == Some(true) + { + tally.approvals += 1; + } + } + _ => tally.changes_requested += 1, + } + } + tally +} + +/// The commit statuses on a head, as one state; `None` when it has none. +fn ci_state(status: &CombinedStatus) -> Option { + if status.total_count.unwrap_or(0) == 0 { + return None; + } + Some(match status.state? { + CommitStatusState::Success | CommitStatusState::Skipped => CiState::Success, + CommitStatusState::Pending | CommitStatusState::Warning => CiState::Pending, + CommitStatusState::Failure | CommitStatusState::Error => CiState::Failure, + }) +} + +fn row( + repo: &str, + pr: &PullRequest, + rules: &[BranchProtection], + reviews: &[PullReview], + status: &CombinedStatus, +) -> Option { + let base_branch = pr.base.as_ref()?.r#ref.clone().unwrap_or_default(); + let head = pr.head.as_ref()?; + let head_sha = head.sha.clone().unwrap_or_default(); + let protection = required_approvals_for(rules, &base_branch); + let required_approvals = protection.unwrap_or(0); + let tally = tally_reviews(reviews, &head_sha); + let logins = |users: &Option>| -> Vec { + users + .iter() + .flatten() + .filter_map(|u| u.login.clone()) + .collect() + }; + Some(PrReportRow { + repo: repo.to_owned(), + number: pr.number?, + title: pr.title.clone().unwrap_or_default(), + html_url: pr.html_url.as_ref().map(ToString::to_string), + author: pr.user.as_ref().and_then(|u| u.login.clone()), + labels: pr + .labels + .iter() + .flatten() + .filter_map(|l| l.name.clone()) + .collect(), + assignees: logins(&pr.assignees), + requested_reviewers: logins(&pr.requested_reviewers), + base_branch, + head_branch: head.r#ref.clone().unwrap_or_default(), + created_unix: pr.created_at.as_ref().map(|t| t.unix_timestamp()), + updated_unix: pr.updated_at.as_ref().map(|t| t.unix_timestamp()), + wip: pr.draft.unwrap_or(false), + has_conflicts: pr.mergeable == Some(false), + targets_protected_branch: protection.is_some(), + required_approvals, + approvals: tally.approvals, + changes_requested: tally.changes_requested, + ci: ci_state(status), + }) +} + +impl Client { + /// Every open pull request in `owner/repo`, resolved for the PR report. + /// + /// The reviews and the commit statuses are one call per PR, run with + /// bounded concurrency like [`Client::issue_report`]'s dependency + /// lookups; the branch protection rules are one call for the repo. + /// + /// # Errors + /// The forge refused or did not answer one of the calls. + pub async fn pr_report(&self, owner: &str, repo: &str) -> Result> { + const CONCURRENCY: usize = 8; + + let query = RepoListPullRequestsQuery { + state: Some(RepoListPullRequestsQueryState::Open), + sort: None, + milestone: None, + labels: None, + poster: None, + base: None, + head: None, + }; + let pulls = self + .api + .call(|api| { + Box::pin( + api.repo_list_pull_requests(owner, repo, query.clone()) + .all(), + ) + }) + .await + .with_context(|| format!("list open pull requests in {owner}/{repo}"))?; + if pulls.is_empty() { + return Ok(Vec::new()); + } + let rules = self + .api + .call(|api| api.repo_list_branch_protection(owner, repo).into_future()) + .await + .with_context(|| format!("list branch protections of {owner}/{repo}"))?; + + let full_name = format!("{owner}/{repo}"); + let rules = &rules; + let full_name = &full_name; + let mut rows: Vec = futures_util::stream::iter(pulls) + .map(|pr| async move { + let Some(number) = pr.number else { + return Ok::<_, anyhow::Error>(None); + }; + let head_sha = pr.head.as_ref().and_then(|h| h.sha.clone()); + let reviews = self + .api + .call(|api| Box::pin(api.repo_list_pull_reviews(owner, repo, number).all())) + .await + .with_context(|| format!("list reviews of {owner}/{repo}#{number}"))?; + let status = match head_sha { + Some(sha) => { + self.api + .call(|api| { + api.repo_get_combined_status_by_ref(owner, repo, &sha) + .into_future() + }) + .await + .with_context(|| { + format!("read the statuses of {owner}/{repo}#{number}") + })? + .1 + } + None => CombinedStatus { + commit_url: None, + repository: None, + sha: None, + state: None, + statuses: None, + total_count: Some(0), + url: None, + }, + }; + Ok(row(full_name, &pr, rules, &reviews, &status)) + }) + .buffer_unordered(CONCURRENCY) + .try_filter_map(|row| async move { Ok(row) }) + .try_collect() + .await?; + rows.sort_by_key(|r| std::cmp::Reverse(r.number)); + Ok(rows) + } + + /// The PR report of every repo with an open pull request. A repo whose + /// report fails is logged and skipped, as [`Client::issue_report_all`] + /// does: one flaky repo should not blank out the others' rows. + /// + /// # Errors + /// The forge refused to list the repos. + pub async fn pr_report_all(&self) -> Result> { + const CONCURRENCY: usize = 4; + + let repos = self.list_repos_with_open_pulls().await?; + let rows: Vec> = futures_util::stream::iter(repos) + .map(|full_name| async move { + let Some((owner, repo)) = full_name.split_once('/') else { + tracing::warn!(%full_name, "pr_report_all: repo full_name has no '/'"); + return Vec::new(); + }; + match self.pr_report(owner, repo).await { + Ok(rows) => rows, + Err(e) => { + tracing::warn!( + error = %format!("{e:#}"), + %full_name, + "pr_report_all: one repo's report failed, skipping it" + ); + Vec::new() + } + } + }) + .buffer_unordered(CONCURRENCY) + .collect() + .await; + Ok(rows.into_iter().flatten().collect()) + } +} + +#[cfg(test)] +mod tests { + use super::*; + + fn rule(name: &str, approvals: i64) -> BranchProtection { + serde_json::from_value(serde_json::json!({ + "rule_name": name, + "required_approvals": approvals, + })) + .expect("a branch protection") + } + + fn review(login: &str, state: &str, commit: &str, flags: &[(&str, bool)]) -> PullReview { + let mut v = serde_json::json!({ + "user": { "login": login }, + "state": state, + "commit_id": commit, + "official": true, + }); + for (k, b) in flags { + v[*k] = serde_json::json!(b); + } + serde_json::from_value(v).expect("a review") + } + + #[test] + fn a_pattern_names_a_branch() { + assert!(glob_matches("main", "main")); + assert!(!glob_matches("main", "main2")); + assert!(glob_matches("release/*", "release/1.0")); + assert!(!glob_matches("release/*", "release/1.0/hotfix")); + assert!(glob_matches("release/**", "release/1.0/hotfix")); + assert!(glob_matches("*", "main")); + assert!(!glob_matches("*", "iris/foo")); + } + + #[test] + fn the_first_matching_rule_sets_the_approvals() { + let rules = [rule("main", 1), rule("*", 2)]; + assert_eq!(required_approvals_for(&rules, "main"), Some(1)); + assert_eq!(required_approvals_for(&rules, "dev"), Some(2)); + assert_eq!(required_approvals_for(&rules, "iris/feature"), None); + assert_eq!(required_approvals_for(&[], "main"), None); + } + + #[test] + fn an_approval_of_the_head_counts_once_per_reviewer() { + let reviews = [ + review("argus", "APPROVED", "h1", &[]), + review("mara", "APPROVED", "h1", &[]), + review("mara", "COMMENT", "h1", &[]), + ]; + let t = tally_reviews(&reviews, "h1"); + assert_eq!( + t, + Tally { + approvals: 2, + changes_requested: 0 + } + ); + } + + #[test] + fn an_old_stale_dismissed_or_unofficial_approval_does_not_count() { + for (r, why) in [ + (review("a", "APPROVED", "old", &[]), "of an older head"), + (review("a", "APPROVED", "h1", &[("stale", true)]), "stale"), + ( + review("a", "APPROVED", "h1", &[("dismissed", true)]), + "dismissed", + ), + ( + review("a", "APPROVED", "h1", &[("official", false)]), + "not official", + ), + ] { + assert_eq!(tally_reviews(&[r], "h1").approvals, 0, "{why}"); + } + } + + #[test] + fn a_later_review_replaces_an_earlier_decision() { + let reviews = [ + review("argus", "REQUEST_CHANGES", "h1", &[]), + review("argus", "APPROVED", "h1", &[]), + ]; + let t = tally_reviews(&reviews, "h1"); + assert_eq!((t.approvals, t.changes_requested), (1, 0)); + + let reviews = [ + review("argus", "APPROVED", "h1", &[]), + review("argus", "REQUEST_CHANGES", "h1", &[]), + ]; + let t = tally_reviews(&reviews, "h1"); + assert_eq!((t.approvals, t.changes_requested), (0, 1)); + + let reviews = [review( + "argus", + "REQUEST_CHANGES", + "h1", + &[("dismissed", true)], + )]; + assert_eq!(tally_reviews(&reviews, "h1").changes_requested, 0); + } + + #[test] + fn the_commit_statuses_read_as_one_state() { + let status = |state: Option, n: i64| CombinedStatus { + commit_url: None, + repository: None, + sha: None, + state, + statuses: None, + total_count: Some(n), + url: None, + }; + assert_eq!(ci_state(&status(None, 0)), None); + assert_eq!( + ci_state(&status(Some(CommitStatusState::Pending), 0)), + None, + "no statuses is not pending" + ); + assert_eq!( + ci_state(&status(Some(CommitStatusState::Success), 3)), + Some(CiState::Success) + ); + assert_eq!( + ci_state(&status(Some(CommitStatusState::Error), 3)), + Some(CiState::Failure) + ); + assert_eq!( + ci_state(&status(Some(CommitStatusState::Pending), 3)), + Some(CiState::Pending) + ); + } + + #[test] + fn a_row_resolves_protection_approvals_and_conflicts() { + let pr: PullRequest = serde_json::from_value(serde_json::json!({ + "number": 7, + "title": "WIP: a thing", + "draft": true, + "mergeable": false, + "user": { "login": "iris" }, + "labels": [{ "name": "type/chore" }], + "assignees": [{ "login": "iris" }], + "requested_reviewers": [{ "login": "argus" }], + "base": { "ref": "main" }, + "head": { "ref": "iris/thing", "sha": "h1" }, + "created_at": "2026-11-07T08:00:00Z", + "updated_at": "2026-11-07T09:00:00Z", + })) + .expect("a pull request"); + let status: CombinedStatus = serde_json::from_value(serde_json::json!({ + "state": "success", "total_count": 2 + })) + .expect("a combined status"); + let reviews = [review("argus", "APPROVED", "h1", &[])]; + let r = row("o/r", &pr, &[rule("main", 2)], &reviews, &status).expect("a row"); + assert_eq!((r.number, r.wip, r.has_conflicts), (7, true, true)); + assert_eq!( + ( + r.targets_protected_branch, + r.required_approvals, + r.approvals + ), + (true, 2, 1) + ); + assert_eq!(r.ci, Some(CiState::Success)); + assert_eq!(r.author.as_deref(), Some("iris")); + assert_eq!(r.requested_reviewers, ["argus"]); + assert_eq!(r.updated_unix, Some(1_794_042_000)); + + // The same PR into a branch no rule names has nothing to approve. + let r = row("o/r", &pr, &[], &reviews, &status).expect("a row"); + assert_eq!( + (r.targets_protected_branch, r.required_approvals), + (false, 0) + ); + } +} diff --git a/swarm-controller/src/main.rs b/swarm-controller/src/main.rs index 59acb7bd..6736a37a 100644 --- a/swarm-controller/src/main.rs +++ b/swarm-controller/src/main.rs @@ -70,6 +70,7 @@ mod notify_flows; mod notify_router_api; mod observability_grant; mod otel_http_client; +mod pr_report; mod proxy_credential; mod pulls; mod queue_identity; diff --git a/swarm-controller/src/pr_report.rs b/swarm-controller/src/pr_report.rs new file mode 100644 index 00000000..53fb7618 --- /dev/null +++ b/swarm-controller/src/pr_report.rs @@ -0,0 +1,149 @@ +//! The swarm UI's PR report: every open pull request of the forge, or of one +//! repo, with the attributes a reviewer or merger filters on already +//! resolved. The logic is [`forge::Client::pr_report`]; this module is the +//! HTTP surface over it, the same shape as [`crate::issue_report`]. The repo +//! dropdown's source is `GET /api/pulls/repos`. + +use axum::{ + Json, + extract::{Path, State}, +}; + +use crate::forge; +use crate::issue_report::issue_report_error_status; +use crate::routes::{AppState, error_problem}; + +/// The 503 detail when the forge did not answer within +/// [`forge::FORGE_DEADLINE`]. +const DEADLINE_DETAIL: &str = "the forge did not answer within the deadline"; + +/// Every open pull request across every repo that has one. +#[utoipa::path( + get, + path = "/api/pr-report", + responses( + (status = 200, description = "every open pull request, newest number first within a repo", body = Vec), + (status = 503, description = "no forge is configured on this host, the report could not be built, or the forge did not answer within two minutes (problem+json)", body = String), + ), + tag = "pulls" +)] +pub async fn get_pr_report_all( + State(state): State, +) -> Result>, problem_details::ProblemDetails> { + let Some(client) = state.forge.get() else { + return Err(error_problem( + axum::http::StatusCode::SERVICE_UNAVAILABLE, + "no forge is configured on this host", + )); + }; + let Some(built) = forge::within_forge_deadline("PR report", client.pr_report_all()).await + else { + return Err(error_problem( + axum::http::StatusCode::SERVICE_UNAVAILABLE, + DEADLINE_DETAIL, + )); + }; + match built { + Ok(rows) => Ok(Json(rows)), + Err(e) => { + let detail = format!("{e:#}"); + tracing::warn!(error = %detail, "building the all-repos PR report failed"); + Err(error_problem( + axum::http::StatusCode::SERVICE_UNAVAILABLE, + &detail, + )) + } + } +} + +/// The open pull requests of `{org}/{repo}`. A 404 from the forge for the +/// repo itself is a 404 here; anything else stays 503, as for the issue +/// report ([`issue_report_error_status`]). +#[utoipa::path( + get, + path = "/api/repos/{org}/{repo}/pr-report", + params( + ("org" = String, Path, description = "repo owner (user or org)"), + ("repo" = String, Path, description = "repo name"), + ), + responses( + (status = 200, description = "every open pull request in this repo", body = Vec), + (status = 404, description = "no such repo (problem+json)", body = String), + (status = 503, description = "no forge is configured on this host, the report could not be built, or the forge did not answer within two minutes (problem+json)", body = String), + ), + tag = "pulls" +)] +pub async fn get_pr_report( + State(state): State, + Path((org, repo)): Path<(String, String)>, +) -> Result>, problem_details::ProblemDetails> { + let Some(client) = state.forge.get() else { + return Err(error_problem( + axum::http::StatusCode::SERVICE_UNAVAILABLE, + "no forge is configured on this host", + )); + }; + let Some(built) = + forge::within_forge_deadline("repo PR report", client.pr_report(&org, &repo)).await + else { + return Err(error_problem( + axum::http::StatusCode::SERVICE_UNAVAILABLE, + DEADLINE_DETAIL, + )); + }; + match built { + Ok(rows) => Ok(Json(rows)), + Err(e) => { + let status = issue_report_error_status(&e); + if status == axum::http::StatusCode::NOT_FOUND { + Err(error_problem(status, &format!("{org}/{repo} not found"))) + } else { + let detail = format!("{e:#}"); + tracing::warn!(error = %detail, %org, %repo, "building PR report failed"); + Err(error_problem(status, &detail)) + } + } + } +} + +#[cfg(test)] +mod tests { + use super::*; + + /// Each route against a forge that accepts the connection and never + /// answers: 503 at the deadline. The paused clock jumps to the deadline + /// once nothing else can run, so this takes no wall time. + #[tokio::test(start_paused = true)] + async fn every_route_is_503_when_the_forge_never_answers() { + use axum::body::Body; + use axum::http::{Request, StatusCode}; + use tower::ServiceExt as _; + + let listener = tokio::net::TcpListener::bind("127.0.0.1:0").await.unwrap(); + let url = url::Url::parse(&format!("http://{}/", listener.local_addr().unwrap())).unwrap(); + for uri in ["/api/pr-report", "/api/repos/o/r/pr-report"] { + let (state, _) = crate::routes::tests::state_with_roster(); + assert!( + state + .forge + .set(std::sync::Arc::new(forge::Client::for_test_forge( + url.clone() + ))) + .is_ok() + ); + let start = tokio::time::Instant::now(); + let req = Request::get(uri).body(Body::empty()).expect("a request"); + let res = crate::startup::build_app(state) + .oneshot(req) + .await + .expect("infallible"); + assert_eq!(res.status(), StatusCode::SERVICE_UNAVAILABLE, "{uri}"); + let bytes = axum::body::to_bytes(res.into_body(), usize::MAX) + .await + .unwrap(); + let body = String::from_utf8_lossy(&bytes); + assert!(body.contains(super::DEADLINE_DETAIL), "{uri}: {body}"); + assert_eq!(start.elapsed(), forge::FORGE_DEADLINE, "{uri}"); + } + } +} diff --git a/swarm-controller/src/startup.rs b/swarm-controller/src/startup.rs index a7a7e60d..ae4f53ae 100644 --- a/swarm-controller/src/startup.rs +++ b/swarm-controller/src/startup.rs @@ -31,9 +31,9 @@ use crate::{ agent_permissions, agent_renewal, agent_state_stream, agent_status, agents_md, alerts, archetypes, auth, backend_env, builder, claude_login, config_drift, config_pr, egress_allow, egress_deny, forge, forge_account, github_account, gitlab_account, inference, issue_report, - linked_accounts, matrix_account, message_stream, notify_flows, notify_router_api, pulls, - queue_identity, review_classifier, schedules, status, subagent_term, term_stream, wanted, - webhook, + linked_accounts, matrix_account, message_stream, notify_flows, notify_router_api, pr_report, + pulls, queue_identity, review_classifier, schedules, status, subagent_term, term_stream, + wanted, webhook, }; /// Where the daemon binds, overridable via `SWARM_CONTROLLER_SOCKET`. @@ -80,7 +80,7 @@ pub(crate) fn socket_path() -> PathBuf { (name = "egress", description = "the destinations agents may reach through their hive's egress proxy"), (name = "notify", description = "the notification router's flows: built-ins switched off, and custom flows"), (name = "review-classifier", description = "trying review classifications against a PR (swarm-ui's try-out page), and the classifier's swarm-wide settings"), - (name = "pulls", description = "open pull requests, for the try-out page's dropdowns"), + (name = "pulls", description = "open pull requests: the try-out page's dropdowns and the PR report"), (name = "alerts", description = "the swarm Grafana's current alerts (swarm-ui's alerts page)"), (name = "builder", description = "the swarm builder's finished builds of agent config PRs, which their commit statuses link to"), ) @@ -667,6 +667,8 @@ pub(crate) fn build_app(state: AppState) -> axum::Router { .routes(routes!(issue_report::get_repos)) .routes(routes!(issue_report::get_issue_report_all)) .routes(routes!(issue_report::get_issue_report)) + .routes(routes!(pr_report::get_pr_report_all)) + .routes(routes!(pr_report::get_pr_report)) .routes(routes!(pulls::get_pull_repos)) .routes(routes!(pulls::get_pulls)) .routes(routes!(review_classifier::post_try_out))