feat(rust): add mergify_core::auth with gh and git-config fallbacks#1364
Merged
mergify[bot] merged 2 commits intomainfrom May 6, 2026
Conversation
Member
Author
|
This pull request is part of a Mergify stack:
|
This was referenced May 5, 2026
Contributor
Merge ProtectionsYour pull request matches the following merge protections and will not be merged until they are valid. 🟢 ⛓️ Depends-On RequirementsWonderful, this rule succeeded.Requirement based on the presence of
🟢 🤖 Continuous IntegrationWonderful, this rule succeeded.
🟢 👀 Review RequirementsWonderful, this rule succeeded.
🟢 Enforce conventional commitWonderful, this rule succeeded.Make sure that we follow https://www.conventionalcommits.org/en/v1.0.0/
🟢 🔎 ReviewsWonderful, this rule succeeded.
🟢 📕 PR descriptionWonderful, this rule succeeded.
|
jd
force-pushed
the
devs/jd/worktree-rust-port/add-mergify-core-auth-gh-git-config-fallbacks--3ae3542b
branch
from
f5aaca2 to
98a9a67
Compare
jd
force-pushed
the
devs/jd/worktree-rust-port/add-port-review-checklist-http-test-ux-parity--1fd9757e
branch
from
6b1d62e to
d8f08a0
Compare
Member
Author
Revision history
|
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<T>`` + ``#[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) <noreply@anthropic.com> Change-Id: I1fd9757e7f0213b0aeecbb6eb52b59e689cdf05a
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) <noreply@anthropic.com> Change-Id: I3ae3542b75bf407150b2607f50255ec91e9e9587
jd
force-pushed
the
devs/jd/worktree-rust-port/add-mergify-core-auth-gh-git-config-fallbacks--3ae3542b
branch
from
98a9a67 to
458b512
Compare
jd
force-pushed
the
devs/jd/worktree-rust-port/add-port-review-checklist-http-test-ux-parity--1fd9757e
branch
from
d8f08a0 to
2b149f4
Compare
sileht
approved these changes
May 6, 2026
Base automatically changed from
devs/jd/worktree-rust-port/add-port-review-checklist-http-test-ux-parity--1fd9757e
to
main
May 6, 2026 12:23
JulianMaurin
approved these changes
May 6, 2026
Contributor
Merge Queue Status
This pull request spent 16 minutes 25 seconds in the queue, including 13 minutes 20 seconds running CI. Required conditions to merge
|
38 tasks
mergify
Bot
deleted the
devs/jd/worktree-rust-port/add-mergify-core-auth-gh-git-config-fallbacks--3ae3542b
branch
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Centralizes the
--token/--api-url/--repositoryresolversinto 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-exitis treated as "no token from gh", same as Python's
CommandErrorcatch inutils.get_default_token).resolve_repository: explicit →GITHUB_REPOSITORY→git config --get remote.origin.urlparsed viaparse_slug(handles HTTPS and SSH shapes, strips
.gitand trailingslashes).
resolve_api_url: explicit →MERGIFY_API_URL→default
https://api.mergify.com.The
ghandgitfallbacks are best-effort: missing binariesor non-zero exits propagate as "fall through to the next source"
rather than surfacing errors. Mirrors the Python implementation.
mergify-config::simulateandmergify-ci::scopes_sendarerefactored 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 precedencefor 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_ghtest 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) noreply@anthropic.com
Depends-On: #1357