tga 7.0.0

//! Async GitHub REST API v3 client for pull-request and issue metadata.

use chrono::Utc;
use rusqlite::params;
use tracing::{debug, warn};

use async_trait::async_trait;

use crate::collect::errors::{CollectError, Result};
use crate::collect::github::budget::{RunBudget, MAX_PAGES};
use crate::collect::github::repo_resolver::{build_http_client, parse_slug};
use crate::collect::github::retry::retry_get;
use crate::collect::github::types::{ApiPull, GitHubIssue, GitHubPrCommit, GitHubReview};
use crate::collect::pr_provider::PrProvider;
// #5734: the PR body is scanned once here and discarded; only the key is kept.
use crate::collect::ticket::pr_body_ticket_key;
use crate::core::config::GithubConfig;
use crate::core::db::Database;
use crate::core::models::{PrState, PullRequest};

/// GitHub REST API base URL.
pub(crate) const GITHUB_API_BASE: &str = "https://api.github.com";
/// Page size for paginated list endpoints (GitHub max is 100).
pub(crate) const PAGE_SIZE: u32 = 100;
/// HTTP `User-Agent` string sent on every request.
pub(crate) const USER_AGENT_VALUE: &str = "trusty-git-analytics/0.1";

/// Async GitHub REST client.
///
/// Supports single-repo and multi-repo PR collection. The `owner` / `repo`
/// pair is the "primary" repository used by issue-oriented endpoints
/// ([`Self::fetch_issue`], [`Self::list_issues`]). The `repos` vector lists
/// every repository the bulk PR fetcher will iterate over and always contains
/// the primary repo as the first entry when one is set.
pub struct GitHubClient {
    pub(crate) client: reqwest::Client,
    pub(crate) token: Option<String>,
    /// Primary `owner` for issue-oriented endpoints.
    pub(crate) owner: String,
    /// Primary `repo` for issue-oriented endpoints.
    pub(crate) repo: String,
    /// Every `(owner, repo)` pair the PR fetcher will scan, in order. Never
    /// empty in single-repo mode; may contain many entries in org / multi-repo
    /// mode (see [`Self::new_for_prs`]).
    pub(crate) repos: Vec<(String, String)>,
    /// REST root every request is built against. [`GITHUB_API_BASE`] unless a
    /// caller overrode it with [`Self::with_api_base`] (#5465).
    pub(crate) api_base: String,
    /// Cached result of the repo-visibility probe (#5980 CRITICAL 1).
    /// `Some(true)` once `GET /repos/{owner}/{repo}` has confirmed this
    /// client's credential (or lack of one) can see the primary repo — see
    /// [`Self::ensure_repo_visible`].
    repo_visible: tokio::sync::OnceCell<bool>,
    /// #6084: the bound every call this client makes charges against, so the
    /// whole PR sweep — every page of every repo, every review, every issue —
    /// shares one ceiling and one breaker.
    ///
    /// #6565: a HANDLE to the run's budget, not a budget of its own. A `collect`
    /// builds several clients, so owning one charged the ceiling once per client
    /// and let each start its own storm. Construction defaults to a fresh run
    /// budget for standalone callers; a run hands its own in with
    /// [`Self::with_run_budget`].
    budget: RunBudget,
}

/// Compute the JSON-encoded `commit_shas` value for a PR row.
///
/// Why: GitHub populates `merge_commit_sha` even for open or
/// closed-without-merge PRs — it's the SHA of a *test* merge commit on
/// `refs/pull/N/merge` (a mergeability probe). That SHA exists on no
/// branch and won't join against the `commits` table (issue #101). Only
/// truly merged PRs (`merged_at` set) carry a joinable merge SHA.
/// What: returns `["<sha>"]` only when the PR is merged and has a SHA;
/// otherwise returns the empty array `[]`.
/// Test: see `commit_shas_gated_on_merged_at` — non-merged PR with a
/// populated SHA yields `"[]"`, merged PR yields `r#"["<sha>"]"#`.
pub(crate) fn commit_shas_for_pull(p: &ApiPull) -> Result<String> {
    match (&p.merge_commit_sha, p.merged_at.is_some()) {
        (Some(s), true) => Ok(serde_json::to_string(&vec![s.clone()])?),
        _ => Ok("[]".to_string()),
    }
}

