merge queue: embarking main (8438ac4) and #1399 together

crates/mergify-cli/src/main.rs

@@ -27,6 +27,7 @@ use mergify_config::simulate::SimulateOptions;
 use mergify_core::OutputMode;
 use mergify_core::StdioOutput;
 use mergify_queue::pause::PauseOptions;
+use mergify_queue::show::ShowOptions;
 use mergify_queue::status::StatusOptions;
 use mergify_queue::unpause::UnpauseOptions;
 
@@ -89,6 +90,7 @@ const NATIVE_COMMANDS: &[(&str, &str)] = &[
     ("queue", "pause"),
     ("queue", "unpause"),
     ("queue", "status"),
+    ("queue", "show"),
 ];
 
 /// Native commands the Rust binary handles without delegating to
@@ -102,6 +104,7 @@ enum NativeCommand {
     QueuePause(QueuePauseOpts),
     QueueUnpause(QueueUnpauseOpts),
     QueueStatus(QueueStatusOpts),
+    QueueShow(QueueShowOpts),
 }
 
 struct ConfigSimulateOpts {
@@ -144,6 +147,15 @@ struct QueueStatusOpts {
     output_json: bool,
 }
 
+struct QueueShowOpts {
+    repository: Option<String>,
+    token: Option<String>,
+    api_url: Option<String>,
+    pr_number: u64,
+    verbose: bool,
+    output_json: bool,
+}
+
 /// Heuristic: does argv look like the user intended a native
 /// subcommand?
 ///
@@ -193,6 +205,7 @@ fn is_help_or_version(err: &clap::Error) -> bool {
 /// — the corresponding exit code is the one chosen by the command
 /// implementation (typically [`mergify_core::ExitCode::Configuration`]
 /// = 8), not 2.
+#[allow(clippy::too_many_lines)] // mostly mechanical match arms
 fn detect_native(argv: &[String]) -> Option<NativeCommand> {
     let looks_native = looks_native(argv);
 
@@ -303,9 +316,28 @@ fn detect_native(argv: &[String]) -> Option<NativeCommand> {
             branch,
             output_json: json,
         })),
+        Subcommands::Queue(QueueArgs {
+            repository,
+            token,
+            api_url,
+            command:
+                QueueSubcommand::Show(ShowCliArgs {
+                    pr_number,
+                    verbose,
+                    json,
+                }),
+        }) => Some(NativeCommand::QueueShow(QueueShowOpts {
+            repository,
+            token,
+            api_url,
+            pr_number,
+            verbose,
+            output_json: json,
+        })),
     }
 }
 
+#[allow(clippy::too_many_lines)] // mostly mechanical match arms
 fn run_native(cmd: NativeCommand) -> ExitCode {
     let rt = match tokio::runtime::Builder::new_current_thread()
         .enable_all()
@@ -394,6 +426,20 @@ fn run_native(cmd: NativeCommand) -> ExitCode {
                 )
                 .await
             }
+            NativeCommand::QueueShow(opts) => {
+                mergify_queue::show::run(
+                    ShowOptions {
+                        repository: opts.repository.as_deref(),
+                        token: opts.token.as_deref(),
+                        api_url: opts.api_url.as_deref(),
+                        pr_number: opts.pr_number,
+                        verbose: opts.verbose,
+                        output_json: opts.output_json,
+                    },
+                    &mut output,
+                )
+                .await
+            }
         }
     });
 
@@ -567,6 +613,8 @@ enum QueueSubcommand {
     Unpause,
     /// Show merge queue status for the repository.
     Status(StatusCliArgs),
+    /// Show detailed state of a pull request in the merge queue.
+    Show(ShowCliArgs),
 }
 
 #[derive(clap::Args)]
@@ -591,3 +639,19 @@ struct StatusCliArgs {
     #[arg(long, default_value_t = false)]
     json: bool,
 }
+
+#[derive(clap::Args)]
+struct ShowCliArgs {
+    /// Pull request number to inspect.
+    #[arg(value_name = "PR_NUMBER")]
+    pr_number: u64,
+
+    /// Show the full checks table and the conditions tree instead
+    /// of compact summaries.
+    #[arg(long, short = 'v', default_value_t = false)]
+    verbose: bool,
+
+    /// Emit the raw API response as a single JSON document.
+    #[arg(long, default_value_t = false)]
+    json: bool,
+}

crates/mergify-core/src/http.rs

@@ -129,6 +129,23 @@ impl Client {
         self.decode_json(resp).await
     }
 
