Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
12 changes: 12 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -8,10 +8,22 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/).

### Fixed

- **`FilePatternCondition._get_changed_files` implementation** -- Replaced
stub that always returned `[]` with a working implementation that extracts
file paths from enriched PR data (`changed_files` list of dicts or plain
strings) and push event commits (`added`/`modified`/`removed` arrays with
deduplication). Added unit tests covering all extraction paths.

## [2026-04-12] -- PR #69

### Fixed

- **Blocking sleep in LLM condition** -- Replaced `time.sleep()` with
`await asyncio.sleep()` in `LLMAssisted` retry backoff to avoid
freezing the event loop during LLM retries.

## [2026-04-08] -- PR #66

### Added

- **AI-powered reviewer recommendation** -- `/reviewers` slash command suggests
Expand Down
37 changes: 27 additions & 10 deletions src/rules/conditions/filesystem.py
Original file line number Diff line number Diff line change
Expand Up @@ -116,16 +116,33 @@ async def validate(self, parameters: dict[str, Any], event: dict[str, Any]) -> b
return len(matching_files) > 0

def _get_changed_files(self, event: dict[str, Any]) -> list[str]:
"""Extract the list of changed files from the event."""
event_type = event.get("event_type", "")
if event_type == "pull_request":
# TODO: Pull request—fetch changed files via GitHub API. Placeholder for now.
return []
elif event_type == "push":
# Push event—files in commits, not implemented.
return []
else:
return []
"""Extract changed file paths from enriched PR data or push commits."""
changed_files = event.get("changed_files", [])
if isinstance(changed_files, list) and changed_files:
extracted: list[str] = []
for item in changed_files:
path = item.get("filename") if isinstance(item, dict) else item
if isinstance(path, str) and path:
extracted.append(path)
if extracted:
return extracted

commits = event.get("commits", [])
if isinstance(commits, list) and commits:
seen: set[str] = set()
for commit in commits:
if not isinstance(commit, dict):
continue
for key in ("added", "modified", "removed"):
paths = commit.get(key, [])
if not isinstance(paths, list):
continue
for path in paths:
if isinstance(path, str) and path:
seen.add(path)
return sorted(seen)
Comment thread
coderabbitai[bot] marked this conversation as resolved.

return []

@staticmethod
def _glob_to_regex(glob_pattern: str) -> str:
Expand Down
99 changes: 99 additions & 0 deletions tests/unit/rules/conditions/test_filesystem.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
Tests for FilePatternCondition, MaxFileSizeCondition, and MaxPrLocCondition classes.
"""

from typing import Any
from unittest.mock import patch

import pytest
Expand Down Expand Up @@ -96,6 +97,104 @@ def test_glob_to_regex_conversion(self) -> None:
assert FilePatternCondition._glob_to_regex("src/*.js") == "^src/.*\\.js$"
assert FilePatternCondition._glob_to_regex("file?.txt") == "^file.\\.txt$"

def test_get_changed_files_from_pr_enriched_data(self) -> None:
"""Test extracting files from enriched PR changed_files (list of dicts)."""
condition = FilePatternCondition()
event = {
"changed_files": [
{"filename": "src/main.py", "status": "modified", "additions": 10, "deletions": 2},
{"filename": "tests/test_main.py", "status": "added", "additions": 30, "deletions": 0},
]
}
result = condition._get_changed_files(event)
assert result == ["src/main.py", "tests/test_main.py"]

def test_get_changed_files_from_pr_plain_strings(self) -> None:
"""Test extracting files when changed_files contains plain strings."""
condition = FilePatternCondition()
event = {"changed_files": ["src/main.py", "README.md"]}
result = condition._get_changed_files(event)
assert result == ["src/main.py", "README.md"]

def test_get_changed_files_from_push_commits(self) -> None:
"""Test extracting files from push event commit arrays."""
condition = FilePatternCondition()
event = {
"commits": [
{"added": ["new_file.py"], "modified": ["src/main.py"], "removed": []},
{"added": [], "modified": ["src/main.py"], "removed": ["old.py"]},
]
}
result = condition._get_changed_files(event)
assert result == ["new_file.py", "old.py", "src/main.py"]

def test_get_changed_files_empty_event(self) -> None:
"""Test that an empty event returns no files."""
condition = FilePatternCondition()
assert condition._get_changed_files({}) == []

def test_get_changed_files_with_malformed_payload(self) -> None:
"""Test that malformed payload entries are filtered out without raising."""
condition = FilePatternCondition()

# changed_files with mixed valid/invalid entries
event_cf: dict[str, Any] = {
"changed_files": [
{"filename": "valid.py", "status": "modified"},
{"status": "added"}, # missing "filename"
None, # type: ignore[list-item]
42, # type: ignore[list-item]
"", # empty string
{"filename": ""}, # empty filename
"also_valid.txt",
]
}
result = condition._get_changed_files(event_cf)
assert result == ["valid.py", "also_valid.txt"]

# commits with non-dict entries and non-list/non-string values
event_commits: dict[str, Any] = {
"commits": [
{"added": ["good.py"], "modified": "not_a_list", "removed": [42, None, "removed.py"]},
"not_a_dict", # type: ignore[list-item]
{"added": [None, "", "another.py"], "modified": [], "removed": []},
]
}
result = condition._get_changed_files(event_commits)
assert result == ["another.py", "good.py", "removed.py"]

@pytest.mark.asyncio
async def test_evaluate_with_real_pr_event(self) -> None:
"""Test full evaluate flow with enriched PR data (no mocking)."""
condition = FilePatternCondition()
context = {
"parameters": {"pattern": "*.py", "condition_type": "files_match_pattern"},
"event": {
"changed_files": [
{"filename": "src/app.py", "status": "modified", "additions": 5, "deletions": 1},
{"filename": "docs/readme.md", "status": "modified", "additions": 2, "deletions": 0},
]
},
}
violations = await condition.evaluate(context)
assert len(violations) == 0

@pytest.mark.asyncio
async def test_evaluate_with_real_push_event(self) -> None:
"""Test full evaluate flow with push commit data (no mocking)."""
condition = FilePatternCondition()
context = {
"parameters": {"pattern": "*.yaml", "condition_type": "files_not_match_pattern"},
"event": {
"commits": [
{"added": ["config/app.yaml"], "modified": [], "removed": []},
]
},
}
violations = await condition.evaluate(context)
assert len(violations) == 1
assert "forbidden pattern" in violations[0].message


class TestMaxFileSizeCondition:
"""Tests for MaxFileSizeCondition class."""
Expand Down
Loading