impl GitHubClient {
    /// Build a client from a [`GithubConfig`].
    ///
    /// The config's `repo` field is expected in `owner/name` form. If the
    /// org-only mode is in use (`org` set, `repo` unset), per-repo calls
    /// will fail until a concrete repo is selected.
    ///
    /// # Errors
    ///
    /// - [`crate::collect::errors::CollectError::Config`] if `repo` is missing or malformed.
    /// - [`crate::collect::errors::CollectError::Http`] if the underlying `reqwest::Client`
    ///   cannot be built.
    pub fn new(config: &GithubConfig) -> Result<Self> {
        use crate::collect::errors::CollectError;
        let repo_slug = config
            .repo
            .as_ref()
            .ok_or_else(|| CollectError::Config("github.repo is required (owner/name)".into()))?;
        let (owner, repo) = parse_slug(repo_slug)?;
        let http = build_http_client(config)?;

        Ok(Self {
            client: http,
            token: config.token.clone(),
            owner: owner.clone(),
            repo: repo.clone(),
            repos: vec![(owner, repo)],
            api_base: GITHUB_API_BASE.to_string(),
            repo_visible: tokio::sync::OnceCell::new(),
            budget: RunBudget::new(),
        })
    }

    /// Construct a client that will fetch pull requests across every
    /// `(owner, repo)` in `repos`.
    ///
    /// Why: org-wide / multi-repo deployments need to drive PR collection
    /// from `repositories[]` (or `github.org` as fallback) rather than a
    /// single `github.repo`. Mirrors the ADO PR-fetcher contract from #84.
    /// What: stores the full list, uses the first entry as the "primary"
    /// for issue-oriented endpoints. Issue endpoints remain single-repo —
    /// the PM adapter still needs a concrete `owner/repo` to hit
    /// `GET /repos/{o}/{r}/issues/{n}`.
    /// Test: covered by `multi_repo_constructor_*` in `client_tests.rs`.
    ///
    /// # Errors
    ///
    /// - [`crate::collect::errors::CollectError::Config`] if `repos` is empty.
    /// - [`crate::collect::errors::CollectError::Http`] if the underlying `reqwest::Client`
    ///   cannot be built.
    pub fn new_for_prs(config: &GithubConfig, repos: Vec<(String, String)>) -> Result<Self> {
        use crate::collect::errors::CollectError;
        if repos.is_empty() {
            return Err(CollectError::Config(
                "GitHubClient::new_for_prs requires at least one (owner, repo)".into(),
            ));
        }
        let (primary_owner, primary_repo) = repos[0].clone();
        let http = build_http_client(config)?;
        Ok(Self {
            client: http,
            token: config.token.clone(),
            owner: primary_owner,
            repo: primary_repo,
            repos,
            api_base: GITHUB_API_BASE.to_string(),
            repo_visible: tokio::sync::OnceCell::new(),
            budget: RunBudget::new(),
        })
    }

    /// Construct a minimal authenticated client for fetching PR reviews only.
    ///
    /// Why: the reviewer-ingestion pass needs an authed client to call
    /// `fetch_pr_reviews_for_repo(owner, repo, pr_number)` without requiring
    /// a dummy repo slug (the old `new_for_prs("_dummy","_dummy")` workaround
    /// was fragile — it relied on the reviews method ignoring `self.owner`).
    /// What: builds the authed client; `owner`/`repo`/`repos` are left empty.
    /// Only use methods that take explicit `(owner, repo)` args.
    /// Test: `new_for_reviews_builds_without_dummy_slugs` in `client_tests.rs`.
    ///
    /// # Errors
    ///
    /// Returns [`crate::collect::errors::CollectError::Http`] if the `reqwest::Client`
    /// cannot be built.
    pub fn new_for_reviews(config: &GithubConfig) -> Result<Self> {
        let http = build_http_client(config)?;
        Ok(Self {
            client: http,
            token: config.token.clone(),
            owner: String::new(),
            repo: String::new(),
            repos: Vec::new(),
            api_base: GITHUB_API_BASE.to_string(),
            repo_visible: tokio::sync::OnceCell::new(),
            budget: RunBudget::new(),
        })
    }

