Skip to content

fix(rust): restore gh and git-config fallbacks in token/repo resolution#1361

Closed
jd wants to merge 4 commits intodevs/jd/worktree-rust-port/port-queue-status-native-rust-phase-1-7--8cebcd32from
devs/jd/worktree-rust-port/restore-gh-git-config-fallbacks-token-repo--6ea0e60f
Closed

fix(rust): restore gh and git-config fallbacks in token/repo resolution#1361
jd wants to merge 4 commits intodevs/jd/worktree-rust-port/port-queue-status-native-rust-phase-1-7--8cebcd32from
devs/jd/worktree-rust-port/restore-gh-git-config-fallbacks-token-repo--6ea0e60f

Conversation

@jd
Copy link
Copy Markdown
Member

@jd jd commented May 5, 2026

Regression introduced by the port: every Rust-native command that
calls Mergify (config simulate, ci scopes-send, queue pause/unpause/
status) errored with please set the 'MERGIFY_TOKEN' or 'GITHUB_TOKEN'… when neither env var was set, even though the
user was authenticated via the GitHub CLI. The Python equivalent
fell back to gh auth token; the Rust ports did not. Same
story for --repository: Python parsed
git config --get remote.origin.url when the flag was absent
and GITHUB_REPOSITORY was unset; the Rust ports just errored.

The three Rust crates each had their own copy of the resolvers,
all with the same regression. Factor a single
mergify_core::auth module that:

  • resolve_token(explicit): explicit → MERGIFY_TOKEN
    GITHUB_TOKENgh auth token (subprocess, swallows
    missing-binary or non-zero-exit errors).
  • resolve_repository(explicit): explicit → GITHUB_REPOSITORY
    git config --get remote.origin.url parsed via a
    parse_slug helper that handles HTTPS
    (https://host/owner/repo.git) and SSH
    (git@host:owner/repo.git) shapes, strips .git and
    trailing slashes.
  • resolve_api_url(explicit): explicit → MERGIFY_API_URL
    default https://api.mergify.com.

Subprocess fallbacks are best-effort: if gh or git aren't
on $PATH, or the command fails, we fall through to the next
source rather than surfacing the subprocess error. Matches the
Python behavior of catching CommandError.

Three call sites refactored to drop their own resolvers and use
mergify_core::auth:

  • mergify-config::simulate: removed local resolve_token /
    resolve_api_url and DEFAULT_API_URL.
  • mergify-ci::scopes_send: removed local resolve_repository
    / resolve_token / resolve_api_url and DEFAULT_API_URL.
  • mergify-queue: deleted mergify_queue::auth entirely; the
    three commands import mergify_core::auth directly.

The redundant tests that lived in simulate.rs and
scopes_send.rs are deleted — their 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 test
resolve_token_error_message_mentions_gh exercises the
"PATH points to a directory with no gh" case to confirm the
fallback degrades cleanly.

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

Depends-On: #1359

@jd
Copy link
Copy Markdown
Member Author

jd commented May 5, 2026

This pull request is part of a Mergify stack:

# Pull Request Link
1 chore(port): drop PORT_STATUS.toml inventory in favor of port-and-delete #1351
2 docs(port): add port review checklist for HTTP/test/UX parity pitfalls #1357
3 feat(rust): port queue pause and unpause to native Rust (Phase 1.5) #1352
4 feat(rust): port ci git-refs and ci queue-info to native Rust (Phase 1.6) #1353
5 feat(rust): port queue status to native Rust (Phase 1.7) #1359
6 fix(rust): restore gh and git-config fallbacks in token/repo resolution #1361 👈

@mergify
Copy link
Copy Markdown
Contributor

mergify Bot commented May 5, 2026

Merge Protections

Your pull request matches the following merge protections and will not be merged until they are valid.

🔴 ⛓️ Depends-On Requirements

Waiting for:

This rule is failing.

Requirement based on the presence of Depends-On in the body of the pull request

🔴 👀 Review Requirements

Waiting for:

  • #approved-reviews-by>=2
This rule is failing.
  • any of:
    • #approved-reviews-by>=2
    • author = dependabot[bot]
    • author = mergify-ci-bot
    • author = renovate[bot]

🔴 🔎 Reviews

Waiting for:

  • #review-requested = 0
This rule is failing.
  • #review-requested = 0
  • #changes-requested-reviews-by = 0
  • #review-threads-unresolved = 0

🟢 🤖 Continuous Integration

Wonderful, this rule succeeded.
  • all of:
    • check-success=ci-gate

🟢 Enforce conventional commit

Wonderful, this rule succeeded.

Make sure that we follow https://www.conventionalcommits.org/en/v1.0.0/

  • title ~= ^(fix|feat|docs|style|refactor|perf|test|build|ci|chore|revert|ui)(?:\(.+\))?:

🟢 📕 PR description

Wonderful, this rule succeeded.
  • body ~= (?ms:.{48,})

jd and others added 4 commits May 5, 2026 15:21
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/<repo>/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<B, T>(path, body) -> Result<T, CliError>`` — mirror of
  ``post``, different verb.
- ``delete_if_exists(path) -> Result<DeleteOutcome, CliError>`` —
  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.

The ``resolve_token`` / ``resolve_api_url`` / ``resolve_repository``
trio is duplicated into ``mergify_queue::auth``. That's now three
copies in the tree (config simulate, ci scopes-send, queue). A
follow-up PR factors them into ``mergify_core::auth`` as soon as
a fourth command needs them.

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) <noreply@anthropic.com>
Change-Id: Idba6fa38caf403fd5f4184cda462b5f7c1eb3ebf
…1.6)

