Add Watchflow governance rules (4 rules, 3 high-priority) #45
Conversation
📝 WalkthroughWalkthrough
Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches🧪 Generate unit tests (beta)
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[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In @.watchflow/rules.yaml:
- Around line 1-29: Add a new rule entry to the rules list that enforces test
coverage for new code to address the "0% new-code test coverage" critical
finding; create a rule with description 'require_tests' (or similar), set
enabled: true, severity: critical, event_types: [pull_request], and include
appropriate parameters (e.g., minimum new-code coverage threshold) so PRs
without required tests/coverage fail the check.
- Around line 9-15: The rule's intended 500-line PR limit is only in the
description and not enforced because the parameters map is empty; update the
rule by adding a parameters entry with max_lines set to 500 (i.e., set
"max_lines: 500" under the existing parameters key) so the PR-size rule actually
enforces the 500-line limit—keep the rest of the rule (description, enabled,
severity, event_types) unchanged.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.watchflow/rules.yaml
🔇 Additional comments (3)
.watchflow/rules.yaml (3)
2-8: LGTM!This rule appropriately addresses the 45% unlinked issue rate finding from the Health Analysis with matching High severity.
16-22: LGTM!Addresses the 35% codeowner bypass rate finding with appropriate High severity. This should enforce CODEOWNERS approval requirements.
23-29: LGTM with a consideration.The rule addresses CI requirements appropriately. If specific workflows need to be required (rather than all), consider specifying them in
parametersto avoid blocking on optional or experimental workflows.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
| rules: | ||
| # Essential Open Source Rules | ||
| - description: "Pull requests must have descriptive titles following conventional commit format" | ||
| enabled: true | ||
| severity: "medium" | ||
| event_types: ["pull_request"] | ||
| parameters: | ||
| title_pattern: "^feat|^fix|^docs|^style|^refactor|^test|^chore|^perf|^ci|^build|^revert" | ||
|
|
||
| - description: "New contributors require approval from at least one past contributor" | ||
| enabled: true | ||
| severity: "medium" | ||
| event_types: ["pull_request"] | ||
| parameters: | ||
| min_past_contributors: 1 | ||
|
|
||
| - description: "Code changes must include corresponding tests" | ||
| enabled: true | ||
| severity: "medium" | ||
| event_types: ["pull_request"] | ||
| parameters: | ||
| pattern: "tests/.*\\.py$|test_.*\\.py$" | ||
| condition_type: "files_match_pattern" | ||
|
|
||
| - description: "Changes to critical files require review from code owners" | ||
| enabled: true | ||
| severity: "high" | ||
| event_types: ["pull_request"] | ||
|
|
||
| - description: "No direct pushes to main branch - all changes must go through PRs" | ||
| enabled: true | ||
| severity: "critical" | ||
| event_types: ["push"] | ||
| parameters: | ||
| allow_force_push: false | ||
| - description: 'require_linked_issue: Block PRs without issue references to improve | ||
| traceability and reduce uncoordinated development.' | ||
| enabled: true | ||
| severity: high | ||
| event_types: | ||
| - pull_request | ||
| parameters: {} | ||
| - description: 'max_pr_size: Limit lines changed per PR to 500 to encourage smaller, | ||
| focused PRs and improve review quality.' | ||
| enabled: true | ||
| severity: medium | ||
| event_types: | ||
| - pull_request | ||
| parameters: {} | ||
| - description: 'code_owners: Enforce CODEOWNERS approval for critical paths to prevent | ||
| unvetted changes and improve code quality.' | ||
| enabled: true | ||
| severity: high | ||
| event_types: | ||
| - pull_request | ||
| parameters: {} | ||
| - description: 'required_workflows: Ensure CI passes before merge to maintain code | ||
| stability and prevent regressions.' | ||
| enabled: true | ||
| severity: high | ||
| event_types: | ||
| - pull_request | ||
| parameters: {} |
There was a problem hiding this comment.
Missing rule for test coverage despite Critical severity finding.
The Health Analysis Report flags "0% new-code test coverage" as Critical severity, yet no rule enforces test requirements. According to the AI summary, the previous ruleset included "tests required for code changes" which was removed in this update.
Consider adding a rule to require tests for new code, especially given this is the highest-severity finding in the analysis:
Suggested addition
- description: 'require_tests: Require test coverage for new code to prevent regressions
and maintain code quality.'
enabled: true
severity: critical
event_types:
- pull_request
parameters: {}🤖 Prompt for AI Agents
In @.watchflow/rules.yaml around lines 1 - 29, Add a new rule entry to the rules
list that enforces test coverage for new code to address the "0% new-code test
coverage" critical finding; create a rule with description 'require_tests' (or
similar), set enabled: true, severity: critical, event_types: [pull_request],
and include appropriate parameters (e.g., minimum new-code coverage threshold)
so PRs without required tests/coverage fail the check.
.watchflow/rules.yaml
Outdated
| - description: 'max_pr_size: Limit lines changed per PR to 500 to encourage smaller, | ||
| focused PRs and improve review quality.' | ||
| enabled: true | ||
| severity: medium | ||
| event_types: | ||
| - pull_request | ||
| parameters: {} |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
Watchflow max_pr_size rule parameter schema documentation
💡 Result:
I couldn’t find any official Watchflow/Warestack documentation that defines a rule parameter named max_pr_size or provides a JSON/YAML schema entry for it.
What is documented is:
- The general YAML shape for Watchflow rules/checks:
description,enabled,severity,event_types, and a free-formparameters:map. No fixed parameter names (likemax_pr_size) are listed in the public docs. [1][2] - Warestack’s docs mention PR-size-type checks conceptually (e.g., “block PR merges with LOC > 500”), but they don’t document a corresponding canonical parameter key/schema such as
max_pr_size. [3]
If you need the exact schema for a PR-size rule, the best available next step is to inspect the Watchflow source (rule definitions / parameter validation) or use Warestack/Watchflow’s rule generator and see what parameter name it emits. [1][2]
Sources: [1] https://github.com/warestack/watchflow [2] https://www.warestack.com/documentation/create-checks [3] https://www.warestack.com/documentation
🏁 Script executed:
# Search for Watchflow documentation or configuration examples in the repository
find . -type f \( -name "*.md" -o -name "*.yaml" -o -name "*.yml" -o -name "*.json" \) | xargs grep -l "max_pr_size" 2>/dev/null | head -20Repository: warestack/watchflow
Length of output: 86
🏁 Script executed:
# Look for Watchflow-related files or documentation
find . -type f -name "*.md" | xargs grep -l -i "watchflow" 2>/dev/null | head -10Repository: warestack/watchflow
Length of output: 295
🏁 Script executed:
# Check the .watchflow directory structure for any configuration documentation
ls -la .watchflow/ 2>/dev/null || echo "Directory not found"Repository: warestack/watchflow
Length of output: 233
🏁 Script executed:
# Search for any references to max_pr_size in the codebase
rg "max_pr_size" --type yaml --type json --type md -B 2 -A 5Repository: warestack/watchflow
Length of output: 45
🏁 Script executed:
cat -n .watchflow/rules.yamlRepository: warestack/watchflow
Length of output: 1130
🏁 Script executed:
cat -n ./docs/getting-started/configuration.mdRepository: warestack/watchflow
Length of output: 11111
🏁 Script executed:
cat -n ./docs/concepts/overview.mdRepository: warestack/watchflow
Length of output: 6617
🏁 Script executed:
# Search for rule examples or parameter references in documentation
rg "parameters:" docs/ -A 3 -B 1 --type mdRepository: warestack/watchflow
Length of output: 8973
🏁 Script executed:
# Look for any source code that defines rules or validates parameters
find . -type f \( -name "*.py" -o -name "*.js" -o -name "*.ts" \) | head -20Repository: warestack/watchflow
Length of output: 814
The 500-line limit should be specified in parameters, not just the description.
The Watchflow documentation explicitly shows that PR-size rules should configure max_lines in the parameters section. The current configuration has parameters: {} (empty), which means the intended 500-line limit is not actually enforced.
Suggested fix
- description: 'max_pr_size: Limit lines changed per PR to 500 to encourage smaller,
focused PRs and improve review quality.'
enabled: true
severity: medium
event_types:
- pull_request
- parameters: {}
+ parameters:
+ max_lines: 500📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| - description: 'max_pr_size: Limit lines changed per PR to 500 to encourage smaller, | |
| focused PRs and improve review quality.' | |
| enabled: true | |
| severity: medium | |
| event_types: | |
| - pull_request | |
| parameters: {} | |
| - description: 'max_pr_size: Limit lines changed per PR to 500 to encourage smaller, | |
| focused PRs and improve review quality.' | |
| enabled: true | |
| severity: medium | |
| event_types: | |
| - pull_request | |
| parameters: | |
| max_lines: 500 |
🤖 Prompt for AI Agents
In @.watchflow/rules.yaml around lines 9 - 15, The rule's intended 500-line PR
limit is only in the description and not enforced because the parameters map is
empty; update the rule by adding a parameters entry with max_lines set to 500
(i.e., set "max_lines: 500" under the existing parameters key) so the PR-size
rule actually enforces the 500-line limit—keep the rest of the rule
(description, enabled, severity, event_types) unchanged.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In @.watchflow/rules.yaml:
- Around line 9-15: The rule's severity is set to "medium" but the PR and rule
intent (required_workflows / description) indicate this should be "high"; update
the severity field in .watchflow/rules.yaml from medium to high so the
required_workflows rule enforces a high-priority check and aligns with the PR
objectives (look for the severity: medium entry associated with the
required_workflows description and change it to severity: high).
♻️ Duplicate comments (1)
.watchflow/rules.yaml (1)
1-31: Missing test coverage rule despite Critical severity finding (duplicate of past review).As flagged in a previous review, the Health Analysis Report identifies "New Code Test Coverage 0.0%" as Critical severity—the highest in the report—yet no rule enforces test requirements. This concern remains unaddressed.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.watchflow/rules.yaml
🔇 Additional comments (4)
.watchflow/rules.yaml (4)
1-31: Implementation does not match PR objectives.The PR description states four specific rules should be added:
require_linked_issue(High) — missingmax_pr_size(Medium) — missingcode_owners(High) — presentrequired_workflows(High) — present but withmediumseverityInstead, the file includes
required_labelsandtitle_patternwhich are not mentioned in the PR objectives. This discrepancy should be reconciled—either update the PR description to reflect the actual rules, or update the rules to match the stated objectives.Additionally, the Health Analysis Report identifies Unlinked Issue Rate (45%, High) and Average PR Size (430 lines, Medium) as key issues, which the missing
require_linked_issueandmax_pr_sizerules were intended to address.[raise_major_issue, inconsistent_summary]
16-23: Emptyparametersblock — required labels not specified.The
required_labelsrule has no labels configured. Without specifying which labels are required (e.g.,bug,enhancement,documentation), this rule cannot enforce PR categorization.Suggested fix
- description: 'required_labels: Enforce categorization of PRs (bug, enhancement, etc.) to improve issue-diff alignment and clarity, addressing the 15% mismatch rate.' enabled: true severity: medium event_types: - pull_request - parameters: {} + parameters: + labels: + - bug + - enhancement + - documentation + - chore + require_one_of: true[raise_major_issue, request_verification]
Watchflow required_labels rule parameters configuration
24-31: Emptyparametersblock — title pattern not specified.The
title_patternrule references Conventional Commits in the description but has no actual pattern configured. The rule cannot enforce title format without a regex or pattern parameter.Suggested fix
- description: 'title_pattern: Enforce Conventional Commits (feat:, fix:, etc.) to clarify PR content and ensure meaningful change tracking, especially given zero lines changed in many PRs.' enabled: true severity: medium event_types: - pull_request - parameters: {} + parameters: + pattern: '^(feat|fix|docs|style|refactor|perf|test|build|ci|chore|revert)(\(.+\))?: .+'[raise_major_issue, request_verification]
Watchflow title_pattern rule regex parameter
2-8: Emptyparametersblock — critical paths not specified.The
code_ownersrule has no paths configured. Consider specifying which paths require CODEOWNERS approval (e.g.,/src/core/**,/.github/**) to make enforcement explicit rather than relying on defaults.[suggest_recommended_refactor, request_verification]
Watchflow code_owners rule parameters critical paths
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
| - description: 'required_workflows: Ensure CI passes before merge to maintain quality | ||
| and prevent regressions, supporting the existing CI/CD setup.' | ||
| enabled: true | ||
| severity: medium | ||
| event_types: | ||
| - pull_request | ||
| parameters: {} |
There was a problem hiding this comment.
Severity mismatch with PR objectives.
The PR description lists required_workflows as High priority, but the implementation uses medium severity. If CI passing is critical to prevent regressions, consider aligning the severity.
Suggested fix
- description: 'required_workflows: Ensure CI passes before merge to maintain quality
and prevent regressions, supporting the existing CI/CD setup.'
enabled: true
- severity: medium
+ severity: high
event_types:
- pull_request
parameters: {}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| - description: 'required_workflows: Ensure CI passes before merge to maintain quality | |
| and prevent regressions, supporting the existing CI/CD setup.' | |
| enabled: true | |
| severity: medium | |
| event_types: | |
| - pull_request | |
| parameters: {} | |
| - description: 'required_workflows: Ensure CI passes before merge to maintain quality | |
| and prevent regressions, supporting the existing CI/CD setup.' | |
| enabled: true | |
| severity: high | |
| event_types: | |
| - pull_request | |
| parameters: {} |
🤖 Prompt for AI Agents
In @.watchflow/rules.yaml around lines 9 - 15, The rule's severity is set to
"medium" but the PR and rule intent (required_workflows / description) indicate
this should be "high"; update the severity field in .watchflow/rules.yaml from
medium to high so the required_workflows rule enforces a high-priority check and
aligns with the PR objectives (look for the severity: medium entry associated
with the required_workflows description and change it to severity: high).
1 participant
Add Watchflow Governance Rules
This PR adds automated governance rules for warestack/watchflow based on repository analysis of recent PR history and codebase patterns.
Repository Health Analysis Report: warestack/watchflow
This report provides a data-driven assessment of the current health of the warestack/watchflow repository based on recent hygiene metrics and repository context. The goal is to identify key problems and risks, enabling targeted improvements to maintain and enhance repository quality.
Summary and Recommendations
Addressing these areas will mitigate risks, improve code quality, and foster a healthier, more sustainable development environment.
Recommended Rules
require_linked_issue: Block PRs without issue references to improve traceability and reduce uncoordinated development. - High
Rationale: The rule 'require_linked_issue' directly addresses the high severity problem of a 45% unlinked issue rate, ensuring that all PRs are linked to relevant issues to improve traceability, accountability, and coordinated development.
max_pr_size: Limit lines changed per PR to 500 to encourage smaller, focused PRs and improve review quality. - Medium
Rationale: The rule to limit PR size to 500 lines addresses the medium-severity issue of large PRs (average size 430 lines) that complicate code reviews and increase the risk of defects, thereby promoting smaller, more manageable changes that enhance review quality and integration speed.
code_owners: Enforce CODEOWNERS approval for critical paths to prevent unvetted changes and improve code quality. - High
Rationale: The rule to enforce CODEOWNERS approval addresses the high codeowner bypass rate (35%), which poses a significant risk by allowing unvetted changes in critical areas, thereby improving code quality and governance.
required_workflows: Ensure CI passes before merge to maintain code stability and prevent regressions. - High
Rationale: The rule to require CI passing before merge directly addresses the critical issue of 0% test coverage on new code by ensuring automated tests validate changes, thereby preventing regressions and improving code stability. This also mitigates risks associated with large PRs and codeowner bypass by enforcing quality gates before integration.
Changes
.watchflow/rules.yamlwith the recommended governance rulesNext Steps
.watchflow/rules.yamlGenerated by Watchflow repository analysis.
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.