    /// Charge this client's calls against `budget` instead of its own.
    ///
    /// Why (#6565): the sleep allowance is meant to bound a RUN. A `collect`
    /// builds an org-discovery client, a PR client, and a reviewer client, so
    /// while each owned its own budget the run could sleep the 120 s ceiling
    /// once per client — and a rate limit that latched the breaker in one pass
    /// left the next pass free to spiral again. Every client a run builds passes
    /// through here so there is one ceiling and one breaker for the whole run.
    /// What: replaces the budget handle with a clone of `budget`, which shares
    /// the same underlying allowance, ledger, and breaker.
    /// Test: `two_clients_draw_down_one_run_budget`.
    #[must_use]
    pub fn with_run_budget(mut self, budget: &RunBudget) -> Self {
        self.budget = budget.clone();
        self
    }

    /// Point every request at `base` instead of [`GITHUB_API_BASE`].
    ///
    /// Why: the write methods added in #5465 create issues and post comments,
    /// and a test that never sends those to github.com is the only kind worth
    /// having. The same seam is what a GitHub Enterprise host would need.
    /// What: replaces the REST root; a trailing `/` is trimmed so callers can
    /// pass a `wiremock` server URI unchanged.
    /// Test: `upsert_comments_on_the_existing_thread_instead_of_opening_a_second`
    /// and the other `issue_writer_tests` drive a local mock through it.
    pub fn with_api_base(mut self, base: impl Into<String>) -> Self {
        let base = base.into();
        self.api_base = base.trim_end_matches('/').to_string();
        self
    }

    /// The REST root this client builds its URLs against.
    pub(crate) fn api_base(&self) -> &str {
        &self.api_base
    }

    /// Fetch all PRs (open + closed + merged) by paginating through the
    /// GitHub REST API.
    ///
    /// # Errors
    ///
    /// Returns [`crate::collect::errors::CollectError::Http`] on transport or
    /// non-success status, and [`crate::collect::errors::CollectError::Json`]
    /// on payload parse failures.
    pub async fn fetch_pull_requests(&self) -> Result<Vec<PullRequest>> {
        let mut out: Vec<PullRequest> = Vec::new();
        for (owner, repo) in &self.repos {
            match self.fetch_pull_requests_for_repo(owner, repo).await {
                Ok(mut prs) => out.append(&mut prs),
                Err(e) => {
                    // Partial-success semantics (issue #87): one bad repo
                    // (404, no token access, transient 5xx after retries)
                    // must not abort PR collection for the rest of the org.
                    warn!(
                        owner = %owner,
                        repo = %repo,
                        error = %e,
                        "GitHub PR fetch failed for repo; continuing with remaining repos"
                    );
                }
            }
        }
        Ok(out)
    }

    /// Fetch all PRs for a single `(owner, repo)` pair, paginating until
    /// exhausted. Internal helper for [`Self::fetch_pull_requests`].
    async fn fetch_pull_requests_for_repo(
        &self,
        owner: &str,
        repo: &str,
    ) -> Result<Vec<PullRequest>> {
        let base = self.api_base();
        let mut out: Vec<PullRequest> = Vec::new();
        let mut page = 1u32;
        loop {
            let url = format!(
                "{base}/repos/{owner}/{repo}/pulls?state=all&per_page={PAGE_SIZE}&page={page}"
            );
            debug!(url = %url, "GET");
            let resp = self.retry_request(&url).await?;

            // Respect rate-limit hints.
            if let Some(rem) = resp
                .headers()
                .get("x-ratelimit-remaining")
                .and_then(|v| v.to_str().ok())
                .and_then(|s| s.parse::<u32>().ok())
            {
                if rem < 5 {
                    warn!(remaining = rem, "GitHub rate limit nearly exhausted");
                }
            }

            let resp = resp.error_for_status()?;
            let pulls: Vec<ApiPull> = resp.json().await?;
            if pulls.is_empty() {
                break;
            }
            let n = pulls.len();
            for p in pulls {
                let state = if p.merged_at.is_some() {
                    PrState::Merged
                } else if p.state == "closed" {
                    PrState::Closed
                } else {
                    PrState::Open
                };
                let commit_shas = commit_shas_for_pull(&p)?;
                // #5734: GitHub always sends `head.ref`, so `Some("")` here is
                // an anomaly the collector reports rather than harvests as
                // nothing. `None` means the payload carried no head block at
                // all — the same "no claim made" value other providers use.
                let head_ref = p.head.map(|h| h.ref_name);
                let body_ticket_id = p.body.as_deref().and_then(pr_body_ticket_key);
                out.push(PullRequest {
                    id: 0,
                    pr_number: p.number,
                    repository: format!("{owner}/{repo}"),
                    title: p.title,
                    author: p.user.map(|u| u.login).unwrap_or_default(),
                    state,
                    created_at: p.created_at,
                    merged_at: p.merged_at,
                    commit_shas,
                    fetched_at: Utc::now().to_rfc3339(),
                    head_ref,
                    body_ticket_id,
                });
            }
            if (n as u32) < PAGE_SIZE {
                break;
            }
            page += 1;
            if self.page_cap_reached("pulls", &format!("{owner}/{repo}"), page) {
                break;
            }
        }
        Ok(out)
    }

