merge queue: checking #1842 on main (c50a1cc), stacked on #1851 - #1852
Closed
mergify[bot] wants to merge 5 commits into
Closed
mergify[bot] wants to merge 5 commits into
mergify[bot] wants to merge 5 commits into
Conversation
…e environment The two tests that pinned "clap must not read `MERGIFY_*` itself" each exported one variable empty and parsed one argv. They were the last two `temp_env` calls in `mergify-cli`, and the only ones whose reader was a dependency rather than our own code. They are two tests now, at the two altitudes the rule lives at. A walk over the built `clap::Command` tree asserts that no argument anywhere declares `env = "…"`, which catches the hook at declaration and covers every argument rather than `--config` and `--test-exit-code`. A parse of each of those two argvs asserts the observable property the deleted tests asserted, because the walk only sees that one spelling: `default_value_t = std::env::var(…).unwrap_or_default()` reproduces monorepo#33423 exactly and declares no hook. Neither test needs an environment. `MERGIFY_BASE_URL` joins the rest in treating exported-but-empty as unset. `install.sh` already guards the same lever with `[ -n … ]`, so the two halves of one feature disagreed: an empty value built the URL `/latest-release.json`, which reqwest rejects as relative, instead of falling back to the default host. The rest is plumbing: `MERGIFY_CLI_TESTING_UTF8_MODE`, `NO_COLOR` / `FORCE_COLOR` / `CLICOLOR_FORCE` and `self_update`'s `MERGIFY_BASE_URL` read through `mergify_core::env`. That is the `env` in scope in both files now; `args()` and `current_exe()` are spelled `std::env::`, since neither is the environment. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Change-Id: I512c50f33a89380f3d9f4a8f7a37725a875d6804
`mergify-auth` landed while this stack was in flight and its production side already reads through `mergify_core::env`, since `var_non_empty` predates the funnel. Only its tests were left: `machine`'s `COMPUTERNAME` / `HOSTNAME` chain, `browser`'s `SSH_CONNECTION` / `DISPLAY` / `WAYLAND_DISPLAY` probes, and `with_mergify_token`. They install an overlay now, like everywhere else, so the crate stops mutating the process environment and drops its `temp-env` dependency. `with_mergify_token` keeps its shape: the overlay is on this thread and the `current_thread` runtime it builds drives the future on that same thread, so the closure form still works and no call site changes. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Change-Id: I7d4c755cee8cc90db511ea671e5c23215300c03c
`mergify stack push` posted a sticky "This pull request is part of a Mergify stack" comment on every member, carrying a table of the whole stack. GitHub's native Stacks UI now renders that list on the pull request page itself, so the comment is a second copy of what the reader is already looking at — one they have to reconcile against the first whenever the two disagree. So a push whose stack GitHub registered posts no table, and deletes the one an earlier push left on each open member. **Keyed off the registration, not off the flag.** This is the same argument as the `Depends-On:` header one step up, and it gets the same treatment for the same reason: `native_stack::register` is explicitly allowed to do nothing — an older GitHub Enterprise, a repo without the feature, a chain GitHub will not accept, or `--no-github-native`. Drop the comment unconditionally and those pushes end up with no member list anywhere, which is a worse place than where they started. A push that did not register keeps posting and updating the table exactly as before. The settling moves to the end of the push (new step 13), next to the header restore it mirrors, because the registration's outcome is only known there. **Comments already on live stacks are deleted, not left or rewritten.** Leaving them is the tempting option and the wrong one: nothing refreshes a table we no longer write, so each would sit frozen at the membership of the last pre-native push, under a live list that keeps moving — stale next to accurate is worse than absent next to accurate. A one-time rewrite to a tombstone would leave permanent noise on every member of every stack ever pushed. Deleting costs nothing recoverable: the comment was ours, bot-authored, and carried no reply thread. The scan that finds it is the GET the upsert already did on every push, so this is not a new request — it is the same one, ending in a DELETE instead of a PATCH. Merged members are skipped, as the upsert always skipped them. Only the fallback upsert is gated on the stack having more than one pull request, where the count saves a genuinely one-change stack a GET that can only come back empty. The removal is not: a stack whose members merge down to a single open pull request keeps its registration, so that last member is still carrying the table it was given while the stack had several — exactly the frozen list this is taking down. The gate costs the removal nothing anyway, since GitHub rejects a stack below two members and an unregistered stack never reaches the removal. The delete is best-effort: it changes nothing about how the stack merges, so a push that has already done everything else reports it and retries on the next push rather than failing. That is the opposite policy from the header restore, which is fatal precisely because an unregistered, unchained stack merges out of order. **The revision-history comment is untouched.** GitHub renders nothing like it, which is what the sticky-comment surface exists for now. Also corrects a stale claim in `stack_comment`'s docstring: it said `mergify stack checkout` rebuilds a stack from the `<!-- mergify-stack-data: -->` JSON marker. It does not, and nothing in this repo reads that marker — `checkout` discovers a stack by chaining each pull request's `head.ref` to the next one's `base.ref`, which has the advantage of working on stacks this CLI never touched. The marker's readers are out of tree, so its wire shape stays pinned. MRGFY-9496 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Change-Id: I4d272321c1cfcb3b836a1fe1ac2ef0ce463ec550
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
2 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.
🎉 This pull request has been checked successfully and will be merged soon. 🎉
#1842 is queued for merge on branch main (c50a1cc).
Stacked behind 1 pull request queued ahead of this batch, not part of it. These checks run on a tip that also carries its commits, so a failure here can come from it as much as from #1842.
Queued ahead of this batch:
This pull request has been created by Mergify to speculatively check the mergeability of #1842.
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: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=maingithub-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