Skip to content

chore: update watchflow rules #54

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
dkargatzis merged 5 commits into from
Feb 26, 2026
Merged

chore: update watchflow rules #54

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
d4cbac3
feat(rules): align default rules with watchflow.dev examples
dkargatzis Feb 26, 2026
562fd20
feat(rules): natively support max_pr_loc parameter alias and update d…
dkargatzis Feb 26, 2026
1b30219
fix(rules): update acknowledgment mappings and tests to exactly match…
dkargatzis Feb 26, 2026
aedfa1d
style: fix pre-commit formatting issues in rules and filesystem condi…
dkargatzis Feb 26, 2026
8a239b6
fix(rules): exclude PR author from required code owner reviewers and …
dkargatzis Feb 26, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
39 changes: 12 additions & 27 deletions .watchflow/rules.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,44 +1,29 @@
rules:
# Essential Open Source Rules
- description: "Pull requests must have descriptive titles following conventional commit format"
- description: "PRs must reference a linked issue in the description or title (e.g. Fixes #123)."
enabled: true
severity: "medium"
severity: "high"
event_types: ["pull_request"]
parameters:
title_pattern: "^feat|^fix|^docs|^style|^refactor|^test|^chore|^perf|^ci|^build|^revert"
require_linked_issue: true

- description: "New contributors require approval from at least one past contributor"
- description: "When a PR modifies paths with CODEOWNERS, those owners must be added as reviewers."
enabled: true
severity: "medium"
severity: "high"
event_types: ["pull_request"]
parameters:
min_past_contributors: 1
require_code_owner_reviewers: true

- description: "Code changes must include corresponding tests"
- description: "PR total lines changed must not exceed a maximum (e.g. 500 lines)."
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"]
parameters:
critical_owners: []
max_pr_loc: 500

- description: "When a PR modifies paths with CODEOWNERS, those owners must be added as reviewers"
- description: "PR titles must follow a pattern (e.g. feat:, fix:, docs:) and descriptions must align diff."
enabled: true
severity: "high"
severity: "medium"
event_types: ["pull_request"]
parameters:
require_code_owner_reviewers: true

- description: "No direct pushes to main branch - all changes must go through PRs"
enabled: true
severity: "critical"
event_types: ["push"]
parameters:
no_force_push: true
title_pattern: "^feat|^fix|^docs|^style|^refactor|^test|^chore|^perf|^ci|^build|^revert"
Copy link

@coderabbitai coderabbitai bot Feb 26, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

cat -n .watchflow/rules.yaml

Repository: warestack/watchflow

Length of output: 1258

title_pattern does not enforce the colon required by Conventional Commits standard.

Line 23 description shows examples with colons (feat:, fix:, docs:), but the pattern allows any string starting with those prefixes. Update the pattern to require the colon and proper format.

Suggested pattern update 
-      title_pattern: "^feat|^fix|^docs|^style|^refactor|^test|^chore|^perf|^ci|^build|^revert"
+      title_pattern: "^(feat|fix|docs|style|refactor|test|chore|perf|ci|build|revert)(\\([^)]+\\))?:\\s.+$"
📝 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.

Suggested change
title_pattern: "^feat|^fix|^docs|^style|^refactor|^test|^chore|^perf|^ci|^build|^revert"
title_pattern: "^(feat|fix|docs|style|refactor|test|chore|perf|ci|build|revert)(\\([^)]+\\))?:\\s.+$"
🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In @.watchflow/rules.yaml at line 28, The title_pattern currently accepts
prefixes without the required colon; update the regex for the title_pattern key
to enforce Conventional Commits (type, optional scope, colon, space, and
description). Replace the value of title_pattern with a pattern like
^(feat|fix|docs|style|refactor|test|chore|perf|ci|build|revert)(\([\w\/\-\s]+\))?:\s.+$
so the rule (title_pattern) requires a colon and a non-empty description after
an optional scope.

min_description_length: 50
Comment on lines +23 to +29
Copy link

@coderabbitai coderabbitai bot Feb 26, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Rule description currently overstates enforcement scope.

The text says descriptions “must align diff,” but configured parameters only enforce title_pattern and min_description_length.

📝 Suggested wording tweak 
-  - description: "PR titles must follow a pattern (e.g. feat:, fix:, docs:) and descriptions must align diff."
+  - description: "PR titles must follow a pattern (e.g. feat:, fix:, docs:) and descriptions must meet minimum detail requirements."
📝 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.