    /// Persist a batch of [`PullRequest`] rows into the database.
    ///
    /// Why: `ON CONFLICT … DO UPDATE` keeps existing `id` so FK-linked
    /// `pr_reviewers` survive re-collection; `INSERT OR REPLACE` wiped them (#752).
    /// A bare `DO UPDATE` still overwrote `state`/`merged_at`/`commit_shas`
    /// unconditionally on every conflict, so a background job re-ingesting an
    /// OLDER snapshot could downgrade an already-`merged` PR back to `open`
    /// (issue #821). The `WHERE excluded.fetched_at > pull_requests.fetched_at`
    /// guard rejects such stale writes: the conflicting row is left untouched
    /// and the statement still succeeds with no error.
    /// What: new rows insert all columns, including `fetched_at`; existing
    /// rows update `title`/`author`/`state`/`merged_at`/`commit_shas`/`fetched_at`
    /// only when the incoming `fetched_at` is strictly newer; `id` and
    /// `created_at` are never overwritten.
    /// Test: reviewer_store tests cover FK-preservation;
    /// `store_pull_requests_stale_write_guard_rejects_older_fetched_at` and
    /// `store_pull_requests_applies_genuinely_newer_fetched_at` (in
    /// `client_tests.rs`) cover the guard itself.
    ///
    /// # Errors
    ///
    /// Propagates [`crate::core::TgaError::DbError`] on SQL failures.
    pub fn store_pull_requests(
        &self,
        db: &Database,
        prs: &[PullRequest],
    ) -> crate::core::Result<usize> {
        let conn = db.connection();
        let mut count = 0usize;
        for pr in prs {
            conn.execute(
                // #5734: head_ref / body_ticket_id ride the same stale-write
                // guard as every other mutable column, so an older snapshot
                // cannot overwrite a newer branch or issue reference.
                "INSERT INTO pull_requests \
                 (provider,repository,pr_number,title,author,state,created_at,merged_at,commit_shas,fetched_at,head_ref,body_ticket_id) \
                 VALUES(?1,?2,?3,?4,?5,?6,?7,?8,?9,?10,?11,?12) \
                 ON CONFLICT(provider,repository,pr_number) DO UPDATE SET \
                   title=excluded.title,author=excluded.author,state=excluded.state,\
                   merged_at=excluded.merged_at,commit_shas=excluded.commit_shas,\
                   fetched_at=excluded.fetched_at,head_ref=excluded.head_ref,\
                   body_ticket_id=excluded.body_ticket_id \
                 WHERE excluded.fetched_at > pull_requests.fetched_at",
                params![
                    "github",
                    pr.repository,
                    pr.pr_number as i64,
                    pr.title,
                    pr.author,
                    pr.state.as_str(),
                    pr.created_at.to_rfc3339(),
                    pr.merged_at.map(|t| t.to_rfc3339()),
                    pr.commit_shas,
                    pr.fetched_at,
                    pr.head_ref.as_deref().unwrap_or(""),
                    pr.body_ticket_id,
                ],
            )?;
            count += 1;
        }
        Ok(count)
    }

    /// Whether this client was constructed with an authentication token.
    pub fn has_token(&self) -> bool {
        self.token.is_some()
    }

