T8782: Restore backport conflict workspace via label_conflicts and assignees#7
Conversation
…signees
Re-enable PR creation on backport cherry-pick conflicts (ignore_conflicts:
true) and add label_conflicts: backport-conflict + assignees: ["{{ author }}"]
so the conflicted PR is discoverable and accountable instead of silent.
The prior ignore_conflicts: false (set after vyos-documentation#1994 →
#1998/#1999 shipped conflict markers) stopped silent merges but eliminated
the workspace where conflicts could be resolved. With this config the PR
exists, carries a distinct label (backport-conflict, not the base-merge
`conflicts` label), and pings the source PR author.
The label itself does not block merge — fleet-wide enforcement (required
CI catching <<<<<<< markers, or branch-protection ruleset on the label)
is governance work tracked under T8850 Cat 1, not central config.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Central YAML (base), Organization UI (inherited) Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📜 Recent review details🧰 Additional context used🔍 Remote MCPAdditional relevant facts for reviewing PR
|
| Layer / File(s) | Summary |
|---|---|
Backport conflict defaults .mergify.yml (lines 21–74) |
Changed defaults.actions.backport.ignore_conflicts from false to true so Mergify proceeds when cherry-pick conflicts occur (commits conflict markers). Added automatic labels: - backport-conflict and assignees: - "{{ author }}" on conflicted backport PRs. |
Merge protection gate .mergify.yml (lines 169–203) |
Added merge_protections rule that prevents Mergify merges while backport-conflict label is present by requiring success_conditions: label != backport-conflict. This forces manual conflict resolution and label removal before auto-merge. |
Possibly related PRs
- vyos/mergify#1: Prior changes to backport conflict handling and labeling; this PR builds on that flow by adding merge-protection enforcement and assignee behavior.
🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
| Check name | Status | Explanation |
|---|---|---|
| Title check | ✅ Passed | Title accurately describes the main change: restoring backport conflict workspace via Mergify configuration updates with label_conflicts and assignees. |
| Description check | ✅ Passed | Description is comprehensive and directly related to the changeset, explaining both the configuration updates and the rationale behind them. |
| Docstring Coverage | ✅ Passed | No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. |
| Linked Issues check | ✅ Passed | Check skipped because no linked issues were found for this pull request. |
| Out of Scope Changes check | ✅ Passed | Check skipped because no linked issues were found for this pull request. |
✏️ Tip: You can configure your own custom pre-merge checks in the settings.
✨ Finishing Touches
✨ Simplify code
- Create PR with simplified code
- Commit simplified code in branch
claude/stoic-saha-f62fe0
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.
Comment @coderabbitai help to get the list of available commands and usage tips.
|
@copilot review |
Here's my review of the changes in this PR: What changed (
Assessment — no issues found:
One minor observation: the |
…_protections Add a merge_protections rule requiring `label != backport-conflict` for merge. The backport action above auto-applies the label on cherry-pick conflicts, so this closes the loop: the conflicted PR exists (workspace preserved), is labeled and assigned, AND cannot be merged until the operator pushes a resolution and removes the label manually. Per Mergify docs, merge_protections from `extends:` merges across parent and child with same-name override semantics; consumer repos can opt out by defining a same-named rule locally with weaker conditions. `dequeue` is not exposed as a YAML action under pull_request_rules — only as a user-facing slash command and a key under commands_restrictions. merge_protections is the declarative equivalent. Manual `@Mergifyio dequeue` remains available to Maintainers via commands_restrictions. Active ejection of an already-queued PR when the label arrives vs. holding the PR indefinitely in queue is not explicitly documented by Mergify; the comment marks this as TBD on first real backport conflict.
|
@copilot review |
Review of the current state (both commits, Changes in scope Commit 1 ( Assessment — no issues found, a few notes worth flagging:
No blocking issues — the implementation is complete and consistent with the stated design. |
Mergify schema validation rejected the prior commit with `Field required @ root → merge_protections → item 0 → if`. The `if:` clause scopes when the rule applies; without it the entry is invalid. Now `if: - label = backport-conflict` so the rule applies only when the label is present, and `success_conditions: - label != backport-conflict` fails whenever the rule applies → merge blocked while the label is set. No behavior change vs. intent — purely fixing schema compliance.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
.mergify.yml (1)
195-201: Operational follow-up: automatebackport-conflictremoval so resolved PRs don't stay gated by forgotten labels.The label is added automatically but only removed manually, so a maintainer who pushes a clean resolution commit but forgets the label change leaves the PR permanently blocked. The defense-in-depth piece you reference (T8850 — CI status matching
<<<<<<<in tracked files) would naturally pair with apull_request_rulestoggleso the label tracks the actual file state, mirroring howconflictsis toggled by theconflictpredicate above.Not blocking — T8850 is the right home for this. Flagging so the link between the two doesn't get lost.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.mergify.yml around lines 195 - 201, Add automation to remove the backport-conflict label when a PR no longer has merge conflicts by adding a pull_request_rules entry that uses the existing conflict predicate (or the conflicts predicate used elsewhere) and a toggle action targeting the backport-conflict label; specifically, create a pull_request_rules rule that matches when conflicts are false/absent and applies toggle: - label: backport-conflict to remove the label, mirroring how the conflicts predicate currently toggles other labels so resolved PRs are no longer blocked.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In @.mergify.yml:
- Around line 195-201: Add automation to remove the backport-conflict label when
a PR no longer has merge conflicts by adding a pull_request_rules entry that
uses the existing conflict predicate (or the conflicts predicate used elsewhere)
and a toggle action targeting the backport-conflict label; specifically, create
a pull_request_rules rule that matches when conflicts are false/absent and
applies toggle: - label: backport-conflict to remove the label, mirroring how
the conflicts predicate currently toggles other labels so resolved PRs are no
longer blocked.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Central YAML (base), Organization UI (inherited)
Review profile: CHILL
Plan: Pro Plus
Run ID: ad24b881-1680-4b62-ad09-81b16ef7dd5b
📒 Files selected for processing (1)
.mergify.yml
📜 Review details
🧰 Additional context used
🔍 Remote MCP
Summary of Additional Context
Based on the Mergify documentation, here are the key facts relevant to reviewing this PR:
Configuration Parameters
ignore_conflicts (backport action)
- Controls whether backport PRs are created when cherry-pick conflicts occur
- Default value is
true, meaning PRs are created even with conflicts - The PR changes the default to explicitly set
trueto ensure a PR workspace is created for manual conflict resolution
label_conflicts (backport action)
- Automatically labels backport PRs that have conflicts during cherry-pick
- Only applies when
ignore_conflictsis enabled - Used here to mark conflicted backports with the
backport-conflictlabel for visibility and filtering
assignees (backport action)
- Assigns the created backport PR to specified users
- The PR uses
["{{ author }}"]to assign backports to the original PR author - This ensures the person who authored the original change is responsible for resolving conflicts
merge_protections rule
- Defines conditions that must be satisfied before a PR can be merged
- Uses
success_conditionsto validate rules (in this case:label != backport-conflict) - Only affects Mergify-driven merges, not manual merges via GitHub UI
- The PR adds this to prevent merging conflicted backports until the label is manually removed
Key Workflow Behavior
- When a backport cherry-pick encounters conflicts with
ignore_conflicts: trueandlabel_conflicts: backport-conflictconfigured, Mergify creates a PR with conflict markers visible, applies thebackport-conflictlabel, and assigns it to the original author - The
merge_protectionsrule then blocks Mergify from automatically merging such PRs while the label is present - Manual label removal is required before the
merge_protectionsrule allows the PR to merge
[::Context7_resolve-library-id::], [::Context7_query-docs::]
🔀 Multi-repo context ansible/ansible
[::ansible/ansible::]
- No repository-level Mergify configuration or references to ".mergify" found in the tree (rg returned no .mergify.yml or "mergify" config matches). Files searched globally; nothing matching mergify-specific keys (label_conflicts, ignore_conflicts, merge_protections) was found.
- Repository contains backport-related tooling/scripts which may interact with backport workflows:
- hacking/backport/backport_of_line_adder.py — utilities to add "Backport of ..." lines and to detect backport PRs. [::ansible/ansible::hacking/backport/backport_of_line_adder.py]
- hacking/backport/README.md — documentation for backport scripts. [::ansible/ansible::hacking/backport/README.md]
- Several other mentions of "backport" in docs/tests (e.g., AGENTS.md, test/sanity files) but no automated Mergify configuration integration points discovered by text search.
Conclusion: No direct Mergify configuration or label/merge-protection consumers found; only repository-local backport helper scripts exist which are relevant to backport workflows but do not show direct integration with Mergify labels/actions from the searches above.
🔇 Additional comments (1)
.mergify.yml (1)
21-74: LGTM!Also applies to: 170-201
Summary
Two-part change to the central backport configuration:
ignore_conflicts: true+label_conflicts: backport-conflict+assignees: ["{{ author }}"]).backport-conflictlabel is present via amerge_protectionsrule, so the conflicted PR cannot proceed to merge until the operator resolves the conflict and removes the label.Mirrored on VyOS-Networks/mergify#5.
Why
The prior
ignore_conflicts: false(set after vyos/vyos-documentation#1994 → #1998 / #1999 shipped literal conflict markers) stopped silent merges but eliminated the in-PR workspace. With no PR, conflict resolution had no tracking surface. This config restores the workspace AND closes the silent-merge hole by gating merge on label absence.Backport-action defaults
ignore_conflicts: truelabel_conflicts: backport-conflictconflictslabel) so triage can tell backport-conflict PRs apartassignees: ["{{ author }}"]Why two distinct labels (
backport-conflictvsconflicts)label_conflictsfires only on backport (cherry-pick) conflicts. The existingLabel conflicting pull requestsrule (predicate:conflict) fires on base-branch merge conflicts and would NOT catch this case — cherry-pick conflict markers live in committed file content, not as unmerged paths, so theconflictattribute staysfalse. Two distinct labels keep triage unambiguous.Why
merge_protectionsand not adequeueactionMergify exposes
dequeueas a slash command and as acommands_restrictionskey, but NOT as a YAML action underpull_request_rules.merge_protectionsis the declarative equivalent — a PR with the label fails the success_conditionlabel != backport-conflictand is held back from merge.Resolution flow: assignee pushes a fix commit, removes the
backport-conflictlabel, CI flips green, Mergify queue-merges.Manual
@Mergifyio dequeueremains available to Maintainers as an escape hatch.Evaluation timing — partial documentation
Mergify documents that
merge_protectionsare "safe to edit live; re-evaluations happen automatically", but does NOT explicitly state whether already-queued PRs are actively ejected when asuccess_conditionstarts failing. Two safe assumptions:Whether the queued PR is ejected vs. held is TBD on first real backport conflict — the comment in the file says so explicitly.
Inheritance
Per Mergify docs,
merge_protectionsfromextends:merges across parent and child, with same-named rules in the child overriding the parent. Consumer repos can disable this rule by defining a same-named rule locally with weaker conditions.Caveat — merge button bypass
merge_protectionsonly governs Mergify-driven merges. A determined operator can still click GitHub's merge button if branch protection doesn't require Mergify checks. Defense-in-depth (required CI status that fails on<<<<<<<in tracked files, or branch-protection ruleset on the label) is tracked under T8850 (Cat 1 branch governance rulesets).Related
🤖 Generated by robots