From 2b149f4468776fb5fef6756644f5414c054ac182 Mon Sep 17 00:00:00 2001 From: Julien Danjou Date: Tue, 5 May 2026 13:16:03 +0200 Subject: [PATCH 1/3] docs(port): add port review checklist for HTTP/test/UX parity pitfalls MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The 2026.5.5.x release shipped a ``ci scopes-send`` regression (fixed in #1356): the Rust port deserialized the response body via ``let _: serde_json::Value = client.post(...)`` while the Python original ignored the body, and the Mergify endpoint returns an empty body on success. ``Value`` still requires valid JSON, so the Rust version surfaced ``parse response JSON: error decoding response body`` to every user once it shipped. Test mocks used ``set_body_json(json!({}))`` instead of an empty body, so the bug hid through unit tests too. The fix is structural — split the HTTP client so callers pick ``post`` (parses) vs ``post_no_response`` (drops) up front — but the bug class is broader than this one command. Capturing the lessons in AGENTS.md so the next porter (human or assistant) walks through them before opening a PR. Three categories of pitfall, all observed during the port so far: 1. **HTTP response handling** — match the Python original. If Python ignores the response, Rust must use ``post_no_response`` etc.; if Python parses, optional/nullable fields must be ``Option`` + ``#[serde(default)]``. 2. **Test fixtures** — mirror the real server. ``ResponseTemplate`` bodies must match what the endpoint actually returns; an empty body is not the same as ``json!({})``. 3. **UX parity with click** — char-counted validators use ``chars().count()`` (not ``len()``); confirmation prompts go to stderr; resolve all required state before prompting. Audit of existing/in-flight ports against the checklist: - ``config validate``, ``config simulate`` (main): match - ``ci scopes-send`` (main): regressed, fix in #1356 - ``queue pause`` / ``unpause`` (#1352): match - ``ci git-refs`` / ``queue-info`` (#1353): no HTTP, no risk Co-Authored-By: Claude Opus 4.7 (1M context) Change-Id: I1fd9757e7f0213b0aeecbb6eb52b59e689cdf05a --- AGENTS.md | 44 ++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 44 insertions(+) diff --git a/AGENTS.md b/AGENTS.md index 0c863e7f..2dc581e5 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -95,6 +95,50 @@ fine when the command is being deprecated/dropped from the CLI — the rule is "no two live copies of the same command", not "every Python copy must be replaced". +### Port review checklist + +Walk through this before opening a port PR. Most regressions caught so +far have been a violation of one of these. + +**HTTP calls — match the Python original's response handling exactly.** + +- If the Python original calls a method on the client and **ignores + the response** (e.g. `await client.post(...)` with no `.json()`), + use `post_no_response` / `delete_if_exists` / equivalent on the + Rust side. Do NOT write `let _: serde_json::Value = client.post(...)` + — `Value` still requires a valid JSON body, and a server that + returns an empty body fails at deserialization with `parse response + JSON: error decoding response body`. This regressed `ci scopes-send` + once (fixed in #1356); the cure is structural — pick the right HTTP + method up-front. +- If the Python original parses the response, keep the same shape on + the Rust side. Fields the Python type-hints as `T | None` (or that + the server has historically tolerated as missing/null) should be + `Option` with `#[serde(default)]` so a missing/null field doesn't + abort deserialization. + +**Test fixtures — mirror the real server, not what's convenient.** + +- Wiremock body templates must match what the real endpoint actually + returns. If the real endpoint sends an empty body, use + `ResponseTemplate::new(200)` — never `set_body_json(json!({}))`. The + latter passes deserialization in tests and masks empty-body bugs + that fail in production. +- Add a regression test for the specific failure mode you're + preventing — exit code, error message, missing-field tolerance — + not just the happy path. + +**Strings & UX parity with click.** + +- Char-bounded validators ("must be ≤ N characters") use + `s.chars().count()`, not `s.len()`. The latter counts UTF-8 bytes + and rejects valid non-ASCII input. +- Confirmation prompts go to stderr, matching click's `confirm` / + `prompt`. Stdout stays pipeable. +- Resolve all required state (repository, token, API URL) before + prompting the user. A missing-token error surfacing after the user + types `y` is bad UX and worse for non-interactive invocations. + ## Documentation When adding or changing a CLI feature, always update the documentation: From 458b512938e5c7977ada88aa0669505b6312164e Mon Sep 17 00:00:00 2001 From: Julien Danjou Date: Tue, 5 May 2026 17:15:06 +0200 Subject: [PATCH 2/3] feat(rust): add mergify_core::auth with gh and git-config fallbacks MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Centralizes the `--token` / `--api-url` / `--repository` resolvers into one module so every ported command (config simulate, ci scopes-send, the queue commands that follow in this stack) shares the same fallback behavior: - ``resolve_token``: explicit → ``MERGIFY_TOKEN`` → ``GITHUB_TOKEN`` → ``gh auth token`` (subprocess; missing-binary or non-zero-exit is treated as "no token from gh", same as Python's ``CommandError`` catch in ``utils.get_default_token``). - ``resolve_repository``: explicit → ``GITHUB_REPOSITORY`` → ``git config --get remote.origin.url`` parsed via ``parse_slug`` (handles HTTPS and SSH shapes, strips ``.git`` and trailing slashes). - ``resolve_api_url``: explicit → ``MERGIFY_API_URL`` → default ``https://api.mergify.com``. The ``gh`` and ``git`` fallbacks are best-effort: missing binaries or non-zero exits propagate as "fall through to the next source" rather than surfacing errors. Mirrors the Python implementation. ``mergify-config::simulate`` and ``mergify-ci::scopes_send`` are refactored in this same commit to use the new module — their local copies of the resolvers (which were missing the gh / git fallbacks) are removed. The duplicate tests that lived in those modules are dropped; the env-precedence and slug-parsing coverage now lives once in ``mergify_core::auth::tests``. 13 new tests in ``mergify_core::auth::tests``: env-var precedence for token/api-url/repository, default API URL, error message mentions ``gh client``, slug parsing for HTTPS / SSH / dot-git / trailing-slash / empty-owner / path-without-repo. The subprocess-fallback paths themselves aren't unit-tested (they shell out to real binaries); the ``resolve_token_error_message_mentions_gh`` test exercises the "PATH points to a directory with no gh" case to confirm the fallback degrades cleanly. The queue ports later in the stack consume this module from the start — there's no transition period where they ship with a private auth resolver. Co-Authored-By: Claude Opus 4.7 (1M context) Change-Id: I3ae3542b75bf407150b2607f50255ec91e9e9587 --- Cargo.lock | 1 + crates/mergify-ci/src/scopes_send.rs | 44 +--- crates/mergify-config/src/simulate.rs | 124 +--------- crates/mergify-core/Cargo.toml | 1 + crates/mergify-core/src/auth.rs | 327 ++++++++++++++++++++++++++ crates/mergify-core/src/lib.rs | 1 + 6 files changed, 344 insertions(+), 154 deletions(-) create mode 100644 crates/mergify-core/src/auth.rs diff --git a/Cargo.lock b/Cargo.lock index 2a65a621..eb64f31d 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -937,6 +937,7 @@ dependencies = [ "reqwest", "serde", "serde_json", + "temp-env", "thiserror", "tokio", "url", diff --git a/crates/mergify-ci/src/scopes_send.rs b/crates/mergify-ci/src/scopes_send.rs index 2993a631..6ca7fa31 100644 --- a/crates/mergify-ci/src/scopes_send.rs +++ b/crates/mergify-ci/src/scopes_send.rs @@ -18,25 +18,24 @@ //! returns success — matches Python's "no PR, nothing to send" //! behavior. //! -//! Auth + API URL resolution follows the same fallback order as -//! ``config simulate``: explicit flag → ``MERGIFY_TOKEN`` / -//! ``MERGIFY_API_URL`` env var → default (or error). +//! Auth + API URL resolution goes through `mergify_core::auth`, +//! which adds a `gh auth token` fallback (matches Python's +//! `utils.get_default_token`) and a `git config remote.origin.url` +//! fallback for the repository slug (matches +//! `utils.get_default_repository`). -use std::env; use std::path::Path; 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; -use url::Url; use crate::detector; -const DEFAULT_API_URL: &str = "https://api.mergify.com"; - pub struct ScopesSendOptions<'a> { pub repository: Option<&'a str>, pub pull_request: Option, @@ -56,8 +55,8 @@ pub async fn run(opts: ScopesSendOptions<'_>, output: &mut dyn Output) -> Result }; let repository = resolve_repository(opts.repository)?; - let token = resolve_token(opts.token)?; - let api_url = resolve_api_url(opts.api_url)?; + let token = auth::resolve_token(opts.token)?; + let api_url = auth::resolve_api_url(opts.api_url)?; // Whenever the deprecated `--file` flag is supplied, surface // the deprecation warning — even when `--scopes-json` is also @@ -111,33 +110,6 @@ fn resolve_pull_request(explicit: Option) -> Result, CliError> detector::get_github_pull_request_number() } -fn resolve_token(explicit: Option<&str>) -> Result { - if let Some(value) = explicit.filter(|s| !s.is_empty()) { - return Ok(value.to_string()); - } - for env_name in ["MERGIFY_TOKEN", "GITHUB_TOKEN"] { - if let Ok(value) = env::var(env_name) { - if !value.is_empty() { - return Ok(value); - } - } - } - Err(CliError::Configuration( - "please set the 'MERGIFY_TOKEN' or 'GITHUB_TOKEN' environment variable, \ - or pass --token explicitly" - .to_string(), - )) -} - -fn resolve_api_url(explicit: Option<&str>) -> Result { - let raw = explicit - .map(str::to_string) - .or_else(|| env::var("MERGIFY_API_URL").ok()) - .filter(|s| !s.is_empty()) - .unwrap_or_else(|| DEFAULT_API_URL.to_string()); - Url::parse(&raw).map_err(|e| CliError::Configuration(format!("invalid --api-url {raw:?}: {e}"))) -} - #[derive(Deserialize)] struct DetectedScopesFile { scopes: Vec, diff --git a/crates/mergify-config/src/simulate.rs b/crates/mergify-config/src/simulate.rs index 3ea8d14d..511d7d81 100644 --- a/crates/mergify-config/src/simulate.rs +++ b/crates/mergify-config/src/simulate.rs @@ -12,13 +12,10 @@ //! 5. Prints the simulator's title + summary. //! //! Token / api-url / config-file all follow the same resolution -//! order as the Python CLI: explicit flag, then env var, then -//! default. Missing token falls back from ``MERGIFY_TOKEN`` to -//! ``GITHUB_TOKEN``. The ``gh auth token`` subprocess fallback from -//! Python isn't ported yet — if neither env var is set the command -//! errors out. +//! order as the Python CLI (`mergify_core::auth`): explicit flag, +//! then env var, then `gh auth token` for the bearer, then the +//! default API URL. -use std::env; use std::io::Write; use std::path::Path; @@ -26,14 +23,12 @@ 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; -use url::Url; use crate::paths::resolve_config_path; -const DEFAULT_API_URL: &str = "https://api.mergify.com"; - /// Deserialized shape of the `(owner/repo, number)` pair parsed from /// a pull-request URL. #[derive(Clone, Debug, Eq, PartialEq)] @@ -75,39 +70,6 @@ pub fn parse_pr_url(url: &str) -> Result { }) } -/// Resolve the Mergify API bearer token. -/// -/// Precedence: explicit `--token`, then `MERGIFY_TOKEN`, then -/// `GITHUB_TOKEN`. Errors out when none of those are set. -fn resolve_token(explicit: Option<&str>) -> Result { - if let Some(token) = explicit.filter(|t| !t.is_empty()) { - return Ok(token.to_string()); - } - for env_name in ["MERGIFY_TOKEN", "GITHUB_TOKEN"] { - if let Ok(value) = env::var(env_name) { - if !value.is_empty() { - return Ok(value); - } - } - } - Err(CliError::Configuration( - "please set the 'MERGIFY_TOKEN' or 'GITHUB_TOKEN' environment variable, \ - or pass --token explicitly" - .to_string(), - )) -} - -/// Resolve the Mergify API base URL. Falls back to `MERGIFY_API_URL` -/// env var, then to the default `https://api.mergify.com`. -fn resolve_api_url(explicit: Option<&str>) -> Result { - let raw = explicit - .map(str::to_string) - .or_else(|| env::var("MERGIFY_API_URL").ok()) - .filter(|s| !s.is_empty()) - .unwrap_or_else(|| DEFAULT_API_URL.to_string()); - Url::parse(&raw).map_err(|e| CliError::Configuration(format!("invalid --api-url {raw:?}: {e}"))) -} - #[derive(Serialize)] struct SimulatorRequest<'a> { mergify_yml: &'a str, @@ -133,8 +95,8 @@ pub async fn run(opts: SimulateOptions<'_>, output: &mut dyn Output) -> Result<( CliError::Configuration(format!("cannot read {}: {e}", config_path.display())) })?; - let token = resolve_token(opts.token)?; - let api_url = resolve_api_url(opts.api_url)?; + let token = auth::resolve_token(opts.token)?; + let api_url = auth::resolve_api_url(opts.api_url)?; output.status(&format!("Simulating against {api_url}…"))?; @@ -218,80 +180,6 @@ mod tests { assert!(parse_pr_url("https://github.com//repo/pull/42").is_err()); } - #[test] - fn resolve_token_prefers_explicit_over_env() { - temp_env::with_vars( - [ - ("MERGIFY_TOKEN", Some("from-env")), - ("GITHUB_TOKEN", Some("github-env")), - ], - || { - assert_eq!(resolve_token(Some("from-cli")).unwrap(), "from-cli"); - }, - ); - } - - #[test] - fn resolve_token_falls_back_to_mergify_env() { - temp_env::with_vars( - [ - ("MERGIFY_TOKEN", Some("mergify-env")), - ("GITHUB_TOKEN", Some("github-env")), - ], - || { - assert_eq!(resolve_token(None).unwrap(), "mergify-env"); - }, - ); - } - - #[test] - fn resolve_token_falls_back_to_github_env() { - temp_env::with_vars( - [ - ("MERGIFY_TOKEN", None::<&str>), - ("GITHUB_TOKEN", Some("github-env")), - ], - || { - assert_eq!(resolve_token(None).unwrap(), "github-env"); - }, - ); - } - - #[test] - fn resolve_token_errors_when_no_source_available() { - temp_env::with_vars( - [ - ("MERGIFY_TOKEN", None::<&str>), - ("GITHUB_TOKEN", None::<&str>), - ], - || { - let err = resolve_token(None).unwrap_err(); - assert!(matches!(err, CliError::Configuration(_))); - assert!(err.to_string().contains("MERGIFY_TOKEN")); - }, - ); - } - - #[test] - fn resolve_api_url_uses_default_when_unset() { - temp_env::with_vars([("MERGIFY_API_URL", None::<&str>)], || { - assert_eq!( - resolve_api_url(None).unwrap().as_str(), - "https://api.mergify.com/", - ); - }); - } - - #[test] - fn resolve_api_url_prefers_explicit() { - temp_env::with_vars([("MERGIFY_API_URL", Some("https://from.env"))], || { - assert_eq!( - resolve_api_url(Some("https://from.cli")).unwrap().as_str(), - "https://from.cli/", - ); - }); - } - #[tokio::test] async fn run_posts_config_and_prints_simulator_result() { let server = MockServer::start().await; diff --git a/crates/mergify-core/Cargo.toml b/crates/mergify-core/Cargo.toml index b57416eb..be6914cc 100644 --- a/crates/mergify-core/Cargo.toml +++ b/crates/mergify-core/Cargo.toml @@ -18,6 +18,7 @@ tokio = { version = "1", default-features = false, features = ["macros", "rt", " url = "2" [dev-dependencies] +temp-env = "0.3" tokio = { version = "1", default-features = false, features = ["macros", "rt", "rt-multi-thread", "time"] } wiremock = "0.6" diff --git a/crates/mergify-core/src/auth.rs b/crates/mergify-core/src/auth.rs new file mode 100644 index 00000000..1b822a43 --- /dev/null +++ b/crates/mergify-core/src/auth.rs @@ -0,0 +1,327 @@ +//! Resolve `--token`, `--api-url`, and `--repository` with the +//! same fallback order the Python CLI used. +//! +//! Token: `--token` flag → `MERGIFY_TOKEN` env → `GITHUB_TOKEN` +//! env → `gh auth token` (the GitHub CLI). Mirrors Python's +//! `utils.get_default_token`. +//! +//! Repository: `--repository` flag → `GITHUB_REPOSITORY` env → +//! `git config --get remote.origin.url` parsed into `/`. +//! Mirrors Python's `utils.get_default_repository` + `utils.get_slug`. +//! +//! API URL: `--api-url` flag → `MERGIFY_API_URL` env → default +//! `https://api.mergify.com`. +//! +//! Each ported command resolves these once before doing any +//! network or interactive work. The Rust copies that previously +//! lived in `mergify-config::simulate`, `mergify-ci::scopes_send`, +//! and `mergify-queue::auth` were missing the `gh auth token` and +//! `git config` fallbacks — that's why this module exists. + +use std::env; +use std::process::Command; + +use url::Url; + +use crate::CliError; + +const DEFAULT_API_URL: &str = "https://api.mergify.com"; + +/// Resolve the Mergify API bearer token. +/// +/// Precedence: explicit `--token`, then `MERGIFY_TOKEN`, then +/// `GITHUB_TOKEN`, then the output of `gh auth token`. Errors when +/// none of those produce a non-empty value. +pub fn resolve_token(explicit: Option<&str>) -> Result { + if let Some(value) = explicit.filter(|s| !s.is_empty()) { + return Ok(value.to_string()); + } + for env_name in ["MERGIFY_TOKEN", "GITHUB_TOKEN"] { + if let Ok(value) = env::var(env_name) { + if !value.is_empty() { + return Ok(value); + } + } + } + if let Ok(token) = gh_auth_token() { + if !token.is_empty() { + return Ok(token); + } + } + Err(CliError::Configuration( + "please set the 'MERGIFY_TOKEN' or 'GITHUB_TOKEN' environment variable, \ + or make sure that the gh client is installed and you are authenticated" + .to_string(), + )) +} + +/// Resolve the Mergify API base URL. Falls back to the +/// `MERGIFY_API_URL` env var, then the default +/// `https://api.mergify.com`. +pub fn resolve_api_url(explicit: Option<&str>) -> Result { + let raw = explicit + .map(str::to_string) + .or_else(|| env::var("MERGIFY_API_URL").ok()) + .filter(|s| !s.is_empty()) + .unwrap_or_else(|| DEFAULT_API_URL.to_string()); + Url::parse(&raw).map_err(|e| CliError::Configuration(format!("invalid --api-url {raw:?}: {e}"))) +} + +/// Resolve the repository (`/`). +/// +/// Precedence: explicit `--repository`, then `GITHUB_REPOSITORY` +/// env, then `git config --get remote.origin.url` parsed via +/// [`parse_slug`]. Errors when none of those yield a slug. +pub fn resolve_repository(explicit: Option<&str>) -> Result { + if let Some(value) = explicit.filter(|s| !s.is_empty()) { + return Ok(value.to_string()); + } + if let Ok(value) = env::var("GITHUB_REPOSITORY") { + if !value.is_empty() { + return Ok(value); + } + } + if let Some(remote) = git_remote_origin_url() { + if let Some(slug) = parse_slug(&remote) { + return Ok(slug); + } + } + Err(CliError::Configuration( + "--repository not provided, GITHUB_REPOSITORY env var is unset, and \ + the local git config has no usable `remote.origin.url`" + .to_string(), + )) +} + +/// Run `gh auth token` and return stdout (trimmed). Returns an +/// `Err` when `gh` is missing or the command fails, which the +/// caller treats as "no token from gh". +fn gh_auth_token() -> Result { + let output = Command::new("gh").args(["auth", "token"]).output()?; + if !output.status.success() { + return Err(std::io::Error::other("`gh auth token` exited non-zero")); + } + let token = String::from_utf8(output.stdout) + .map_err(|e| std::io::Error::other(format!("`gh auth token` non-UTF-8 output: {e}")))? + .trim() + .to_string(); + Ok(token) +} + +/// Run `git config --get remote.origin.url` in the current +/// directory and return stdout (trimmed). Returns `None` when git +/// isn't available, the working tree isn't a git repo, or the +/// remote isn't configured. +fn git_remote_origin_url() -> Option { + let output = Command::new("git") + .args(["config", "--get", "remote.origin.url"]) + .output() + .ok()?; + if !output.status.success() { + return None; + } + let value = String::from_utf8(output.stdout).ok()?.trim().to_string(); + (!value.is_empty()).then_some(value) +} + +/// Parse a git remote URL into `/`. +/// +/// Handles both HTTPS (`https://github.com/owner/repo.git`) and +/// SSH (`git@github.com:owner/repo.git`) shapes; `.git` suffix and +/// trailing slashes are stripped. Returns `None` when the URL +/// doesn't decompose into at least two path segments. +fn parse_slug(url: &str) -> Option { + let url = url.trim(); + + // SSH form: `git@host:owner/repo[.git]` — no scheme, the + // delimiter between user@host and path is `:`. We detect this + // by checking for `@…:` before the first `/`. + let path = if let Some(scheme_end) = url.find("://") { + let after_scheme = &url[scheme_end + 3..]; + after_scheme.split_once('/')?.1.to_string() + } else if let Some(colon) = url.find(':') { + url[colon + 1..].to_string() + } else { + return None; + }; + + let path = path.trim_end_matches('/').trim_start_matches('/'); + let (owner, rest) = path.split_once('/')?; + let repo = rest + .trim_end_matches('/') + .strip_suffix(".git") + .unwrap_or(rest); + let repo = repo.trim_end_matches('/'); + if owner.is_empty() || repo.is_empty() { + return None; + } + Some(format!("{owner}/{repo}")) +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn resolve_token_prefers_explicit_over_env() { + temp_env::with_vars( + [ + ("MERGIFY_TOKEN", Some("env-mergify")), + ("GITHUB_TOKEN", Some("env-github")), + ], + || { + assert_eq!( + resolve_token(Some("explicit-token")).unwrap(), + "explicit-token", + ); + }, + ); + } + + #[test] + fn resolve_token_falls_back_to_mergify_env() { + temp_env::with_vars( + [ + ("MERGIFY_TOKEN", Some("env-mergify")), + ("GITHUB_TOKEN", Some("env-github")), + ], + || { + assert_eq!(resolve_token(None).unwrap(), "env-mergify"); + }, + ); + } + + #[test] + fn resolve_token_falls_back_to_github_env_when_mergify_unset() { + temp_env::with_vars( + [ + ("MERGIFY_TOKEN", None), + ("GITHUB_TOKEN", Some("env-github")), + ], + || { + assert_eq!(resolve_token(None).unwrap(), "env-github"); + }, + ); + } + + #[test] + fn resolve_token_error_message_mentions_gh() { + // When env vars are unset and `gh auth token` is unavailable + // (or fails), the user-facing error must mention the gh + // fallback so the user knows there's a third option. + // Forcing PATH to a directory with no `gh` keeps the test + // hermetic on machines that do have the GitHub CLI installed. + temp_env::with_vars( + [ + ("MERGIFY_TOKEN", None), + ("GITHUB_TOKEN", None), + ("PATH", Some("/nonexistent-directory-for-test")), + ], + || { + let err = resolve_token(None).unwrap_err(); + let msg = err.to_string(); + assert!(msg.contains("MERGIFY_TOKEN"), "got {msg:?}"); + assert!(msg.contains("gh client"), "got {msg:?}"); + }, + ); + } + + #[test] + fn resolve_api_url_default() { + temp_env::with_var("MERGIFY_API_URL", None::<&str>, || { + let url = resolve_api_url(None).unwrap(); + assert_eq!(url.as_str(), "https://api.mergify.com/"); + }); + } + + #[test] + fn resolve_api_url_prefers_explicit() { + temp_env::with_var("MERGIFY_API_URL", Some("https://from-env.example/"), || { + let url = resolve_api_url(Some("https://explicit.example/")).unwrap(); + assert_eq!(url.as_str(), "https://explicit.example/"); + }); + } + + #[test] + fn resolve_api_url_uses_env_var_when_explicit_empty() { + temp_env::with_var("MERGIFY_API_URL", Some("https://from-env.example/"), || { + let url = resolve_api_url(None).unwrap(); + assert_eq!(url.as_str(), "https://from-env.example/"); + }); + } + + #[test] + fn resolve_api_url_rejects_garbage() { + temp_env::with_var("MERGIFY_API_URL", None::<&str>, || { + let err = resolve_api_url(Some("not a url")).unwrap_err(); + assert!(err.to_string().contains("invalid --api-url")); + }); + } + + #[test] + fn resolve_repository_prefers_explicit() { + temp_env::with_var("GITHUB_REPOSITORY", Some("owner-from-env/repo"), || { + assert_eq!( + resolve_repository(Some("explicit/repo")).unwrap(), + "explicit/repo", + ); + }); + } + + #[test] + fn resolve_repository_falls_back_to_env() { + temp_env::with_var("GITHUB_REPOSITORY", Some("owner/repo"), || { + assert_eq!(resolve_repository(None).unwrap(), "owner/repo"); + }); + } + + #[test] + fn parse_slug_https_with_dot_git() { + assert_eq!( + parse_slug("https://github.com/owner/repo.git").as_deref(), + Some("owner/repo"), + ); + } + + #[test] + fn parse_slug_https_without_dot_git() { + assert_eq!( + parse_slug("https://github.com/owner/repo").as_deref(), + Some("owner/repo"), + ); + } + + #[test] + fn parse_slug_https_with_trailing_slash() { + assert_eq!( + parse_slug("https://github.com/owner/repo/").as_deref(), + Some("owner/repo"), + ); + } + + #[test] + fn parse_slug_ssh_form() { + assert_eq!( + parse_slug("git@github.com:owner/repo.git").as_deref(), + Some("owner/repo"), + ); + } + + #[test] + fn parse_slug_ssh_without_dot_git() { + assert_eq!( + parse_slug("git@github.com:owner/repo").as_deref(), + Some("owner/repo"), + ); + } + + #[test] + fn parse_slug_rejects_empty_owner() { + assert!(parse_slug("https://github.com//repo.git").is_none()); + } + + #[test] + fn parse_slug_rejects_path_without_repo() { + assert!(parse_slug("https://github.com/owner").is_none()); + } +} diff --git a/crates/mergify-core/src/lib.rs b/crates/mergify-core/src/lib.rs index 9c27f27a..6b9da36b 100644 --- a/crates/mergify-core/src/lib.rs +++ b/crates/mergify-core/src/lib.rs @@ -15,6 +15,7 @@ //! Git operations, interactive prompts, and config loading arrive //! in subsequent sub-phases. +pub mod auth; pub mod error; pub mod exit_code; pub mod http; From 41c21095d1649ea7878f241c52f53f054c4a4869 Mon Sep 17 00:00:00 2001 From: "renovate[bot]" <29139614+renovate[bot]@users.noreply.github.com> Date: Wed, 6 May 2026 12:03:31 +0000 Subject: [PATCH 3/3] chore(deps): update mergifyio/gha-mergify-ci action to v17 --- .github/workflows/ci.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/ci.yaml b/.github/workflows/ci.yaml index b56f595b..bdc7acd6 100644 --- a/.github/workflows/ci.yaml +++ b/.github/workflows/ci.yaml @@ -161,7 +161,7 @@ jobs: runs-on: ubuntu-latest steps: - name: Verify all jobs succeeded - uses: Mergifyio/gha-mergify-ci@v16 + uses: Mergifyio/gha-mergify-ci@v17 with: action: wait-jobs jobs: ${{ toJSON(needs) }}