merge queue: checking #1823 on main (e1f1c11) - #1826
Closed
mergify[bot] wants to merge 3 commits into
Closed
Conversation
`MERGIFY_TOKEN` is the natural place to put the credential `mergify auth login` mints, and `stack` reads that same variable for its GitHub calls. A user who exports one would get `401 Bad credentials` from api.github.com, which says nothing about why. Skip it there. The cost is nothing: `mut_` is Mergify's prefix, registered with GitHub's own secret scanning, so a value carrying it was never going to authenticate against GitHub. `stack` falls through to `GITHUB_TOKEN` or `gh auth token` as it would have if the variable were unset. `gh auth token` needs the same check, and not only because somebody could have logged `gh` in with the wrong string: it prints `$GITHUB_TOKEN` when that variable is set, so skipping the variable does not, on its own, keep its value out of the request -- the fallback hands the same token straight back. That is the GitHub Actions case, where `GITHUB_TOKEN` is exported for you. So the check is on the value, wherever it came from. The failure says so, and names the variable it actually skipped. Telling someone to set `MERGIFY_TOKEN` when they have set it, and it was skipped for a reason visible only at `-vv`, is the worst version of this message -- and `GITHUB_TOKEN` can carry a `mut_` value too, since it is the variable GitHub Actions already exports and therefore at least as likely a place to paste an `auth login` token. There the old wording named `MERGIFY_TOKEN`, which is unset, and prescribed `GITHUB_TOKEN`, which is the broken one. Where `gh` is one of the sources skipped it is installed and authenticated already, so the message asks for a login to a GitHub account rather than an install. An explicit `--token` is not second-guessed. The user aimed it at this command, and quietly using a different credential instead would be the more surprising failure of the two. This is a failure mode the rest of this stack creates -- there was no reason to hold a `mut_` token before it -- which is why it lands here rather than as a separate ticket. Fixes MRGFY-8703 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Change-Id: If3c422baf7dde5e912971015b991046ccbc886eb
`key_for` files `https://host/api` and `https://host/api/` under one entry, and its comment justified that by saying every request "replaces the base path outright". That stopped being true with #1819, which made `Client::join()` append a request path to the base's path prefix instead of replacing it, so GitHub Enterprise's `/api/v3` survives. The conclusion did not change, only the reason. `join()` now gives the base a trailing slash and strips the path's leading one, so both spellings send `/v1/user` to `https://host/api/v1/user`, and one credential entry for both is still right. The comment now says that. Comment only. Fixes MRGFY-8703 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Change-Id: Ibd81a3c529cb558d9e67d061ffe1d3d02dad2e34
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
1 participant
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.
🎉 This pull request has been checked successfully and will be merged soon. 🎉
#1823 is queued for merge on branch main (e1f1c11).
This pull request has been created by Mergify to check the mergeability of #1823.
You don't need to do anything. Mergify will close this pull request automatically when it is complete.
Required conditions of queue rule
defaultfor merge:depends-on = Mergifyio/mergify-cli#1808[⛓️ fix(core): never send a Mergify user token to GitHub #1808]github-review-approved[🛡 GitHub branch protection]github-review-approved[🛡 GitHub repository ruleset ruleRequire pull request for default branch]Enforce conventional commit]:title ~= ^(fix|feat|internal|docs|style|refactor|perf|test|build|ci|chore|revert|ui)(?:\(.+\))?!?:👀 Review Requirements]:#approved-reviews-by>=2author = dependabot[bot]author = mergify-ci-botauthor = renovate[bot]📕 PR description]:body ~= (?ms:.{48,})🔎 Reviews]:#changes-requested-reviews-by = 0#review-requested = 0#review-threads-unresolved = 0🤖 Continuous Integration]:check-success=ci-gateRequired conditions to stay in the queue:
base=maindepends-on = Mergifyio/mergify-cli#1808[⛓️ fix(core): never send a Mergify user token to GitHub #1808]github-review-approved[🛡 GitHub branch protection]github-review-approved[🛡 GitHub repository ruleset ruleRequire pull request for default branch]label!=manual mergeEnforce conventional commit]:title ~= ^(fix|feat|internal|docs|style|refactor|perf|test|build|ci|chore|revert|ui)(?:\(.+\))?!?:👀 Review Requirements]:#approved-reviews-by>=2author = dependabot[bot]author = mergify-ci-botauthor = renovate[bot]📕 PR description]:body ~= (?ms:.{48,})🔎 Reviews]:#changes-requested-reviews-by = 0#review-requested = 0#review-threads-unresolved = 0🤖 Continuous Integration]:check-success=ci-gate