Suggested change
- description: "PR titles must follow a pattern (e.g. feat:, fix:, docs:) and descriptions must align diff."
enabled: true
severity: "high"
severity: "medium"
event_types: ["pull_request"]
parameters:
require_code_owner_reviewers: true
- description: "No direct pushes to main branch - all changes must go through PRs"
enabled: true
severity: "critical"
event_types: ["push"]
parameters:
no_force_push: true
title_pattern: "^feat|^fix|^docs|^style|^refactor|^test|^chore|^perf|^ci|^build|^revert"
min_description_length: 50
- description: "PR titles must follow a pattern (e.g. feat:, fix:, docs:) and descriptions must meet minimum detail requirements."
enabled: true
severity: "medium"
event_types: ["pull_request"]
parameters:
title_pattern: "^feat|^fix|^docs|^style|^refactor|^test|^chore|^perf|^ci|^build|^revert"
min_description_length: 50
🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In @.watchflow/rules.yaml around lines 23 - 29, Update the rule description
string so it accurately reflects what the rule enforces: mention that only the
PR title is validated against the title_pattern parameter and the PR body is
checked for a minimum length via min_description_length, and remove or reword
the misleading phrase "must align diff"; reference the rule's parameters
title_pattern and min_description_length and keep the description concise and
accurate about enforcement scope.

