diff --git a/hive-forge/src/verbs/pr_merge.rs b/hive-forge/src/verbs/pr_merge.rs index 5564218c..ac4c4755 100644 --- a/hive-forge/src/verbs/pr_merge.rs +++ b/hive-forge/src/verbs/pr_merge.rs @@ -15,10 +15,10 @@ use anyhow::{Result, bail}; use clap::{Args as ClapArgs, ValueEnum}; use forgejo_api::structs::{ - MergePullRequestOption, MergePullRequestOptionDo, PullRequest, StateType, + CommitStatusState, MergePullRequestOption, MergePullRequestOptionDo, PullRequest, StateType, }; -use crate::client::{Client, index}; +use crate::client::{Client, index, split_repo}; /// Merge strategy. Squash is deliberately omitted (hive convention: keep the /// per-commit history, so a squash option isn't exposed). @@ -136,11 +136,17 @@ fn check_ready(client: &Client, repo: &str, number: u64, pull: &PullRequest) -> } if let Some(sha) = pull.head.as_ref().and_then(|h| h.sha.as_deref()) { - let combined = super::pr_status::fetch_combined_in(client, repo, sha)?; + let (owner, name) = split_repo(repo)?; + let (_, combined) = client + .api() + .repo_get_combined_status_by_ref(owner, name, sha) + .send()?; + let state = combined.state; + let has_statuses = combined.statuses.as_ref().is_some_and(|a| !a.is_empty()); // An empty status set means no CI is configured — not a blocker. // Anything other than success once CI exists blocks the merge. - if !combined.statuses.is_empty() && combined.state != "success" { - let state = &combined.state; + if has_statuses && state != Some(CommitStatusState::Success) { + let state = super::pr_status::status_state_str(state); bail!( "pr-merge: PR #{number} CI is not green (state: {state}). Wait for green, or pass --force." ); diff --git a/hive-forge/src/verbs/pr_status.rs b/hive-forge/src/verbs/pr_status.rs index dc1f35d8..1703381d 100644 --- a/hive-forge/src/verbs/pr_status.rs +++ b/hive-forge/src/verbs/pr_status.rs @@ -11,7 +11,7 @@ use anyhow::{Context, Result, bail}; use clap::Args as ClapArgs; -use forgejo_api::structs::IssueGetCommentsQuery; +use forgejo_api::structs::{CommitStatusState, IssueGetCommentsQuery}; use serde_json::Value; use crate::client::{Client, index}; @@ -44,21 +44,19 @@ pub fn run(client: &Client, args: Args) -> Result<()> { } } -/// Lenient mirror of the combined-status response. -/// -/// The typed client models `state` as an enum, but a commit with no CI -/// contexts at all comes back as `"state": ""` — not a member of that -/// enum — so the typed call fails to deserialize and the whole verb -/// dies on exactly the PRs where "no CI ran" is the useful answer. -/// Keeping `state` a plain `String` (and the per-context statuses as -/// opaque `Value`s, which the render helpers already walk) means an -/// unknown or empty state is reported rather than fatal. -#[derive(serde::Deserialize, Default)] -pub(crate) struct CombinedStatus { - #[serde(default)] - pub state: String, - #[serde(default)] - pub statuses: Vec, +/// The wire string for a combined/per-context CI status state, matching +/// what the raw API emitted (`""` when absent). Shared with `pr-merge` +/// for its "CI is not green" message. +pub(crate) fn status_state_str(state: Option) -> &'static str { + match state { + Some(CommitStatusState::Pending) => "pending", + Some(CommitStatusState::Success) => "success", + Some(CommitStatusState::Error) => "error", + Some(CommitStatusState::Failure) => "failure", + Some(CommitStatusState::Warning) => "warning", + Some(CommitStatusState::Skipped) => "skipped", + None => "", + } } /// CI-only path for an explicit commit. Exit code mirrors the CI verdict. @@ -162,27 +160,23 @@ fn pr_status(client: &Client, repo: &str, pr: u64) -> Result<()> { } } -/// Fetch the combined commit status for `sha` in the client's repo. -/// Goes through the raw-JSON escape hatch rather than the typed client -/// so an empty (no-CI) state stays reportable — see [`CombinedStatus`]. -/// -/// # Errors -/// -/// Returns an error if `repo` isn't a well-formed `owner/name`, or if -/// the status GET fails (transport, non-2xx, or a body that isn't even -/// loosely the expected shape). An empty state is NOT an error — that's -/// the case this function exists to report. -pub(crate) fn fetch_combined_in(client: &Client, repo: &str, sha: &str) -> Result { - let (owner, name) = crate::client::split_repo(repo)?; - client.get_api_json(&format!("/repos/{owner}/{name}/commits/{sha}/status"), &[]) -} - /// Fetch the combined commit status: `(overall_state, statuses[])`. -/// Statuses ride as their API-shape JSON so the render helpers stay -/// pure `Value` walkers. +/// Statuses ride as their serialized (API-shape) JSON so the render +/// helpers stay pure `Value` walkers. fn fetch_combined(client: &Client, sha: &str) -> Result<(String, Vec)> { - let combined = fetch_combined_in(client, client.repo(), sha)?; - Ok((combined.state, combined.statuses)) + let (owner, name) = client.owner_repo()?; + let (_, combined) = client + .api() + .repo_get_combined_status_by_ref(owner, name, sha) + .send()?; + let state = status_state_str(combined.state).to_owned(); + let statuses = combined + .statuses + .unwrap_or_default() + .iter() + .map(serde_json::to_value) + .collect::, _>>()?; + Ok((state, statuses)) } /// The most recent issue comment on the PR, as `(login, created_at)`. @@ -387,30 +381,16 @@ mod tests { } #[test] - fn combined_status_accepts_empty_state() { - // A commit with no CI contexts: Forgejo emits `"state": ""`, - // which the typed client's enum rejects outright. - let c: CombinedStatus = - serde_json::from_str(r#"{"state":"","statuses":[],"sha":"abc"}"#).unwrap(); - assert_eq!(c.state, ""); - assert!(c.statuses.is_empty()); - } - - #[test] - fn combined_status_keeps_known_state_and_contexts() { - let c: CombinedStatus = serde_json::from_str( - r#"{"state":"success","statuses":[{"status":"success","context":"ci"}]}"#, - ) - .unwrap(); - assert_eq!(c.state, "success"); - assert_eq!(c.statuses.len(), 1); - } - - #[test] - fn combined_status_tolerates_missing_fields() { - let c: CombinedStatus = serde_json::from_str("{}").unwrap(); - assert_eq!(c.state, ""); - assert!(c.statuses.is_empty()); + fn status_state_str_matches_wire_names() { + assert_eq!( + status_state_str(Some(CommitStatusState::Success)), + "success" + ); + assert_eq!( + status_state_str(Some(CommitStatusState::Failure)), + "failure" + ); + assert_eq!(status_state_str(None), ""); } #[test]