Use single invalid-title label for both title and commit-message rules#4
Conversation
The two PR-message-format rules check the same `T<digits>: <text>` headline regex against two surfaces — the PR title and each commit's first line. Splitting that into two labels (`invalid-title` + `invalid-commit-title`) creates near-identical labels on the same PR and adds no information for the author: the fix in both cases is "rewrite the headline to match the format." Collapse onto `invalid-title`. The two rules stay separate so an operator scanning rule logs can still see which surface tripped, but the user-facing surface gets one label. 🤖 Generated by [robots](https://vyos.io)
|
@copilot 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 MCP Context7Relevant findings for reviewing the PR
Sources used: Context7 documentation and changelog for Mergify (label action / toggle guidance). 📝 WalkthroughSummary by CodeRabbit
WalkthroughConsolidates separate PR title and commit-first-line headline checks into a single ChangesMergify label consolidation
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches✨ Simplify code
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 |
Review findings for commit The change is clean and achieves its stated goal. One thing worth confirming: Both rules now independently
Depending on which rule Mergify evaluates last, the label may be incorrectly cleared. In practice this edge case is rare (authors who fix commits usually fix the title too), and a subsequent push re-triggers evaluation, but it's a subtle gotcha. A fully safe alternative would be using separate Everything else looks good:
|
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
.mergify.yml (1)
83-99:⚠️ Potential issue | 🟠 Major | 🏗️ Heavy liftMultiple rules toggling
invalid-titlewith different conditions may produce indeterminate label states.Rules at lines 70–81 (title check) and 83–99 (commit message check) both toggle
invalid-titlebased on different conditions. Mergify documentation warns that "multiple toggles can conflict, the outcome may be indeterminate without explicit rule ordering, which is not documented." When one rule matches and the other doesn't (e.g., valid title but invalid commit), the final label state becomes unpredictable.Recommended fixes:
- Use mutually exclusive conditions so only one rule can match, or
- Replace
togglewith explicitaddandremoveactions with clearly defined precedence🤖 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 83 - 99, The two rules ("Flag commit message not matching T-ID format" which uses condition 'commits[*].commit_message~=^(?!(([a-zA-Z0-9\-_.]+:\s)?)T\d+:\s+[^\s]+).*' and the title-check rule that also toggles the invalid-title label) both use toggle on the same label and can conflict; update the commit-message rule to avoid indeterminate state by either making its conditions mutually exclusive with the title rule (so only one rule can match), or replace the toggle action with explicit add/remove actions (use label:add invalid-title when match and label:remove invalid-title in a complementary rule or via an else-style rule) so label state is deterministic and precedence is clear.
🤖 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.
Outside diff comments:
In @.mergify.yml:
- Around line 83-99: The two rules ("Flag commit message not matching T-ID
format" which uses condition
'commits[*].commit_message~=^(?!(([a-zA-Z0-9\-_.]+:\s)?)T\d+:\s+[^\s]+).*' and
the title-check rule that also toggles the invalid-title label) both use toggle
on the same label and can conflict; update the commit-message rule to avoid
indeterminate state by either making its conditions mutually exclusive with the
title rule (so only one rule can match), or replace the toggle action with
explicit add/remove actions (use label:add invalid-title when match and
label:remove invalid-title in a complementary rule or via an else-style rule) so
label state is deterministic and precedence is clear.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Central YAML (base), Organization UI (inherited)
Review profile: CHILL
Plan: Pro Plus
Run ID: 3daf0ef2-009e-4c48-93a5-b278e2206a2b
📒 Files selected for processing (1)
.mergify.yml
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: copilot
🧰 Additional context used
🔍 Remote MCP
I'll search for relevant information about Mergify configuration and best practices that would help in reviewing this PR effectively.Based on the gathered information from Mergify documentation and best practices, here's the relevant context for reviewing this PR:
Mergify Label Configuration Context
Label Action Functionality
The label action allows Mergify to automatically add or remove labels from a pull request based on the conditions specified in your rules, making your workflow more organized and efficient. The consolidation from two labels to one is a valid approach.
Pull Request Rules Structure
The Mergify configuration file consists of two primary components: rules and actions. Rules define the conditions under which specific actions should be triggered, while actions specify the automated tasks to be performed on the pull requests. The PR maintains this separation by keeping two distinct rules while consolidating their labeling behavior.
Regex Pattern Support
Mergify leverages Python regular expressions to match rules, and regex101, PyRegex, or Pythex can be used to test regular expressions. This is relevant since the PR references a shared regex pattern applied to both PR titles and commit messages.
Best Practices for Label Consolidation
You can set up rules to automatically update dependencies, rebase branches, and label PRs based on specific conditions. Having these standard processes reduces mistakes and makes it much easier to handle lots of PRs efficiently. Consolidating labels aligns with this principle of streamlined automation.
Configuration Management
Instead of defining the same options repeatedly for different rules, you can specify them once in the defaults section. If a particular option is directly defined under pull_request_rules or queue_rules, it will take precedence; otherwise, the system resorts to the values defined in the defaults section. This pattern may be relevant for understanding how the shared invalid-title label is applied.
Code reviewFound 1 issue:
Lines 78 to 100 in 618d166 🤖 Generated with Claude Code - If this code review was useful, please react with 👍. Otherwise, react with 👎. |
The previous revision of this PR moved both rules' `toggle` action to the same `invalid-title` label but kept them as two separate rules. That has a structural conflict: each `toggle` rule fires in both directions on every evaluation (add when condition true, remove when false). When the title conforms but a commit doesn't (or vice versa), the two rules issue conflicting add/remove operations on the same label — order-dependent and likely to flap. Mergify's `toggle` docs frame the action as a single-rule pattern that subsumes the earlier two-rule "add on match / remove on inverse" idiom (mergify changelog 2022-11-15). Using `toggle` on two rules pointing at the same label is outside the documented use case. Collapse the title-check condition and the commit-message-check condition into one rule with an `or`-combined condition. One rule, one label, one toggle — no conflict. The diagnostic separation between "title failed" vs "commits failed" was already given up when the label was unified; the rule description still lists both surfaces so an operator reading the rule log knows what to look at. 🤖 Generated by [robots](https://vyos.io)
|
Verified the structural conflict — Mergify’s Collapsed both checks into one rule with |
|
@coderabbitai addressed in 890a369 — collapsed the two |
|
Tip For best results, initiate chat on the files or code changes.
ᓚᘏᗢ ✓ If you found this review helpful, would you consider giving us a shout-out on X? Thank you for using CodeRabbit! |
Change Summary
Both
Flag PR title not matching T-ID formatandFlag commit message not matching T-ID formatcheck the same^(([a-zA-Z0-9\-_.]+:\s)?)T\d+:\s+[^\s]+.*headline regex — one against the PR title, one against each commit's first line. They labelinvalid-titleandinvalid-commit-titlerespectively.The two labels are near-identical and add no signal: a PR that fails one usually fails the other, and the author's fix in both cases is "rewrite the headline." Splitting them just produces two near-identical labels on the same PR.
This PR collapses both rules onto a single
invalid-titlelabel. The two rules stay separate so an operator scanning Mergify rule logs can still tell which surface tripped, but the user-facing label surface gets one entry.Rollout impact
extends: mergify(the org-wide central baseline).invalid-commit-titlewill keep that label until something removes it. Once this merges, the rule that togglesinvalid-commit-titleno longer exists in the central config, so the label is orphaned (Mergify won't re-evaluate it). Manual cleanup or a one-shot label removal is fine — the label stops appearing on new PRs immediately.Flag commit message not matching T-ID formatrule by name (e.g. vyos-documentation#2009) are unaffected — they replace the rule entirely, so the parent's label choice doesn't reach them.Related Task(s)
Related PR(s)
invalid-titleandinvalid-commit-title, surfacing the duplication.🤖 Generated by robots