    /// Fetch a single issue by number from the GitHub REST API.
    ///
    /// Hits `GET /repos/{owner}/{repo}/issues/{number}`. Uses the same
    /// `Bearer` token (if any) as the bulk PR fetch. Retries on 429/5xx the
    /// same as [`Self::list_issues`] and [`Self::fetch_pr_commits`] (#5980
    /// MEDIUM 2 — this used to call `self.client` directly and skip retry).
    ///
    /// Returns `Ok(None)` when the API responds with `404 Not Found` AND the
    /// repo itself is confirmed visible to this credential (see
    /// [`Self::ensure_repo_visible`]) — a genuinely deleted or nonexistent
    /// issue. A 404 on a repo this credential cannot see at all (private repo,
    /// missing/invalid token) is a different failure — GitHub answers that
    /// with 404 on every request under it, so treating it as "no such issue"
    /// would silently report zero issues collected instead of a credential
    /// problem (#5980 CRITICAL 1). All other non-success statuses, as well as
    /// transport and JSON-parse failures, are propagated as
    /// [`crate::collect::errors::CollectError`].
    ///
    /// # Errors
    ///
    /// - [`crate::collect::errors::CollectError::GithubApi`] when the repo
    ///   itself is not visible to this credential.
    /// - [`crate::collect::errors::CollectError::Http`] on transport or non-`404`
    ///   non-success HTTP responses.
    /// - [`crate::collect::errors::CollectError::Json`] on payload parse failures.
    pub async fn fetch_issue(&self, number: u64) -> Result<Option<GitHubIssue>> {
        let base = self.api_base();
        let url = format!("{base}/repos/{}/{}/issues/{number}", self.owner, self.repo);
        debug!(url = %url, "GET");
        let resp = self.retry_request(&url).await?;

        if resp.status() == reqwest::StatusCode::NOT_FOUND {
            self.ensure_repo_visible().await?;
            return Ok(None);
        }

        let resp = resp.error_for_status()?;
        let issue: GitHubIssue = resp.json().await?;
        Ok(Some(issue))
    }

    /// Confirm the primary `(owner, repo)` is visible to this client's
    /// credential, once per client instance.
    ///
    /// Why (#5980 CRITICAL 1): GitHub 404s EVERY request under a private
    /// repository the credential can't see — `fetch_issue` alone cannot tell
    /// that apart from "this one issue doesn't exist". Probing
    /// `GET /repos/{owner}/{repo}` resolves the ambiguity: the repo endpoint
    /// 404s only when the repo is invisible to this credential (or genuinely
    /// does not exist), never because of one missing issue number.
    /// What: on the first call, issues the probe and caches `true` on
    /// success so later `fetch_issue` calls that also 404 don't repeat the
    /// request. A failed probe is NOT cached — a transient network error
    /// should not permanently poison every later call on this client.
    /// Test: `fetch_issue_tests::a_private_repo_with_no_visible_credential_errors_instead_of_returning_none`,
    /// `fetch_issue_tests::a_genuinely_missing_issue_on_a_visible_repo_returns_none`,
    /// `fetch_issue_tests::the_repo_visibility_probe_runs_at_most_once_per_client`.
    ///
    /// # Errors
    ///
    /// [`crate::collect::errors::CollectError::GithubApi`] when the probe
    /// itself 404s; [`crate::collect::errors::CollectError::Http`] on any
    /// other transport or non-success response.
    async fn ensure_repo_visible(&self) -> Result<()> {
        if self.repo_visible.get().is_some() {
            return Ok(());
        }
        let base = self.api_base();
        let url = format!("{base}/repos/{}/{}", self.owner, self.repo);
        debug!(url = %url, "GET (repo-visibility probe)");
        let resp = self.retry_request(&url).await?;

        if resp.status() == reqwest::StatusCode::NOT_FOUND {
            return Err(CollectError::GithubApi {
                status: 404,
                endpoint: url,
                message: "repository not visible to the configured credential \
                          (private repo with no or invalid token, or the repo \
                          genuinely does not exist)"
                    .to_string(),
            });
        }
        // Any other non-success (401/403/5xx) is a real transport failure,
        // not the "invisible repo" shape this probe exists to catch.
        resp.error_for_status()?;
        let _ = self.repo_visible.set(true);
        Ok(())
    }