6 changes: 4 additions & 2 deletions src/presentation/github_formatter.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,8 @@ def format_check_run_output(
text += f"## {emoji} {severity.title()} Severity\n\n"

for violation in severity_groups[severity]:
text += f"### {violation.rule_description or 'Unknown Rule'}\n"
text += f"### {violation.rule_description or 'Unknown Rule'}\n\n"
text += f"{violation.message}\n\n"
text += f"Rule validation failed with severity: **{violation.severity}**\n"
if violation.how_to_fix:
text += f"**How to fix:** {violation.how_to_fix}\n"
Expand Down Expand Up @@ -169,7 +170,8 @@ def format_violations_comment(violations: list[Violation]) -> str:
comment += f"### {emoji} {severity.title()} Severity\n\n"

for violation in severity_groups[severity]:
comment += f"**{violation.rule_description or 'Unknown Rule'}**\n"
comment += f"### {violation.rule_description or 'Unknown Rule'}\n\n"
comment += f"{violation.message}\n\n"
comment += f"Rule validation failed with severity: **{violation.severity}**\n"
if violation.how_to_fix:
comment += f"**How to fix:** {violation.how_to_fix}\n"
Expand Down
20 changes: 10 additions & 10 deletions src/rules/acknowledgment.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,17 +38,17 @@ class RuleID(StrEnum):

# Mapping from violation text patterns to RuleID
VIOLATION_TEXT_TO_RULE_MAPPING: dict[str, RuleID] = {
"Pull request does not have the minimum required": RuleID.MIN_PR_APPROVALS,
"Pull request is missing required label": RuleID.REQUIRED_LABELS,
"Pull request title does not match the required pattern": RuleID.PR_TITLE_PATTERN,
"Pull request description is too short": RuleID.PR_DESCRIPTION_REQUIRED,
"Individual files cannot exceed": RuleID.FILE_SIZE_LIMIT,
"Pull request exceeds maximum lines changed": RuleID.MAX_PR_LOC,
"approvals, requires": RuleID.MIN_PR_APPROVALS,
"Missing required labels": RuleID.REQUIRED_LABELS,
"does not match required pattern": RuleID.PR_TITLE_PATTERN,
"PR description is too short": RuleID.PR_DESCRIPTION_REQUIRED,
"exceed size limit of": RuleID.FILE_SIZE_LIMIT,
"exceeds maximum lines changed": RuleID.MAX_PR_LOC,
"does not reference a linked issue": RuleID.REQUIRE_LINKED_ISSUE,
"Force pushes are not allowed": RuleID.NO_FORCE_PUSH,
"Direct pushes to main/master branches": RuleID.PROTECTED_BRANCH_PUSH,
"Paths without a code owner in CODEOWNERS": RuleID.PATH_HAS_CODE_OWNER,
"Code owners for modified paths must be added as reviewers": RuleID.REQUIRE_CODE_OWNER_REVIEWERS,
"Force push detected": RuleID.NO_FORCE_PUSH,
"targets protected branch": RuleID.PROTECTED_BRANCH_PUSH,
"Paths without a code owner": RuleID.PATH_HAS_CODE_OWNER,
"Code owners for modified paths": RuleID.REQUIRE_CODE_OWNER_REVIEWERS,
}

# Mapping from RuleID to human-readable descriptions
Expand Down
5 changes: 5 additions & 0 deletions src/rules/conditions/access_control.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -298,6 +298,11 @@ def _required_code_owner_reviewers(event: dict[str, Any]) -> tuple[list[str], li
requested_logins = {u.get("login") for u in requested_users if u.get("login")}
requested_slugs = {t.get("slug") for t in requested_teams if t.get("slug")}

# The PR author is inherently a reviewer of their own code
author_login = pr.get("user", {}).get("login")
if author_login:
requested_logins.add(author_login)

# Owner can be a user (login) or a team (slug or org/slug). Match user by login, team by slug.
requested_identifiers = requested_logins | requested_slugs

Expand Down
12 changes: 8 additions & 4 deletions src/rules/conditions/filesystem.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -225,9 +225,9 @@ class MaxPrLocCondition(BaseCondition):

name = "max_pr_loc"
description = "Validates that total lines changed (additions + deletions) in a PR do not exceed a maximum; enforces a maximum LOC per pull request."
parameter_patterns = ["max_lines"]
parameter_patterns = ["max_lines", "max_pr_loc", "max_changed_lines"]
event_types = ["pull_request"]
examples = [{"max_lines": 500}, {"max_lines": 1000}]
examples = [{"max_lines": 500}, {"max_pr_loc": 1000}]

async def evaluate(self, context: Any) -> list[Violation]:
"""Evaluate max PR LOC condition.
Expand All @@ -241,7 +241,9 @@ async def evaluate(self, context: Any) -> list[Violation]:
parameters = context.get("parameters", {})
event = context.get("event", {})

max_lines = parameters.get("max_lines", 0)
max_lines = (
parameters.get("max_lines") or parameters.get("max_pr_loc") or parameters.get("max_changed_lines") or 0
)
Comment on lines +244 to +246
Copy link

@coderabbitai coderabbitai bot Feb 26, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Read the relevant sections of the file
echo "=== Lines 228-250 (evaluate method) ==="
sed -n '228,250p' src/rules/conditions/filesystem.py

echo ""
echo "=== Lines 265-280 (validate method) ==="
sed -n '265,280p' src/rules/conditions/filesystem.py

Repository: warestack/watchflow

Length of output: 1664

🏁 Script executed:

#!/bin/bash
# Check the full structure around these lines to understand the methods better
wc -l src/rules/conditions/filesystem.py

Repository: warestack/watchflow

Length of output: 101

🏁 Script executed:

#!/bin/bash
# Search for usages of the parameters (max_lines, max_pr_loc, max_changed_lines) to understand intent
rg "(max_lines|max_pr_loc|max_changed_lines)" src/rules/conditions/filesystem.py -B 2 -A 2

Repository: warestack/watchflow

Length of output: 2196

🏁 Script executed:

#!/bin/bash
# Check if there are tests that use these parameters with explicit 0
find . -name "*.py" -type f | xargs rg "max_lines.*0|max_pr_loc.*0|max_changed_lines.*0" -l

Repository: warestack/watchflow

Length of output: 148

🏁 Script executed:

#!/bin/bash
# Check the test file to understand the expected behavior
echo "=== Test file content for max_pr_loc ==="
rg "MaxPrLocCondition|max_lines|max_pr_loc|max_changed_lines" tests/unit/rules/conditions/test_filesystem.py -B 1 -A 3

Repository: warestack/watchflow

Length of output: 3063

🏁 Script executed:

#!/bin/bash
# Look for any documentation or configuration examples
find . -name "*.md" -o -name "*.yaml" -o -name "*.yml" | xargs rg "max_lines|max_pr_loc|max_changed_lines" 2>/dev/null || echo "No matches in docs"

Repository: warestack/watchflow

Length of output: 2511

🏁 Script executed:

#!/bin/bash
# Check if there's any explicit 0 usage in test fixtures or examples
rg "max_lines.*0|max_pr_loc.*0|max_changed_lines.*0" tests/ -B 3 -A 3

Repository: warestack/watchflow

Length of output: 5105

Extract max_lines alias resolution to a static method to fix precedence logic and eliminate duplication.

The current or chain treats 0 as missing, causing an explicit disable value to be silently overridden if multiple aliases coexist in the parameters dict. The test test_evaluate_missing_max_lines_returns_empty confirms max_lines: 0 is intended to disable the check. This pattern is duplicated across evaluate and validate methods.

Refactor to fix precedence 
+    `@staticmethod`
+    def _resolve_max_lines(parameters: dict[str, Any]) -> int:
+        for key in ("max_lines", "max_pr_loc", "max_changed_lines"):
+            if key in parameters and parameters[key] is not None:
+                return int(parameters[key])
+        return 0
+
     async def evaluate(self, context: Any) -> list[Violation]:
-        max_lines = (
-            parameters.get("max_lines") or parameters.get("max_pr_loc") or parameters.get("max_changed_lines") or 0
-        )
+        max_lines = self._resolve_max_lines(parameters)

     async def validate(self, parameters: dict[str, Any], event: dict[str, Any]) -> bool:
-        max_lines = (
-            parameters.get("max_lines") or parameters.get("max_pr_loc") or parameters.get("max_changed_lines") or 0
-        )
+        max_lines = self._resolve_max_lines(parameters)
🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@src/rules/conditions/filesystem.py` around lines 244 - 246, The code
incorrectly uses an "or" chain to derive max_lines (treating 0 as falsy) and
duplicates this logic in evaluate and validate; add a static helper (e.g.,
Filesystem._resolve_max_lines(parameters)) that checks parameter keys in the
intended precedence (check "max_lines", then "max_pr_loc", then
"max_changed_lines") using explicit "in" membership tests and returns the exact
value (including 0) or None/default, then replace the duplicated inline
expression in both evaluate and validate with calls to this helper to preserve
precedence and handle explicit 0 correctly.

if not max_lines:
logger.debug("MaxPrLocCondition: No max_lines specified, skipping validation")
return []
Expand All @@ -266,7 +268,9 @@ async def evaluate(self, context: Any) -> list[Violation]:

async def validate(self, parameters: dict[str, Any], event: dict[str, Any]) -> bool:
"""Legacy validation interface for backward compatibility."""
max_lines = parameters.get("max_lines", 0)
max_lines = (
parameters.get("max_lines") or parameters.get("max_pr_loc") or parameters.get("max_changed_lines") or 0
)
if not max_lines:
return True

Expand Down
3 changes: 0 additions & 3 deletions src/rules/loaders/github_loader.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -94,9 +94,6 @@ def _parse_rule(rule_data: dict[str, Any]) -> Rule:
# Get parameters (strip internal "validator" key; engine infers validator from parameter names)
parameters = dict(rule_data.get("parameters", {}))
parameters.pop("validator", None)
# Normalize aliases so conditions match (e.g. max_changed_lines -> max_lines for MaxPrLocCondition)
if "max_changed_lines" in parameters and "max_lines" not in parameters:
parameters["max_lines"] = parameters["max_changed_lines"]

# Instantiate conditions using Registry (matches on parameter keys, e.g. max_lines, require_linked_issue)
conditions = ConditionRegistry.get_conditions_for_parameters(parameters)
Expand Down
2 changes: 1 addition & 1 deletion tests/unit/event_processors/pull_request/test_enricher.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ async def test_enrich_event_data(enricher, mock_task, mock_github_client):
async def test_fetch_acknowledgments(enricher, mock_github_client):
mock_github_client.get_issue_comments.return_value = [
{
"body": "🚨 Watchflow Rule Violations Detected\n\n**Reason:** valid reason\n\n---\nThe following violations have been overridden:\n• **Rule** - Pull request does not have the minimum required approvals\n",
"body": "🚨 Watchflow Rule Violations Detected\n\n**Reason:** valid reason\n\n---\nThe following violations have been overridden:\n• PR has 1 approvals, requires 2\n",
Copy link

@coderabbitai coderabbitai bot Feb 26, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Grammatical issue in violation text: "1 approvals" should be "1 approval".

The test expectation contains "PR has 1 approvals, requires 2" which has incorrect pluralization. If this reflects the actual production output, the source should be fixed to use proper grammar (e.g., "PR has 1 approval, requires 2" or dynamic pluralization).

🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@tests/unit/event_processors/pull_request/test_enricher.py` at line 68, The
violation text incorrectly uses "1 approvals"; update the pluralization so the
message uses "approval" when the count equals 1 and "approvals" otherwise.
Locate the code that composes the violation line (the place that sets the "body"
string in the PR watcher/enricher logic or helper that formats lines like "PR
has {n} approvals, requires {m}") and change it to choose singular for n==1,
then update the test expectation in
tests/unit/event_processors/pull_request/test_enricher.py to assert "PR has 1
approval, requires 2".

"user": {"login": "reviewer"},
}
]
Expand Down
6 changes: 3 additions & 3 deletions tests/unit/presentation/test_github_formatter.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,9 +19,9 @@ def test_format_violations_comment_groups_by_severity():
assert "## 🚨 Watchflow Rule Violations Detected" in comment
assert "### 🔴 Critical Severity" in comment
assert "### 🟠 High Severity" in comment
assert "**Rule 2**" in comment
assert "**Rule 1**" in comment
assert "**Rule 3**" in comment
assert "### Rule 2" in comment
assert "### Rule 1" in comment
assert "### Rule 3" in comment
assert "Fix 1" in comment
assert "Fix 2" in comment

Expand Down
26 changes: 13 additions & 13 deletions tests/unit/rules/test_acknowledgment.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -140,18 +140,18 @@ class TestMapViolationTextToRuleId:
@pytest.mark.parametrize(
"text,expected_rule_id",
[
("Pull request does not have the minimum required approvals", RuleID.MIN_PR_APPROVALS),
("Pull request is missing required label: security", RuleID.REQUIRED_LABELS),
("Pull request title does not match the required pattern", RuleID.PR_TITLE_PATTERN),
("Pull request description is too short (20 chars)", RuleID.PR_DESCRIPTION_REQUIRED),
("Individual files cannot exceed 10MB limit", RuleID.FILE_SIZE_LIMIT),
("PR has 1 approvals, requires 2", RuleID.MIN_PR_APPROVALS),
("Missing required labels: security", RuleID.REQUIRED_LABELS),
("PR title 'foo' does not match required pattern '^feat'", RuleID.PR_TITLE_PATTERN),
("PR description is too short", RuleID.PR_DESCRIPTION_REQUIRED),
("Files exceed size limit of 10MB", RuleID.FILE_SIZE_LIMIT),
("Pull request exceeds maximum lines changed (1234 > 500)", RuleID.MAX_PR_LOC),
(
"PR does not reference a linked issue (e.g. #123 or closes #123 in body/title)",
RuleID.REQUIRE_LINKED_ISSUE,
),
("Force pushes are not allowed on this branch", RuleID.NO_FORCE_PUSH),
("Direct pushes to main/master branches prohibited", RuleID.PROTECTED_BRANCH_PUSH),
("Force push detected on protected branch", RuleID.NO_FORCE_PUSH),
("PR targets protected branch 'main'", RuleID.PROTECTED_BRANCH_PUSH),
("Paths without a code owner in CODEOWNERS: src/bar.py", RuleID.PATH_HAS_CODE_OWNER),
(
"Code owners for modified paths must be added as reviewers: alice",
Expand All @@ -175,7 +175,7 @@ class TestMapViolationTextToRuleDescription:

def test_maps_to_description(self):
"""Should map violation text to human-readable description."""
text = "Pull request does not have the minimum required approvals"
text = "PR has 1 approvals, requires 2"
description = map_violation_text_to_rule_description(text)
assert description == "Pull requests require at least 2 approvals"

Expand All @@ -193,7 +193,7 @@ def test_parses_single_violation(self):
**Reason:** Emergency fix

The following violations have been overridden:
• Pull request does not have the minimum required approvals
• PR has 1 approvals, requires 2

---
*This acknowledgment was validated.*"""
Expand All @@ -211,8 +211,8 @@ def test_parses_multiple_violations(self):
**Reason:** Sprint deadline

The following violations have been overridden:
• Pull request does not have the minimum required approvals
• Pull request is missing required label: review
• PR has 1 approvals, requires 2
• Missing required labels: review

---"""

Expand All @@ -230,7 +230,7 @@ def test_empty_comment_returns_empty_list(self):
def test_returns_acknowledgment_models(self):
"""Should return proper Acknowledgment model instances."""
comment = """The following violations have been overridden:
• Force pushes are not allowed"""
• Force push detected on protected branch"""

acknowledgments = parse_acknowledgment_comment(comment, "admin")

Expand All @@ -240,7 +240,7 @@ def test_returns_acknowledgment_models(self):
def test_stops_at_section_delimiter(self):
"""Should stop parsing when hitting section delimiters."""
comment = """The following violations have been overridden:
• Pull request title does not match the required pattern
• PR title 'foo' does not match required pattern '^feat'
---
⚠️ Other content that should be ignored
• Some other bullet that is NOT a violation"""
Expand Down