Adds `github_event` (GitHub Actions event payload deserialization) and
`queue_metadata` (MQ YAML fenced-block extraction) shared modules to the
mergify-ci crate, and ports two commands on top of them:

- `ci queue-info` — prints MQ batch metadata as pretty JSON; errors
  INVALID_STATE (exit 7) outside an MQ context. Appends to
  `$GITHUB_OUTPUT` with the same `ghadelimiter_<uuid>` heredoc the
  Python version uses.

- `ci git-refs` — detects base/head refs from Buildkite env, GitHub
  event payload, `refs/notes/mergify/<branch>` git notes, MQ PR body,
  or falls back to `HEAD^..HEAD`. Supports `text`/`shell`/`json`
  output formats, writes `base`/`head` to `$GITHUB_OUTPUT`, and calls
  `buildkite-agent meta-data set` when `BUILDKITE=true`.

The notes reader is injected as a trait-object callback so unit tests
can exercise the note-driven detection path without touching a real
git repository; the production path shells out via
`real_notes_reader`.

The Python implementations of `ci git-refs` and `ci queue-info` are
removed in the same PR — the Rust binary now owns both commands
end-to-end. The looks-native heuristic in `main.rs` learns the two
new subcommand names so argv errors surface as clap exit 2 instead
of silently falling through to the Python shim.

Adds `serde_yaml_ng` (YAML parser) and `uuid` (ghadelimiter) deps to
the mergify-ci crate. Binary grows ~100KB to 8.6MB.

19 new Rust tests (8 event/metadata, 3 queue-info, 12 git-refs + 2
format round-trips merged in). Full workspace: 79 tests green,
compat harness 4/4.

Change-Id: I8d3f96e6cb4eb51e6cd195951b3e622cee7efdd4
The Rust binary now serves ``mergify queue status`` natively. The
Python implementation (``mergify_cli/queue/cli.py:status`` plus the
batch/scope/topology helpers it depended on) is removed in the
same PR — the port-and-delete rule we adopted in #1322 keeps a
single live copy of every command.

## What ports

``mergify queue status [-r REPO] [-t TOKEN] [-u URL] [-b BRANCH]
[--json]``:

