From 43c5e172540651dea5c3dd3d495fc3025d773356 Mon Sep 17 00:00:00 2001 From: Julien Danjou Date: Thu, 23 Apr 2026 17:35:28 +0200 Subject: [PATCH] feat(rust): port queue pause and unpause to native Rust MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two queue commands in one PR — both are idempotent one-shot API calls that share the same auth + repository resolution. Pause exercises the new PUT method; unpause exercises the new DELETE-with-status-check method. PUTs ``{"reason": "..."}`` to ``/v1/repos//merge-queue/pause``, prints a confirmation line with the reason and timestamp. Safety rails match Python: - ``--yes-i-am-sure`` skips confirmation outright. - Interactive (TTY): prompts "Proceed? [y/N]". Anything other than ``y``/``yes`` aborts as a generic error. - Non-interactive without the flag: refuses with INVALID_STATE (exit 7), matching Python's ``raise SystemExit(ExitCode.INVALID_STATE)``. ``--reason`` has a 255-char cap enforced by clap's ``value_parser`` — bad input exits 2. DELETEs the same path. On 404 the API is telling us the queue wasn't paused, so the command prints "Queue is not currently paused" and exits MERGIFY_API_ERROR (matches Python). On 2xx it prints "Queue resumed." and exits 0. Two new methods on ``mergify_core::HttpClient``: - ``put(path, body) -> Result`` — mirror of ``post``, different verb. - ``delete_if_exists(path) -> Result`` — returns ``Deleted`` on 2xx, ``NotFound`` on 404, errors on any other non-success status. Lets commands like ``unpause`` give a friendlier 404 message without parsing error strings. Auth resolution (token / api-url / repository) goes through the shared ``mergify_core::auth`` module added earlier in the stack — no per-crate copy of the resolvers in ``mergify-queue``. 5 new unit tests in the queue crate: - ``parse_reason`` accepts short strings and rejects > 255 chars - ``run`` pauses and prints the API-returned reason + timestamp - ``run`` prints "Queue resumed" on 2xx - ``run`` errors with MERGIFY_API_ERROR on 404 carrying the "not currently paused" message End-to-end smoke tested three paths: ``queue pause --reason X -r owner/repo`` → exit 8 (missing token), ``queue unpause -r owner/repo`` → exit 8 (missing token), ``echo n | queue pause --reason X`` → exit 7 (non-TTY, no --sure). The Python ``queue pause`` / ``queue unpause`` implementations and their tests are removed in the same PR — the Rust binary now owns both commands end-to-end. Binary: 8.4 MB → 8.5 MB. 56 Rust tests. Co-Authored-By: Claude Opus 4.7 (1M context) Change-Id: Idba6fa38caf403fd5f4184cda462b5f7c1eb3ebf --- .github/workflows/func-tests-live.yaml | 11 +- Cargo.lock | 99 ++++----- crates/mergify-cli/Cargo.toml | 1 + crates/mergify-cli/src/main.rs | 134 +++++++++++- crates/mergify-core/src/http.rs | 116 +++++++++- crates/mergify-core/src/lib.rs | 2 +- crates/mergify-queue/Cargo.toml | 23 ++ crates/mergify-queue/src/lib.rs | 11 + crates/mergify-queue/src/pause.rs | 287 +++++++++++++++++++++++++ crates/mergify-queue/src/unpause.rs | 145 +++++++++++++ func-tests/conftest.py | 25 ++- func-tests/test_live_smoke.py | 63 +++++- mergify_cli/queue/api.py | 28 --- mergify_cli/queue/cli.py | 88 -------- mergify_cli/tests/queue/test_cli.py | 123 ----------- mergify_cli/tests/queue/test_skill.py | 17 +- 16 files changed, 853 insertions(+), 320 deletions(-) create mode 100644 crates/mergify-queue/Cargo.toml create mode 100644 crates/mergify-queue/src/lib.rs create mode 100644 crates/mergify-queue/src/pause.rs create mode 100644 crates/mergify-queue/src/unpause.rs diff --git a/.github/workflows/func-tests-live.yaml b/.github/workflows/func-tests-live.yaml index f708e9bf..d2242620 100644 --- a/.github/workflows/func-tests-live.yaml +++ b/.github/workflows/func-tests-live.yaml @@ -35,5 +35,14 @@ jobs: - name: Live smoke tests shell: bash env: - LIVE_TEST_MERGIFY_TOKEN: ${{ secrets.MERGIFY_CLI_LIVE_TEST_MERGIFY_TOKEN }} + # Two tokens with different scopes: + # - _CI exercises read-only / pull-scoped endpoints + # (scopes-send, junit-process, queue status, queue show, + # ci git-refs / queue-info). + # - _ADMIN exercises destructive queue-admin endpoints + # (queue pause / unpause). + # Tests select the appropriate fixture; absent tokens + # cause individual tests to skip rather than fail. + LIVE_TEST_MERGIFY_TOKEN_CI: ${{ secrets.MERGIFY_CLI_LIVE_TEST_MERGIFY_TOKEN_CI }} + LIVE_TEST_MERGIFY_TOKEN_ADMIN: ${{ secrets.MERGIFY_CLI_LIVE_TEST_MERGIFY_TOKEN_ADMIN }} run: uv run --locked poe live-test diff --git a/Cargo.lock b/Cargo.lock index eb38a258..d172079b 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -184,9 +184,9 @@ checksum = "1e748733b7cbc798e1434b6ac524f0c1ff2ab456fe201501e6497c8417a4fc33" [[package]] name = "cc" -version = "1.2.60" +version = "1.2.61" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "43c5703da9466b66a946814e1adf53ea2c90f10063b86290cc9eb67ce3478a20" +checksum = "d16d90359e986641506914ba71350897565610e87ce0ad9e6f28569db3dd5c6d" dependencies = [ "find-msvc-tools", "jobserver", @@ -416,9 +416,9 @@ dependencies = [ [[package]] name = "fraction" -version = "0.15.3" +version = "0.15.4" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "0f158e3ff0a1b334408dc9fb811cd99b446986f4d8b741bb08f9df1604085ae7" +checksum = "e076045bb43dac435333ed5f04caf35c7463631d0dae2deb2638d94dd0a5b872" dependencies = [ "lazy_static", "num", @@ -560,9 +560,9 @@ dependencies = [ [[package]] name = "h2" -version = "0.4.13" +version = "0.4.14" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "2f44da3a8150a6703ed5d34e164b875fd14c2cdab9af1252a9a1020bde2bdc54" +checksum = "171fefbc92fe4a4de27e0698d6a5b392d6a0e333506bc49133760b3bcf948733" dependencies = [ "atomic-waker", "bytes", @@ -846,16 +846,6 @@ version = "2.12.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "d98f6fed1fde3f8c21bc40a1abb88dd75e67924f9cffc3ef95607bad8017f8e2" -[[package]] -name = "iri-string" -version = "0.7.12" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "25e659a4bb38e810ebc252e53b5814ff908a8c58c2a9ce2fae1bbec24cbf4e20" -dependencies = [ - "memchr", - "serde", -] - [[package]] name = "is_terminal_polyfill" version = "1.70.2" @@ -929,9 +919,9 @@ dependencies = [ [[package]] name = "js-sys" -version = "0.3.95" +version = "0.3.98" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "2964e92d1d9dc3364cae4d718d93f227e3abb088e747d92e0395bfdedf1c12ca" +checksum = "67df7112613f8bfd9150013a0314e196f4800d3201ae742489d999db2f979f08" dependencies = [ "cfg-if", "futures-util", @@ -980,9 +970,9 @@ checksum = "09edd9e8b54e49e587e4f6295a7d29c3ea94d469cb40ab8ca70b288248a81db2" [[package]] name = "libc" -version = "0.2.185" +version = "0.2.186" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "52ff2c0fe9bc6cb6b14a0592c2ff4fa9ceb83eea9db979b0487cd054946a2b8f" +checksum = "68ab91017fe16c622486840e4c83c9a37afeff978bd239b5293d61ece587de66" [[package]] name = "linux-raw-sys" @@ -1046,6 +1036,7 @@ dependencies = [ "mergify-config", "mergify-core", "mergify-py-shim", + "mergify-queue", "tokio", ] @@ -1088,6 +1079,18 @@ dependencies = [ "thiserror", ] +[[package]] +name = "mergify-queue" +version = "0.0.0" +dependencies = [ + "mergify-core", + "serde", + "serde_json", + "tokio", + "url", + "wiremock", +] + [[package]] name = "micromap" version = "0.3.0" @@ -1552,9 +1555,9 @@ dependencies = [ [[package]] name = "rustls" -version = "0.23.38" +version = "0.23.40" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "69f9466fb2c14ea04357e91413efb882e2a6d4a406e625449bc0a5d360d53a21" +checksum = "ef86cd5876211988985292b91c96a8f2d298df24e75989a43a3c73f2d4d8168b" dependencies = [ "aws-lc-rs", "once_cell", @@ -1578,9 +1581,9 @@ dependencies = [ [[package]] name = "rustls-pki-types" -version = "1.14.0" +version = "1.14.1" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "be040f8b0a225e40375822a563fa9524378b9d63112f53e19ffff34df5d33fdd" +checksum = "30a7197ae7eb376e574fe940d068c30fe0462554a3ddbe4eca7838e049c937a9" dependencies = [ "web-time", "zeroize", @@ -1973,20 +1976,20 @@ dependencies = [ [[package]] name = "tower-http" -version = "0.6.8" +version = "0.6.10" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "d4e6559d53cc268e5031cd8429d05415bc4cb4aefc4aa5d6cc35fbf5b924a1f8" +checksum = "68d6fdd9f81c2819c9a8b0e0cd91660e7746a8e6ea2ba7c6b2b057985f6bcb51" dependencies = [ "bitflags", "bytes", "futures-util", "http", "http-body", - "iri-string", "pin-project-lite", "tower", "tower-layer", "tower-service", + "url", ] [[package]] @@ -2129,11 +2132,11 @@ checksum = "ccf3ec651a847eb01de73ccad15eb7d99f80485de043efb2f370cd654f4ea44b" [[package]] name = "wasip2" -version = "1.0.3+wasi-0.2.9" +version = "1.0.1+wasi-0.2.4" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "20064672db26d7cdc89c7798c48a0fdfac8213434a1186e5ef29fd560ae223d6" +checksum = "0562428422c63773dad2c345a1882263bbf4d65cf3f42e90921f787ef5ad58e7" dependencies = [ - "wit-bindgen 0.57.1", + "wit-bindgen 0.46.0", ] [[package]] @@ -2147,9 +2150,9 @@ dependencies = [ [[package]] name = "wasm-bindgen" -version = "0.2.118" +version = "0.2.121" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "0bf938a0bacb0469e83c1e148908bd7d5a6010354cf4fb73279b7447422e3a89" +checksum = "49ace1d07c165b0864824eee619580c4689389afa9dc9ed3a4c75040d82e6790" dependencies = [ "cfg-if", "once_cell", @@ -2160,9 +2163,9 @@ dependencies = [ [[package]] name = "wasm-bindgen-futures" -version = "0.4.68" +version = "0.4.71" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "f371d383f2fb139252e0bfac3b81b265689bf45b6874af544ffa4c975ac1ebf8" +checksum = "96492d0d3ffba25305a7dc88720d250b1401d7edca02cc3bcd50633b424673b8" dependencies = [ "js-sys", "wasm-bindgen", @@ -2170,9 +2173,9 @@ dependencies = [ [[package]] name = "wasm-bindgen-macro" -version = "0.2.118" +version = "0.2.121" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "eeff24f84126c0ec2db7a449f0c2ec963c6a49efe0698c4242929da037ca28ed" +checksum = "8e68e6f4afd367a562002c05637acb8578ff2dea1943df76afb9e83d177c8578" dependencies = [ "quote", "wasm-bindgen-macro-support", @@ -2180,9 +2183,9 @@ dependencies = [ [[package]] name = "wasm-bindgen-macro-support" -version = "0.2.118" +version = "0.2.121" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "9d08065faf983b2b80a79fd87d8254c409281cf7de75fc4b773019824196c904" +checksum = "d95a9ec35c64b2a7cb35d3fead40c4238d0940c86d107136999567a4703259f2" dependencies = [ "bumpalo", "proc-macro2", @@ -2193,9 +2196,9 @@ dependencies = [ [[package]] name = "wasm-bindgen-shared" -version = "0.2.118" +version = "0.2.121" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "5fd04d9e306f1907bd13c6361b5c6bfc7b3b3c095ed3f8a9246390f8dbdee129" +checksum = "c4e0100b01e9f0d03189a92b96772a1fb998639d981193d7dbab487302513441" dependencies = [ "unicode-ident", ] @@ -2236,9 +2239,9 @@ dependencies = [ [[package]] name = "web-sys" -version = "0.3.95" +version = "0.3.98" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "4f2dfbb17949fa2088e5d39408c48368947b86f7834484e87b73de55bc14d97d" +checksum = "4b572dff8bcf38bad0fa19729c89bb5748b2b9b1d8be70cf90df697e3a8f32aa" dependencies = [ "js-sys", "wasm-bindgen", @@ -2383,6 +2386,12 @@ dependencies = [ "url", ] +[[package]] +name = "wit-bindgen" +version = "0.46.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "f17a85883d4e6d00e8a97c586de764dabcc06133f7f1d55dce5cdc070ad7fe59" + [[package]] name = "wit-bindgen" version = "0.51.0" @@ -2392,12 +2401,6 @@ dependencies = [ "wit-bindgen-rust-macro", ] -[[package]] -name = "wit-bindgen" -version = "0.57.1" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "1ebf944e87a7c253233ad6766e082e3cd714b5d03812acc24c318f549614536e" - [[package]] name = "wit-bindgen-core" version = "0.51.0" diff --git a/crates/mergify-cli/Cargo.toml b/crates/mergify-cli/Cargo.toml index 6665503b..7d28db2c 100644 --- a/crates/mergify-cli/Cargo.toml +++ b/crates/mergify-cli/Cargo.toml @@ -19,6 +19,7 @@ mergify-ci = { path = "../mergify-ci" } mergify-config = { path = "../mergify-config" } mergify-core = { path = "../mergify-core" } mergify-py-shim = { path = "../mergify-py-shim" } +mergify-queue = { path = "../mergify-queue" } tokio = { version = "1", default-features = false, features = ["macros", "rt", "time"] } [lints] diff --git a/crates/mergify-cli/src/main.rs b/crates/mergify-cli/src/main.rs index 7b69caa8..599c9379 100644 --- a/crates/mergify-cli/src/main.rs +++ b/crates/mergify-cli/src/main.rs @@ -24,6 +24,8 @@ use mergify_config::simulate::PullRequestRef; use mergify_config::simulate::SimulateOptions; use mergify_core::OutputMode; use mergify_core::StdioOutput; +use mergify_queue::pause::PauseOptions; +use mergify_queue::unpause::UnpauseOptions; fn main() -> ExitCode { let argv: Vec = env::args().skip(1).collect(); @@ -63,6 +65,8 @@ enum NativeCommand { ConfigValidate { config_file: Option }, ConfigSimulate(ConfigSimulateOpts), CiScopesSend(CiScopesSendOpts), + QueuePause(QueuePauseOpts), + QueueUnpause(QueueUnpauseOpts), } struct ConfigSimulateOpts { @@ -83,13 +87,26 @@ struct CiScopesSendOpts { file_deprecated: Option, } +struct QueuePauseOpts { + repository: Option, + token: Option, + api_url: Option, + reason: String, + yes_i_am_sure: bool, +} + +struct QueueUnpauseOpts { + repository: Option, + token: Option, + api_url: Option, +} + /// Heuristic: does argv look like the user intended a native -/// subcommand (`config validate`, `config simulate`, `ci -/// scopes-send`)? +/// subcommand? /// /// Used as a fallback when clap rejects the input — if the user -/// clearly meant a native command, surface clap's error rather than -/// silently dispatching to the Python shim. We look for two +/// clearly meant a native command, surface clap's error rather +/// than silently dispatching to the Python shim. We look for two /// *consecutive* tokens forming a `(group, subcommand)` pair so a /// flag value like `--repository config` doesn't accidentally /// classify the invocation as native. @@ -97,7 +114,9 @@ fn looks_native(argv: &[String]) -> bool { argv.windows(2).any(|pair| { matches!( (pair[0].as_str(), pair[1].as_str()), - ("config", "validate" | "simulate") | ("ci", "scopes-send"), + ("config", "validate" | "simulate") + | ("ci", "scopes-send") + | ("queue", "pause" | "unpause"), ) }) } @@ -121,12 +140,12 @@ fn is_help_or_version(err: &clap::Error) -> bool { /// Returns ``None`` when the argv doesn't look like a native /// command — callers fall back to the Python shim, which produces /// the same error messages as before the port started. When the -/// argv obviously targets a native command (contains ``config`` -/// and ``validate``/``simulate``) but clap can't parse it — e.g. -/// the user gave a bad flag or an invalid URL — this function -/// prints clap's formatted error to stderr and exits the process -/// with clap's exit code (2), matching the Python CLI's behavior -/// for argument errors. +/// argv obviously targets a native command (per [`looks_native`]) +/// but clap can't parse it — e.g. the user gave an unknown flag +/// or omitted a required argument — this function prints clap's +/// formatted error to stderr and exits the process with clap's +/// exit code (2), matching the Python CLI's behavior for argument +/// errors. /// /// Argument *values* that are accepted by clap as `String` but /// fail later domain validation (e.g. an `--api-url` that doesn't @@ -200,6 +219,32 @@ fn detect_native(argv: &[String]) -> Option { scopes_file, file_deprecated, })), + Subcommands::Queue(QueueArgs { + repository, + token, + api_url, + command: + QueueSubcommand::Pause(PauseCliArgs { + reason, + yes_i_am_sure, + }), + }) => Some(NativeCommand::QueuePause(QueuePauseOpts { + repository, + token, + api_url, + reason, + yes_i_am_sure, + })), + Subcommands::Queue(QueueArgs { + repository, + token, + api_url, + command: QueueSubcommand::Unpause, + }) => Some(NativeCommand::QueueUnpause(QueueUnpauseOpts { + repository, + token, + api_url, + })), } } @@ -250,6 +295,30 @@ fn run_native(cmd: NativeCommand) -> ExitCode { ) .await } + NativeCommand::QueuePause(opts) => { + mergify_queue::pause::run( + PauseOptions { + repository: opts.repository.as_deref(), + token: opts.token.as_deref(), + api_url: opts.api_url.as_deref(), + reason: &opts.reason, + yes_i_am_sure: opts.yes_i_am_sure, + }, + &mut output, + ) + .await + } + NativeCommand::QueueUnpause(opts) => { + mergify_queue::unpause::run( + UnpauseOptions { + repository: opts.repository.as_deref(), + token: opts.token.as_deref(), + api_url: opts.api_url.as_deref(), + }, + &mut output, + ) + .await + } } }); @@ -277,6 +346,8 @@ enum Subcommands { Config(ConfigArgs), /// Mergify CI-related commands. Ci(CiArgs), + /// Manage the Mergify merge queue. + Queue(QueueArgs), } #[derive(clap::Args)] @@ -372,3 +443,44 @@ struct ScopesSendCliArgs { #[arg(long = "file", short = 'f', hide = true)] file_deprecated: Option, } + +#[derive(clap::Args)] +struct QueueArgs { + /// Mergify or GitHub token. Falls back to ``MERGIFY_TOKEN`` and + /// then ``GITHUB_TOKEN`` env vars. + #[arg(long, short = 't', global = true)] + token: Option, + + /// Mergify API URL. Falls back to ``MERGIFY_API_URL`` env var, + /// then to the default. + #[arg(long = "api-url", short = 'u', global = true)] + api_url: Option, + + /// Repository full name (owner/repo). Falls back to + /// ``GITHUB_REPOSITORY`` env var. + #[arg(long, short = 'r', global = true)] + repository: Option, + + #[command(subcommand)] + command: QueueSubcommand, +} + +#[derive(Subcommand)] +enum QueueSubcommand { + /// Pause the merge queue for the repository. + Pause(PauseCliArgs), + /// Unpause the merge queue for the repository. + Unpause, +} + +#[derive(clap::Args)] +struct PauseCliArgs { + /// Reason for pausing the queue (max 255 characters). + #[arg(long, value_parser = mergify_queue::pause::parse_reason)] + reason: String, + + /// Skip the confirmation prompt. Required in non-interactive + /// sessions. + #[arg(long = "yes-i-am-sure", default_value_t = false)] + yes_i_am_sure: bool, +} diff --git a/crates/mergify-core/src/http.rs b/crates/mergify-core/src/http.rs index ec657ff5..42dcb057 100644 --- a/crates/mergify-core/src/http.rs +++ b/crates/mergify-core/src/http.rs @@ -42,6 +42,15 @@ pub enum ApiFlavor { Mergify, } +/// Outcome of [`Client::delete_if_exists`]. +#[derive(Copy, Clone, Debug, Eq, PartialEq)] +pub enum DeleteOutcome { + /// 2xx: the resource was deleted. + Deleted, + /// 404: the resource didn't exist (or was already gone). + NotFound, +} + /// Retry policy for transient failures. Only 5xx responses and /// connect/timeout errors are retried; 4xx responses are never /// retried — those are caller errors and retrying would hide bugs. @@ -149,6 +158,30 @@ impl Client { .map(drop) } + /// PUT `body` as JSON to `path` and deserialize the JSON + /// response as `T`. + pub async fn put( + &self, + path: &str, + body: &B, + ) -> Result { + let url = self.join(path)?; + let resp = self.execute_request(self.inner.put(url).json(body)).await?; + self.decode_json(resp).await + } + + /// DELETE `path`, returning whether the resource existed. + /// + /// Returns `Ok(DeleteOutcome::Deleted)` on 2xx responses and + /// `Ok(DeleteOutcome::NotFound)` on 404 — useful for idempotent + /// "turn this thing off if it's on" operations where 404 means + /// "nothing to do". 4xx-other and 5xx map to the normal API + /// errors. + pub async fn delete_if_exists(&self, path: &str) -> Result { + let url = self.join(path)?; + self.execute_status(self.inner.delete(url)).await + } + fn join(&self, path: &str) -> Result { // `Url::join` accepts absolute URLs and protocol-relative // paths (`//host/...`), which would let a caller-supplied @@ -164,6 +197,58 @@ impl Client { .map_err(|e| self.api_error(format!("invalid path {path:?}: {e}"))) } + /// Execute 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 { + let mut backoff = self.retry.initial_backoff; + let mut last_message = String::from("HTTP request failed without response"); + + 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, + }; + + match req.send().await { + Ok(resp) => { + let status = resp.status(); + if status.is_success() { + return Ok(DeleteOutcome::Deleted); + } + if status == StatusCode::NOT_FOUND { + return Ok(DeleteOutcome::NotFound); + } + 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))); + } + } + } + Err(self.api_error(last_message)) + } + async fn execute_request( &self, builder: reqwest::RequestBuilder, @@ -202,17 +287,7 @@ impl Client { backoff *= 2; } Err(e) => { - let msg = if e.is_timeout() { - format!( - "{} did not respond in time. The request was aborted — please retry.", - self.service_name() - ) - } else if e.is_connect() { - format!("could not reach {}: {e}", self.service_name()) - } else { - format!("request failed: {e}") - }; - return Err(self.api_error(msg)); + return Err(self.api_error(self.terminal_send_error_message(&e))); } } } @@ -241,6 +316,25 @@ impl Client { ApiFlavor::Mergify => "Mergify", } } + + /// Render a non-retried `reqwest` send error as the message + /// body for `CliError`. Shared between the GET/POST/PUT path + /// (`execute_request`) and the DELETE-style status-only path + /// (`execute_status`) so verbs don't drift on user-facing + /// diagnostics — timeouts and connect failures must read the + /// same regardless of HTTP method. + fn terminal_send_error_message(&self, e: &reqwest::Error) -> String { + if e.is_timeout() { + format!( + "{} did not respond in time. The request was aborted — please retry.", + self.service_name() + ) + } else if e.is_connect() { + format!("could not reach {}: {e}", self.service_name()) + } else { + format!("request failed: {e}") + } + } } fn is_transient(e: &reqwest::Error) -> bool { diff --git a/crates/mergify-core/src/lib.rs b/crates/mergify-core/src/lib.rs index 6b9da36b..8b731465 100644 --- a/crates/mergify-core/src/lib.rs +++ b/crates/mergify-core/src/lib.rs @@ -23,7 +23,7 @@ pub mod output; pub use error::CliError; pub use exit_code::ExitCode; -pub use http::{ApiFlavor, Client as HttpClient, RetryPolicy}; +pub use http::{ApiFlavor, Client as HttpClient, DeleteOutcome, RetryPolicy}; pub use output::{Output, OutputMode, StdioOutput}; /// Compile-time version string taken from the crate package metadata diff --git a/crates/mergify-queue/Cargo.toml b/crates/mergify-queue/Cargo.toml new file mode 100644 index 00000000..bdd0ceca --- /dev/null +++ b/crates/mergify-queue/Cargo.toml @@ -0,0 +1,23 @@ +[package] +name = "mergify-queue" +version = "0.0.0" +edition.workspace = true +rust-version.workspace = true +license.workspace = true +repository.workspace = true +authors.workspace = true +description = "Native implementation of `mergify queue` subcommands." +publish = false + +[dependencies] +mergify-core = { path = "../mergify-core" } +serde = { version = "1.0", features = ["derive"] } +url = "2" + +[dev-dependencies] +serde_json = "1.0" +tokio = { version = "1", default-features = false, features = ["macros", "rt", "time"] } +wiremock = "0.6" + +[lints] +workspace = true diff --git a/crates/mergify-queue/src/lib.rs b/crates/mergify-queue/src/lib.rs new file mode 100644 index 00000000..1de93ff8 --- /dev/null +++ b/crates/mergify-queue/src/lib.rs @@ -0,0 +1,11 @@ +//! Native Rust implementation of the `mergify queue` subcommands. +//! +//! Phase 1.5 ports `pause` and `unpause` — two idempotent API +//! calls that rest on the HTTP client added in 1.2b and the new +//! `put`/`delete_if_exists` methods added alongside this crate. +//! `queue status` and `queue show` stay shimmed until their +//! JSON-output contracts are locked (they carry considerable +//! structured data and want careful schema work). + +pub mod pause; +pub mod unpause; diff --git a/crates/mergify-queue/src/pause.rs b/crates/mergify-queue/src/pause.rs new file mode 100644 index 00000000..02ea2ed9 --- /dev/null +++ b/crates/mergify-queue/src/pause.rs @@ -0,0 +1,287 @@ +//! `mergify queue pause` — pause the merge queue for a repository. +//! +//! PUTs ``{"reason": "..."}`` to +//! ``/v1/repos//merge-queue/pause``. Prints a "Queue paused" +//! confirmation with the reason (and the raw pause timestamp if +//! the API returned one). +//! +//! Confirmation flow: +//! +//! - ``--yes-i-am-sure`` skips the prompt. +//! - Interactive (TTY): asks "Proceed? [y/N]"; anything other than +//! "y"/"yes" aborts with a generic error. +//! - Non-interactive (no TTY, no ``--yes-i-am-sure``): refuses with +//! an ``INVALID_STATE`` error matching Python's behavior. + +use std::io::IsTerminal; +use std::io::Write; + +use mergify_core::ApiFlavor; +use mergify_core::CliError; +use mergify_core::HttpClient; +use mergify_core::Output; +use mergify_core::auth; +use serde::Deserialize; +use serde::Serialize; + +const MAX_REASON_LEN: usize = 255; + +pub struct PauseOptions<'a> { + pub repository: Option<&'a str>, + pub token: Option<&'a str>, + pub api_url: Option<&'a str>, + pub reason: &'a str, + pub yes_i_am_sure: bool, +} + +/// Clap value-parser for the positional `--reason` flag. +/// +/// # Errors +/// +/// Returns a message when `value` exceeds 255 user-visible +/// characters. We count via [`str::chars`] (Unicode scalar values) +/// rather than [`str::len`] (UTF-8 bytes), so non-ASCII reasons +/// such as `"déploiement"` aren't rejected for being below 255 +/// chars but above 255 bytes. +pub fn parse_reason(value: &str) -> Result { + if value.chars().count() > MAX_REASON_LEN { + Err("must be 255 characters or fewer".to_string()) + } else { + Ok(value.to_string()) + } +} + +#[derive(Serialize)] +struct PauseRequest<'a> { + reason: &'a str, +} + +#[derive(Deserialize)] +struct PauseResponse { + // Both fields are optional defensively: the API has historically + // tolerated `reason: null` (the deleted Python `QueuePauseResponse` + // typed it as `str | None`), so the Rust port matches that + // shape rather than aborting deserialization on a missing or + // null value. + #[serde(default)] + reason: Option, + #[serde(default)] + paused_at: Option, +} + +/// Run the `queue pause` command. +pub async fn run(opts: PauseOptions<'_>, output: &mut dyn Output) -> Result<(), CliError> { + // Resolve auth/repo first so the prompt names the *actual* repo + // (including the `GITHUB_REPOSITORY` fallback) and so a missing + // repo or token fails loudly *before* we ask for confirmation. + let repository = auth::resolve_repository(opts.repository)?; + let token = auth::resolve_token(opts.token)?; + let api_url = auth::resolve_api_url(opts.api_url)?; + + confirm( + opts.yes_i_am_sure, + std::io::stdin().is_terminal(), + &repository, + )?; + + output.status(&format!("Pausing merge queue for {repository}…"))?; + + let client = HttpClient::new(api_url, token, ApiFlavor::Mergify)?; + let path = format!("/v1/repos/{repository}/merge-queue/pause"); + let resp: PauseResponse = client + .put( + &path, + &PauseRequest { + reason: opts.reason, + }, + ) + .await?; + + emit_confirmation(output, &resp)?; + Ok(()) +} + +/// Decide whether to proceed with a destructive queue-pause. +/// +/// `is_tty` is taken as a parameter (rather than read from +/// `std::io::stdin()` here) so unit tests can exercise the +/// non-TTY branch deterministically — `cargo test` happens to run +/// without a terminal stdin most of the time, but that's not +/// something we want to depend on, especially when developers run +/// `cargo test` from inside an interactive shell. +fn confirm(skip: bool, is_tty: bool, repository: &str) -> Result<(), CliError> { + if skip { + return Ok(()); + } + if !is_tty { + return Err(CliError::InvalidState( + "refusing to pause without confirmation. Pass --yes-i-am-sure to proceed.".to_string(), + )); + } + // Prompt goes to stderr (matches click's `confirm`/`prompt` + // behavior) so users can pipe stdout cleanly without the + // prompt text mixed in. + let mut err = std::io::stderr().lock(); + write!( + err, + "You are about to pause the merge queue for {repository}. Proceed? [y/N]: ", + ) + .map_err(CliError::from)?; + err.flush().map_err(CliError::from)?; + drop(err); + + let mut line = String::new(); + std::io::stdin() + .read_line(&mut line) + .map_err(CliError::from)?; + match line.trim().to_ascii_lowercase().as_str() { + "y" | "yes" => Ok(()), + _ => Err(CliError::Generic("aborted by user".to_string())), + } +} + +fn emit_confirmation(output: &mut dyn Output, response: &PauseResponse) -> std::io::Result<()> { + let reason = response.reason.clone(); + let paused_at = response.paused_at.clone(); + output.emit(&(), &mut |w: &mut dyn Write| { + match &reason { + Some(r) => write!(w, "Queue paused: \"{r}\"")?, + None => write!(w, "Queue paused")?, + } + if let Some(ts) = &paused_at { + write!(w, " (since {ts})")?; + } + writeln!(w) + }) +} + +#[cfg(test)] +mod tests { + use mergify_core::OutputMode; + use mergify_core::StdioOutput; + use wiremock::Mock; + use wiremock::MockServer; + use wiremock::ResponseTemplate; + use wiremock::matchers::body_json; + use wiremock::matchers::header; + use wiremock::matchers::method; + use wiremock::matchers::path; + + use super::*; + + type SharedBytes = std::sync::Arc>>; + + struct Captured { + output: StdioOutput, + stdout: SharedBytes, + } + + fn make_output() -> Captured { + let stdout: SharedBytes = std::sync::Arc::new(std::sync::Mutex::new(Vec::new())); + let stderr: SharedBytes = std::sync::Arc::new(std::sync::Mutex::new(Vec::new())); + let output = StdioOutput::with_sinks( + OutputMode::Human, + SharedWriter(std::sync::Arc::clone(&stdout)), + SharedWriter(std::sync::Arc::clone(&stderr)), + ); + Captured { output, stdout } + } + + #[test] + fn parse_reason_accepts_short() { + assert_eq!( + parse_reason("deploying hotfix").unwrap(), + "deploying hotfix" + ); + } + + #[test] + fn parse_reason_rejects_over_255() { + let long = "a".repeat(256); + assert!(parse_reason(&long).is_err()); + } + + #[test] + fn parse_reason_counts_chars_not_bytes() { + // 200 user-visible characters but well above 255 *bytes* + // because each `é` is a 2-byte UTF-8 sequence — we keep it. + let multibyte = "é".repeat(200); + assert!(multibyte.len() > MAX_REASON_LEN); + assert!(multibyte.chars().count() <= MAX_REASON_LEN); + assert!(parse_reason(&multibyte).is_ok()); + } + + #[test] + fn confirm_refuses_without_yes_when_non_tty() { + // Pass `is_tty = false` explicitly so the test exercises + // the non-TTY branch without depending on the runtime + // shape of `cargo test`'s stdin (which can be a TTY when + // run from an interactive shell). + let err = confirm(false, false, "owner/repo").unwrap_err(); + assert!( + matches!(err, CliError::InvalidState(_)), + "expected InvalidState, got {err:?}" + ); + assert_eq!(err.exit_code(), mergify_core::ExitCode::InvalidState); + assert!( + err.to_string().contains("--yes-i-am-sure"), + "message should mention the override flag, got: {err}" + ); + } + + #[test] + fn confirm_skips_when_yes_i_am_sure_is_set() { + // `skip = true` must bypass the TTY check — it's the + // contract of `--yes-i-am-sure`, including in CI where + // stdin isn't a terminal. The `is_tty` value passed here + // is irrelevant. + confirm(true, false, "owner/repo").unwrap(); + } + + #[tokio::test] + async fn run_pauses_and_prints_confirmation() { + let server = MockServer::start().await; + Mock::given(method("PUT")) + .and(path("/v1/repos/owner/repo/merge-queue/pause")) + .and(header("Authorization", "Bearer t")) + .and(body_json(serde_json::json!({"reason": "deploy freeze"}))) + .respond_with(ResponseTemplate::new(200).set_body_json(serde_json::json!({ + "reason": "deploy freeze", + "paused_at": "2026-04-23T12:34:56Z", + }))) + .expect(1) + .mount(&server) + .await; + + let mut cap = make_output(); + let api_url = server.uri(); + run( + PauseOptions { + repository: Some("owner/repo"), + token: Some("t"), + api_url: Some(&api_url), + reason: "deploy freeze", + yes_i_am_sure: true, + }, + &mut cap.output, + ) + .await + .unwrap(); + + let stdout = String::from_utf8(cap.stdout.lock().unwrap().clone()).unwrap(); + assert!(stdout.contains("Queue paused"), "got: {stdout:?}"); + assert!(stdout.contains("deploy freeze"), "got: {stdout:?}"); + assert!(stdout.contains("2026-04-23"), "got: {stdout:?}"); + } + + struct SharedWriter(SharedBytes); + impl Write for SharedWriter { + fn write(&mut self, bytes: &[u8]) -> std::io::Result { + self.0.lock().unwrap().extend_from_slice(bytes); + Ok(bytes.len()) + } + fn flush(&mut self) -> std::io::Result<()> { + Ok(()) + } + } +} diff --git a/crates/mergify-queue/src/unpause.rs b/crates/mergify-queue/src/unpause.rs new file mode 100644 index 00000000..c88f8858 --- /dev/null +++ b/crates/mergify-queue/src/unpause.rs @@ -0,0 +1,145 @@ +//! `mergify queue unpause` — resume the merge queue for a +//! repository. +//! +//! DELETEs ``/v1/repos//merge-queue/pause``. When the API +//! responds 404 the command prints "Queue is not currently paused" +//! and exits with `MERGIFY_API_ERROR` — matches Python's behavior. + +use std::io::Write; + +use mergify_core::ApiFlavor; +use mergify_core::CliError; +use mergify_core::DeleteOutcome; +use mergify_core::HttpClient; +use mergify_core::Output; +use mergify_core::auth; + +pub struct UnpauseOptions<'a> { + pub repository: Option<&'a str>, + pub token: Option<&'a str>, + pub api_url: Option<&'a str>, +} + +/// Run the `queue unpause` command. +pub async fn run(opts: UnpauseOptions<'_>, output: &mut dyn Output) -> Result<(), CliError> { + let repository = auth::resolve_repository(opts.repository)?; + let token = auth::resolve_token(opts.token)?; + let api_url = auth::resolve_api_url(opts.api_url)?; + + output.status(&format!("Unpausing merge queue for {repository}…"))?; + + let client = HttpClient::new(api_url, token, ApiFlavor::Mergify)?; + let path = format!("/v1/repos/{repository}/merge-queue/pause"); + + match client.delete_if_exists(&path).await? { + DeleteOutcome::Deleted => { + emit_resumed(output)?; + Ok(()) + } + DeleteOutcome::NotFound => Err(CliError::MergifyApi( + "Queue is not currently paused".to_string(), + )), + } +} + +fn emit_resumed(output: &mut dyn Output) -> std::io::Result<()> { + output.emit(&(), &mut |w: &mut dyn Write| writeln!(w, "Queue resumed.")) +} + +#[cfg(test)] +mod tests { + use mergify_core::OutputMode; + use mergify_core::StdioOutput; + use wiremock::Mock; + use wiremock::MockServer; + use wiremock::ResponseTemplate; + use wiremock::matchers::header; + use wiremock::matchers::method; + use wiremock::matchers::path; + + use super::*; + + type SharedBytes = std::sync::Arc>>; + + struct Captured { + output: StdioOutput, + stdout: SharedBytes, + } + + fn make_output() -> Captured { + let stdout: SharedBytes = std::sync::Arc::new(std::sync::Mutex::new(Vec::new())); + let stderr: SharedBytes = std::sync::Arc::new(std::sync::Mutex::new(Vec::new())); + let output = StdioOutput::with_sinks( + OutputMode::Human, + SharedWriter(std::sync::Arc::clone(&stdout)), + SharedWriter(std::sync::Arc::clone(&stderr)), + ); + Captured { output, stdout } + } + + #[tokio::test] + async fn run_unpauses_on_2xx() { + let server = MockServer::start().await; + Mock::given(method("DELETE")) + .and(path("/v1/repos/owner/repo/merge-queue/pause")) + .and(header("Authorization", "Bearer t")) + .respond_with(ResponseTemplate::new(204)) + .expect(1) + .mount(&server) + .await; + + let mut cap = make_output(); + let api_url = server.uri(); + run( + UnpauseOptions { + repository: Some("owner/repo"), + token: Some("t"), + api_url: Some(&api_url), + }, + &mut cap.output, + ) + .await + .unwrap(); + + let stdout = String::from_utf8(cap.stdout.lock().unwrap().clone()).unwrap(); + assert!(stdout.contains("Queue resumed"), "got: {stdout:?}"); + } + + #[tokio::test] + async fn run_reports_not_currently_paused_on_404() { + let server = MockServer::start().await; + Mock::given(method("DELETE")) + .and(path("/v1/repos/owner/repo/merge-queue/pause")) + .respond_with(ResponseTemplate::new(404)) + .expect(1) + .mount(&server) + .await; + + let mut cap = make_output(); + let api_url = server.uri(); + let err = run( + UnpauseOptions { + repository: Some("owner/repo"), + token: Some("t"), + api_url: Some(&api_url), + }, + &mut cap.output, + ) + .await + .unwrap_err(); + assert!(matches!(err, CliError::MergifyApi(_))); + assert!(err.to_string().contains("not currently paused")); + assert_eq!(err.exit_code(), mergify_core::ExitCode::MergifyApiError); + } + + struct SharedWriter(SharedBytes); + impl Write for SharedWriter { + fn write(&mut self, bytes: &[u8]) -> std::io::Result { + self.0.lock().unwrap().extend_from_slice(bytes); + Ok(bytes.len()) + } + fn flush(&mut self) -> std::io::Result<()> { + Ok(()) + } + } +} diff --git a/func-tests/conftest.py b/func-tests/conftest.py index 7435446f..eca552c8 100644 --- a/func-tests/conftest.py +++ b/func-tests/conftest.py @@ -75,10 +75,29 @@ class CliResult: @pytest.fixture def live_token() -> str: - """Skip the live test if `LIVE_TEST_MERGIFY_TOKEN` isn't set.""" - token = os.environ.get("LIVE_TEST_MERGIFY_TOKEN", "").strip() + """Token for read-only / pull-scoped live endpoints. + + Skips the test if ``LIVE_TEST_MERGIFY_TOKEN_CI`` isn't set. + Use this for everything that doesn't need queue-admin rights + (scopes-send, junit-process, queue status / show, etc.). + """ + token = os.environ.get("LIVE_TEST_MERGIFY_TOKEN_CI", "").strip() + if not token: + pytest.skip("LIVE_TEST_MERGIFY_TOKEN_CI unset") + return token + + +@pytest.fixture +def live_admin_token() -> str: + """Token for queue-admin live endpoints (pause / unpause). + + Skips the test if ``LIVE_TEST_MERGIFY_TOKEN_ADMIN`` isn't + set. Separated from ``live_token`` so the CI-scoped token + can stay narrow. + """ + token = os.environ.get("LIVE_TEST_MERGIFY_TOKEN_ADMIN", "").strip() if not token: - pytest.skip("LIVE_TEST_MERGIFY_TOKEN unset") + pytest.skip("LIVE_TEST_MERGIFY_TOKEN_ADMIN unset") return token diff --git a/func-tests/test_live_smoke.py b/func-tests/test_live_smoke.py index 6fcc7b5c..7a4323c9 100644 --- a/func-tests/test_live_smoke.py +++ b/func-tests/test_live_smoke.py @@ -17,7 +17,9 @@ Driven by `func-tests-live.yaml` on every PR against `mergify-clients-testing/mergify-cli-repo` PR #1. Each test fires when the real API's URL, auth, or wire format diverges from what -the CLI expects. Skipped unless `LIVE_TEST_MERGIFY_TOKEN` is set. +the CLI expects. API-hitting tests skip unless their token +(`LIVE_TEST_MERGIFY_TOKEN_CI` or `_ADMIN`) is set; locally-evaluated +tests run unconditionally. """ from __future__ import annotations @@ -38,6 +40,65 @@ JUNIT_FAIL = pathlib.Path(__file__).parent / "fixtures" / "junit_fail.xml" +def test_queue_pause_unpause_roundtrip( + live_admin_token: str, + cli: typing.Callable[..., typing.Any], +) -> None: + """`PUT` + `DELETE /v1/repos/{owner}/{repo}/merge-queue/pause`. + + Uses the admin-scoped token because pause/unpause hits the + queue-admin endpoint and the CI-scoped token is rejected + (403) by design. + + Runs the pause and unpause commands as a single round-trip so + the test repo's queue is left in the same state we found it + in, even when an assertion fails (the unpause runs from + ``finally``). This means the test is also tolerant of a leaked + paused state from a previous interrupted run — the second pause + just refreshes the reason. + """ + pause = cli( + "queue", + "pause", + "--api-url", + API_URL, + "--token", + live_admin_token, + "--repository", + REPOSITORY, + "--reason", + "func-tests-live-smoke", + "--yes-i-am-sure", + ) + try: + assert pause.returncode == 0, ( + f"queue pause failed\nstdout:\n{pause.stdout}\nstderr:\n{pause.stderr}" + ) + assert "Queue paused" in pause.stdout, ( + f"queue pause did not print confirmation\n" + f"stdout:\n{pause.stdout}\nstderr:\n{pause.stderr}" + ) + finally: + unpause = cli( + "queue", + "unpause", + "--api-url", + API_URL, + "--token", + live_admin_token, + "--repository", + REPOSITORY, + ) + + assert unpause.returncode == 0, ( + f"queue unpause failed\nstdout:\n{unpause.stdout}\nstderr:\n{unpause.stderr}" + ) + assert "Queue resumed" in unpause.stdout, ( + f"queue unpause did not print confirmation\n" + f"stdout:\n{unpause.stdout}\nstderr:\n{unpause.stderr}" + ) + + def test_scopes_send( live_token: str, cli: typing.Callable[..., typing.Any], diff --git a/mergify_cli/queue/api.py b/mergify_cli/queue/api.py index f48d79b4..8c2b3fa1 100644 --- a/mergify_cli/queue/api.py +++ b/mergify_cli/queue/api.py @@ -55,12 +55,6 @@ class QueuePause(typing.TypedDict): paused_at: str -class QueuePauseResponse(typing.TypedDict): - paused: bool - reason: str | None - paused_at: str | None - - class QueueStatusResponse(typing.TypedDict, total=False): batches: typing.Required[list[QueueBatch]] waiting_pull_requests: typing.Required[list[QueuePullRequest]] @@ -137,25 +131,3 @@ async def get_queue_pull( f"/v1/repos/{repository}/merge-queue/pull/{pr_number}", ) return response.json() # type: ignore[no-any-return] - - -async def pause_queue( - client: httpx.AsyncClient, - repository: str, - *, - reason: str, -) -> QueuePauseResponse: - response = await client.put( - f"/v1/repos/{repository}/merge-queue/pause", - json={"reason": reason}, - ) - return response.json() # type: ignore[no-any-return] - - -async def unpause_queue( - client: httpx.AsyncClient, - repository: str, -) -> None: - await client.delete( - f"/v1/repos/{repository}/merge-queue/pause", - ) diff --git a/mergify_cli/queue/cli.py b/mergify_cli/queue/cli.py index 0246f5b8..7acb5216 100644 --- a/mergify_cli/queue/cli.py +++ b/mergify_cli/queue/cli.py @@ -8,7 +8,6 @@ from rich.tree import Tree from mergify_cli import console -from mergify_cli import console_error from mergify_cli import utils from mergify_cli.dym import DYMGroup from mergify_cli.exit_codes import ExitCode @@ -433,93 +432,6 @@ async def status(ctx: click.Context, *, branch: str | None, output_json: bool) - _print_waiting_prs(waiting) -def _validate_pause_reason( - ctx: click.Context, # noqa: ARG001 - param: click.Parameter, # noqa: ARG001 - value: str, -) -> str: - if len(value) > 255: - msg = "must be 255 characters or fewer" - raise click.BadParameter(msg) - return value - - -@queue.command(help="Pause the merge queue for the repository") -@click.option( - "--reason", - required=True, - callback=_validate_pause_reason, - help="Reason for pausing the queue (max 255 chars)", -) -@click.option( - "--yes-i-am-sure", - is_flag=True, - default=False, - help="Skip confirmation prompt (required in non-interactive mode)", -) -@click.pass_context -@utils.run_with_asyncio -async def pause(ctx: click.Context, *, reason: str, yes_i_am_sure: bool) -> None: - repository = ctx.obj["repository"] - - if not yes_i_am_sure: - import os - - if not os.isatty(0): - console_error( - "refusing to pause without confirmation. " - "Pass --yes-i-am-sure to proceed.", - ) - raise SystemExit(ExitCode.INVALID_STATE) - click.confirm( - f"You are about to pause the merge queue for {repository}. Proceed?", - abort=True, - ) - - async with utils.get_mergify_http_client( - ctx.obj["api_url"], - ctx.obj["token"], - ) as client: - data = await queue_api.pause_queue( - client, - repository, - reason=reason, - ) - - pause_text = Text() - pause_text.append("⚠ Queue paused", style="bold yellow") - pause_text.append(f': "{data["reason"]}"') - if data.get("paused_at"): - pause_rel = _relative_time(data["paused_at"]) - if pause_rel: - pause_text.append(f" (since {pause_rel})", style="dim") - console.print(pause_text) - - -@queue.command(help="Unpause the merge queue for the repository") -@click.pass_context -@utils.run_with_asyncio -async def unpause(ctx: click.Context) -> None: - import httpx - - try: - async with utils.get_mergify_http_client( - ctx.obj["api_url"], - ctx.obj["token"], - ) as client: - await queue_api.unpause_queue(client, ctx.obj["repository"]) - except httpx.HTTPStatusError as e: - if e.response.status_code == 404: - console.print( - "Queue is not currently paused", - style="yellow", - ) - raise SystemExit(ExitCode.MERGIFY_API_ERROR) from None - raise - - console.print("[green]Queue unpaused successfully.[/]") - - @queue.command(help="Show detailed state of a pull request in the merge queue") @click.argument("pr_number", type=int) @click.option( diff --git a/mergify_cli/tests/queue/test_cli.py b/mergify_cli/tests/queue/test_cli.py index 97e5ea69..387380aa 100644 --- a/mergify_cli/tests/queue/test_cli.py +++ b/mergify_cli/tests/queue/test_cli.py @@ -8,7 +8,6 @@ from httpx import Response import respx -from mergify_cli.exit_codes import ExitCode from mergify_cli.queue.cli import _relative_time from mergify_cli.queue.cli import _topological_sort from mergify_cli.queue.cli import queue @@ -404,125 +403,3 @@ def test_checks_omitted_when_zero(self) -> None: ) assert result.exit_code == 0, result.output assert "0/0" not in result.output - - -FAKE_PAUSE_RESPONSE = { - "paused": True, - "reason": "Deploying hotfix", - "paused_at": "2025-11-05T14:00:00Z", -} - - -class TestPauseCommand: - def test_pause_with_confirmation(self) -> None: - with respx.mock(base_url="https://api.mergify.com") as mock: - mock.put("/v1/repos/owner/repo/merge-queue/pause").mock( - return_value=Response(200, json=FAKE_PAUSE_RESPONSE), - ) - runner = CliRunner() - with patch("os.isatty", return_value=True): - result = runner.invoke( - queue, - [*BASE_ARGS, "pause", "--reason", "Deploying hotfix"], - input="y\n", - ) - assert result.exit_code == 0, result.output - assert "paused" in result.output.lower() - assert "Deploying hotfix" in result.output - - def test_pause_with_yes_flag(self) -> None: - with respx.mock(base_url="https://api.mergify.com") as mock: - mock.put("/v1/repos/owner/repo/merge-queue/pause").mock( - return_value=Response(200, json=FAKE_PAUSE_RESPONSE), - ) - runner = CliRunner() - result = runner.invoke( - queue, - [ - *BASE_ARGS, - "pause", - "--reason", - "Deploying hotfix", - "--yes-i-am-sure", - ], - ) - assert result.exit_code == 0, result.output - assert "paused" in result.output.lower() - assert "Deploying hotfix" in result.output - - def test_pause_confirmation_denied(self) -> None: - runner = CliRunner() - with patch("os.isatty", return_value=True): - result = runner.invoke( - queue, - [*BASE_ARGS, "pause", "--reason", "test"], - input="n\n", - ) - assert result.exit_code != 0 - - def test_pause_non_tty_without_flag(self) -> None: - runner = CliRunner() - with patch("os.isatty", return_value=False): - result = runner.invoke( - queue, - [*BASE_ARGS, "pause", "--reason", "test"], - ) - assert result.exit_code == ExitCode.INVALID_STATE - assert "--yes-i-am-sure" in result.output - - def test_pause_requires_reason(self) -> None: - runner = CliRunner() - result = runner.invoke( - queue, - [*BASE_ARGS, "pause"], - ) - assert result.exit_code != 0 - - def test_pause_reason_too_long(self) -> None: - runner = CliRunner() - result = runner.invoke( - queue, - [*BASE_ARGS, "pause", "--reason", "x" * 256, "--yes-i-am-sure"], - ) - assert result.exit_code != 0 - assert "255 characters" in result.output - - def test_pause_api_error(self) -> None: - with respx.mock(base_url="https://api.mergify.com") as mock: - mock.put("/v1/repos/owner/repo/merge-queue/pause").mock( - return_value=Response(422, json={"message": "Invalid reason"}), - ) - runner = CliRunner() - result = runner.invoke( - queue, - [ - *BASE_ARGS, - "pause", - "--reason", - "test", - "--yes-i-am-sure", - ], - ) - assert result.exit_code != 0 - - -class TestUnpauseCommand: - def test_unpause(self) -> None: - with respx.mock(base_url="https://api.mergify.com") as mock: - mock.delete("/v1/repos/owner/repo/merge-queue/pause").mock( - return_value=Response(204), - ) - runner = CliRunner() - result = runner.invoke(queue, [*BASE_ARGS, "unpause"]) - assert result.exit_code == 0, result.output - assert "unpaused" in result.output.lower() - - def test_unpause_not_paused(self) -> None: - with respx.mock(base_url="https://api.mergify.com") as mock: - mock.delete("/v1/repos/owner/repo/merge-queue/pause").mock( - return_value=Response(404, json={"message": "Not paused"}), - ) - runner = CliRunner() - result = runner.invoke(queue, [*BASE_ARGS, "unpause"]) - assert result.exit_code == ExitCode.MERGIFY_API_ERROR - assert "not currently paused" in result.output.lower() diff --git a/mergify_cli/tests/queue/test_skill.py b/mergify_cli/tests/queue/test_skill.py index f2096995..1982d735 100644 --- a/mergify_cli/tests/queue/test_skill.py +++ b/mergify_cli/tests/queue/test_skill.py @@ -62,18 +62,25 @@ def test_skill_has_required_sections() -> None: assert section in content, f"Skill is missing required section: {section}" +# Rust-native queue commands. Each port PR appends to this list when +# it deletes the Python copy, so the validation below stays accurate +# without needing to spawn the Rust binary at test time. +NATIVE_QUEUE_COMMANDS: frozenset[str] = frozenset({"pause", "unpause"}) + + def test_skill_references_valid_commands() -> None: - """Check that commands referenced in the skill exist in the CLI.""" + """Every `mergify queue ` reference in the skill must resolve + to either a registered click command (still-shimmed) or a known + Rust-native command. Catches typos and skill drift after a port.""" from mergify_cli.queue.cli import queue content = _get_skill_content() - # Extract `mergify queue ` references referenced = set(re.findall(r"mergify queue ([\w-]+)", content)) - - available = set(queue.commands.keys()) + available = set(queue.commands.keys()) | NATIVE_QUEUE_COMMANDS for cmd in referenced: assert cmd in available, ( - f"Skill references 'mergify queue {cmd}' but it's not a registered command. " + f"Skill references 'mergify queue {cmd}' but it's neither a " + f"registered click command nor a known Rust-native command. " f"Available: {sorted(available)}" )