chore(test): Mergify routing — docs area#386
Open
coseto6125 wants to merge 1 commit into
Open
Conversation
Contributor
|
[] |
coseto6125
added a commit
that referenced
this pull request
May 23, 2026
…lock) (#390) T14 round-2 routing test (PRs #386 / #387 / #388) surfaced two bugs: 1. Comment-only and docs PRs analyzed with area=null even when the only changed path was a clear area match (README.md, parser comment, cli WHY-comment). `ImpactJson::changed_files()` derived from `changed_symbols[].filePath`, but `ecp impact` returns 0 changed_symbols for comment-only diffs (no symbol semantically changed). Empty changed_files → `classify_area(&[]) → None` → no `ecp:area-*` label → wrong Mergify queue. Pull the diff file list from git directly via a new `git_diff_files(baseline, pr_head)` helper that runs `git diff --name-only $baseline..$pr_head`. Now docs PRs route to docs-only queue and parser-comment PRs route to parser-changes. The unused `ImpactJson::changed_files` is removed; the `file_path` field on `ChangedSymbol` stays for future consumers and to keep the deserializer happy. 2. Cross-PR conflict deadlock: when 3 PRs pushed in parallel, each `analyze` run reads the other two's caches before they've been written. Old code reported each missing cache as a conflict (`__pending_analysis__` sentinel), so all 3 PRs ended up with `ecp/cross-pr-conflict=pending` and Mergify queued none. Spec actually said missing-cache should fall through to "let Mergify speculative trial catch real conflicts" — the conservative implementation was inconsistent with the design. Switch the `None => …` arm to `continue` so uncached siblings are skipped, not blocked. Real symbol overlap on cached siblings still reports normally. Updates `cross_pr_conflict_missing_cache_is_conservative` test (renamed `_skipped`) to assert the new behaviour, and tightens `cross_pr_conflict_excludes_self` to use cached impact (so it actually exercises the overlap path rather than the now-skipped missing-cache path).
2201a58 to
2594a56
Compare
2 tasks
coseto6125
added a commit
that referenced
this pull request
May 23, 2026
The 'Detect code changes' job did: git fetch --no-tags --depth=1 origin "$BASE_REF" diff_range="origin/$BASE_REF...HEAD" The checkout step already used fetch-depth: 0, so all history was present locally. The explicit --depth=1 overrode it and truncated origin/main to just its tip, leaving no common ancestor with HEAD: fatal: origin/main...HEAD: no merge base That failed the Detect-code-changes job, which fed the Test matrix's path filter, which then skipped every shard, which caused the 'Test (all platforms)' aggregator to fail with 'Test was skipped on PR'. Every Mergify routing PR (#386 #387 #388) was stuck behind this.
2594a56 to
827ef99
Compare
Owner
Author
|
@Mergifyio queue main-docs-only |
Contributor
Merge Queue Status
This pull request spent 1 minute 11 seconds in the queue, including 50 seconds running CI. Waiting for
All conditions
ReasonThe merge conditions cannot be satisfied due to failing checks Failing checks: HintYou may have to fix your CI before adding the pull request to the queue again. |
76 tasks
3 tasks
coseto6125
added a commit
that referenced
this pull request
May 23, 2026
PR #386 entered main-docs-only then immediately dequeued with 'Checks failed' — Mergify's required-conditions box showed `check-success=ecp/cross-pr-conflict` still unchecked even though the PR's HEAD commit had that status as success. Cause: Mergify's speculative trial creates a fresh commit (PR diff + main tip). merge_conditions are evaluated on THAT commit, not the PR's HEAD. ecp-pr-analyze.yml triggers only on `pull_request` events, so it never fires against the trial commit → no cross-pr-conflict status ever reports → merge_conditions blocked → dequeue. Move the check from merge_conditions into queue_conditions. The question 'is it safe to queue this PR?' is evaluated on the PR HEAD, where the status exists. Once queued, Mergify's own speculative CI (branch protection's required checks running on the trial commit) is the merge gate.
coseto6125
added a commit
that referenced
this pull request
May 23, 2026
…ions Symptom: every PR satisfying the routing rules sat at 'Mergify Merge Queue / completed / neutral / Merge queue is ready' indefinitely, never embarking. Manual '@Mergifyio queue main-docs-only' on PR #386 worked immediately, proving queue_rules themselves are wired up. Root cause: in modern Mergify, pull_request_rules with a 'queue' action is an EXPLICIT-enqueue mechanism (for commands / advanced filters). It does not auto-route based on its 'conditions:' block. Auto-routing lives in queue_rules.queue_conditions: Mergify evaluates each PR against every queue_rule's queue_conditions and embarks it into the first match. Fix: move the routing conditions (base, labels, required-checks) from each pull_request_rule into the corresponding queue_rule's queue_conditions, and drop pull_request_rules entirely. The queue_rule order matters — most-specific (area=test/parser/cli/docs) before the fallback (no area label) so the catch-all doesn't grab specialized PRs.
coseto6125
added a commit
that referenced
this pull request
May 23, 2026
PR #386 entered main-docs-only then immediately dequeued with 'Checks failed' — Mergify's required-conditions box showed `check-success=ecp/cross-pr-conflict` still unchecked even though the PR's HEAD commit had that status as success. Cause: Mergify's speculative trial creates a fresh commit (PR diff + main tip). merge_conditions are evaluated on THAT commit, not the PR's HEAD. ecp-pr-analyze.yml triggers only on `pull_request` events, so it never fires against the trial commit → no cross-pr-conflict status ever reports → merge_conditions blocked → dequeue. Move the check from merge_conditions into queue_conditions. The question 'is it safe to queue this PR?' is evaluated on the PR HEAD, where the status exists. Once queued, Mergify's own speculative CI (branch protection's required checks running on the trial commit) is the merge gate.
coseto6125
added a commit
that referenced
this pull request
May 23, 2026
…#393) * fix(mergify): migrate routing from pull_request_rules to queue_conditions Symptom: every PR satisfying the routing rules sat at 'Mergify Merge Queue / completed / neutral / Merge queue is ready' indefinitely, never embarking. Manual '@Mergifyio queue main-docs-only' on PR #386 worked immediately, proving queue_rules themselves are wired up. Root cause: in modern Mergify, pull_request_rules with a 'queue' action is an EXPLICIT-enqueue mechanism (for commands / advanced filters). It does not auto-route based on its 'conditions:' block. Auto-routing lives in queue_rules.queue_conditions: Mergify evaluates each PR against every queue_rule's queue_conditions and embarks it into the first match. Fix: move the routing conditions (base, labels, required-checks) from each pull_request_rule into the corresponding queue_rule's queue_conditions, and drop pull_request_rules entirely. The queue_rule order matters — most-specific (area=test/parser/cli/docs) before the fallback (no area label) so the catch-all doesn't grab specialized PRs. * fix(ci): drop ci:run label gate (replaced by merge-queue label) ci:run predated Mergify and served as a manual opt-in to fire the Test matrix on PRs (the GitHub-personal-repo substitute for self-approve, when no merge queue existed). With Mergify queue_conditions taking over the merge gate, ci:run became redundant AND actively harmful: - Mergify queue_conditions require check-success=Test (all platforms), but Test would only fire after the label landed → chicken/egg. - Toggling ci:run between pushes triggered cancellation races with in-flight CI workflows (confirmed via the t-mergify-* test PRs). Remove: - 'labeled' / 'unlabeled' from pull_request event types (no longer needed to trigger workflow runs). - The contains(...ci:run) check from the test job's if-guard. Test now fires unconditionally on PRs; detect-changes still short-circuits the matrix to skipped for docs-only changes. - The PR-skipped FAIL branch in the test-result aggregator — skip only happens via [skip ci] now, which is a legitimate opt-out. The new merge gate is the 'merge-queue' label (applied by author when ready), evaluated by Mergify queue_conditions alongside the green-check requirement. * fix(mergify): move cross-pr-conflict to queue_conditions PR #386 entered main-docs-only then immediately dequeued with 'Checks failed' — Mergify's required-conditions box showed `check-success=ecp/cross-pr-conflict` still unchecked even though the PR's HEAD commit had that status as success. Cause: Mergify's speculative trial creates a fresh commit (PR diff + main tip). merge_conditions are evaluated on THAT commit, not the PR's HEAD. ecp-pr-analyze.yml triggers only on `pull_request` events, so it never fires against the trial commit → no cross-pr-conflict status ever reports → merge_conditions blocked → dequeue. Move the check from merge_conditions into queue_conditions. The question 'is it safe to queue this PR?' is evaluated on the PR HEAD, where the status exists. Once queued, Mergify's own speculative CI (branch protection's required checks running on the trial commit) is the merge gate.
827ef99 to
6609e17
Compare
3 tasks
coseto6125
added a commit
that referenced
this pull request
May 23, 2026
#398) PRs #386 #387 #388 reached every required-check green, mergeStateStatus=CLEAN, correct ecp:area-* + merge-queue labels — but Mergify check stayed at 'completed/neutral · Merge queue is ready' with payload queue_rule_name=null. None of our queue_conditions matched. Diagnosis: ecp/cross-pr-conflict is a *commit status* set via `gh api -X POST repos/.../statuses/$HEAD_SHA` in ecp-pr-analyze.yml (line 1300 area). Mergify's `check-success=` condition queries the GitHub Checks API, not the Statuses API. The two are separate stores in GitHub. So our `check-success=ecp/cross-pr-conflict` could never match no matter how green the status was. Mergify's documented keyword for matching commit statuses is `status-success=` (parallel keyword to check-success, same syntax, queries the Statuses API instead). Switch all 5 queue_rules. After this lands, the 3 test PRs should auto-embark on next sync.
6609e17 to
1cf93a4
Compare
3 tasks
github-actions Bot
pushed a commit
that referenced
this pull request
May 23, 2026
…date (#401) * refactor(ci): replace Mergify with GitHub-native auto-merge + auto-update Delete .mergify.yml + Mergify GitHub App dependency. Replace with two small GitHub Actions workflows that together cover Mergify's value for this repo's actual scale (single-author, low PR volume): 1. .github/workflows/auto-merge.yml On non-draft PR events, enable GitHub native auto-merge via `gh pr merge --auto --squash --delete-branch`. GitHub then waits for required checks + up-to-date branch, merges automatically. 2. .github/workflows/auto-update-pr-branches.yml On push to main (and 15-min cron safety net), iterate eligible open PRs and call /update-branch API — programmatic equivalent of the 'Update branch' button. Conflicts → one-time marker comment + job failure so GitHub emails the author. What we lose vs Mergify: - Speculative trial (PR diff + main tip CI before merge) — substituted by 'Require branches to be up to date' + auto-update. - Batched trial (multiple PRs in one CI run) — never exercised on this repo; batch_size config was aspirational. - Per-area parallel queue UI categorization — ecp:area-* labels (set by ecp-pr-analyze.yml) survive and remain readable for humans / LLM agents. What we gain: - No more 'Mergify Merge Queue: neutral / Merge queue is ready' noise blocking dogfood PRs (see today's stuck #386 #387 #388). - One less third-party service in the merge path. - Workflows readable end-to-end in the repo, no Mergify dashboard hop. Also bundle 2 small hardcode fixes in release.yml: - owner: coseto6125 → owner: ${{ github.repository_owner }} - coseto6125/homebrew-tap → env.HOMEBREW_TAP_REPO (single source of truth) * fix(ci): dedupe overlapping case patterns + silence false SC2016 actionlint surfaced 3 shellcheck warnings on auto-update-pr-branches.yml: - SC2221: `*"already up"*` always overrides `*"up to date"*` (the former matches "already up to date" too) → drop the redundant pattern. - SC2222: `*conflict*` always wins over `*"Merge conflict"*` (subset) → drop the redundant pattern. - SC2016: backticks inside the printf single-quoted format string look like un-expanded command substitution, but they are intentional Markdown formatting (`main`, `%s`). False positive → disable with shellcheck directive + comment explaining why.
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
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.
T14 round 2. Expected: area-docs + risk-low → main-docs-only queue.