1. Resolves repository / token / API URL via the shared
   ``mergify_queue::auth`` resolver introduced in #1352.
2. Fetches ``GET /v1/repos/<repo>/merge-queue/status``, optionally
   with ``?branch=<branch>`` (URL-encoded via
   ``url::form_urlencoded::byte_serialize``).
3. With ``--json``: pretty-prints the raw response. The schema is
   Mergify's API contract, not this CLI's, so we deserialize into
   ``serde_json::Value`` and emit verbatim — unknown fields and
   future schema additions survive the round trip.
4. Without ``--json``: deserializes into a typed ``StatusView``
   that uses ``#[serde(default)] Option<…>`` for every field the
   Mergify API has historically treated as optional/nullable
   (matches the port checklist from #1357), then renders a
   header, an optional pause indicator, the batch tree (grouped
   by scope when there is more than one), and the waiting-PR
   list. Status icons (``● ◑ ◌ ✓ ✗ ◎ ⏳ ↻ ⏰ ❄``) and relative
   times (``5m ago`` / ``~1h``) match the Python implementation.

## Rendering departures

Python used Rich's ``Tree`` for batches. The Rust port emits
flat indented text instead — same data, simpler rendering. Both
are line-oriented and round-trip cleanly through pipes; the
fancy box-drawing was visual sugar that didn't survive piping
anyway.

## Tests (24 new, in mergify-queue::status)

- ``build_path`` covers no-branch, branch, and URL-encoding of a
  branch name with slashes + spaces (e.g. ``feature/foo bar``
  becomes ``feature%2Ffoo+bar`` in the query).
- ``relative_time`` covers seconds / minutes / hours / days,
  future prefix, and graceful empty-string return on a malformed
  timestamp (matches Python's "degrade rather than fail").
- ``topological_sort`` covers parents-before-children ordering
  and tolerance of ``parent_ids`` that reference missing batches.
- ``group_by_scope`` covers the ``[]`` → ``"default"`` fallback
  and multi-scope batches appearing under each scope they claim.
- ``status_icon`` covers known + unknown codes.
- End-to-end wiremock tests: empty queue, paused queue, batches +
  waiting PRs, multi-scope grouping, ``?branch=…`` query
  threading, JSON-passthrough preserving an ``extra_field``,
  and tolerance of a response that omits all optional fields.

## Wiring

- ``crates/mergify-queue/Cargo.toml``: adds ``chrono`` (relative
  time math), ``indexmap`` (scope groups in insertion order),
  promotes ``serde_json`` from dev to runtime (used for the
  ``serde_json::Value`` passthrough).
- ``crates/mergify-cli/src/main.rs``: registers the ``status``
  subcommand under ``QueueSubcommand``, threads the
  ``--branch``/``--json`` flags, dispatches to
  ``mergify_queue::status::run``. Adds ``status`` to
  ``looks_native``.
- ``mergify_cli/queue/api.py``: removes ``QueueStatusResponse``,
  ``QueueBatch``, ``QueuePause``, ``QueueChecksSummary``,
  ``QueueBatchStatus``, ``QueuePullRequest``,
  ``QueuePullRequestAuthor``, and ``get_queue_status`` — all
  now Rust-native. ``QueuePullResponse`` and friends stay for
  the still-shimmed ``queue show`` (next phase).
- ``mergify_cli/queue/cli.py``: removes the ``@queue.command
  status`` block and the helpers it owned (``STATUS_STYLES``,
  ``_status_text``, ``_batch_label``, ``_pr_label``,
  ``_topological_sort``, ``_group_batches_by_scope``,
  ``_print_batches``, ``_print_waiting_prs``).
  ``_relative_time`` stays — ``show`` still uses it.
- ``mergify_cli/tests/queue/test_cli.py``: deletes
  ``TestStatusCommand``, ``TestTopologicalSort``, and the
  ``_invoke_status`` helper. ``TestRelativeTime`` stays.

Workspace: 138 Rust tests green, 590 Python tests green.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Change-Id: I8cebcd325f05173dfa41083da2ec6516a6ec3a3f
Regression introduced by the port: every Rust-native command that
calls Mergify (config simulate, ci scopes-send, queue pause/unpause/
status) errored with ``please set the 'MERGIFY_TOKEN' or
'GITHUB_TOKEN'…`` when neither env var was set, even though the
user was authenticated via the GitHub CLI. The Python equivalent
fell back to ``gh auth token``; the Rust ports did not. Same
story for ``--repository``: Python parsed
``git config --get remote.origin.url`` when the flag was absent
and ``GITHUB_REPOSITORY`` was unset; the Rust ports just errored.

The three Rust crates each had their own copy of the resolvers,
all with the same regression. Factor a single
``mergify_core::auth`` module that:

- ``resolve_token(explicit)``: explicit → ``MERGIFY_TOKEN`` →
  ``GITHUB_TOKEN`` → ``gh auth token`` (subprocess, swallows
  missing-binary or non-zero-exit errors).
- ``resolve_repository(explicit)``: explicit → ``GITHUB_REPOSITORY``
  → ``git config --get remote.origin.url`` parsed via a
  ``parse_slug`` helper that handles HTTPS
  (``https://host/owner/repo.git``) and SSH
  (``git@host:owner/repo.git``) shapes, strips ``.git`` and
  trailing slashes.
- ``resolve_api_url(explicit)``: explicit → ``MERGIFY_API_URL`` →
  default ``https://api.mergify.com``.

Subprocess fallbacks are best-effort: if ``gh`` or ``git`` aren't
on ``$PATH``, or the command fails, we fall through to the next
source rather than surfacing the subprocess error. Matches the
Python behavior of catching ``CommandError``.

Three call sites refactored to drop their own resolvers and use
``mergify_core::auth``:

- ``mergify-config::simulate``: removed local ``resolve_token`` /
  ``resolve_api_url`` and ``DEFAULT_API_URL``.
- ``mergify-ci::scopes_send``: removed local ``resolve_repository``
  / ``resolve_token`` / ``resolve_api_url`` and ``DEFAULT_API_URL``.
- ``mergify-queue``: deleted ``mergify_queue::auth`` entirely; the
  three commands import ``mergify_core::auth`` directly.

The redundant tests that lived in ``simulate.rs`` and
``scopes_send.rs`` are deleted — their 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 test
``resolve_token_error_message_mentions_gh`` exercises the
"PATH points to a directory with no gh" case to confirm the
fallback degrades cleanly.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Change-Id: I6ea0e60fb21c1debea7ee005333b30aa86df6f0e
@jd jd force-pushed the devs/jd/worktree-rust-port/restore-gh-git-config-fallbacks-token-repo--6ea0e60f branch from 433e7a8 to 6f890be Compare May 5, 2026 13:21
@jd jd force-pushed the devs/jd/worktree-rust-port/port-queue-status-native-rust-phase-1-7--8cebcd32 branch from d733d15 to aa25245 Compare May 5, 2026 13:21
@jd
Copy link
Copy Markdown
Member Author

jd commented May 5, 2026

Revision history

# Type Changes Reason Date
1 initial 433e7a8 2026-05-05 13:22 UTC
2 rebase 433e7a8 → 6f890be 2026-05-05 13:22 UTC

@mergify mergify Bot had a problem deploying to Mergify Merge Protections May 5, 2026 13:22 Failure
@mergify mergify Bot requested a review from a team May 5, 2026 13:34
@jd jd force-pushed the devs/jd/worktree-rust-port/port-queue-status-native-rust-phase-1-7--8cebcd32 branch from aa25245 to 53d4832 Compare May 5, 2026 15:18
@jd jd closed this May 5, 2026
@jd jd deleted the devs/jd/worktree-rust-port/restore-gh-git-config-fallbacks-token-repo--6ea0e60f branch May 5, 2026 15:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

1 participant