    /// Send a GET request with exponential backoff on transient failures.
    ///
    /// Why: GitHub occasionally returns 502/504 under load and 429 when the
    /// per-token rate limit drains; a tiny retry loop avoids surfacing those
    /// as pipeline failures.
    /// What: delegates to the free [`retry_get`] helper, passing `self.client`
    /// and this client's shared [`FetchBudget`] so every call on it charges
    /// against one run-wide ceiling (#6084).
    /// Test: covered indirectly by callers and by `wiremock` integration tests.
    async fn retry_request(&self, url: &str) -> Result<reqwest::Response> {
        retry_get(&self.client, url, self.budget.shared()).await
    }

    /// Whether a paginated walk has reached [`MAX_PAGES`] and must stop.
    ///
    /// Why (#6084): every listing here looped until GitHub returned a short
    /// page, which trusts the server to eventually end the walk. A cap makes
    /// each walk finite; recording the stop is what keeps the shortened result
    /// from reading downstream exactly like a complete one.
    /// What: returns `false` below the cap. At the cap, warns, appends a
    /// truncation notice to the budget, and returns `true` for the caller to
    /// break on.
    /// Test: `client_tests::a_listing_that_never_ends_stops_at_the_page_cap_and_says_so`.
    fn page_cap_reached(&self, endpoint: &str, scope: &str, page: u32) -> bool {
        if page <= MAX_PAGES {
            return false;
        }
        warn!(
            endpoint,
            scope,
            pages = MAX_PAGES,
            "GitHub listing hit the page cap; results for this scope are partial"
        );
        self.budget.shared().note_truncation(format!(
            "GitHub {endpoint} listing for {scope} stopped at the {MAX_PAGES}-page cap \
             ({} items); these results are PARTIAL",
            MAX_PAGES * PAGE_SIZE
        ));
        true
    }

    /// Bounds this client hit while fetching, in operator-facing wording.
    ///
    /// Empty when nothing was truncated. Non-empty means the data this client
    /// returned is incomplete and the caller must say so (#6084).
    pub(crate) fn fetch_notices(&self) -> Vec<String> {
        self.budget.shared().notices()
    }

    /// Fetch all reviews for a given pull request, paginating until exhausted.
    ///
    /// Why: review counts, approval status, and review latency are core PR
    /// metrics; the bulk-PR endpoint omits reviews entirely. Taking explicit
    /// `(owner, repo)` rather than using `self.owner`/`self.repo` is
    /// critical for multi-repo clients where the primary owner/repo is
    /// unrelated to the PR being reviewed (issue #742 bug fix — the old
    /// signature silently fetched reviews from the wrong repo).
    /// What: `GET /repos/{owner}/{repo}/pulls/{pr_number}/reviews?per_page=100`,
    /// looping pages until a short page indicates end-of-list.
    /// Test: deserialization shape covered by `github_review_deserializes`;
    /// correct routing verified by the reviewer-ingestion integration path.
    ///
    /// # Errors
    ///
    /// - [`crate::collect::errors::CollectError::Http`] on transport / non-success
    ///   HTTP responses after retries are exhausted.
    /// - [`crate::collect::errors::CollectError::Json`] on payload parse failures.
    pub async fn fetch_pr_reviews_for_repo(
        &self,
        owner: &str,
        repo: &str,
        pr_number: u64,
    ) -> Result<Vec<GitHubReview>> {
        let base = self.api_base();
        let mut out = Vec::new();
        let mut page = 1u32;
        loop {
            let url = format!(
                "{base}/repos/{owner}/{repo}/pulls/{pr_number}/reviews?per_page={PAGE_SIZE}&page={page}"
            );
            let resp = self.retry_request(&url).await?.error_for_status()?;
            let batch: Vec<GitHubReview> = resp.json().await?;
            let n = batch.len();
            out.extend(batch);
            if (n as u32) < PAGE_SIZE {
                break;
            }
            page += 1;
            if self.page_cap_reached("reviews", &format!("{owner}/{repo}#{pr_number}"), page) {
                break;
            }
        }
        Ok(out)
    }

    /// Expose the internal HTTP client for org-discovery requests.
    ///
    /// Why: `discover_org_repos` lives in a sibling module and needs the
    /// same authenticated `reqwest::Client` without duplicating the header
    /// build logic.
    /// What: returns a shared reference to the underlying `reqwest::Client`.
    /// Test: used by the reviewer-ingestion path in `collector.rs`.
    pub fn http_client(&self) -> &reqwest::Client {
        &self.client
    }

