fix(queue): treat a not-queued PR as a normal state, not an API error (#1678)

`queue show` mapped a PR that isn't in the merge queue to exit 6
(Mergify API error) with empty stdout — so `--json` consumers got
nothing parseable for the most common case, and scripts saw an
API-failure code for a routine state.

Report it on stdout and exit 0 instead: a `{"number": N, "queued":
false}` document under `--json`, the `PR #N is not in the merge
queue` line in human mode.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

Author: JulianMaurin

crates/mergify-cli/tests/live_smoke.rs

@@ -397,9 +397,9 @@ fn queue_show_not_in_queue() {
     //
     // Calls with a PR number that is almost certainly not in the
     // queue (the test repo has far fewer than this many PRs).
-    // Both Python and Rust special-case 404 with the same
-    // user-facing message and `MERGIFY_API_ERROR` exit code (6)
-    // — that contract is what this test pins.
+    // A not-queued PR is a normal queryable state, so the command
+    // reports it on stdout and exits 0 — that contract is what this
+    // test pins.
     //
     // Testing the 404 path (instead of a real queued PR) makes
     // the test independent of whether PR #1 happens to be queued
@@ -421,17 +421,17 @@ fn queue_show_not_in_queue() {
     ]);
     assert_eq!(
         result.returncode,
-        6,
-        "expected MERGIFY_API_ERROR (6), got {}{}",
+        0,
+        "expected a not-queued PR to exit 0, got {}{}",
         result.returncode,
         result.context()
     );
     assert!(
         result
-            .combined()
+            .stdout
             .to_lowercase()
             .contains("not in the merge queue"),
-        "expected 'not in the merge queue' message{}",
+        "expected 'not in the merge queue' message on stdout{}",
         result.context()
     );
 }

crates/mergify-queue/src/show.rs

@@ -14,15 +14,15 @@
 //!   a tree.
 //!
 //! 404 responses are special-cased: the API returns 404 for "PR is
-//! not currently in the merge queue", which is a routine caller
-//! branch rather than a server failure. The command returns a
-//! [`CliError::MergifyApi`] whose message is `PR #N is not in the
-//! merge queue`; the binary's top-level handler prints it to
-//! stderr as `mergify: PR #N is not in the merge queue` (the
-//! `mergify: ` prefix is the binary's standard error envelope)
-//! and exits with the Mergify-API error code. Live smoke tests
-//! assert against the substring, which is stable across the
-//! Python and Rust implementations.
+//! not currently in the merge queue", which is a routine queryable
+//! state rather than a server failure. The command reports it on
+//! stdout and exits 0 — a not-queued PR is a normal answer, not an
+//! error a script should branch on as an API failure. Under `--json`
+//! the not-queued state is a `{"number": N, "queued": false}`
+//! document so pipeline consumers always get parseable output; in
+//! human mode it is the line `PR #N is not in the merge queue`. Live
+//! smoke tests assert against that substring, which is stable across
+//! the Python and Rust implementations.
 
 use std::io::Write;
 
@@ -119,10 +119,8 @@ pub async fn run(opts: ShowOptions<'_>, output: &mut dyn Output) -> Result<(), C
 
     let raw: Option<serde_json::Value> = client.get_if_exists(&path).await?;
     let Some(raw) = raw else {
-        return Err(CliError::MergifyApi(format!(
-            "PR #{n} is not in the merge queue",
-            n = opts.pr_number,
-        )));
+        emit_not_queued(output, opts.pr_number, opts.output_json)?;
+        return Ok(());
     };
 
     if opts.output_json {
@@ -136,6 +134,28 @@ pub async fn run(opts: ShowOptions<'_>, output: &mut dyn Output) -> Result<(), C
     Ok(())
 }
 
+/// Emit the "PR is not in the merge queue" state. This is a normal
+/// answer, not a failure, so the command exits 0 — see the module
+/// docs. Under `--json` we emit a `{number, queued: false}` document
+/// (a machine consumer always gets parseable output); in human mode
+/// a single notice line. The wording is load-bearing: live smoke
+/// tests assert on the "is not in the merge queue" substring.
+fn emit_not_queued(
+    output: &mut dyn Output,
+    pr_number: u64,
+    output_json: bool,
+) -> Result<(), CliError> {
+    if output_json {
+        let payload = serde_json::json!({ "number": pr_number, "queued": false });
+        output.emit_json_value(&payload)?;
+    } else {
+        output.emit(&(), &mut |w: &mut dyn Write| {
+            writeln!(w, "PR #{pr_number} is not in the merge queue")
+        })?;
+    }
+    Ok(())
+}
+
 fn emit_human(output: &mut dyn Output, view: &PullView, verbose: bool) -> std::io::Result<()> {
     let now = Utc::now();
     let theme = Theme::detect();
@@ -657,7 +677,10 @@ mod tests {
     }
 
     #[tokio::test]
-    async fn run_404_is_not_in_queue() {
+    async fn run_404_human_is_not_in_queue_and_succeeds() {
+        // A not-queued PR is a normal queryable state, not an API
+        // failure: human mode prints the notice to stdout and the
+        // command returns Ok (exit 0).
         let server = MockServer::start().await;
         Mock::given(method("GET"))
             .and(path("/v1/repos/owner/repo/merge-queue/pull/999"))
@@ -668,7 +691,7 @@ mod tests {
 
         let mut cap = Captured::human();
         let api_url = server.uri();
-        let err = run(
+        run(
             ShowOptions {
                 repository: Some("owner/repo"),
                 token: Some("t"),
@@ -680,19 +703,51 @@ mod tests {
             &mut cap.output,
         )
         .await
-        .unwrap_err();
+        .unwrap();
 
+        let stdout = cap.stdout();
         assert!(
-            matches!(err, CliError::MergifyApi(_)),
-            "expected MergifyApi, got {err:?}",
-        );
-        assert_eq!(err.exit_code(), mergify_core::ExitCode::MergifyApiError);
-        assert!(
-            err.to_string().contains("not in the merge queue"),
-            "got: {err}"
+            stdout.contains("PR #999 is not in the merge queue"),
+            "got: {stdout:?}",
         );
     }
 
+    #[tokio::test]
+    async fn run_404_json_emits_not_queued_document() {
+        // Under `--json`, the not-queued state is a parseable
+        // `{number, queued: false}` document on stdout (exit 0), so
+        // pipeline consumers never get empty output for the common
+        // case.
+        let server = MockServer::start().await;
+        Mock::given(method("GET"))
+            .and(path("/v1/repos/owner/repo/merge-queue/pull/999"))
+            .respond_with(ResponseTemplate::new(404))
+            .expect(1)
+            .mount(&server)
+            .await;
+
+        let mut cap = Captured::new(OutputMode::Json);
+        let api_url = server.uri();
+        run(
+            ShowOptions {
+                repository: Some("owner/repo"),
+                token: Some("t"),
+                api_url: Some(&api_url),
+                pr_number: 999,
+                verbose: false,
+                output_json: true,
+            },
+            &mut cap.output,
+        )
+        .await
+        .unwrap();
+
+        let stdout = cap.stdout();
+        let parsed: serde_json::Value = serde_json::from_str(&stdout).unwrap();
+        assert_eq!(parsed["number"], json!(999));
+        assert_eq!(parsed["queued"], json!(false));
+    }
+
     #[tokio::test]
     async fn run_no_mergeability_check() {
         let server = MockServer::start().await;