+    /// GET `path`, returning `None` on 404. Other 4xx/5xx responses
+    /// surface as the normal `CliError` API failure. Mirrors
+    /// [`Self::delete_if_exists`] but for read-only endpoints where
+    /// "not found" is a meaningful caller branch (e.g. `queue show`
+    /// must distinguish "PR not in queue" from a genuine API
+    /// failure).
+    pub async fn get_if_exists<T: DeserializeOwned>(
+        &self,
+        path: &str,
+    ) -> Result<Option<T>, CliError> {
+        let url = self.join(path)?;
+        match self.execute_request_optional(self.inner.get(url)).await? {
+            Some(resp) => self.decode_json(resp).await.map(Some),
+            None => Ok(None),
+        }
+    }
+
     /// POST `body` as JSON to `path` and deserialize the JSON
     /// response as `T`.
     pub async fn post<B: Serialize + ?Sized, T: DeserializeOwned>(
@@ -197,14 +214,29 @@ impl Client {
             .map_err(|e| self.api_error(format!("invalid path {path:?}: {e}")))
     }
 
-    /// Execute a request that cares only about the HTTP status.
+    /// Single retry/auth/error driver behind every public verb.
     ///
-    /// Used by [`Self::delete_if_exists`] — the response body (if
-    /// any) is discarded.
-    async fn execute_status(
+    /// `tolerate_not_found` lets callers opt into "404 is a
+    /// caller-branch, not an error" semantics:
+    ///
+    /// - `false` (default for `get` / `post` / `put`): 404 surfaces
+    ///   as a [`CliError`] like any other 4xx.
+    /// - `true` (for `get_if_exists` / `delete_if_exists`): 404
+    ///   short-circuits to `Ok(None)`. The HTTP body is dropped.
+    ///
+    /// Success (2xx) always returns `Ok(Some(response))` — the
+    /// caller decides whether to decode the body, drop it, or
+    /// map it to a domain type.
+    ///
+    /// 5xx is retried with exponential backoff (`self.retry`);
+    /// transient send errors (timeout / connect) are retried with
+    /// the same backoff. Other terminal errors and non-5xx 4xx
+    /// fail immediately.
+    async fn execute_with_retry(
         &self,
         builder: reqwest::RequestBuilder,
-    ) -> Result<DeleteOutcome, CliError> {
+        tolerate_not_found: bool,
+    ) -> Result<Option<reqwest::Response>, CliError> {
         let mut backoff = self.retry.initial_backoff;
         let mut last_message = String::from("HTTP request failed without response");
 
@@ -223,10 +255,10 @@ impl Client {
                 Ok(resp) => {
                     let status = resp.status();
                     if status.is_success() {
-                        return Ok(DeleteOutcome::Deleted);
+                        return Ok(Some(resp));
                     }
-                    if status == StatusCode::NOT_FOUND {
-                        return Ok(DeleteOutcome::NotFound);
+                    if tolerate_not_found && status == StatusCode::NOT_FOUND {
+                        return Ok(None);
                     }
                     last_message = error_message(status, resp).await;
                     if status.is_server_error() && attempt + 1 < self.retry.max_attempts {
@@ -249,49 +281,40 @@ impl Client {
         Err(self.api_error(last_message))
     }
 
+    /// Send a request that must return a response. 404 is treated
+    /// like any other 4xx (caller error → [`CliError`]).
     async fn execute_request(
         &self,
         builder: reqwest::RequestBuilder,
     ) -> Result<reqwest::Response, CliError> {
-        let mut backoff = self.retry.initial_backoff;
-        let mut last_message = String::from("HTTP request failed without response");
+        // `tolerate_not_found = false` means the driver never
+        // returns `None`; `Option::expect` documents that invariant.
+        Ok(self
+            .execute_with_retry(builder, false)
+            .await?
+            .expect("execute_with_retry returned None despite tolerate_not_found=false"))
+    }
 
-        for attempt in 0..self.retry.max_attempts {
-            let Some(cloned) = builder.try_clone() else {
-                return Err(self.api_error(
-                    "request body is not cloneable (streaming?) — cannot retry".into(),
-                ));
-            };
-            let req = match &self.token {
-                Some(token) => cloned.bearer_auth(token),
-                None => cloned,
-            };
+    /// Send a request where 404 is a routine caller branch
+    /// rather than a server failure. Used by [`Self::get_if_exists`].
+    async fn execute_request_optional(
+        &self,
+        builder: reqwest::RequestBuilder,
+    ) -> Result<Option<reqwest::Response>, CliError> {
+        self.execute_with_retry(builder, true).await
+    }
 
-            match req.send().await {
-                Ok(resp) => {
-                    let status = resp.status();
-                    if status.is_success() {
-                        return Ok(resp);
-                    }
-                    last_message = error_message(status, resp).await;
-                    if status.is_server_error() && attempt + 1 < self.retry.max_attempts {
-                        tokio::time::sleep(backoff).await;
-                        backoff *= 2;
-                        continue;
-                    }
-                    return Err(self.api_error(last_message));
-                }
-                Err(e) if is_transient(&e) && attempt + 1 < self.retry.max_attempts => {
-                    last_message = format!("network error: {e}");
-                    tokio::time::sleep(backoff).await;
-                    backoff *= 2;
-                }
-                Err(e) => {
-                    return Err(self.api_error(self.terminal_send_error_message(&e)));
-                }
-            }
+    /// Send a request that cares only about the HTTP status.
+    /// Used by [`Self::delete_if_exists`] — the response body
+    /// (if any) is discarded.
+    async fn execute_status(
+        &self,
+        builder: reqwest::RequestBuilder,
+    ) -> Result<DeleteOutcome, CliError> {
+        match self.execute_with_retry(builder, true).await? {
+            Some(_) => Ok(DeleteOutcome::Deleted),
+            None => Ok(DeleteOutcome::NotFound),
         }
-        Err(self.api_error(last_message))
     }
 
     async fn decode_json<T: DeserializeOwned>(
@@ -708,4 +731,92 @@ mod tests {
             msg.len()
         );
     }
+
+    #[tokio::test]
+    async fn get_if_exists_returns_some_on_2xx() {
+        let server = MockServer::start().await;
+        Mock::given(method("GET"))
+            .and(path("/foo"))
+            .and(header("Authorization", "Bearer test-token"))
+            .respond_with(ResponseTemplate::new(200).set_body_json(Foo { bar: 7 }))
+            .expect(1)
+            .mount(&server)
+            .await;
+
+        let client = fast_client(&server, ApiFlavor::Mergify);
+        let got: Option<Foo> = client.get_if_exists("/foo").await.unwrap();
+        assert_eq!(got, Some(Foo { bar: 7 }));
+    }
+
+    #[tokio::test]
+    async fn get_if_exists_returns_none_on_404() {
+        let server = MockServer::start().await;
+        Mock::given(method("GET"))
+            .and(path("/missing"))
+            .respond_with(ResponseTemplate::new(404).set_body_string("not found"))
+            .expect(1)
+            .mount(&server)
+            .await;
+
+        let client = fast_client(&server, ApiFlavor::Mergify);
+        let got: Option<Foo> = client.get_if_exists("/missing").await.unwrap();
+        assert!(got.is_none(), "expected None on 404, got {got:?}");
+    }
+
+    #[tokio::test]
+    async fn get_if_exists_surfaces_other_4xx_as_error() {
+        // 403 / 401 / 422 etc. are real failures, not "doesn't
+        // exist" — they must surface as `CliError`. Only 404 is
+        // mapped to `None`.
+        let server = MockServer::start().await;
+        Mock::given(method("GET"))
+            .and(path("/forbidden"))
+            .respond_with(ResponseTemplate::new(403).set_body_string(r#"{"detail":"nope"}"#))
+            .expect(1)
+            .mount(&server)
+            .await;
+
+        let client = fast_client(&server, ApiFlavor::Mergify);
+        let err = client.get_if_exists::<Foo>("/forbidden").await.unwrap_err();
+        assert!(
+            matches!(err, CliError::MergifyApi(_)),
+            "expected MergifyApi, got {err:?}",
+        );
+        assert!(err.to_string().contains("403"));
+    }
+
+    #[tokio::test]
+    async fn get_if_exists_retries_5xx_then_succeeds() {
+        // Same retry semantics as `get`: a 500 on the first
+        // attempt should not short-circuit; the second attempt's
+        // 200 must be returned as `Some`.
+        struct FlakyRespond {
+            calls: Arc<AtomicU32>,
+        }
+        impl Respond for FlakyRespond {
+            fn respond(&self, _req: &Request) -> ResponseTemplate {
+                let n = self.calls.fetch_add(1, Ordering::SeqCst);
+                if n == 0 {
+                    ResponseTemplate::new(500)
+                } else {
+                    ResponseTemplate::new(200).set_body_json(Foo { bar: 9 })
+                }
+            }
+        }
+        let server = MockServer::start().await;
+        let calls = Arc::new(AtomicU32::new(0));
+        Mock::given(method("GET"))
+            .and(path("/flaky"))
+            .respond_with(FlakyRespond {
+                calls: Arc::clone(&calls),
+            })
+            .expect(2)
+            .mount(&server)
+            .await;
+
+        let client = fast_client(&server, ApiFlavor::Mergify);
+        let got: Option<Foo> = client.get_if_exists("/flaky").await.unwrap();
+        assert_eq!(got, Some(Foo { bar: 9 }));
+        assert_eq!(calls.load(Ordering::SeqCst), 2, "expected two attempts");
+    }
 }

crates/mergify-queue/src/lib.rs

@@ -10,5 +10,6 @@
 //! ports next.
 
 pub mod pause;
+pub mod show;
 pub mod status;
 pub mod unpause;