    /// Fetch all commits attached to a pull request, paginating until exhausted.
    ///
    /// Why: PR-level commit lists let us attribute work to the PR author and
    /// reconstruct review-window churn even when the merge commit alone is
    /// recorded on the default branch.
    /// What: `GET /repos/{owner}/{repo}/pulls/{pr_number}/commits?per_page=100`.
    /// Test: deserialization shape covered by `github_pr_commit_deserializes`.
    ///
    /// # Errors
    ///
    /// - [`crate::collect::errors::CollectError::Http`] on transport / non-success
    ///   HTTP responses after retries are exhausted.
    /// - [`crate::collect::errors::CollectError::Json`] on payload parse failures.
    pub async fn fetch_pr_commits(&self, pr_number: u64) -> Result<Vec<GitHubPrCommit>> {
        let base = self.api_base();
        let mut out = Vec::new();
        let mut page = 1u32;
        loop {
            let url = format!(
                "{base}/repos/{}/{}/pulls/{pr_number}/commits?per_page={PAGE_SIZE}&page={page}",
                self.owner, self.repo
            );
            let resp = self.retry_request(&url).await?.error_for_status()?;
            let batch: Vec<GitHubPrCommit> = resp.json().await?;
            let n = batch.len();
            out.extend(batch);
            if (n as u32) < PAGE_SIZE {
                break;
            }
            page += 1;
            let scope = format!("{}/{}#{pr_number}", self.owner, self.repo);
            if self.page_cap_reached("pr commits", &scope, page) {
                break;
            }
        }
        Ok(out)
    }

    /// List issues on the configured repository, paginating until exhausted.
    ///
    /// Note: the GitHub `issues` endpoint includes pull requests in its
    /// response. Callers needing pure issues should call [`Self::fetch_pull_requests`]
    /// for PR-specific work.
    ///
    /// Why: bulk issue listing is needed for backfilling ticket metadata
    /// when commit messages reference `#NNN` without a project prefix.
    /// What: `GET /repos/{owner}/{repo}/issues?state={state}&since={since}&per_page=100`.
    /// Test: integration-tested via the `pm` adapter suite; deserialization
    /// reuses `GitHubIssue` whose shape is unit-tested above.
    ///
    /// # Arguments
    ///
    /// * `state` — one of `"open"`, `"closed"`, or `"all"`.
    /// * `since` — optional ISO8601 timestamp; only issues updated at or
    ///   after this time are returned.
    ///
    /// # Errors
    ///
    /// - [`crate::collect::errors::CollectError::Http`] on transport / non-success
    ///   HTTP responses after retries are exhausted.
    /// - [`crate::collect::errors::CollectError::Json`] on payload parse failures.
    pub async fn list_issues(&self, state: &str, since: Option<&str>) -> Result<Vec<GitHubIssue>> {
        let base = self.api_base();
        let mut out = Vec::new();
        let mut page = 1u32;
        loop {
            let mut url = format!(
                "{base}/repos/{}/{}/issues?state={state}&per_page={PAGE_SIZE}&page={page}",
                self.owner, self.repo
            );
            if let Some(s) = since {
                url.push_str("&since=");
                url.push_str(s);
            }
            let resp = self.retry_request(&url).await?.error_for_status()?;
            let batch: Vec<GitHubIssue> = resp.json().await?;
            let n = batch.len();
            out.extend(batch);
            if (n as u32) < PAGE_SIZE {
                break;
            }
            page += 1;
            let scope = format!("{}/{}", self.owner, self.repo);
            if self.page_cap_reached("issues", &scope, page) {
                break;
            }
        }
        Ok(out)
    }
}

#[async_trait]
impl PrProvider for GitHubClient {
    fn name(&self) -> &str {
        "github"
    }

    async fn fetch_pull_requests(&self) -> Result<Vec<PullRequest>> {
        GitHubClient::fetch_pull_requests(self).await
    }

    fn store_pull_requests(
        &self,
        db: &Database,
        prs: &[PullRequest],
    ) -> crate::core::Result<usize> {
        GitHubClient::store_pull_requests(self, db, prs)
    }

    fn fetch_notices(&self) -> Vec<String> {
        GitHubClient::fetch_notices(self)
    }
}

#[cfg(test)]
#[path = "client_tests.rs"]
mod tests;