diff --git a/apps/backend/__tests__/fixtures/gitlab.py b/apps/backend/__tests__/fixtures/gitlab.py new file mode 100644 index 00000000..cd78c8bd --- /dev/null +++ b/apps/backend/__tests__/fixtures/gitlab.py @@ -0,0 +1,243 @@ +""" +GitLab Test Fixtures +==================== + +Mock data and fixtures for GitLab integration tests. +""" + +# Sample GitLab MR data +SAMPLE_MR_DATA = { + "iid": 123, + "id": 12345, + "title": "Add user authentication feature", + "description": "Implement OAuth2 login with Google and GitHub providers", + "author": { + "id": 1, + "username": "john_doe", + "name": "John Doe", + "email": "john@example.com", + }, + "source_branch": "feature/oauth-auth", + "target_branch": "main", + "state": "opened", + "draft": False, + "merge_status": "can_be_merged", + "web_url": "https://gitlab.com/group/project/-/merge_requests/123", + "created_at": "2025-01-14T10:00:00.000Z", + "updated_at": "2025-01-14T12:00:00.000Z", + "labels": ["feature", "authentication"], + "assignees": [], +} + +SAMPLE_MR_CHANGES = { + "id": 12345, + "iid": 123, + "project_id": 1, + "title": "Add user authentication feature", + "description": "Implement OAuth2 login", + "state": "opened", + "created_at": "2025-01-14T10:00:00.000Z", + "updated_at": "2025-01-14T12:00:00.000Z", + "merge_status": "can_be_merged", + "additions": 150, + "deletions": 20, + "changed_files_count": 5, + "changes": [ + { + "old_path": "src/auth/__init__.py", + "new_path": "src/auth/__init__.py", + "diff": "@@ -0,0 +1,5 @@\n+from .oauth import OAuthHandler\n+from .providers import GoogleProvider, GitHubProvider", + "new_file": False, + "renamed_file": False, + "deleted_file": False, + }, + { + "old_path": "src/auth/oauth.py", + "new_path": "src/auth/oauth.py", + "diff": "@@ -0,0 +1,50 @@\n+class OAuthHandler:\n+ def handle_callback(self, request):\n+ pass", + "new_file": True, + "renamed_file": False, + "deleted_file": False, + }, + ], +} + +SAMPLE_MR_COMMITS = [ + { + "id": "abc123def456", + "short_id": "abc123de", + "title": "Add OAuth handler", + "message": "Add OAuth handler", + "author_name": "John Doe", + "author_email": "john@example.com", + "authored_date": "2025-01-14T10:00:00.000Z", + "created_at": "2025-01-14T10:00:00.000Z", + }, + { + "id": "def456ghi789", + "short_id": "def456gh", + "title": "Add Google provider", + "message": "Add Google provider", + "author_name": "John Doe", + "author_email": "john@example.com", + "authored_date": "2025-01-14T11:00:00.000Z", + "created_at": "2025-01-14T11:00:00.000Z", + }, +] + +# Sample GitLab issue data +SAMPLE_ISSUE_DATA = { + "iid": 42, + "id": 42, + "title": "Bug: Login button not working", + "description": "Clicking the login button does nothing", + "author": { + "id": 2, + "username": "jane_smith", + "name": "Jane Smith", + "email": "jane@example.com", + }, + "state": "opened", + "labels": ["bug", "urgent"], + "assignees": [], + "milestone": None, + "web_url": "https://gitlab.com/group/project/-/issues/42", + "created_at": "2025-01-14T09:00:00.000Z", + "updated_at": "2025-01-14T09:30:00.000Z", +} + +# Sample GitLab pipeline data +SAMPLE_PIPELINE_DATA = { + "id": 1001, + "iid": 1, + "project_id": 1, + "ref": "feature/oauth-auth", + "sha": "abc123def456", + "status": "success", + "source": "merge_request_event", + "created_at": "2025-01-14T10:30:00.000Z", + "updated_at": "2025-01-14T10:35:00.000Z", + "finished_at": "2025-01-14T10:35:00.000Z", + "duration": 300, + "web_url": "https://gitlab.com/group/project/-/pipelines/1001", +} + +SAMPLE_PIPELINE_JOBS = [ + { + "id": 5001, + "name": "test", + "stage": "test", + "status": "success", + "started_at": "2025-01-14T10:31:00.000Z", + "finished_at": "2025-01-14T10:34:00.000Z", + "duration": 180, + "allow_failure": False, + }, + { + "id": 5002, + "name": "lint", + "stage": "test", + "status": "success", + "started_at": "2025-01-14T10:31:00.000Z", + "finished_at": "2025-01-14T10:32:00.000Z", + "duration": 60, + "allow_failure": False, + }, +] + +# Sample GitLab discussion/note data +SAMPLE_MR_DISCUSSIONS = [ + { + "id": "d1", + "notes": [ + { + "id": 1001, + "type": "DiscussionNote", + "author": {"username": "coderabbit[bot]"}, + "body": "Consider adding error handling for OAuth failures", + "created_at": "2025-01-14T11:00:00.000Z", + "system": False, + "resolvable": True, + } + ], + } +] + +SAMPLE_MR_NOTES = [ + { + "id": 2001, + "type": "DiscussionNote", + "author": {"username": "reviewer_user"}, + "body": "LGTM, just one comment", + "created_at": "2025-01-14T12:00:00.000Z", + "system": False, + } +] + +# Mock GitLab config +MOCK_GITLAB_CONFIG = { + "token": "glpat-test-token-12345", + "project": "group/project", + "instance_url": "https://gitlab.example.com", +} + + +def mock_mr_data(**overrides): + """Create mock MR data with optional overrides.""" + data = SAMPLE_MR_DATA.copy() + data.update(overrides) + return data + + +def mock_mr_changes(**overrides): + """Create mock MR changes with optional overrides.""" + data = SAMPLE_MR_CHANGES.copy() + data.update(overrides) + return data + + +def mock_issue_data(**overrides): + """Create mock issue data with optional overrides.""" + data = SAMPLE_ISSUE_DATA.copy() + data.update(overrides) + return data + + +def mock_pipeline_data(**overrides): + """Create mock pipeline data with optional overrides.""" + data = SAMPLE_PIPELINE_DATA.copy() + data.update(overrides) + return data + + +def mock_pipeline_jobs(**overrides): + """Create mock pipeline jobs with optional overrides.""" + data = SAMPLE_PIPELINE_JOBS.copy() + if overrides: + data[0].update(overrides) + return data + + +def get_mock_diff() -> str: + """Get a mock diff string for testing.""" + return """diff --git a/src/auth/oauth.py b/src/auth/oauth.py +new file mode 100644 +index 0000000..abc1234 +--- /dev/null ++++ b/src/auth/oauth.py +@@ -0,0 +1,50 @@ ++class OAuthHandler: ++ def handle_callback(self, request): ++ pass +diff --git a/src/auth/providers.py b/src/auth/providers.py +new file mode 100644 +index 0000000..def5678 +--- /dev/null ++++ b/src/auth/providers.py +@@ -0,0 +1,30 @@ ++class GoogleProvider: ++ pass ++ ++class GitHubProvider: ++ pass +""" diff --git a/apps/backend/__tests__/test_gitlab_bot_detection.py b/apps/backend/__tests__/test_gitlab_bot_detection.py new file mode 100644 index 00000000..b0cdcda0 --- /dev/null +++ b/apps/backend/__tests__/test_gitlab_bot_detection.py @@ -0,0 +1,255 @@ +""" +GitLab Bot Detection Tests +========================== + +Tests for bot detection to prevent infinite review loops. +""" + +import json +from datetime import datetime, timezone +from pathlib import Path +from unittest.mock import MagicMock, Mock, patch + +import pytest +from tests.fixtures.gitlab import ( + MOCK_GITLAB_CONFIG, + mock_mr_data, +) + + +class TestBotDetector: + """Test bot detection prevents infinite loops.""" + + @pytest.fixture + def detector(self, tmp_path): + """Create a BotDetector instance for testing.""" + from runners.gitlab.bot_detection import BotDetector + + return BotDetector( + state_dir=tmp_path, + bot_username="auto-claude-bot", + review_own_mrs=False, + ) + + def test_bot_detection_init(self, detector): + """Test detector initializes correctly.""" + assert detector.bot_username == "auto-claude-bot" + assert detector.review_own_mrs is False + assert detector.state.reviewed_commits == {} + + def test_is_bot_mr_self_authored(self, detector): + """Test MR authored by bot is detected.""" + mr_data = mock_mr_data(author="auto-claude-bot") + + assert detector.is_bot_mr(mr_data) is True + + def test_is_bot_mr_pattern_match(self, detector): + """Test MR with bot pattern in username is detected.""" + mr_data = mock_mr_data(author="coderabbit[bot]") + + assert detector.is_bot_mr(mr_data) is True + + def test_is_bot_mr_human_authored(self, detector): + """Test MR authored by human is not detected as bot.""" + mr_data = mock_mr_data(author="john_doe") + + assert detector.is_bot_mr(mr_data) is False + + def test_is_bot_commit_self_authored(self, detector): + """Test commit by bot is detected.""" + commit = { + "author": {"username": "auto-claude-bot"}, + "message": "Fix issue", + } + + assert detector.is_bot_commit(commit) is True + + def test_is_bot_commit_ai_coauthored(self, detector): + """Test commit with AI co-authorship is detected.""" + commit = { + "author": {"username": "human"}, + "message": "Co-authored-by: claude ", + } + + assert detector.is_bot_commit(commit) is True + + def test_is_bot_commit_human(self, detector): + """Test human commit is not detected as bot.""" + commit = { + "author": {"username": "john_doe"}, + "message": "Fix bug", + } + + assert detector.is_bot_commit(commit) is False + + def test_should_skip_mr_bot_authored(self, detector): + """Test should skip MR when bot authored.""" + mr_data = mock_mr_data(author="auto-claude-bot") + commits = [] + + should_skip, reason = detector.should_skip_mr_review(123, mr_data, commits) + + assert should_skip is True + assert "auto-claude-bot" in reason.lower() + + def test_should_skip_mr_in_cooling_off(self, detector): + """Test should skip MR when in cooling off period.""" + # First, mark as reviewed + detector.mark_reviewed(123, "abc123") + + # Immediately try to review again + mr_data = mock_mr_data() + commits = [{"id": "abc123", "sha": "abc123"}] + + should_skip, reason = detector.should_skip_mr_review(123, mr_data, commits) + + assert should_skip is True + assert "cooling" in reason.lower() + + def test_should_skip_mr_already_reviewed(self, detector): + """Test should skip MR when commit already reviewed.""" + # Mark as reviewed + detector.mark_reviewed(123, "abc123") + + # Try to review same commit + mr_data = mock_mr_data() + commits = [{"id": "abc123", "sha": "abc123"}] + + # Wait past cooling off (manually update time) + detector.state.last_review_times["123"] = ( + datetime.now(timezone.utc) - __import__("datetime").timedelta(minutes=10) + ).isoformat() + + should_skip, reason = detector.should_skip_mr_review(123, mr_data, commits) + + assert should_skip is True + assert "already reviewed" in reason.lower() + + def test_should_not_skip_safe_mr(self, detector): + """Test should not skip when MR is safe to review.""" + mr_data = mock_mr_data() + commits = [{"id": "new123", "sha": "new123"}] + + should_skip, reason = detector.should_skip_mr_review(456, mr_data, commits) + + assert should_skip is False + assert reason == "" + + def test_mark_reviewed(self, detector): + """Test marking MR as reviewed.""" + detector.mark_reviewed(123, "abc123") + + assert "123" in detector.state.reviewed_commits + assert "abc123" in detector.state.reviewed_commits["123"] + assert "123" in detector.state.last_review_times + + def test_mark_reviewed_multiple_commits(self, detector): + """Test marking multiple commits for same MR.""" + detector.mark_reviewed(123, "commit1") + detector.mark_reviewed(123, "commit2") + detector.mark_reviewed(123, "commit3") + + assert len(detector.state.reviewed_commits["123"]) == 3 + + def test_clear_mr_state(self, detector): + """Test clearing MR state.""" + detector.mark_reviewed(123, "abc123") + detector.clear_mr_state(123) + + assert "123" not in detector.state.reviewed_commits + assert "123" not in detector.state.last_review_times + + def test_get_stats(self, detector): + """Test getting detector statistics.""" + detector.mark_reviewed(123, "abc123") + detector.mark_reviewed(124, "def456") + + stats = detector.get_stats() + + assert stats["bot_username"] == "auto-claude-bot" + assert stats["total_mrs_tracked"] == 2 + assert stats["total_reviews_performed"] == 2 + + def test_cleanup_stale_mrs(self, detector): + """Test cleanup of old MR state.""" + # Add an old MR (manually set old timestamp) + old_time = ( + datetime.now(timezone.utc) - __import__("datetime").timedelta(days=40) + ).isoformat() + detector.state.last_review_times["999"] = old_time + detector.state.reviewed_commits["999"] = ["old123"] + + # Add a recent MR + detector.mark_reviewed(123, "abc123") + + cleaned = detector.cleanup_stale_mrs(max_age_days=30) + + assert cleaned == 1 + assert "999" not in detector.state.reviewed_commits + assert "123" in detector.state.reviewed_commits + + def test_state_persistence(self, tmp_path): + """Test state is saved and loaded correctly.""" + from runners.gitlab.bot_detection import BotDetector + + # Create detector and mark as reviewed + detector1 = BotDetector( + state_dir=tmp_path, + bot_username="test-bot", + ) + detector1.mark_reviewed(123, "abc123") + + # Create new detector instance (should load state) + detector2 = BotDetector( + state_dir=tmp_path, + bot_username="test-bot", + ) + + assert "123" in detector2.state.reviewed_commits + assert "abc123" in detector2.state.reviewed_commits["123"] + + +class TestBotDetectionState: + """Test BotDetectionState model.""" + + def test_to_dict(self): + """Test converting state to dictionary.""" + from runners.gitlab.bot_detection import BotDetectionState + + state = BotDetectionState( + reviewed_commits={"123": ["abc123", "def456"]}, + last_review_times={"123": "2025-01-14T10:00:00"}, + ) + + data = state.to_dict() + + assert data["reviewed_commits"]["123"] == ["abc123", "def456"] + + def test_from_dict(self): + """Test loading state from dictionary.""" + from runners.gitlab.bot_detection import BotDetectionState + + data = { + "reviewed_commits": {"123": ["abc123"]}, + "last_review_times": {"123": "2025-01-14T10:00:00"}, + } + + state = BotDetectionState.from_dict(data) + + assert state.reviewed_commits["123"] == ["abc123"] + assert state.last_review_times["123"] == "2025-01-14T10:00:00" + + def test_save_and_load(self, tmp_path): + """Test saving and loading state from disk.""" + from runners.gitlab.bot_detection import BotDetectionState + + state = BotDetectionState( + reviewed_commits={"123": ["abc123"]}, + last_review_times={"123": "2025-01-14T10:00:00"}, + ) + + state.save(tmp_path) + + loaded = BotDetectionState.load(tmp_path) + + assert loaded.reviewed_commits["123"] == ["abc123"] diff --git a/apps/backend/__tests__/test_gitlab_ci_checker.py b/apps/backend/__tests__/test_gitlab_ci_checker.py new file mode 100644 index 00000000..1ba73d54 --- /dev/null +++ b/apps/backend/__tests__/test_gitlab_ci_checker.py @@ -0,0 +1,376 @@ +""" +GitLab CI Checker Tests +======================== + +Tests for CI/CD pipeline status checking. +""" + +from datetime import datetime, timezone +from pathlib import Path +from unittest.mock import MagicMock, Mock, patch + +import pytest +from tests.fixtures.gitlab import ( + MOCK_GITLAB_CONFIG, + mock_mr_data, + mock_pipeline_data, + mock_pipeline_jobs, +) + + +class TestCIChecker: + """Test CI/CD pipeline checking functionality.""" + + @pytest.fixture + def checker(self, tmp_path): + """Create a CIChecker instance for testing.""" + from runners.gitlab.glab_client import GitLabConfig + from runners.gitlab.services.ci_checker import CIChecker + + config = GitLabConfig( + token="test-token", + project="group/project", + instance_url="https://gitlab.example.com", + ) + + with patch("runners.gitlab.services.ci_checker.GitLabClient"): + return CIChecker( + project_dir=tmp_path, + config=config, + ) + + def test_init(self, checker): + """Test checker initializes correctly.""" + assert checker.client is not None + + def test_check_mr_pipeline_success(self, checker): + """Test checking MR with successful pipeline.""" + pipeline_data = mock_pipeline_data(status="success") + + async def mock_get_pipelines(mr_iid): + return [pipeline_data] + + async def mock_get_pipeline_status(pipeline_id): + return pipeline_data + + async def mock_get_pipeline_jobs(pipeline_id): + return mock_pipeline_jobs() + + # Setup async mocks + import asyncio + + async def test(): + with patch.object( + checker.client, "get_mr_pipelines_async", mock_get_pipelines + ): + with patch.object( + checker.client, + "get_pipeline_status_async", + mock_get_pipeline_status, + ): + with patch.object( + checker.client, + "get_pipeline_jobs_async", + mock_get_pipeline_jobs, + ): + pipeline = await checker.check_mr_pipeline(123) + + assert pipeline is not None + assert pipeline.pipeline_id == 1001 + assert pipeline.status.value == "success" + assert pipeline.has_failures is False + + asyncio.run(test()) + + def test_check_mr_pipeline_failed(self, checker): + """Test checking MR with failed pipeline.""" + pipeline_data = mock_pipeline_data(status="failed") + jobs_data = mock_pipeline_jobs() + jobs_data[0]["status"] = "failed" + + import asyncio + + async def test(): + async def mock_get_pipelines(mr_iid): + return [pipeline_data] + + async def mock_get_pipeline_status(pipeline_id): + return pipeline_data + + async def mock_get_pipeline_jobs(pipeline_id): + return jobs_data + + with patch.object( + checker.client, "get_mr_pipelines_async", mock_get_pipelines + ): + with patch.object( + checker.client, + "get_pipeline_status_async", + mock_get_pipeline_status, + ): + with patch.object( + checker.client, + "get_pipeline_jobs_async", + mock_get_pipeline_jobs, + ): + pipeline = await checker.check_mr_pipeline(123) + + assert pipeline.has_failures is True + assert pipeline.is_blocking is True + + asyncio.run(test()) + + def test_check_mr_pipeline_no_pipeline(self, checker): + """Test checking MR with no pipeline.""" + import asyncio + + async def test(): + async def mock_get_pipelines(mr_iid): + return [] + + with patch.object( + checker.client, "get_mr_pipelines_async", mock_get_pipelines + ): + pipeline = await checker.check_mr_pipeline(123) + + assert pipeline is None + + asyncio.run(test()) + + def test_get_blocking_reason_success(self, checker): + """Test getting blocking reason for successful pipeline.""" + from runners.gitlab.services.ci_checker import PipelineInfo, PipelineStatus + + pipeline = PipelineInfo( + pipeline_id=1001, + status=PipelineStatus.SUCCESS, + ref="main", + sha="abc123", + created_at="2025-01-14T10:00:00", + updated_at="2025-01-14T10:05:00", + failed_jobs=[], + ) + + reason = checker.get_blocking_reason(pipeline) + + assert reason == "" + + def test_get_blocking_reason_failed(self, checker): + """Test getting blocking reason for failed pipeline.""" + from runners.gitlab.services.ci_checker import ( + JobStatus, + PipelineInfo, + PipelineStatus, + ) + + pipeline = PipelineInfo( + pipeline_id=1001, + status=PipelineStatus.FAILED, + ref="main", + sha="abc123", + created_at="2025-01-14T10:00:00", + updated_at="2025-01-14T10:05:00", + failed_jobs=[ + JobStatus( + name="test", + status="failed", + stage="test", + failure_reason="AssertionError", + ) + ], + ) + + reason = checker.get_blocking_reason(pipeline) + + assert "failed" in reason.lower() + + def test_format_pipeline_summary(self, checker): + """Test formatting pipeline summary.""" + from runners.gitlab.services.ci_checker import ( + JobStatus, + PipelineInfo, + PipelineStatus, + ) + + pipeline = PipelineInfo( + pipeline_id=1001, + status=PipelineStatus.SUCCESS, + ref="main", + sha="abc123", + created_at="2025-01-14T10:00:00", + updated_at="2025-01-14T10:05:00", + duration=300, + jobs=[ + JobStatus( + name="test", + status="success", + stage="test", + ), + JobStatus( + name="lint", + status="success", + stage="lint", + ), + ], + ) + + summary = checker.format_pipeline_summary(pipeline) + + assert "Pipeline #1001" in summary + assert "SUCCESS" in summary + assert "2 total" in summary + + def test_security_scan_detection(self, checker): + """Test detection of security scan failures.""" + from runners.gitlab.services.ci_checker import JobStatus + + jobs = [ + JobStatus( + name="sast", + status="failed", + stage="test", + failure_reason="Vulnerability found", + ), + JobStatus( + name="secret_detection", + status="failed", + stage="test", + failure_reason="Secret leaked", + ), + JobStatus( + name="test", + status="success", + stage="test", + ), + ] + + issues = checker._check_security_scans(jobs) + + assert len(issues) == 2 + assert any(i["type"] == "Static Application Security Testing" for i in issues) + assert any(i["type"] == "Secret Detection" for i in issues) + + +class TestPipelineStatus: + """Test PipelineStatus enum.""" + + def test_status_values(self): + """Test all status values exist.""" + from runners.gitlab.services.ci_checker import PipelineStatus + + assert PipelineStatus.PENDING.value == "pending" + assert PipelineStatus.RUNNING.value == "running" + assert PipelineStatus.SUCCESS.value == "success" + assert PipelineStatus.FAILED.value == "failed" + assert PipelineStatus.CANCELED.value == "canceled" + + +class TestJobStatus: + """Test JobStatus model.""" + + def test_job_status_creation(self): + """Test creating JobStatus.""" + from runners.gitlab.services.ci_checker import JobStatus + + job = JobStatus( + name="test", + status="success", + stage="test", + started_at="2025-01-14T10:00:00", + finished_at="2025-01-14T10:01:00", + duration=60, + ) + + assert job.name == "test" + assert job.status == "success" + assert job.duration == 60 + + +class TestPipelineInfo: + """Test PipelineInfo model.""" + + def test_pipeline_info_creation(self): + """Test creating PipelineInfo.""" + from runners.gitlab.services.ci_checker import PipelineInfo, PipelineStatus + + pipeline = PipelineInfo( + pipeline_id=1001, + status=PipelineStatus.SUCCESS, + ref="main", + sha="abc123", + created_at="2025-01-14T10:00:00", + updated_at="2025-01-14T10:05:00", + ) + + assert pipeline.pipeline_id == 1001 + assert pipeline.has_failures is False + assert pipeline.is_blocking is False + + def test_has_failures_property(self): + """Test has_failures property.""" + from runners.gitlab.services.ci_checker import ( + JobStatus, + PipelineInfo, + PipelineStatus, + ) + + pipeline = PipelineInfo( + pipeline_id=1001, + status=PipelineStatus.FAILED, + ref="main", + sha="abc123", + created_at="2025-01-14T10:00:00", + updated_at="2025-01-14T10:05:00", + failed_jobs=[ + JobStatus(name="test", status="failed", stage="test"), + ], + ) + + assert pipeline.has_failures is True + assert len(pipeline.failed_jobs) == 1 + + def test_is_blocking_success(self): + """Test is_blocking for successful pipeline.""" + from runners.gitlab.services.ci_checker import PipelineInfo, PipelineStatus + + pipeline = PipelineInfo( + pipeline_id=1001, + status=PipelineStatus.SUCCESS, + ref="main", + sha="abc123", + created_at="2025-01-14T10:00:00", + updated_at="2025-01-14T10:05:00", + ) + + assert pipeline.is_blocking is False + + def test_is_blocking_failed(self): + """Test is_blocking for failed pipeline.""" + from runners.gitlab.services.ci_checker import PipelineInfo, PipelineStatus + + pipeline = PipelineInfo( + pipeline_id=1001, + status=PipelineStatus.FAILED, + ref="main", + sha="abc123", + created_at="2025-01-14T10:00:00", + updated_at="2025-01-14T10:05:00", + ) + + assert pipeline.is_blocking is True + + def test_is_blocking_running(self): + """Test is_blocking for running pipeline.""" + from runners.gitlab.services.ci_checker import PipelineInfo, PipelineStatus + + pipeline = PipelineInfo( + pipeline_id=1001, + status=PipelineStatus.RUNNING, + ref="main", + sha="abc123", + created_at="2025-01-14T10:00:00", + updated_at="2025-01-14T10:05:00", + ) + + # Running with no failed jobs is not blocking + assert pipeline.is_blocking is False diff --git a/apps/backend/__tests__/test_gitlab_file_lock.py b/apps/backend/__tests__/test_gitlab_file_lock.py new file mode 100644 index 00000000..47a67b2f --- /dev/null +++ b/apps/backend/__tests__/test_gitlab_file_lock.py @@ -0,0 +1,420 @@ +""" +GitLab File Lock Tests +======================= + +Tests for file locking utilities for concurrent safety. +""" + +import json +import tempfile +import threading +import time +from pathlib import Path +from unittest.mock import patch + +import pytest + + +class TestFileLock: + """Test FileLock for concurrent-safe operations.""" + + @pytest.fixture + def lock_file(self, tmp_path): + """Create a temporary lock file path.""" + return tmp_path / "test.lock" + + def test_acquire_lock(self, lock_file): + """Test acquiring a lock.""" + from runners.gitlab.utils.file_lock import FileLock + + with FileLock(lock_file, timeout=5.0): + # Lock is held here + assert lock_file.exists() + + def test_lock_release(self, lock_file): + """Test lock is released after context.""" + from runners.gitlab.utils.file_lock import FileLock + + with FileLock(lock_file, timeout=5.0): + pass + + # Lock file should be cleaned up + assert not lock_file.exists() + + def test_lock_timeout(self, lock_file): + """Test lock timeout when held by another process.""" + from runners.gitlab.utils.file_lock import FileLock, FileLockTimeout + + # Hold lock in separate thread + def hold_lock(): + with FileLock(lock_file, timeout=5.0): + time.sleep(0.5) + + thread = threading.Thread(target=hold_lock) + thread.start() + + # Wait a bit for lock to be acquired + time.sleep(0.1) + + # Try to acquire with short timeout + with pytest.raises(FileLockTimeout): + FileLock(lock_file, timeout=0.1).acquire() + + thread.join() + + def test_exclusive_lock(self, lock_file): + """Test exclusive lock prevents concurrent writes.""" + from runners.gitlab.utils.file_lock import FileLock + + results = [] + + def try_write(value): + try: + with FileLock(lock_file, timeout=1.0, exclusive=True): + with open(lock_file.with_suffix(".txt"), "w") as f: + f.write(str(value)) + results.append(value) + except Exception: + results.append(None) + + threads = [ + threading.Thread(target=try_write, args=(1,)), + threading.Thread(target=try_write, args=(2,)), + ] + + for t in threads: + t.start() + + for t in threads: + t.join() + + # Only one should have succeeded + successful = [r for r in results if r is not None] + assert len(successful) == 1 + + def test_lock_cleanup_on_error(self, lock_file): + """Test lock is cleaned up even on error.""" + from runners.gitlab.utils.file_lock import FileLock + + try: + with FileLock(lock_file, timeout=5.0): + raise ValueError("Simulated error") + except ValueError: + pass + + # Lock should be cleaned up despite error + assert not lock_file.exists() + + +class TestAtomicWrite: + """Test atomic_write for safe file writes.""" + + @pytest.fixture + def target_file(self, tmp_path): + """Create a temporary target file.""" + return tmp_path / "target.txt" + + def test_atomic_write_creates_file(self, target_file): + """Test atomic write creates target file.""" + from runners.gitlab.utils.file_lock import atomic_write + + with atomic_write(target_file) as f: + f.write("test content") + + assert target_file.exists() + assert target_file.read_text() == "test content" + + def test_atomic_write_preserves_on_error(self, target_file): + """Test atomic write doesn't corrupt on error.""" + from runners.gitlab.utils.file_lock import atomic_write + + # Create initial content + target_file.write_text("original content") + + try: + with atomic_write(target_file) as f: + f.write("new content") + raise ValueError("Simulated error") + except ValueError: + pass + + # Original content should be preserved + assert target_file.read_text() == "original content" + + def test_atomic_write_context_manager(self, target_file): + """Test atomic write context manager.""" + from runners.gitlab.utils.file_lock import atomic_write + + with atomic_write(target_file) as f: + f.write("line 1\n") + f.write("line 2\n") + + content = target_file.read_text() + assert "line 1" in content + assert "line 2" in content + + +class TestLockedJsonOperations: + """Test locked JSON operations.""" + + @pytest.fixture + def data_file(self, tmp_path): + """Create a temporary data file.""" + return tmp_path / "data.json" + + def test_locked_json_write(self, data_file): + """Test writing JSON with file locking.""" + from runners.gitlab.utils.file_lock import locked_json_write + + data = {"key": "value", "number": 42} + + locked_json_write(data_file, data) + + assert data_file.exists() + with open(data_file) as f: + loaded = json.load(f) + assert loaded == data + + def test_locked_json_read(self, data_file): + """Test reading JSON with file locking.""" + from runners.gitlab.utils.file_lock import locked_json_read, locked_json_write + + data = {"key": "value", "nested": {"item": 1}} + locked_json_write(data_file, data) + + loaded = locked_json_read(data_file) + + assert loaded == data + + def test_locked_json_update(self, data_file): + """Test updating JSON with file locking.""" + from runners.gitlab.utils.file_lock import ( + locked_json_read, + locked_json_update, + locked_json_write, + ) + + initial = {"key": "value"} + locked_json_write(data_file, initial) + + def update_fn(data): + data["new_key"] = "new_value" + return data + + locked_json_update(data_file, update_fn) + + loaded = locked_json_read(data_file) + assert loaded["key"] == "value" + assert loaded["new_key"] == "new_value" + + def test_locked_json_read_missing_file(self, tmp_path): + """Test reading missing JSON file returns None.""" + from runners.gitlab.utils.file_lock import locked_json_read + + result = locked_json_read(tmp_path / "nonexistent.json") + + assert result is None + + def test_concurrent_json_writes(self, tmp_path): + """Test concurrent JSON writes are safe.""" + from runners.gitlab.utils.file_lock import ( + locked_json_read, + locked_json_update, + locked_json_write, + ) + + data_file = tmp_path / "concurrent.json" + + # Initialize + locked_json_write(data_file, {"counter": 0}) + + results = [] + + def increment(): + def updater(data): + data["counter"] += 1 + return data + + locked_json_update(data_file, updater) + result = locked_json_read(data_file) + results.append(result["counter"]) + + threads = [ + threading.Thread(target=increment), + threading.Thread(target=increment), + threading.Thread(target=increment), + ] + + for t in threads: + t.start() + + for t in threads: + t.join() + + # Final value should be 3 + final = locked_json_read(data_file) + assert final["counter"] == 3 + + +class TestLockedReadWrite: + """Test general locked read/write operations.""" + + @pytest.fixture + def data_file(self, tmp_path): + """Create a temporary data file.""" + return tmp_path / "data.txt" + + def test_locked_write(self, data_file): + """Test writing with lock.""" + from runners.gitlab.utils.file_lock import locked_write + + with locked_write(data_file) as f: + f.write("test content") + + assert data_file.read_text() == "test content" + + def test_locked_read(self, data_file): + """Test reading with lock.""" + from runners.gitlab.utils.file_lock import locked_read, locked_write + + with locked_write(data_file) as f: + f.write("read test") + + with locked_read(data_file) as f: + content = f.read() + + assert content == "read test" + + def test_locked_write_file_lock(self, data_file): + """Test locked_write with custom FileLock.""" + from runners.gitlab.utils.file_lock import FileLock, locked_write + + with FileLock(data_file, timeout=5.0): + with locked_write(data_file, lock=None) as f: + f.write("custom lock") + + assert data_file.read_text() == "custom lock" + + +class TestFileLockError: + """Test FileLockError exceptions.""" + + def test_file_lock_error(self): + """Test FileLockError is raised correctly.""" + from runners.gitlab.utils.file_lock import FileLockError + + error = FileLockError("Custom error message") + assert str(error) == "Custom error message" + + def test_file_lock_timeout(self): + """Test FileLockTimeout is raised correctly.""" + from runners.gitlab.utils.file_lock import FileLockTimeout + + error = FileLockTimeout("Timeout message") + assert "Timeout" in str(error) + + +class TestConcurrentSafety: + """Test concurrent safety scenarios.""" + + def test_multiple_readers(self, tmp_path): + """Test multiple readers can access file concurrently.""" + from runners.gitlab.utils.file_lock import locked_json_read, locked_json_write + + data_file = tmp_path / "readers.json" + locked_json_write(data_file, {"value": 42}) + + results = [] + + def read_value(): + data = locked_json_read(data_file) + results.append(data["value"]) + + threads = [threading.Thread(target=read_value) for _ in range(5)] + + for t in threads: + t.start() + + for t in threads: + t.join() + + assert len(results) == 5 + assert all(r == 42 for r in results) + + def test_writers_exclusive(self, tmp_path): + """Test writers have exclusive access.""" + from runners.gitlab.utils.file_lock import ( + locked_json_read, + locked_json_update, + locked_json_write, + ) + + data_file = tmp_path / "writers.json" + locked_json_write(data_file, {"counter": 0}) + + results = [] + + def increment(): + def updater(data): + data["counter"] += 1 + return data + + locked_json_update(data_file, updater) + result = locked_json_read(data_file) + results.append(result["counter"]) + + threads = [threading.Thread(target=increment) for _ in range(10)] + + for t in threads: + t.start() + + for t in threads: + t.join() + + # All increments should be applied + final = locked_json_read(data_file) + assert final["counter"] == 10 + assert len(results) == 10 + + def test_reader_writer_conflict(self, tmp_path): + """Test readers and writers don't conflict.""" + from runners.gitlab.utils.file_lock import ( + locked_json_read, + locked_json_update, + locked_json_write, + ) + + data_file = tmp_path / "rw.json" + locked_json_write(data_file, {"reads": 0, "writes": 0}) + + read_results = [] + + def reader(): + for _ in range(10): + data = locked_json_read(data_file) + read_results.append(data["reads"]) + + def writer(): + for _ in range(5): + + def updater(data): + data["writes"] += 1 + return data + + locked_json_update(data_file, updater) + + threads = [ + threading.Thread(target=reader), + threading.Thread(target=writer), + ] + + for t in threads: + t.start() + + for t in threads: + t.join() + + # All operations should complete + final = locked_json_read(data_file) + assert final["writes"] == 5 + assert len(read_results) == 10 diff --git a/apps/backend/__tests__/test_gitlab_mr_e2e.py b/apps/backend/__tests__/test_gitlab_mr_e2e.py new file mode 100644 index 00000000..fe824792 --- /dev/null +++ b/apps/backend/__tests__/test_gitlab_mr_e2e.py @@ -0,0 +1,564 @@ +""" +GitLab MR E2E Tests +=================== + +End-to-end tests for MR review lifecycle. +""" + +import json +from datetime import datetime, timezone +from pathlib import Path +from unittest.mock import AsyncMock, MagicMock, Mock, patch + +import pytest +from tests.fixtures.gitlab import ( + MOCK_GITLAB_CONFIG, + mock_mr_changes, + mock_mr_commits, + mock_mr_data, + mock_pipeline_data, + mock_pipeline_jobs, +) + + +class TestMREndToEnd: + """End-to-end MR review lifecycle tests.""" + + @pytest.fixture + def mock_orchestrator(self, tmp_path): + """Create a mock orchestrator for testing.""" + from runners.gitlab.models import GitLabRunnerConfig + from runners.gitlab.orchestrator import GitLabOrchestrator + + config = GitLabRunnerConfig( + token="test-token", + project="group/project", + instance_url="https://gitlab.example.com", + model="claude-sonnet-4-20250514", + ) + + with patch("runners.gitlab.orchestrator.GitLabClient"): + orchestrator = GitLabOrchestrator( + project_dir=tmp_path, + config=config, + enable_bot_detection=False, + enable_ci_checking=False, + ) + return orchestrator + + @pytest.mark.asyncio + async def test_full_mr_review_lifecycle(self, mock_orchestrator): + """Test complete MR review from start to finish.""" + # Mock MR data + mock_orchestrator.client.get_mr_async.return_value = mock_mr_data() + mock_orchestrator.client.get_mr_commits_async.return_value = mock_mr_commits() + mock_orchestrator.client.get_mr_changes_async.return_value = mock_mr_changes() + + # Mock review engine + with patch( + "runners.gitlab.services.context_gatherer.MRContextGatherer" + ) as mock_gatherer: + from runners.gitlab.models import ( + MergeVerdict, + MRContext, + MRReviewFinding, + ReviewCategory, + ReviewSeverity, + ) + + mock_gatherer.return_value.gather.return_value = MRContext( + mr_iid=123, + title="Add feature", + description="Implementation", + author="john_doe", + source_branch="feature", + target_branch="main", + state="opened", + changed_files=[], + diff="", + commits=[], + ) + + # Mock review engine to return findings + with patch("runners.gitlab.services.MRReviewEngine") as mock_engine: + findings = [ + MRReviewFinding( + id="find-1", + severity=ReviewSeverity.MEDIUM, + category=ReviewCategory.QUALITY, + title="Code style", + description="Fix formatting", + file="file.py", + line=10, + ) + ] + + mock_engine.return_value.run_review.return_value = ( + findings, + MergeVerdict.MERGE_WITH_CHANGES, + "Consider the suggestions", + [], + ) + + result = await mock_orchestrator.review_mr(123) + + assert result.success is True + assert result.mr_iid == 123 + assert len(result.findings) == 1 + assert result.verdict == MergeVerdict.MERGE_WITH_CHANGES + + @pytest.mark.asyncio + async def test_mr_review_with_ci_failure(self, mock_orchestrator): + """Test MR review blocked by CI failure.""" + from runners.gitlab.services.ci_checker import PipelineInfo, PipelineStatus + + # Setup CI failure + with patch("runners.gitlab.orchestrator.MRContextGatherer"): + with patch("runners.gitlab.services.ci_checker.CIChecker") as mock_checker: + pipeline_info = PipelineInfo( + pipeline_id=1001, + status=PipelineStatus.FAILED, + ref="feature", + sha="abc123", + created_at="2025-01-14T10:00:00", + updated_at="2025-01-14T10:05:00", + failed_jobs=[ + Mock( + status="failed", + name="test", + stage="test", + failure_reason="Assert failed", + ) + ], + ) + + mock_checker.return_value.check_mr_pipeline.return_value = pipeline_info + mock_checker.return_value.get_blocking_reason.return_value = ( + "Test job failed" + ) + mock_checker.return_value.format_pipeline_summary.return_value = ( + "CI Failed" + ) + + mock_orchestrator.client.get_mr_async.return_value = mock_mr_data() + mock_orchestrator.client.get_mr_commits_async.return_value = [] + + with patch("runners.gitlab.services.MRReviewEngine") as mock_engine: + from runners.gitlab.models import MergeVerdict + + mock_engine.return_value.run_review.return_value = ( + [], + MergeVerdict.READY_TO_MERGE, + "Looks good", + [], + ) + + result = await mock_orchestrator.review_mr(123) + + assert result.ci_status == "failed" + assert result.ci_pipeline_id == 1001 + assert "CI" in result.summary + + @pytest.mark.asyncio + async def test_followup_review_lifecycle(self, mock_orchestrator): + """Test follow-up review after initial review.""" + from runners.gitlab.models import MergeVerdict, MRReviewResult + + # Create initial review + initial_review = MRReviewResult( + mr_iid=123, + project="group/project", + success=True, + findings=[ + Mock(id="find-1", title="Fix bug"), + Mock(id="find-2", title="Add tests"), + ], + reviewed_commit_sha="abc123", + verdict=MergeVerdict.NEEDS_REVISION, + verdict_reasoning="Issues found", + blockers=["find-1"], + ) + + # Save initial review + initial_review.save(mock_orchestrator.gitlab_dir) + + # Mock new commits + new_commits = mock_mr_commits() + [ + { + "id": "new456", + "sha": "new456", + "message": "Fix the issues", + } + ] + + mock_orchestrator.client.get_mr_async.return_value = mock_mr_data() + mock_orchestrator.client.get_mr_commits_async.return_value = new_commits + + # Mock follow-up review + with patch("runners.gitlab.orchestrator.MRContextGatherer"): + with patch("runners.gitlab.services.MRReviewEngine") as mock_engine: + mock_engine.return_value.run_review.return_value = ( + [], # No new findings + MergeVerdict.READY_TO_MERGE, + "All fixed", + [], + ) + + result = await mock_orchestrator.followup_review_mr(123) + + assert result.is_followup_review is True + assert result.reviewed_commit_sha == "new456" + + @pytest.mark.asyncio + async def test_bot_detection_skips_review(self, tmp_path): + """Test bot detection skips bot-authored MRs.""" + from runners.gitlab.models import GitLabRunnerConfig + from runners.gitlab.orchestrator import GitLabOrchestrator + + config = GitLabRunnerConfig( + token="test-token", + project="group/project", + ) + + with patch("runners.gitlab.orchestrator.GitLabClient"): + orchestrator = GitLabOrchestrator( + project_dir=tmp_path, + config=config, + bot_username="auto-claude-bot", + ) + + # Bot-authored MR + bot_mr = mock_mr_data(author="auto-claude-bot") + orchestrator.client.get_mr_async.return_value = bot_mr + orchestrator.client.get_mr_commits_async.return_value = [] + + result = await orchestrator.review_mr(123) + + assert result.success is False + assert "bot" in result.error.lower() + + @pytest.mark.asyncio + async def test_cooling_off_prevents_re_review(self, tmp_path): + """Test cooling off period prevents immediate re-review.""" + from runners.gitlab.models import GitLabRunnerConfig + from runners.gitlab.orchestrator import GitLabOrchestrator + + config = GitLabRunnerConfig( + token="test-token", + project="group/project", + ) + + with patch("runners.gitlab.orchestrator.GitLabClient"): + orchestrator = GitLabOrchestrator( + project_dir=tmp_path, + config=config, + ) + + # First review + orchestrator.client.get_mr_async.return_value = mock_mr_data() + orchestrator.client.get_mr_commits_async.return_value = mock_mr_commits() + + with patch("runners.gitlab.orchestrator.MRContextGatherer"): + with patch("runners.gitlab.services.MRReviewEngine") as mock_engine: + from runners.gitlab.models import MergeVerdict + + mock_engine.return_value.run_review.return_value = ( + [], + MergeVerdict.READY_TO_MERGE, + "Good", + [], + ) + + result1 = await orchestrator.review_mr(123) + + assert result1.success is True + + # Immediate second review should be skipped + result2 = await orchestrator.review_mr(123) + + assert result2.success is False + assert "cooling" in result2.error.lower() + + +class TestMRReviewEngineIntegration: + """Test MR review engine integration.""" + + @pytest.fixture + def engine(self, tmp_path): + """Create review engine for testing.""" + from runners.gitlab.models import GitLabRunnerConfig + from runners.gitlab.services.mr_review_engine import MRReviewEngine + + config = GitLabRunnerConfig( + token="test-token", + project="group/project", + ) + + gitlab_dir = tmp_path / ".auto-claude" / "gitlab" + gitlab_dir.mkdir(parents=True, exist_ok=True) + + return MRReviewEngine( + project_dir=tmp_path, + gitlab_dir=gitlab_dir, + config=config, + ) + + def test_engine_initialization(self, engine): + """Test engine initializes correctly.""" + assert engine.project_dir + assert engine.gitlab_dir + assert engine.config + + def test_generate_summary(self, engine): + """Test summary generation.""" + from runners.gitlab.models import ( + MergeVerdict, + MRReviewFinding, + ReviewCategory, + ReviewSeverity, + ) + + findings = [ + MRReviewFinding( + id="find-1", + severity=ReviewSeverity.CRITICAL, + category=ReviewCategory.SECURITY, + title="SQL injection", + description="Vulnerability", + file="file.py", + line=10, + ), + MRReviewFinding( + id="find-2", + severity=ReviewSeverity.LOW, + category=ReviewCategory.STYLE, + title="Formatting", + description="Style issue", + file="file.py", + line=20, + ), + ] + + summary = engine.generate_summary( + findings=findings, + verdict=MergeVerdict.BLOCKED, + verdict_reasoning="Critical security issue", + blockers=["SQL injection"], + ) + + assert "BLOCKED" in summary + assert "SQL injection" in summary + assert "Critical" in summary + + +class TestMRContextGatherer: + """Test MR context gatherer.""" + + @pytest.fixture + def gatherer(self, tmp_path): + """Create context gatherer for testing.""" + from runners.gitlab.glab_client import GitLabConfig + from runners.gitlab.services.context_gatherer import MRContextGatherer + + config = GitLabConfig( + token="test-token", + project="group/project", + ) + + with patch("runners.gitlab.services.context_gatherer.GitLabClient"): + return MRContextGatherer( + project_dir=tmp_path, + mr_iid=123, + config=config, + ) + + @pytest.mark.asyncio + async def test_gather_context(self, gatherer): + """Test gathering MR context.""" + from runners.gitlab.models import MRContext + + # Mock client responses + gatherer.client.get_mr_async.return_value = mock_mr_data() + gatherer.client.get_mr_changes_async.return_value = mock_mr_changes() + gatherer.client.get_mr_commits_async.return_value = mock_mr_commits() + gatherer.client.get_mr_notes_async.return_value = [] + + context = await gatherer.gather() + + assert isinstance(context, MRContext) + assert context.mr_iid == 123 + assert context.title == "Add user authentication feature" + assert context.author == "john_doe" + + @pytest.mark.asyncio + async def test_gather_ai_bot_comments(self, gatherer): + """Test gathering AI bot comments.""" + # Mock AI bot comments + ai_notes = [ + { + "id": 1001, + "author": {"username": "coderabbit[bot]"}, + "body": "Consider adding error handling", + "created_at": "2025-01-14T10:00:00", + }, + { + "id": 1002, + "author": {"username": "human_user"}, + "body": "Regular comment", + "created_at": "2025-01-14T11:00:00", + }, + ] + + gatherer.client.get_mr_notes_async.return_value = ai_notes + + # First call should parse comments + from runners.gitlab.services.context_gatherer import AIBotComment + + # Note: _fetch_ai_bot_comments is called internally during gather() + gatherer.client.get_mr_async.return_value = mock_mr_data() + gatherer.client.get_mr_changes_async.return_value = mock_mr_changes() + gatherer.client.get_mr_commits_async.return_value = mock_mr_commits() + + context = await gatherer.gather() + + # Verify AI bot comments were detected (context would have them if implemented) + assert context.mr_iid == 123 + + +class TestFollowupContextGatherer: + """Test follow-up context gatherer.""" + + @pytest.fixture + def previous_review(self): + """Create a previous review for testing.""" + from runners.gitlab.models import MergeVerdict, MRReviewResult + + return MRReviewResult( + mr_iid=123, + project="group/project", + success=True, + findings=[ + Mock(id="find-1", title="Bug"), + ], + reviewed_commit_sha="abc123", + verdict=MergeVerdict.NEEDS_REVISION, + verdict_reasoning="Issues found", + blockers=[], + ) + + @pytest.fixture + def gatherer(self, tmp_path, previous_review): + """Create follow-up context gatherer.""" + from runners.gitlab.glab_client import GitLabConfig + from runners.gitlab.services.context_gatherer import FollowupMRContextGatherer + + config = GitLabConfig( + token="test-token", + project="group/project", + ) + + with patch("runners.gitlab.services.context_gatherer.GitLabClient"): + return FollowupMRContextGatherer( + project_dir=tmp_path, + mr_iid=123, + previous_review=previous_review, + config=config, + ) + + @pytest.mark.asyncio + async def test_gather_followup_context(self, gatherer): + """Test gathering follow-up context.""" + from runners.gitlab.models import FollowupMRContext + + # Mock new commits since previous review + new_commits = [ + { + "id": "new456", + "sha": "new456", + "message": "Fix bug", + } + ] + + gatherer.client.get_mr_async.return_value = mock_mr_data() + gatherer.client.get_mr_commits_async.return_value = new_commits + gatherer.client.get_mr_changes_async.return_value = mock_mr_changes() + + context = await gatherer.gather() + + assert isinstance(context, FollowupMRContext) + assert context.mr_iid == 123 + assert context.previous_commit_sha == "abc123" + assert context.current_commit_sha == "new456" + assert len(context.commits_since_review) == 1 + + @pytest.mark.asyncio + async def test_no_new_commits(self, gatherer): + """Test follow-up when no new commits.""" + from runners.gitlab.models import FollowupMRContext + + # Same commits as previous review + gatherer.client.get_mr_async.return_value = mock_mr_data() + gatherer.client.get_mr_commits_async.return_value = mock_mr_commits() + gatherer.client.get_mr_changes_async.return_value = mock_mr_changes() + + context = await gatherer.gather() + + assert context.current_commit_sha == "abc123" # Same as previous + + +class TestAIBotComment: + """Test AI bot comment detection.""" + + def test_parse_coderabbit_comment(self): + """Test parsing CodeRabbit comment.""" + from runners.gitlab.services.context_gatherer import AIBotComment + + note = { + "id": 1001, + "author": {"username": "coderabbit[bot]"}, + "body": "Add error handling", + "created_at": "2025-01-14T10:00:00", + } + + from runners.gitlab.services.context_gatherer import MRContextGatherer + + gatherer_class = MRContextGatherer.__class__ + + comment = gatherer_class._parse_ai_comment(None, note) + + assert comment is not None + assert comment.tool_name == "CodeRabbit" + assert comment.comment_id == 1001 + + def test_parse_human_comment(self): + """Test human comment is not detected as AI.""" + from runners.gitlab.services.context_gatherer import MRContextGatherer + + note = { + "id": 1002, + "author": {"username": "john_doe"}, + "body": "Regular comment", + "created_at": "2025-01-14T10:00:00", + } + + comment = MRContextGatherer._parse_ai_comment(None, note) + + assert comment is None + + def test_parse_greptile_comment(self): + """Test parsing Greptile comment.""" + from runners.gitlab.services.context_gatherer import AIBotComment + + note = { + "id": 1003, + "author": {"username": "greptile[bot]"}, + "body": "Consider this", + "created_at": "2025-01-14T10:00:00", + } + + from runners.gitlab.services.context_gatherer import MRContextGatherer + + comment = MRContextGatherer._parse_ai_comment(None, note) + + assert comment is not None + assert comment.tool_name == "Greptile" diff --git a/apps/backend/__tests__/test_gitlab_mr_review.py b/apps/backend/__tests__/test_gitlab_mr_review.py new file mode 100644 index 00000000..ce8d8eca --- /dev/null +++ b/apps/backend/__tests__/test_gitlab_mr_review.py @@ -0,0 +1,514 @@ +""" +GitLab MR Review Tests +====================== + +Tests for MR review models, findings, verdicts. +""" + +import json +from datetime import datetime, timezone +from pathlib import Path +from unittest.mock import MagicMock, Mock, patch + +import pytest +from tests.fixtures.gitlab import ( + MOCK_GITLAB_CONFIG, + mock_issue_data, + mock_mr_data, +) + + +class TestMRReviewFinding: + """Test MRReviewFinding model.""" + + def test_finding_creation(self): + """Test creating a review finding.""" + from runners.gitlab.models import ( + MRReviewFinding, + ReviewCategory, + ReviewSeverity, + ) + + finding = MRReviewFinding( + id="find-1", + severity=ReviewSeverity.HIGH, + category=ReviewCategory.SECURITY, + title="SQL injection vulnerability", + description="User input not sanitized in query", + file="src/auth.py", + line=42, + end_line=45, + suggested_fix="Use parameterized query", + fixable=True, + ) + + assert finding.id == "find-1" + assert finding.severity == ReviewSeverity.HIGH + assert finding.category == ReviewCategory.SECURITY + assert finding.file == "src/auth.py" + assert finding.line == 42 + assert finding.fixable is True + + def test_finding_to_dict(self): + """Test converting finding to dictionary.""" + from runners.gitlab.models import ( + MRReviewFinding, + ReviewCategory, + ReviewSeverity, + ) + + finding = MRReviewFinding( + id="find-1", + severity=ReviewSeverity.HIGH, + category=ReviewCategory.SECURITY, + title="SQL injection", + description="Vulnerability", + file="src/auth.py", + line=42, + ) + + data = finding.to_dict() + + assert data["id"] == "find-1" + assert data["severity"] == "high" + assert data["category"] == "security" + + def test_finding_from_dict(self): + """Test loading finding from dictionary.""" + from runners.gitlab.models import MRReviewFinding + + data = { + "id": "find-1", + "severity": "high", + "category": "security", + "title": "SQL injection", + "description": "Vulnerability", + "file": "src/auth.py", + "line": 42, + "end_line": 45, + "suggested_fix": "Fix it", + "fixable": True, + } + + finding = MRReviewFinding.from_dict(data) + + assert finding.id == "find-1" + assert finding.severity.value == "high" + assert finding.line == 42 + + def test_finding_with_evidence_code(self): + """Test finding with evidence code.""" + from runners.gitlab.models import ( + MRReviewFinding, + ReviewCategory, + ReviewPass, + ReviewSeverity, + ) + + finding = MRReviewFinding( + id="find-1", + severity=ReviewSeverity.CRITICAL, + category=ReviewCategory.SECURITY, + title="Command injection", + description="User input in subprocess", + file="src/exec.py", + line=10, + evidence_code="subprocess.call(user_input, shell=True)", + found_by_pass=ReviewPass.SECURITY, + ) + + assert finding.evidence_code == "subprocess.call(user_input, shell=True)" + assert finding.found_by_pass == ReviewPass.SECURITY + + +class TestStructuralIssue: + """Test StructuralIssue model.""" + + def test_structural_issue_creation(self): + """Test creating a structural issue.""" + from runners.gitlab.models import ReviewSeverity, StructuralIssue + + issue = StructuralIssue( + id="struct-1", + type="feature_creep", + title="Additional features added", + description="MR includes features beyond original scope", + severity=ReviewSeverity.MEDIUM, + files_affected=["src/auth.py", "src/users.py"], + ) + + assert issue.id == "struct-1" + assert issue.type == "feature_creep" + assert issue.files_affected == ["src/auth.py", "src/users.py"] + + def test_structural_issue_to_dict(self): + """Test converting structural issue to dictionary.""" + from runners.gitlab.models import StructuralIssue + + issue = StructuralIssue( + id="struct-1", + type="scope_change", + title="Scope increased", + description="MR scope changed significantly", + files_affected=["file1.py"], + ) + + data = issue.to_dict() + + assert data["id"] == "struct-1" + assert data["type"] == "scope_change" + + def test_structural_issue_from_dict(self): + """Test loading structural issue from dictionary.""" + from runners.gitlab.models import StructuralIssue + + data = { + "id": "struct-1", + "type": "feature_creep", + "title": "Extra features", + "description": "Beyond scope", + "severity": "medium", + "files_affected": ["file.py"], + } + + issue = StructuralIssue.from_dict(data) + + assert issue.type == "feature_creep" + + +class TestAICommentTriage: + """Test AICommentTriage model.""" + + def test_triage_creation(self): + """Test creating AI comment triage.""" + from runners.gitlab.models import AICommentTriage + + triage = AICommentTriage( + comment_id=1001, + tool_name="CodeRabbit", + original_comment="Consider adding error handling", + triage_result="valid", + reasoning="Good point about error handling", + file="src/auth.py", + line=50, + created_at="2025-01-14T10:00:00", + ) + + assert triage.comment_id == 1001 + assert triage.tool_name == "CodeRabbit" + assert triage.triage_result == "valid" + + def test_triage_to_dict(self): + """Test converting triage to dictionary.""" + from runners.gitlab.models import AICommentTriage + + triage = AICommentTriage( + comment_id=1001, + tool_name="CodeRabbit", + original_comment="Add tests", + triage_result="false_positive", + reasoning="Tests already exist", + ) + + data = triage.to_dict() + + assert data["comment_id"] == 1001 + assert data["triage_result"] == "false_positive" + + def test_triage_from_dict(self): + """Test loading triage from dictionary.""" + from runners.gitlab.models import AICommentTriage + + data = { + "comment_id": 1001, + "tool_name": "Cursor", + "original_comment": "Fix bug", + "triage_result": "questionable", + "reasoning": "Unclear if bug exists", + "file": "file.py", + "line": 10, + } + + triage = AICommentTriage.from_dict(data) + + assert triage.tool_name == "Cursor" + assert triage.triage_result == "questionable" + + +class TestMRReviewResult: + """Test MRReviewResult model.""" + + def test_result_creation(self): + """Test creating review result.""" + from runners.gitlab.models import ( + MergeVerdict, + MRReviewFinding, + MRReviewResult, + ReviewCategory, + ReviewSeverity, + ) + + findings = [ + MRReviewFinding( + id="find-1", + severity=ReviewSeverity.HIGH, + category=ReviewCategory.SECURITY, + title="Bug", + description="Issue", + file="file.py", + line=1, + ) + ] + + result = MRReviewResult( + mr_iid=123, + project="group/project", + success=True, + findings=findings, + summary="Review complete", + overall_status="approve", + verdict=MergeVerdict.READY_TO_MERGE, + verdict_reasoning="No issues found", + blockers=[], + ) + + assert result.mr_iid == 123 + assert result.findings == findings + assert result.verdict == MergeVerdict.READY_TO_MERGE + + def test_result_with_structural_issues(self): + """Test result with structural issues.""" + from runners.gitlab.models import ( + MergeVerdict, + MRReviewResult, + StructuralIssue, + ) + + structural_issues = [ + StructuralIssue( + id="struct-1", + type="feature_creep", + title="Extra features", + description="Beyond scope", + ) + ] + + result = MRReviewResult( + mr_iid=123, + project="group/project", + success=True, + structural_issues=structural_issues, + verdict=MergeVerdict.MERGE_WITH_CHANGES, + verdict_reasoning="Feature creep detected", + blockers=[], + ) + + assert len(result.structural_issues) == 1 + assert result.verdict == MergeVerdict.MERGE_WITH_CHANGES + + def test_result_with_ai_triages(self): + """Test result with AI comment triages.""" + from runners.gitlab.models import ( + AICommentTriage, + MergeVerdict, + MRReviewResult, + ) + + ai_triages = [ + AICommentTriage( + comment_id=1001, + tool_name="CodeRabbit", + original_comment="Fix bug", + triage_result="valid", + reasoning="Correct", + ) + ] + + result = MRReviewResult( + mr_iid=123, + project="group/project", + success=True, + ai_triages=ai_triages, + verdict=MergeVerdict.READY_TO_MERGE, + verdict_reasoning="All good", + blockers=[], + ) + + assert len(result.ai_triages) == 1 + + def test_result_with_ci_status(self): + """Test result with CI/CD status.""" + from runners.gitlab.models import MergeVerdict, MRReviewResult + + result = MRReviewResult( + mr_iid=123, + project="group/project", + success=True, + ci_status="failed", + ci_pipeline_id=1001, + verdict=MergeVerdict.BLOCKED, + verdict_reasoning="CI failed", + blockers=["CI Pipeline Failed"], + ) + + assert result.ci_status == "failed" + assert result.ci_pipeline_id == 1001 + assert result.verdict == MergeVerdict.BLOCKED + + def test_result_to_dict(self): + """Test converting result to dictionary.""" + from runners.gitlab.models import MergeVerdict, MRReviewResult + + result = MRReviewResult( + mr_iid=123, + project="group/project", + success=True, + verdict=MergeVerdict.READY_TO_MERGE, + verdict_reasoning="Good", + blockers=[], + ) + + data = result.to_dict() + + assert data["mr_iid"] == 123 + assert data["verdict"] == "ready_to_merge" + + def test_result_from_dict(self): + """Test loading result from dictionary.""" + from runners.gitlab.models import MergeVerdict, MRReviewResult + + data = { + "mr_iid": 123, + "project": "group/project", + "success": True, + "findings": [], + "summary": "Review", + "overall_status": "approve", + "verdict": "ready_to_merge", + "verdict_reasoning": "Good", + "blockers": [], + } + + result = MRReviewResult.from_dict(data) + + assert result.mr_iid == 123 + assert result.verdict == MergeVerdict.READY_TO_MERGE + + def test_result_save_and_load(self, tmp_path): + """Test saving and loading result from disk.""" + from runners.gitlab.models import MergeVerdict, MRReviewResult + + result = MRReviewResult( + mr_iid=123, + project="group/project", + success=True, + verdict=MergeVerdict.READY_TO_MERGE, + verdict_reasoning="Good", + blockers=[], + ) + + result.save(tmp_path) + + loaded = MRReviewResult.load(tmp_path, 123) + + assert loaded is not None + assert loaded.mr_iid == 123 + + def test_followup_review_fields(self): + """Test follow-up review fields.""" + from runners.gitlab.models import MergeVerdict, MRReviewResult + + result = MRReviewResult( + mr_iid=123, + project="group/project", + success=True, + is_followup_review=True, + reviewed_commit_sha="abc123", + resolved_findings=["find-1"], + unresolved_findings=["find-2"], + new_findings_since_last_review=["find-3"], + verdict=MergeVerdict.READY_TO_MERGE, + verdict_reasoning="Good", + blockers=[], + ) + + assert result.is_followup_review is True + assert result.reviewed_commit_sha == "abc123" + assert len(result.resolved_findings) == 1 + + +class TestReviewPass: + """Test ReviewPass enum.""" + + def test_all_passes_defined(self): + """Test all review passes are defined.""" + from runners.gitlab.models import ReviewPass + + assert ReviewPass.QUICK_SCAN + assert ReviewPass.SECURITY + assert ReviewPass.QUALITY + assert ReviewPass.DEEP_ANALYSIS + assert ReviewPass.STRUCTURAL + assert ReviewPass.AI_COMMENT_TRIAGE + + def test_pass_values(self): + """Test pass enum values.""" + from runners.gitlab.models import ReviewPass + + assert ReviewPass.QUICK_SCAN.value == "quick_scan" + assert ReviewPass.SECURITY.value == "security" + assert ReviewPass.QUALITY.value == "quality" + assert ReviewPass.DEEP_ANALYSIS.value == "deep_analysis" + assert ReviewPass.STRUCTURAL.value == "structural" + assert ReviewPass.AI_COMMENT_TRIAGE.value == "ai_comment_triage" + + +class TestMergeVerdict: + """Test MergeVerdict enum.""" + + def test_all_verdicts_defined(self): + """Test all verdicts are defined.""" + from runners.gitlab.models import MergeVerdict + + assert MergeVerdict.READY_TO_MERGE + assert MergeVerdict.MERGE_WITH_CHANGES + assert MergeVerdict.NEEDS_REVISION + assert MergeVerdict.BLOCKED + + def test_verdict_values(self): + """Test verdict enum values.""" + from runners.gitlab.models import MergeVerdict + + assert MergeVerdict.READY_TO_MERGE.value == "ready_to_merge" + assert MergeVerdict.MERGE_WITH_CHANGES.value == "merge_with_changes" + assert MergeVerdict.NEEDS_REVISION.value == "needs_revision" + assert MergeVerdict.BLOCKED.value == "blocked" + + +class TestReviewSeverity: + """Test ReviewSeverity enum.""" + + def test_all_severities(self): + """Test all severity levels.""" + from runners.gitlab.models import ReviewSeverity + + assert ReviewSeverity.CRITICAL + assert ReviewSeverity.HIGH + assert ReviewSeverity.MEDIUM + assert ReviewSeverity.LOW + + +class TestReviewCategory: + """Test ReviewCategory enum.""" + + def test_all_categories(self): + """Test all categories.""" + from runners.gitlab.models import ReviewCategory + + assert ReviewCategory.SECURITY + assert ReviewCategory.QUALITY + assert ReviewCategory.STYLE + assert ReviewCategory.TEST + assert ReviewCategory.DOCS + assert ReviewCategory.PATTERN + assert ReviewCategory.PERFORMANCE diff --git a/apps/backend/__tests__/test_gitlab_provider.py b/apps/backend/__tests__/test_gitlab_provider.py new file mode 100644 index 00000000..3bc8339d --- /dev/null +++ b/apps/backend/__tests__/test_gitlab_provider.py @@ -0,0 +1,236 @@ +""" +GitLab Provider Tests +===================== + +Tests for GitLabProvider implementation of the GitProvider protocol. +""" + +import json +from datetime import datetime, timezone +from pathlib import Path +from unittest.mock import MagicMock, Mock, patch + +import pytest +from tests.fixtures.gitlab import ( + MOCK_GITLAB_CONFIG, + mock_issue_data, + mock_mr_data, + mock_pipeline_data, +) + +# Tests for GitLabProvider + + +class TestGitLabProvider: + """Test GitLabProvider implements GitProvider protocol correctly.""" + + @pytest.fixture + def provider(self, tmp_path): + """Create a GitLabProvider instance for testing.""" + from runners.gitlab.providers.gitlab_provider import GitLabProvider + + with patch( + "runners.gitlab.providers.gitlab_provider.GitLabClient" + ) as mock_client: + provider = GitLabProvider( + _repo="group/project", + _token="test-token", + _instance_url="https://gitlab.example.com", + _project_dir=tmp_path, + _glab_client=mock_client.return_value, + ) + return provider + + def test_provider_type_property(self, provider): + """Test provider type is GitLab.""" + from runners.github.providers.protocol import ProviderType + + assert provider.provider_type == ProviderType.GITLAB + + def test_repo_property(self, provider): + """Test repo property returns the repository.""" + assert provider.repo == "group/project" + + def test_fetch_pr(self, provider): + """Test fetching a single MR.""" + # Mock client responses + provider._glab_client.get_mr.return_value = mock_mr_data() + provider._glab_client.get_mr_changes.return_value = { + "changes": [ + { + "diff": "@@ -0,0 +1,10 @@\n+new line", + "new_path": "test.py", + "old_path": "test.py", + } + ] + } + + # Fetch MR + pr = await_if_needed(provider.fetch_pr(123)) + + assert pr.number == 123 + assert pr.title == "Add user authentication feature" + assert pr.author == "john_doe" + assert pr.state == "opened" + assert pr.source_branch == "feature/oauth-auth" + assert pr.target_branch == "main" + assert pr.provider.name == "GITLAB" + + def test_fetch_prs_with_filters(self, provider): + """Test fetching multiple MRs with filters.""" + provider._glab_client._fetch.return_value = [ + mock_mr_data(iid=100), + mock_mr_data(iid=101, state="closed"), + ] + + prs = await_if_needed(provider.fetch_prs()) + + assert len(prs) == 2 + + def test_fetch_pr_diff(self, provider): + """Test fetching MR diff.""" + expected_diff = "diff content here" + provider._glab_client.get_mr_diff.return_value = expected_diff + + diff = await_if_needed(provider.fetch_pr_diff(123)) + + assert diff == expected_diff + + def test_fetch_issue(self, provider): + """Test fetching a single issue.""" + from tests.fixtures.gitlab import SAMPLE_ISSUE_DATA + + provider._glab_client._fetch.return_value = SAMPLE_ISSUE_DATA + + issue = await_if_needed(provider.fetch_issue(42)) + + assert issue.number == 42 + assert issue.title == "Bug: Login button not working" + assert issue.author == "jane_smith" + assert issue.state == "opened" + + def test_fetch_issues_with_filters(self, provider): + """Test fetching issues with filters.""" + provider._glab_client._fetch.return_value = [ + mock_issue_data(iid=10), + mock_issue_data(iid=11), + ] + + issues = await_if_needed(provider.fetch_issues()) + + assert len(issues) == 2 + + def test_post_review(self, provider): + """Test posting a review to an MR.""" + from runners.github.providers.protocol import ReviewData + + provider._glab_client.post_mr_note.return_value = {"id": 999} + provider._glab_client._fetch.return_value = {} # approve MR response + + review = ReviewData( + body="LGTM with minor suggestions", + event="approve", + comments=[], + ) + + note_id = await_if_needed(provider.post_review(123, review)) + + assert note_id == 999 + provider._glab_client.post_mr_note.assert_called_once() + + def test_merge_pr(self, provider): + """Test merging an MR.""" + provider._glab_client.merge_mr.return_value = {"status": "success"} + + result = await_if_needed(provider.merge_pr(123, merge_method="merge")) + + assert result is True + + def test_close_pr(self, provider): + """Test closing an MR.""" + provider._glab_client._fetch.return_value = {} + + result = await_if_needed( + provider.close_pr(123, comment="Closing as not needed") + ) + + assert result is True + + def test_create_label(self, provider): + """Test creating a label.""" + from runners.github.providers.protocol import LabelData + + provider._glab_client._fetch.return_value = {} + + label = LabelData( + name="bug", + color="#ff0000", + description="Bug report", + ) + + await_if_needed(provider.create_label(label)) + + # Verify call was made (checking that it didn't raise) + provider._glab_client._fetch.assert_called() + + def test_list_labels(self, provider): + """Test listing labels.""" + provider._glab_client._fetch.return_value = [ + {"name": "bug", "color": "ff0000", "description": "Bug"}, + {"name": "feature", "color": "00ff00", "description": "Feature"}, + ] + + labels = await_if_needed(provider.list_labels()) + + assert len(labels) == 2 + assert labels[0].name == "bug" + assert labels[0].color == "#ff0000" + + def test_get_repository_info(self, provider): + """Test getting repository info.""" + provider._glab_client._fetch.return_value = { + "name": "project", + "path_with_namespace": "group/project", + "default_branch": "main", + } + + info = await_if_needed(provider.get_repository_info()) + + assert info["default_branch"] == "main" + + def test_get_default_branch(self, provider): + """Test getting default branch.""" + provider._glab_client._fetch.return_value = { + "default_branch": "main", + } + + branch = await_if_needed(provider.get_default_branch()) + + assert branch == "main" + + def test_api_get(self, provider): + """Test low-level API GET.""" + provider._glab_client._fetch.return_value = {"data": "value"} + + result = await_if_needed(provider.api_get("/projects/1")) + + assert result["data"] == "value" + + def test_api_post(self, provider): + """Test low-level API POST.""" + provider._glab_client._fetch.return_value = {"id": 123} + + result = await_if_needed( + provider.api_post("/projects/1/notes", {"body": "test"}) + ) + + assert result["id"] == 123 + + +def await_if_needed(coro_or_result): + """Helper to await async functions if needed.""" + import asyncio + + if hasattr(coro_or_result, "__await__"): + return asyncio.run(coro_or_result) + return coro_or_result diff --git a/apps/backend/__tests__/test_gitlab_rate_limiter.py b/apps/backend/__tests__/test_gitlab_rate_limiter.py new file mode 100644 index 00000000..a9c2b860 --- /dev/null +++ b/apps/backend/__tests__/test_gitlab_rate_limiter.py @@ -0,0 +1,519 @@ +""" +GitLab Rate Limiter Tests +========================= + +Tests for token bucket rate limiting. +""" + +import asyncio +import time +from unittest.mock import patch + +import pytest + + +class TestTokenBucket: + """Test TokenBucket for rate limiting.""" + + def test_token_bucket_initialization(self): + """Test token bucket initializes correctly.""" + from runners.gitlab.utils.rate_limiter import TokenBucket + + bucket = TokenBucket(capacity=10, refill_rate=5.0) + + assert bucket.capacity == 10 + assert bucket.refill_rate == 5.0 + assert bucket.tokens == 10 + + def test_token_bucket_consume_success(self): + """Test consuming tokens when available.""" + from runners.gitlab.utils.rate_limiter import TokenBucket + + bucket = TokenBucket(capacity=10, refill_rate=5.0) + + success = bucket.consume(1) + + assert success is True + assert bucket.tokens == 9 + + def test_token_bucket_consume_multiple(self): + """Test consuming multiple tokens.""" + from runners.gitlab.utils.rate_limiter import TokenBucket + + bucket = TokenBucket(capacity=10, refill_rate=5.0) + + success = bucket.consume(5) + + assert success is True + assert bucket.tokens == 5 + + def test_token_bucket_consume_insufficient(self): + """Test consuming when insufficient tokens.""" + from runners.gitlab.utils.rate_limiter import TokenBucket + + bucket = TokenBucket(capacity=10, refill_rate=5.0) + + # Consume more than available + success = bucket.consume(15) + + assert success is False + assert bucket.tokens == 10 # Should not change + + def test_token_bucket_refill(self): + """Test token refill over time.""" + from runners.gitlab.utils.rate_limiter import TokenBucket + + bucket = TokenBucket(capacity=10, refill_rate=10.0) + + # Consume all tokens + bucket.consume(10) + assert bucket.tokens == 0 + + # Wait for refill (0.1 seconds at 10 tokens/sec = 1 token) + time.sleep(0.11) + + # Check refill + available = bucket.tokens + assert available >= 1 + + def test_token_bucket_refill_cap(self): + """Test tokens don't exceed capacity.""" + from runners.gitlab.utils.rate_limiter import TokenBucket + + bucket = TokenBucket(capacity=10, refill_rate=100.0) + + # Wait long time for refill + time.sleep(0.2) + + # Should not exceed capacity + assert bucket.tokens <= 10 + + def test_token_bucket_wait_for_token(self): + """Test waiting for token availability.""" + from runners.gitlab.utils.rate_limiter import TokenBucket + + bucket = TokenBucket(capacity=5, refill_rate=10.0) + + # Consume all + bucket.consume(5) + + # Should wait for refill + start = time.time() + bucket.consume(1, wait=True) + elapsed = time.time() - start + + # Should have waited at least 0.1 seconds + assert elapsed >= 0.1 + + def test_token_bucket_wait_with_tokens(self): + """Test wait returns immediately when tokens available.""" + from runners.gitlab.utils.rate_limiter import TokenBucket + + bucket = TokenBucket(capacity=10, refill_rate=5.0) + + start = time.time() + bucket.consume(1, wait=True) + elapsed = time.time() - start + + # Should be immediate + assert elapsed < 0.01 + + def test_token_bucket_get_available(self): + """Test getting available token count.""" + from runners.gitlab.utils.rate_limiter import TokenBucket + + bucket = TokenBucket(capacity=10, refill_rate=5.0) + + assert bucket.get_available() == 10 + + bucket.consume(3) + assert bucket.get_available() == 7 + + def test_token_bucket_reset(self): + """Test resetting token bucket.""" + from runners.gitlab.utils.rate_limiter import TokenBucket + + bucket = TokenBucket(capacity=10, refill_rate=5.0) + + bucket.consume(5) + assert bucket.tokens == 5 + + bucket.reset() + assert bucket.tokens == 10 + + +class TestRateLimiter: + """Test RateLimiter for API rate limiting.""" + + @pytest.fixture + def limiter(self): + """Create a rate limiter for testing.""" + from runners.gitlab.utils.rate_limiter import RateLimiter + + return RateLimiter( + requests_per_minute=60, + burst_size=10, + ) + + def test_rate_limiter_initialization(self): + """Test rate limiter initializes correctly.""" + from runners.gitlab.utils.rate_limiter import RateLimiter + + limiter = RateLimiter( + requests_per_minute=60, + burst_size=10, + ) + + assert limiter.requests_per_minute == 60 + assert limiter.burst_size == 10 + + def test_acquire_request(self, limiter): + """Test acquiring a request slot.""" + success = limiter.acquire() + + assert success is True + + def test_acquire_burst(self, limiter): + """Test burst requests.""" + # Should be able to make burst_size requests immediately + for _ in range(10): + success = limiter.acquire() + assert success is True + + def test_acquire_exceeds_burst(self, limiter): + """Test exceeding burst limit.""" + # Consume burst capacity + for _ in range(10): + limiter.acquire() + + # Next request should fail + success = limiter.acquire() + assert success is False + + def test_acquire_with_wait(self, limiter): + """Test acquire with wait option.""" + # Consume burst + for _ in range(10): + limiter.acquire() + + # Should wait for refill + start = time.time() + success = limiter.acquire(wait=True) + elapsed = time.time() - start + + assert success is True + # At 60 req/min, 1 request = 1 second + assert elapsed >= 0.9 + + def test_get_wait_time(self, limiter): + """Test getting wait time.""" + # No wait needed initially + wait_time = limiter.get_wait_time() + assert wait_time == 0 + + # Consume burst + for _ in range(10): + limiter.acquire() + + # Should need to wait + wait_time = limiter.get_wait_time() + assert wait_time > 0 + + def test_reset(self, limiter): + """Test resetting rate limiter.""" + # Consume some capacity + for _ in range(5): + limiter.acquire() + + limiter.reset() + + # Should have full capacity + success = limiter.acquire() + assert success is True + + def test_rate_limiter_state_tracking(self, limiter): + """Test rate limiter tracks request state.""" + from runners.gitlab.utils.rate_limiter import RateLimiterState + + state = limiter.get_state() + + assert isinstance(state, RateLimiterState) + assert state.available_tokens >= 0 + assert state.available_tokens <= limiter.burst_size + + def test_concurrent_requests(self, limiter): + """Test concurrent request handling.""" + import threading + + results = [] + + def make_request(): + success = limiter.acquire(wait=True) + results.append(success) + + threads = [threading.Thread(target=make_request) for _ in range(15)] + + for t in threads: + t.start() + + for t in threads: + t.join() + + # All requests should succeed (some wait for refill) + assert all(results) + + def test_rate_limiter_persistence(self, limiter, tmp_path): + """Test saving and loading rate limiter state.""" + state_file = tmp_path / "rate_limiter_state.json" + + # Consume some tokens + for _ in range(5): + limiter.acquire() + + # Save state + limiter.save_state(state_file) + + # Create new limiter and load state + from runners.gitlab.utils.rate_limiter import RateLimiter + + new_limiter = RateLimiter( + requests_per_minute=60, + burst_size=10, + ) + new_limiter.load_state(state_file) + + # Should have same state + original_state = limiter.get_state() + loaded_state = new_limiter.get_state() + + assert abs(original_state.available_tokens - loaded_state.available_tokens) < 1 + + +class TestRateLimiterIntegration: + """Integration tests for rate limiting with API calls.""" + + def test_rate_limiter_with_api_client(self): + """Test rate limiter integrates with API client.""" + from runners.gitlab.utils.rate_limiter import RateLimiter + + limiter = RateLimiter( + requests_per_minute=60, + burst_size=5, + ) + + call_count = 0 + + def mock_api_call(): + nonlocal call_count + if limiter.acquire(wait=True): + call_count += 1 + return {"data": "success"} + return {"error": "rate limited"} + + # Make several calls + results = [mock_api_call() for _ in range(8)] + + # Should have made all calls successfully (some waited) + assert call_count == 8 + assert all(r.get("data") for r in results) + + def test_rate_limiter_respects_backoff(self): + """Test rate limiter handles backoff correctly.""" + from runners.gitlab.utils.rate_limiter import RateLimiter + + limiter = RateLimiter( + requests_per_minute=30, # 0.5 req/sec + burst_size=3, + ) + + times = [] + + def track_time(): + times.append(time.time()) + return limiter.acquire(wait=True) + + # Make burst + 1 requests + for _ in range(4): + track_time() + + # First 3 should be immediate (burst) + # 4th should have waited + burst_duration = times[2] - times[0] + wait_duration = times[3] - times[2] + + # 4th request should have taken longer + assert wait_duration > burst_duration + + @pytest.mark.asyncio + async def test_async_rate_limiting(self): + """Test rate limiting with async operations.""" + from runners.gitlab.utils.rate_limiter import RateLimiter + + limiter = RateLimiter( + requests_per_minute=60, + burst_size=5, + ) + + async def make_request(i): + if limiter.acquire(wait=True): + await asyncio.sleep(0.01) # Simulate API call + return f"request-{i}" + return "rate-limited" + + results = await asyncio.gather(*[make_request(i) for i in range(8)]) + + # All should succeed + assert len(results) == 8 + assert all("rate-limited" not in r for r in results) + + +class TestRateLimiterState: + """Test RateLimiterState model.""" + + def test_state_creation(self): + """Test creating state object.""" + from runners.gitlab.utils.rate_limiter import RateLimiterState + + state = RateLimiterState( + available_tokens=5.0, + last_refill_time=1234567890.0, + ) + + assert state.available_tokens == 5.0 + assert state.last_refill_time == 1234567890.0 + + def test_state_to_dict(self): + """Test converting state to dict.""" + from runners.gitlab.utils.rate_limiter import RateLimiterState + + state = RateLimiterState( + available_tokens=7.5, + last_refill_time=1234567890.0, + ) + + data = state.to_dict() + + assert data["available_tokens"] == 7.5 + assert data["last_refill_time"] == 1234567890.0 + + def test_state_from_dict(self): + """Test loading state from dict.""" + from runners.gitlab.utils.rate_limiter import RateLimiterState + + data = { + "available_tokens": 8.0, + "last_refill_time": 1234567890.0, + } + + state = RateLimiterState.from_dict(data) + + assert state.available_tokens == 8.0 + assert state.last_refill_time == 1234567890.0 + + +class TestRateLimiterDecorators: + """Test rate limiter decorators.""" + + def test_rate_limit_decorator(self): + """Test rate limit decorator for functions.""" + from runners.gitlab.utils.rate_limiter import rate_limit + + limiter = type( + "MockLimiter", + (), + { + "acquire": lambda wait=True: True, + }, + )() + + @rate_limit(limiter) + def api_function(): + return "success" + + result = api_function() + assert result == "success" + + def test_rate_limit_decorator_with_wait(self): + """Test rate limit decorator respects wait parameter.""" + from runners.gitlab.utils.rate_limiter import rate_limit + + call_count = 0 + + class MockLimiter: + def acquire(self, wait=True): + nonlocal call_count + call_count += 1 + return call_count <= 3 # Fail after 3 calls + + limiter = MockLimiter() + + @rate_limit(limiter, wait=True) + def api_function(): + return "success" + + # First 3 succeed + for _ in range(3): + result = api_function() + assert result == "success" + + # 4th should fail (would wait but our mock returns False) + result = api_function() + assert result is None + + +class TestAdaptiveRateLimiting: + """Test adaptive rate limiting based on responses.""" + + def test_adaptive_backoff_on_429(self): + """Test adaptive backoff on rate limit errors.""" + from runners.gitlab.utils.rate_limiter import AdaptiveRateLimiter + + limiter = AdaptiveRateLimiter( + requests_per_minute=60, + burst_size=10, + ) + + # Simulate rate limit response + limiter.handle_response(status_code=429) + + # Should reduce rate + state = limiter.get_state() + assert state.adaptive_factor < 1.0 + + def test_adaptive_recovery_on_success(self): + """Test adaptive recovery on successful requests.""" + from runners.gitlab.utils.rate_limiter import AdaptiveRateLimiter + + limiter = AdaptiveRateLimiter( + requests_per_minute=60, + burst_size=10, + ) + + # Trigger backoff + limiter.handle_response(status_code=429) + + # Recover with successful requests + for _ in range(10): + limiter.handle_response(status_code=200) + + # Should recover rate + state = limiter.get_state() + assert state.adaptive_factor >= 0.9 + + def test_adaptive_minimum_rate(self): + """Test adaptive rate has minimum floor.""" + from runners.gitlab.utils.rate_limiter import AdaptiveRateLimiter + + limiter = AdaptiveRateLimiter( + requests_per_minute=60, + burst_size=10, + min_adaptive_factor=0.1, + ) + + # Trigger many backoffs + for _ in range(100): + limiter.handle_response(status_code=429) + + # Should not go below minimum + state = limiter.get_state() + assert state.adaptive_factor >= 0.1 diff --git a/apps/backend/__tests__/test_glab_client.py b/apps/backend/__tests__/test_glab_client.py new file mode 100644 index 00000000..ebcc372b --- /dev/null +++ b/apps/backend/__tests__/test_glab_client.py @@ -0,0 +1,699 @@ +""" +GitLab Client Tests +=================== + +Tests for GitLab client timeout, retry, and async operations. +""" + +import asyncio +import json +from datetime import datetime, timezone +from pathlib import Path +from unittest.mock import AsyncMock, MagicMock, Mock, patch + +import pytest +from requests.exceptions import ConnectionError, RequestException, Timeout + + +class TestGitLabClient: + """Test GitLab client basic operations.""" + + @pytest.fixture + def client(self): + """Create a GitLab client for testing.""" + from runners.gitlab.glab_client import GitLabClient + + return GitLabClient( + token="test-token", + project="group/project", + instance_url="https://gitlab.example.com", + ) + + def test_client_initialization(self, client): + """Test client initializes correctly.""" + assert client.token == "test-token" + assert client.project == "group/project" + assert client.instance_url == "https://gitlab.example.com" + assert client.timeout == 30 + assert client.max_retries == 3 + + def test_client_custom_timeout(self): + """Test client with custom timeout.""" + from runners.gitlab.glab_client import GitLabClient + + client = GitLabClient( + token="test-token", + project="group/project", + timeout=60, + ) + + assert client.timeout == 60 + + def test_client_custom_retries(self): + """Test client with custom retry count.""" + from runners.gitlab.glab_client import GitLabClient + + client = GitLabClient( + token="test-token", + project="group/project", + max_retries=5, + ) + + assert client.max_retries == 5 + + def test_build_url(self, client): + """Test URL building.""" + url = client._build_url("projects", "group%2Fproject", "merge_requests") + + assert "group%2Fproject" in url + assert "merge_requests" in url + + def test_build_url_with_params(self, client): + """Test URL building with query parameters.""" + url = client._build_url( + "projects", + "group%2Fproject", + "merge_requests", + state="opened", + per_page=50, + ) + + assert "state=opened" in url + assert "per_page=50" in url + + +class TestGitLabClientRetry: + """Test GitLab client retry logic.""" + + @pytest.fixture + def client(self): + """Create a GitLab client for testing.""" + from runners.gitlab.glab_client import GitLabClient + + return GitLabClient( + token="test-token", + project="group/project", + max_retries=3, + timeout=1, + ) + + def test_retry_on_timeout(self, client): + """Test retry on timeout exception.""" + call_count = 0 + + def mock_request(*args, **kwargs): + nonlocal call_count + call_count += 1 + if call_count < 3: + raise Timeout("Request timed out") + return {"data": "success"} + + with patch.object(client, "_make_request", mock_request): + result = client.get_mr(123) + + assert call_count == 3 # Initial + 2 retries + assert result["data"] == "success" + + def test_retry_on_connection_error(self, client): + """Test retry on connection error.""" + call_count = 0 + + def mock_request(*args, **kwargs): + nonlocal call_count + call_count += 1 + if call_count < 2: + raise ConnectionError("Connection failed") + return {"id": 123} + + with patch.object(client, "_make_request", mock_request): + result = client.get_mr(123) + + assert call_count == 2 # Initial + 1 retry + assert result["id"] == 123 + + def test_retry_exhausted(self, client): + """Test failure after retry exhaustion.""" + + def mock_request(*args, **kwargs): + raise Timeout("Request timed out") + + with patch.object(client, "_make_request", mock_request): + with pytest.raises(Timeout): + client.get_mr(123) + + def test_retry_with_backoff(self, client): + """Test retry uses exponential backoff.""" + call_times = [] + + def mock_request(*args, **kwargs): + call_times.append(time.time()) + if len(call_times) < 3: + raise Timeout("Request timed out") + return {"data": "success"} + + import time + + with patch.object(client, "_make_request", mock_request): + result = client.get_mr(123) + + # Check delays between retries increase (exponential backoff) + if len(call_times) > 2: + delay1 = call_times[1] - call_times[0] + delay2 = call_times[2] - call_times[1] + # Second delay should be longer + assert delay2 > delay1 + + def test_no_retry_on_client_error(self, client): + """Test no retry on 4xx client errors.""" + from requests.exceptions import HTTPError + + def mock_request(*args, **kwargs): + response = Mock() + response.status_code = 404 + response.raise_for_status.side_effect = HTTPError(response=response) + return response + + with patch.object(client, "_make_request", mock_request): + with pytest.raises(HTTPError): + client.get_mr(123) + + def test_retry_on_server_error(self, client): + """Test retry on 5xx server errors.""" + from requests.exceptions import HTTPError + + call_count = 0 + + def mock_request(*args, **kwargs): + nonlocal call_count + call_count += 1 + if call_count < 2: + response = Mock() + response.status_code = 503 + response.raise_for_status.side_effect = HTTPError(response=response) + return response + return {"id": 123} + + with patch.object(client, "_make_request", mock_request): + result = client.get_mr(123) + + assert call_count == 2 + + +class TestGitLabClientAsync: + """Test GitLab client async operations.""" + + @pytest.fixture + def client(self): + """Create a GitLab client for testing.""" + from runners.gitlab.glab_client import GitLabClient + + return GitLabClient( + token="test-token", + project="group/project", + ) + + @pytest.mark.asyncio + async def test_get_mr_async(self, client): + """Test async get MR.""" + mock_data = { + "iid": 123, + "title": "Test MR", + "state": "opened", + } + + with patch.object(client, "get_mr", return_value=mock_data): + result = await client.get_mr_async(123) + + assert result["iid"] == 123 + assert result["title"] == "Test MR" + + @pytest.mark.asyncio + async def test_get_mr_changes_async(self, client): + """Test async get MR changes.""" + mock_data = { + "changes": [ + { + "old_path": "file.py", + "new_path": "file.py", + "diff": "@@ -1,1 +1,2 @@", + } + ] + } + + with patch.object(client, "get_mr_changes", return_value=mock_data): + result = await client.get_mr_changes_async(123) + + assert len(result["changes"]) == 1 + + @pytest.mark.asyncio + async def test_get_mr_commits_async(self, client): + """Test async get MR commits.""" + mock_data = [ + {"id": "abc123", "message": "Commit 1"}, + {"id": "def456", "message": "Commit 2"}, + ] + + with patch.object(client, "get_mr_commits", return_value=mock_data): + result = await client.get_mr_commits_async(123) + + assert len(result) == 2 + assert result[0]["id"] == "abc123" + + @pytest.mark.asyncio + async def test_get_mr_notes_async(self, client): + """Test async get MR notes.""" + mock_data = [ + {"id": 1001, "body": "Comment 1"}, + {"id": 1002, "body": "Comment 2"}, + ] + + with patch.object(client, "get_mr_notes", return_value=mock_data): + result = await client.get_mr_notes_async(123) + + assert len(result) == 2 + + @pytest.mark.asyncio + async def test_get_mr_pipelines_async(self, client): + """Test async get MR pipelines.""" + mock_data = [ + {"id": 1001, "status": "success"}, + {"id": 1002, "status": "failed"}, + ] + + with patch.object(client, "get_mr_pipelines", return_value=mock_data): + result = await client.get_mr_pipelines_async(123) + + assert len(result) == 2 + + @pytest.mark.asyncio + async def test_get_issue_async(self, client): + """Test async get issue.""" + mock_data = { + "iid": 456, + "title": "Test Issue", + "state": "opened", + } + + with patch.object(client, "get_issue", return_value=mock_data): + result = await client.get_issue_async(456) + + assert result["iid"] == 456 + + @pytest.mark.asyncio + async def test_get_pipeline_async(self, client): + """Test async get pipeline.""" + mock_data = { + "id": 1001, + "status": "running", + "ref": "main", + } + + with patch.object(client, "get_pipeline", return_value=mock_data): + result = await client.get_pipeline_async(1001) + + assert result["id"] == 1001 + + @pytest.mark.asyncio + async def test_get_pipeline_jobs_async(self, client): + """Test async get pipeline jobs.""" + mock_data = [ + {"id": 2001, "name": "test", "status": "success"}, + {"id": 2002, "name": "build", "status": "failed"}, + ] + + with patch.object(client, "get_pipeline_jobs", return_value=mock_data): + result = await client.get_pipeline_jobs_async(1001) + + assert len(result) == 2 + + @pytest.mark.asyncio + async def test_concurrent_async_requests(self, client): + """Test concurrent async requests.""" + + async def fetch_mr(iid): + return await client.get_mr_async(iid) + + mock_data = { + "iid": 123, + "title": "Test MR", + } + + with patch.object(client, "get_mr", return_value=mock_data): + results = await asyncio.gather( + fetch_mr(123), + fetch_mr(456), + fetch_mr(789), + ) + + assert len(results) == 3 + + @pytest.mark.asyncio + async def test_async_error_handling(self, client): + """Test async error handling.""" + with patch.object(client, "get_mr", side_effect=Exception("API Error")): + with pytest.raises(Exception, match="API Error"): + await client.get_mr_async(123) + + +class TestGitLabClientAPI: + """Test GitLab client API methods.""" + + @pytest.fixture + def client(self): + """Create a GitLab client for testing.""" + from runners.gitlab.glab_client import GitLabClient + + return GitLabClient( + token="test-token", + project="group/project", + ) + + def test_get_mr(self, client): + """Test getting MR details.""" + mock_response = { + "iid": 123, + "title": "Test MR", + "description": "Test description", + "state": "opened", + "author": {"username": "john_doe"}, + } + + with patch.object(client, "_make_request", return_value=mock_response): + result = client.get_mr(123) + + assert result["iid"] == 123 + assert result["title"] == "Test MR" + + def test_get_mr_changes(self, client): + """Test getting MR changes.""" + mock_response = { + "changes": [ + { + "old_path": "src/file.py", + "new_path": "src/file.py", + "diff": "@@ -1,1 +1,2 @@", + } + ] + } + + with patch.object(client, "_make_request", return_value=mock_response): + result = client.get_mr_changes(123) + + assert len(result["changes"]) == 1 + + def test_get_mr_commits(self, client): + """Test getting MR commits.""" + mock_response = [ + {"id": "abc123", "message": "First commit"}, + {"id": "def456", "message": "Second commit"}, + ] + + with patch.object(client, "_make_request", return_value=mock_response): + result = client.get_mr_commits(123) + + assert len(result) == 2 + + def test_get_mr_notes(self, client): + """Test getting MR discussion notes.""" + mock_response = [ + {"id": 1001, "body": "Review comment", "author": {"username": "reviewer"}}, + ] + + with patch.object(client, "_make_request", return_value=mock_response): + result = client.get_mr_notes(123) + + assert len(result) == 1 + + def test_post_mr_note(self, client): + """Test posting note to MR.""" + mock_response = {"id": 1002, "body": "New comment"} + + with patch.object(client, "_make_request", return_value=mock_response): + result = client.post_mr_note(123, "New comment") + + assert result["id"] == 1002 + + def test_get_mr_pipelines(self, client): + """Test getting MR pipelines.""" + mock_response = [ + {"id": 1001, "status": "success", "ref": "feature"}, + ] + + with patch.object(client, "_make_request", return_value=mock_response): + result = client.get_mr_pipelines(123) + + assert len(result) == 1 + + def test_get_pipeline(self, client): + """Test getting pipeline details.""" + mock_response = { + "id": 1001, + "status": "success", + "ref": "main", + "sha": "abc123", + } + + with patch.object(client, "_make_request", return_value=mock_response): + result = client.get_pipeline(1001) + + assert result["id"] == 1001 + + def test_get_pipeline_jobs(self, client): + """Test getting pipeline jobs.""" + mock_response = [ + {"id": 2001, "name": "test", "stage": "test", "status": "passed"}, + {"id": 2002, "name": "build", "stage": "build", "status": "failed"}, + ] + + with patch.object(client, "_make_request", return_value=mock_response): + result = client.get_pipeline_jobs(1001) + + assert len(result) == 2 + assert result[1]["status"] == "failed" + + def test_get_issue(self, client): + """Test getting issue details.""" + mock_response = { + "iid": 456, + "title": "Test Issue", + "description": "Issue description", + "state": "opened", + } + + with patch.object(client, "_make_request", return_value=mock_response): + result = client.get_issue(456) + + assert result["iid"] == 456 + + def test_list_issues(self, client): + """Test listing issues.""" + mock_response = [ + {"iid": 456, "title": "Issue 1"}, + {"iid": 457, "title": "Issue 2"}, + ] + + with patch.object(client, "_make_request", return_value=mock_response): + result = client.list_issues(state="opened") + + assert len(result) == 2 + + def test_post_issue_note(self, client): + """Test posting note to issue.""" + mock_response = {"id": 2001, "body": "Issue comment"} + + with patch.object(client, "_make_request", return_value=mock_response): + result = client.post_issue_note(456, "Issue comment") + + assert result["id"] == 2001 + + def test_get_file(self, client): + """Test getting file from repository.""" + mock_response = { + "file_name": "README.md", + "content": "SGVsbG8gV29ybGQ=", # Base64 encoded + "encoding": "base64", + } + + with patch.object(client, "_make_request", return_value=mock_response): + result = client.get_file("README.md", ref="main") + + assert result["file_name"] == "README.md" + + def test_list_projects(self, client): + """Test listing projects.""" + mock_response = [ + {"id": 1, "name": "project1"}, + {"id": 2, "name": "project2"}, + ] + + with patch.object(client, "_make_request", return_value=mock_response): + result = client.list_projects() + + assert len(result) == 2 + + +class TestGitLabClientAuth: + """Test GitLab client authentication.""" + + def test_token_in_headers(self): + """Test token is included in request headers.""" + from runners.gitlab.glab_client import GitLabClient + + client = GitLabClient( + token="test-token-12345", + project="group/project", + ) + + with patch("requests.request") as mock_request: + mock_request.return_value = Mock(json=lambda: {}) + + client.get_mr(123) + + call_kwargs = mock_request.call_args[1] + headers = call_kwargs.get("headers", {}) + + assert "PRIVATE-TOKEN" in headers + assert headers["PRIVATE-TOKEN"] == "test-token-12345" + + def test_custom_instance_url(self): + """Test custom instance URL.""" + from runners.gitlab.glab_client import GitLabClient + + client = GitLabClient( + token="test-token", + project="group/project", + instance_url="https://gitlab.custom.com", + ) + + with patch("requests.request") as mock_request: + mock_request.return_value = Mock(json=lambda: {}) + + client.get_mr(123) + + call_args = mock_request.call_args[0] + url = call_args[0] + + assert "gitlab.custom.com" in url + + +class TestGitLabClientConfig: + """Test GitLab configuration model.""" + + def test_config_creation(self): + """Test creating GitLab config.""" + from runners.gitlab.glab_client import GitLabConfig + + config = GitLabConfig( + token="test-token", + project="group/project", + instance_url="https://gitlab.example.com", + ) + + assert config.token == "test-token" + assert config.project == "group/project" + + def test_config_defaults(self): + """Test config has sensible defaults.""" + from runners.gitlab.glab_client import GitLabConfig + + config = GitLabConfig( + token="test-token", + project="group/project", + ) + + assert config.instance_url == "https://gitlab.com" + assert config.timeout == 30 + assert config.max_retries == 3 + + def test_config_to_dict(self): + """Test converting config to dict.""" + from runners.gitlab.glab_client import GitLabConfig + + config = GitLabConfig( + token="test-token", + project="group/project", + ) + + data = config.to_dict() + + assert data["token"] == "test-token" + assert data["project"] == "group/project" + + def test_config_from_dict(self): + """Test loading config from dict.""" + from runners.gitlab.glab_client import GitLabConfig + + data = { + "token": "test-token", + "project": "group/project", + "instance_url": "https://gitlab.example.com", + } + + config = GitLabConfig.from_dict(data) + + assert config.token == "test-token" + assert config.instance_url == "https://gitlab.example.com" + + +class TestGitLabClientErrorHandling: + """Test GitLab client error handling.""" + + @pytest.fixture + def client(self): + """Create a GitLab client for testing.""" + from runners.gitlab.glab_client import GitLabClient + + return GitLabClient( + token="test-token", + project="group/project", + ) + + def test_http_404_handling(self, client): + """Test 404 error handling.""" + from requests.exceptions import HTTPError + + def mock_request(*args, **kwargs): + response = Mock() + response.status_code = 404 + response.text = "404 Not Found" + response.raise_for_status.side_effect = HTTPError(response=response) + return response + + with patch.object(client, "_make_request", mock_request): + with pytest.raises(HTTPError): + client.get_mr(99999) + + def test_http_403_handling(self, client): + """Test 403 forbidden error handling.""" + from requests.exceptions import HTTPError + + def mock_request(*args, **kwargs): + response = Mock() + response.status_code = 403 + response.text = "403 Forbidden" + response.raise_for_status.side_effect = HTTPError(response=response) + return response + + with patch.object(client, "_make_request", mock_request): + with pytest.raises(HTTPError): + client.get_mr(123) + + def test_network_error_handling(self, client): + """Test network error handling.""" + from requests.exceptions import ConnectionError + + with patch.object( + client, "_make_request", side_effect=ConnectionError("Network error") + ): + with pytest.raises(ConnectionError): + client.get_mr(123) + + def test_timeout_handling(self, client): + """Test timeout handling.""" + from requests.exceptions import Timeout + + with patch.object( + client, "_make_request", side_effect=Timeout("Request timed out") + ): + with pytest.raises(Timeout): + client.get_mr(123) diff --git a/apps/backend/runners/gitlab/bot_detection.py b/apps/backend/runners/gitlab/bot_detection.py new file mode 100644 index 00000000..31948924 --- /dev/null +++ b/apps/backend/runners/gitlab/bot_detection.py @@ -0,0 +1,507 @@ +""" +Bot Detection for GitLab Automation +==================================== + +Prevents infinite loops by detecting when the bot is reviewing its own work. + +Key Features: +- Identifies bot user from configured token +- Skips MRs authored by the bot +- Skips re-reviewing bot commits +- Implements "cooling off" period to prevent rapid re-reviews +- Tracks reviewed commits to avoid duplicate reviews + +Usage: + detector = BotDetector( + state_dir=Path("/path/to/state"), + bot_username="auto-claude-bot", + review_own_mrs=False + ) + + # Check if MR should be skipped + should_skip, reason = detector.should_skip_mr_review(mr_iid=123, mr_data={}, commits=[]) + if should_skip: + print(f"Skipping MR: {reason}") + return + + # After successful review, mark as reviewed + detector.mark_reviewed(mr_iid=123, commit_sha="abc123") +""" + +from __future__ import annotations + +import json +import logging +from dataclasses import dataclass, field +from datetime import datetime, timedelta +from pathlib import Path + +logger = logging.getLogger(__name__) + +try: + from .utils.file_lock import FileLock, atomic_write +except (ImportError, ValueError, SystemError): + from utils.file_lock import FileLock, atomic_write + + +@dataclass +class BotDetectionState: + """State for tracking reviewed MRs and commits.""" + + # MR IID -> set of reviewed commit SHAs + reviewed_commits: dict[int, list[str]] = field(default_factory=dict) + + # MR IID -> last review timestamp (ISO format) + last_review_times: dict[int, str] = field(default_factory=dict) + + def to_dict(self) -> dict: + """Convert to dictionary for JSON serialization.""" + return { + "reviewed_commits": self.reviewed_commits, + "last_review_times": self.last_review_times, + } + + @classmethod + def from_dict(cls, data: dict) -> BotDetectionState: + """Load from dictionary.""" + return cls( + reviewed_commits=data.get("reviewed_commits", {}), + last_review_times=data.get("last_review_times", {}), + ) + + def save(self, state_dir: Path) -> None: + """Save state to disk with file locking for concurrent safety.""" + state_dir.mkdir(parents=True, exist_ok=True) + state_file = state_dir / "bot_detection_state.json" + + # Use file locking to prevent concurrent write corruption + with FileLock(state_file, timeout=5.0, exclusive=True): + with atomic_write(state_file) as f: + json.dump(self.to_dict(), f, indent=2) + + @classmethod + def load(cls, state_dir: Path) -> BotDetectionState: + """Load state from disk.""" + state_file = state_dir / "bot_detection_state.json" + + if not state_file.exists(): + return cls() + + with open(state_file) as f: + return cls.from_dict(json.load(f)) + + +# Known GitLab bot account patterns +GITLAB_BOT_PATTERNS = [ + # GitLab official bots + "gitlab-bot", + "gitlab", + # Bot suffixes + "[bot]", + "-bot", + "_bot", + ".bot", + # AI coding assistants + "coderabbit", + "greptile", + "cursor", + "sweep", + "codium", + "dependabot", + "renovate", + # Auto-generated patterns + "project_", + "bot_", +] + + +class BotDetector: + """ + Detects bot-authored MRs and commits to prevent infinite review loops. + + Configuration: + - bot_username: GitLab username of the bot account + - review_own_mrs: Whether bot can review its own MRs + + Automatic safeguards: + - 1-minute cooling off period between reviews of same MR + - Tracks reviewed commit SHAs to avoid duplicate reviews + - Identifies bot user by username to skip bot-authored content + """ + + # Cooling off period in minutes + COOLING_OFF_MINUTES = 1 + + def __init__( + self, + state_dir: Path, + bot_username: str | None = None, + review_own_mrs: bool = False, + ): + """ + Initialize bot detector. + + Args: + state_dir: Directory for storing detection state + bot_username: GitLab username of the bot (to identify bot user) + review_own_mrs: Whether to allow reviewing bot's own MRs + """ + self.state_dir = state_dir + self.bot_username = bot_username + self.review_own_mrs = review_own_mrs + + # Load or initialize state + self.state = BotDetectionState.load(state_dir) + + logger.info( + f"Initialized BotDetector: bot_user={bot_username}, review_own_mrs={review_own_mrs}" + ) + + def _is_bot_username(self, username: str | None) -> bool: + """ + Check if a username matches known bot patterns. + + Args: + username: Username to check + + Returns: + True if username matches bot patterns + """ + if not username: + return False + + username_lower = username.lower() + + # Check against known patterns + for pattern in GITLAB_BOT_PATTERNS: + if pattern.lower() in username_lower: + return True + + return False + + def is_bot_mr(self, mr_data: dict) -> bool: + """ + Check if MR was created by the bot. + + Args: + mr_data: MR data from GitLab API (must have 'author' field) + + Returns: + True if MR author matches bot username or bot patterns + """ + author_data = mr_data.get("author", {}) + if not author_data: + return False + + author = author_data.get("username") + + # Check if matches configured bot username + if not self.review_own_mrs and author == self.bot_username: + logger.info(f"MR is bot-authored: {author}") + return True + + # Check if matches bot patterns + if not self.review_own_mrs and self._is_bot_username(author): + logger.info(f"MR matches bot pattern: {author}") + return True + + return False + + def is_bot_commit(self, commit_data: dict) -> bool: + """ + Check if commit was authored by the bot. + + Args: + commit_data: Commit data from GitLab API (must have 'author' field) + + Returns: + True if commit author matches bot username or bot patterns + """ + author_data = commit_data.get("author") or commit_data.get("author_email") + if not author_data: + return False + + if isinstance(author_data, dict): + author = author_data.get("username") or author_data.get("email") + else: + author = author_data + + # Extract username from email if needed + if "@" in str(author): + author = str(author).split("@")[0] + + # Check if matches configured bot username + if not self.review_own_mrs and author == self.bot_username: + logger.info(f"Commit is bot-authored: {author}") + return True + + # Check if matches bot patterns + if not self.review_own_mrs and self._is_bot_username(author): + logger.info(f"Commit matches bot pattern: {author}") + return True + + # Check for AI commit patterns + commit_message = commit_data.get("message", "") + if not self.review_own_mrs and self._is_ai_commit(commit_message): + logger.info("Commit has AI pattern in message") + return True + + return False + + def _is_ai_commit(self, commit_message: str) -> bool: + """ + Check if commit message indicates AI-generated commit. + + Args: + commit_message: Commit message text + + Returns: + True if commit appears to be AI-generated + """ + if not commit_message: + return False + + message_lower = commit_message.lower() + + # Check for AI co-authorship patterns + ai_patterns = [ + "co-authored-by: claude", + "co-authored-by: gpt", + "co-authored-by: gemini", + "co-authored-by: ai assistant", + "generated by ai", + "auto-generated", + ] + + for pattern in ai_patterns: + if pattern in message_lower: + return True + + return False + + def get_last_commit_sha(self, commits: list[dict]) -> str | None: + """ + Get the SHA of the most recent commit. + + Args: + commits: List of commit data from GitLab API + + Returns: + SHA of latest commit or None if no commits + """ + if not commits: + return None + + # GitLab API returns commits in chronological order (oldest first, newest last) + latest = commits[-1] + return latest.get("id") or latest.get("sha") + + def is_within_cooling_off(self, mr_iid: int) -> tuple[bool, str]: + """ + Check if MR is within cooling off period. + + Args: + mr_iid: The MR IID + + Returns: + Tuple of (is_cooling_off, reason_message) + """ + last_review_str = self.state.last_review_times.get(str(mr_iid)) + + if not last_review_str: + return False, "" + + try: + last_review = datetime.fromisoformat(last_review_str) + time_since = datetime.now() - last_review + + if time_since < timedelta(minutes=self.COOLING_OFF_MINUTES): + minutes_left = self.COOLING_OFF_MINUTES - ( + time_since.total_seconds() / 60 + ) + reason = ( + f"Cooling off period active (reviewed {int(time_since.total_seconds() / 60)}m ago, " + f"{int(minutes_left)}m remaining)" + ) + logger.info(f"MR !{mr_iid}: {reason}") + return True, reason + + except (ValueError, TypeError) as e: + logger.error(f"Error parsing last review time: {e}") + + return False, "" + + def has_reviewed_commit(self, mr_iid: int, commit_sha: str) -> bool: + """ + Check if we've already reviewed this specific commit. + + Args: + mr_iid: The MR IID + commit_sha: The commit SHA to check + + Returns: + True if this commit was already reviewed + """ + reviewed = self.state.reviewed_commits.get(str(mr_iid), []) + return commit_sha in reviewed + + def should_skip_mr_review( + self, + mr_iid: int, + mr_data: dict, + commits: list[dict] | None = None, + ) -> tuple[bool, str]: + """ + Determine if we should skip reviewing this MR. + + This is the main entry point for bot detection logic. + + Args: + mr_iid: The MR IID + mr_data: MR data from GitLab API + commits: Optional list of commits in the MR + + Returns: + Tuple of (should_skip, reason) + """ + # Check 1: Is this a bot-authored MR? + if not self.review_own_mrs and self.is_bot_mr(mr_data): + reason = f"MR authored by bot user ({self.bot_username or 'bot pattern'})" + logger.info(f"SKIP MR !{mr_iid}: {reason}") + return True, reason + + # Check 2: Is the latest commit by the bot? + # Note: GitLab API returns commits oldest-first, so commits[-1] is the latest + if commits and not self.review_own_mrs: + latest_commit = commits[-1] if commits else None + if latest_commit and self.is_bot_commit(latest_commit): + reason = "Latest commit authored by bot (likely an auto-fix)" + logger.info(f"SKIP MR !{mr_iid}: {reason}") + return True, reason + + # Check 3: Are we in the cooling off period? + is_cooling, reason = self.is_within_cooling_off(mr_iid) + if is_cooling: + logger.info(f"SKIP MR !{mr_iid}: {reason}") + return True, reason + + # Check 4: Have we already reviewed this exact commit? + head_sha = self.get_last_commit_sha(commits) if commits else None + if head_sha and self.has_reviewed_commit(mr_iid, head_sha): + reason = f"Already reviewed commit {head_sha[:8]}" + logger.info(f"SKIP MR !{mr_iid}: {reason}") + return True, reason + + # All checks passed - safe to review + logger.info(f"MR !{mr_iid} is safe to review") + return False, "" + + def mark_reviewed(self, mr_iid: int, commit_sha: str) -> None: + """ + Mark an MR as reviewed at a specific commit. + + This should be called after successfully posting a review. + + Args: + mr_iid: The MR IID + commit_sha: The commit SHA that was reviewed + """ + mr_key = str(mr_iid) + + # Add to reviewed commits + if mr_key not in self.state.reviewed_commits: + self.state.reviewed_commits[mr_key] = [] + + if commit_sha not in self.state.reviewed_commits[mr_key]: + self.state.reviewed_commits[mr_key].append(commit_sha) + + # Update last review time + self.state.last_review_times[mr_key] = datetime.now().isoformat() + + # Save state + self.state.save(self.state_dir) + + logger.info( + f"Marked MR !{mr_iid} as reviewed at {commit_sha[:8]} " + f"({len(self.state.reviewed_commits[mr_key])} total commits reviewed)" + ) + + def clear_mr_state(self, mr_iid: int) -> None: + """ + Clear tracking state for an MR (e.g., when MR is closed/merged). + + Args: + mr_iid: The MR IID + """ + mr_key = str(mr_iid) + + if mr_key in self.state.reviewed_commits: + del self.state.reviewed_commits[mr_key] + + if mr_key in self.state.last_review_times: + del self.state.last_review_times[mr_key] + + self.state.save(self.state_dir) + + logger.info(f"Cleared state for MR !{mr_iid}") + + def get_stats(self) -> dict: + """ + Get statistics about bot detection activity. + + Returns: + Dictionary with stats + """ + total_mrs = len(self.state.reviewed_commits) + total_reviews = sum( + len(commits) for commits in self.state.reviewed_commits.values() + ) + + return { + "bot_username": self.bot_username, + "review_own_mrs": self.review_own_mrs, + "total_mrs_tracked": total_mrs, + "total_reviews_performed": total_reviews, + "cooling_off_minutes": self.COOLING_OFF_MINUTES, + } + + def cleanup_stale_mrs(self, max_age_days: int = 30) -> int: + """ + Remove tracking state for MRs that haven't been reviewed recently. + + This prevents unbounded growth of the state file by cleaning up + entries for MRs that are likely closed/merged. + + Args: + max_age_days: Remove MRs not reviewed in this many days (default: 30) + + Returns: + Number of MRs cleaned up + """ + cutoff = datetime.now() - timedelta(days=max_age_days) + mrs_to_remove: list[str] = [] + + for mr_key, last_review_str in self.state.last_review_times.items(): + try: + last_review = datetime.fromisoformat(last_review_str) + if last_review < cutoff: + mrs_to_remove.append(mr_key) + except (ValueError, TypeError): + # Invalid timestamp - mark for removal + mrs_to_remove.append(mr_key) + + # Remove stale MRs + for mr_key in mrs_to_remove: + if mr_key in self.state.reviewed_commits: + del self.state.reviewed_commits[mr_key] + if mr_key in self.state.last_review_times: + del self.state.last_review_times[mr_key] + + if mrs_to_remove: + self.state.save(self.state_dir) + logger.info( + f"Cleaned up {len(mrs_to_remove)} stale MRs " + f"(older than {max_age_days} days)" + ) + + return len(mrs_to_remove) diff --git a/apps/backend/runners/gitlab/glab_client.py b/apps/backend/runners/gitlab/glab_client.py index 4b2d47d1..4d39f923 100644 --- a/apps/backend/runners/gitlab/glab_client.py +++ b/apps/backend/runners/gitlab/glab_client.py @@ -4,10 +4,14 @@ GitLab API Client Client for GitLab API operations. Uses direct API calls with PRIVATE-TOKEN authentication. + +Supports both synchronous and asynchronous methods for compatibility +with provider-agnostic interfaces. """ from __future__ import annotations +import asyncio import json import time import urllib.parse @@ -244,6 +248,337 @@ class GitLabClient: data={"assignee_ids": user_ids}, ) + # ------------------------------------------------------------------------- + # Issue Operations + # ------------------------------------------------------------------------- + + def get_issue(self, issue_iid: int) -> dict: + """Get issue details.""" + encoded_project = encode_project_path(self.config.project) + return self._fetch(f"/projects/{encoded_project}/issues/{issue_iid}") + + def list_issues( + self, + state: str | None = None, + labels: list[str] | None = None, + author: str | None = None, + assignee: str | None = None, + per_page: int = 100, + ) -> list[dict]: + """List issues with optional filters.""" + encoded_project = encode_project_path(self.config.project) + params = {"per_page": per_page} + + if state: + params["state"] = state + if labels: + params["labels"] = ",".join(labels) + if author: + params["author_username"] = author + if assignee: + params["assignee_username"] = assignee + + return self._fetch(f"/projects/{encoded_project}/issues", params=params) + + def create_issue( + self, + title: str, + description: str, + labels: list[str] | None = None, + assignee_ids: list[int] | None = None, + ) -> dict: + """Create a new issue.""" + encoded_project = encode_project_path(self.config.project) + data = { + "title": title, + "description": description, + } + + if labels: + data["labels"] = ",".join(labels) + if assignee_ids: + data["assignee_ids"] = assignee_ids + + return self._fetch( + f"/projects/{encoded_project}/issues", + method="POST", + data=data, + ) + + def update_issue( + self, + issue_iid: int, + state_event: str | None = None, + labels: list[str] | None = None, + ) -> dict: + """Update an issue.""" + encoded_project = encode_project_path(self.config.project) + data = {} + + if state_event: + data["state_event"] = state_event # "close" or "reopen" + if labels: + data["labels"] = ",".join(labels) + + return self._fetch( + f"/projects/{encoded_project}/issues/{issue_iid}", + method="PUT", + data=data if data else None, + ) + + def post_issue_note(self, issue_iid: int, body: str) -> dict: + """Post a note (comment) to an issue.""" + encoded_project = encode_project_path(self.config.project) + return self._fetch( + f"/projects/{encoded_project}/issues/{issue_iid}/notes", + method="POST", + data={"body": body}, + ) + + def get_issue_notes(self, issue_iid: int) -> list[dict]: + """Get all notes (comments) for an issue.""" + encoded_project = encode_project_path(self.config.project) + return self._fetch( + f"/projects/{encoded_project}/issues/{issue_iid}/notes", + params={"per_page": 100}, + ) + + # ------------------------------------------------------------------------- + # MR Discussion and Comment Operations + # ------------------------------------------------------------------------- + + def get_mr_discussions(self, mr_iid: int) -> list[dict]: + """Get all discussions for an MR.""" + encoded_project = encode_project_path(self.config.project) + return self._fetch( + f"/projects/{encoded_project}/merge_requests/{mr_iid}/discussions", + params={"per_page": 100}, + ) + + def get_mr_notes(self, mr_iid: int) -> list[dict]: + """Get all notes (comments) for an MR.""" + encoded_project = encode_project_path(self.config.project) + return self._fetch( + f"/projects/{encoded_project}/merge_requests/{mr_iid}/notes", + params={"per_page": 100}, + ) + + def post_mr_discussion_note( + self, + mr_iid: int, + discussion_id: str, + body: str, + ) -> dict: + """Post a note to an existing discussion.""" + encoded_project = encode_project_path(self.config.project) + return self._fetch( + f"/projects/{encoded_project}/merge_requests/{mr_iid}/discussions/{discussion_id}/notes", + method="POST", + data={"body": body}, + ) + + def resolve_mr_discussion(self, mr_iid: int, discussion_id: str) -> dict: + """Resolve a discussion thread.""" + encoded_project = encode_project_path(self.config.project) + return self._fetch( + f"/projects/{encoded_project}/merge_requests/{mr_iid}/discussions/{discussion_id}", + method="PUT", + data={"resolved": True}, + ) + + # ------------------------------------------------------------------------- + # Pipeline and CI Operations + # ------------------------------------------------------------------------- + + def get_mr_pipelines(self, mr_iid: int) -> list[dict]: + """Get all pipelines for an MR.""" + encoded_project = encode_project_path(self.config.project) + return self._fetch( + f"/projects/{encoded_project}/merge_requests/{mr_iid}/pipelines", + params={"per_page": 50}, + ) + + def get_pipeline_status(self, pipeline_id: int) -> dict: + """Get detailed status for a specific pipeline.""" + encoded_project = encode_project_path(self.config.project) + return self._fetch(f"/projects/{encoded_project}/pipelines/{pipeline_id}") + + def get_pipeline_jobs(self, pipeline_id: int) -> list[dict]: + """Get all jobs for a pipeline.""" + encoded_project = encode_project_path(self.config.project) + return self._fetch( + f"/projects/{encoded_project}/pipelines/{pipeline_id}/jobs", + params={"per_page": 100}, + ) + + def get_project_pipelines( + self, + ref: str | None = None, + status: str | None = None, + per_page: int = 50, + ) -> list[dict]: + """Get pipelines for the project.""" + encoded_project = encode_project_path(self.config.project) + params = {"per_page": per_page} + + if ref: + params["ref"] = ref + if status: + params["status"] = status + + return self._fetch( + f"/projects/{encoded_project}/pipelines", + params=params, + ) + + # ------------------------------------------------------------------------- + # Commit Operations + # ------------------------------------------------------------------------- + + def get_commit(self, sha: str) -> dict: + """Get details for a specific commit.""" + encoded_project = encode_project_path(self.config.project) + return self._fetch(f"/projects/{encoded_project}/repository/commits/{sha}") + + def get_commit_diff(self, sha: str) -> list[dict]: + """Get diff for a specific commit.""" + encoded_project = encode_project_path(self.config.project) + return self._fetch(f"/projects/{encoded_project}/repository/commits/{sha}/diff") + + # ------------------------------------------------------------------------- + # User and Permission Operations + # ------------------------------------------------------------------------- + + def get_user_by_username(self, username: str) -> dict | None: + """Get user details by username.""" + users = self._fetch("/users", params={"username": username}) + return users[0] if users else None + + def get_project_members(self, query: str | None = None) -> list[dict]: + """Get members of the project.""" + encoded_project = encode_project_path(self.config.project) + params = {"per_page": 100} + + if query: + params["query"] = query + + return self._fetch( + f"/projects/{encoded_project}/members/all", + params=params, + ) + + # ------------------------------------------------------------------------- + # Async Methods + # ------------------------------------------------------------------------- + + async def _fetch_async( + self, + endpoint: str, + method: str = "GET", + data: dict | None = None, + timeout: float | None = None, + ) -> Any: + """Async wrapper around _fetch that runs in thread pool.""" + loop = asyncio.get_event_loop() + return await loop.run_in_executor( + None, + lambda: self._fetch( + endpoint, + method=method, + data=data, + timeout=timeout, + ), + ) + + async def get_mr_async(self, mr_iid: int) -> dict: + """Async version of get_mr.""" + return await self._fetch_async( + f"/projects/{encode_project_path(self.config.project)}/merge_requests/{mr_iid}" + ) + + async def get_mr_changes_async(self, mr_iid: int) -> dict: + """Async version of get_mr_changes.""" + encoded_project = encode_project_path(self.config.project) + return await self._fetch_async( + f"/projects/{encoded_project}/merge_requests/{mr_iid}/changes" + ) + + async def get_mr_diff_async(self, mr_iid: int) -> str: + """Async version of get_mr_diff.""" + changes = await self.get_mr_changes_async(mr_iid) + diffs = [] + for change in changes.get("changes", []): + diff = change.get("diff", "") + if diff: + diffs.append(diff) + return "\n".join(diffs) + + async def get_mr_commits_async(self, mr_iid: int) -> list[dict]: + """Async version of get_mr_commits.""" + encoded_project = encode_project_path(self.config.project) + return await self._fetch_async( + f"/projects/{encoded_project}/merge_requests/{mr_iid}/commits" + ) + + async def post_mr_note_async(self, mr_iid: int, body: str) -> dict: + """Async version of post_mr_note.""" + encoded_project = encode_project_path(self.config.project) + return await self._fetch_async( + f"/projects/{encoded_project}/merge_requests/{mr_iid}/notes", + method="POST", + data={"body": body}, + ) + + async def approve_mr_async(self, mr_iid: int) -> dict: + """Async version of approve_mr.""" + encoded_project = encode_project_path(self.config.project) + return await self._fetch_async( + f"/projects/{encoded_project}/merge_requests/{mr_iid}/approve", + method="POST", + ) + + async def merge_mr_async(self, mr_iid: int, squash: bool = False) -> dict: + """Async version of merge_mr.""" + encoded_project = encode_project_path(self.config.project) + data = {} + if squash: + data["squash"] = True + return await self._fetch_async( + f"/projects/{encoded_project}/merge_requests/{mr_iid}/merge", + method="PUT", + data=data if data else None, + ) + + async def get_issue_async(self, issue_iid: int) -> dict: + """Async version of get_issue.""" + encoded_project = encode_project_path(self.config.project) + return await self._fetch_async( + f"/projects/{encoded_project}/issues/{issue_iid}" + ) + + async def get_mr_discussions_async(self, mr_iid: int) -> list[dict]: + """Async version of get_mr_discussions.""" + encoded_project = encode_project_path(self.config.project) + return await self._fetch_async( + f"/projects/{encoded_project}/merge_requests/{mr_iid}/discussions", + params={"per_page": 100}, + ) + + async def get_mr_pipelines_async(self, mr_iid: int) -> list[dict]: + """Async version of get_mr_pipelines.""" + encoded_project = encode_project_path(self.config.project) + return await self._fetch_async( + f"/projects/{encoded_project}/merge_requests/{mr_iid}/pipelines", + params={"per_page": 50}, + ) + + async def get_pipeline_status_async(self, pipeline_id: int) -> dict: + """Async version of get_pipeline_status.""" + encoded_project = encode_project_path(self.config.project) + return await self._fetch_async( + f"/projects/{encoded_project}/pipelines/{pipeline_id}" + ) + def load_gitlab_config(project_dir: Path) -> GitLabConfig | None: """Load GitLab config from project's .auto-claude/gitlab/config.json.""" diff --git a/apps/backend/runners/gitlab/models.py b/apps/backend/runners/gitlab/models.py index b0ccb4d6..e5f5c996 100644 --- a/apps/backend/runners/gitlab/models.py +++ b/apps/backend/runners/gitlab/models.py @@ -43,6 +43,8 @@ class ReviewPass(str, Enum): SECURITY = "security" QUALITY = "quality" DEEP_ANALYSIS = "deep_analysis" + STRUCTURAL = "structural" + AI_COMMENT_TRIAGE = "ai_comment_triage" class MergeVerdict(str, Enum): @@ -68,6 +70,10 @@ class MRReviewFinding: end_line: int | None = None suggested_fix: str | None = None fixable: bool = False + # Evidence-based findings - code snippet proving the issue + evidence_code: str | None = None + # Pass that found this issue + found_by_pass: ReviewPass | None = None def to_dict(self) -> dict: return { @@ -81,10 +87,13 @@ class MRReviewFinding: "end_line": self.end_line, "suggested_fix": self.suggested_fix, "fixable": self.fixable, + "evidence_code": self.evidence_code, + "found_by_pass": self.found_by_pass.value if self.found_by_pass else None, } @classmethod def from_dict(cls, data: dict) -> MRReviewFinding: + found_by_pass = data.get("found_by_pass") return cls( id=data["id"], severity=ReviewSeverity(data["severity"]), @@ -96,6 +105,77 @@ class MRReviewFinding: end_line=data.get("end_line"), suggested_fix=data.get("suggested_fix"), fixable=data.get("fixable", False), + evidence_code=data.get("evidence_code"), + found_by_pass=ReviewPass(found_by_pass) if found_by_pass else None, + ) + + +@dataclass +class StructuralIssue: + """A structural issue detected during review (feature creep, scope changes).""" + + id: str + type: str # "feature_creep", "scope_change", "missing_requirement", etc. + title: str + description: str + severity: ReviewSeverity = ReviewSeverity.MEDIUM + files_affected: list[str] = field(default_factory=list) + + def to_dict(self) -> dict: + return { + "id": self.id, + "type": self.type, + "title": self.title, + "description": self.description, + "severity": self.severity.value, + "files_affected": self.files_affected, + } + + @classmethod + def from_dict(cls, data: dict) -> StructuralIssue: + return cls( + id=data["id"], + type=data["type"], + title=data["title"], + description=data["description"], + severity=ReviewSeverity(data.get("severity", "medium")), + files_affected=data.get("files_affected", []), + ) + + +@dataclass +class AICommentTriage: + """Result of triaging another AI tool's comment.""" + + comment_id: str + tool_name: str # "CodeRabbit", "Cursor", etc. + original_comment: str + triage_result: str # "valid", "false_positive", "questionable", "addressed" + reasoning: str + file: str | None = None + line: int | None = None + + def to_dict(self) -> dict: + return { + "comment_id": self.comment_id, + "tool_name": self.tool_name, + "original_comment": self.original_comment, + "triage_result": self.triage_result, + "reasoning": self.reasoning, + "file": self.file, + "line": self.line, + } + + @classmethod + def from_dict(cls, data: dict) -> AICommentTriage: + return cls( + comment_id=data["comment_id"], + tool_name=data["tool_name"], + original_comment=data["original_comment"], + triage_result=data["triage_result"], + reasoning=data["reasoning"], + file=data.get("file"), + line=data.get("line"), ) @@ -117,8 +197,13 @@ class MRReviewResult: verdict_reasoning: str = "" blockers: list[str] = field(default_factory=list) + # Multi-pass review results + structural_issues: list[StructuralIssue] = field(default_factory=list) + ai_triages: list[AICommentTriage] = field(default_factory=list) + # Follow-up review tracking reviewed_commit_sha: str | None = None + reviewed_file_blobs: dict[str, str] = field(default_factory=dict) is_followup_review: bool = False previous_review_id: int | None = None resolved_findings: list[str] = field(default_factory=list) @@ -129,6 +214,10 @@ class MRReviewResult: has_posted_findings: bool = False posted_finding_ids: list[str] = field(default_factory=list) + # CI/CD status + ci_status: str | None = None + ci_pipeline_id: int | None = None + def to_dict(self) -> dict: return { "mr_iid": self.mr_iid, @@ -142,7 +231,10 @@ class MRReviewResult: "verdict": self.verdict.value, "verdict_reasoning": self.verdict_reasoning, "blockers": self.blockers, + "structural_issues": [s.to_dict() for s in self.structural_issues], + "ai_triages": [t.to_dict() for t in self.ai_triages], "reviewed_commit_sha": self.reviewed_commit_sha, + "reviewed_file_blobs": self.reviewed_file_blobs, "is_followup_review": self.is_followup_review, "previous_review_id": self.previous_review_id, "resolved_findings": self.resolved_findings, @@ -150,6 +242,8 @@ class MRReviewResult: "new_findings_since_last_review": self.new_findings_since_last_review, "has_posted_findings": self.has_posted_findings, "posted_finding_ids": self.posted_finding_ids, + "ci_status": self.ci_status, + "ci_pipeline_id": self.ci_pipeline_id, } @classmethod @@ -166,7 +260,14 @@ class MRReviewResult: verdict=MergeVerdict(data.get("verdict", "ready_to_merge")), verdict_reasoning=data.get("verdict_reasoning", ""), blockers=data.get("blockers", []), + structural_issues=[ + StructuralIssue.from_dict(s) for s in data.get("structural_issues", []) + ], + ai_triages=[ + AICommentTriage.from_dict(t) for t in data.get("ai_triages", []) + ], reviewed_commit_sha=data.get("reviewed_commit_sha"), + reviewed_file_blobs=data.get("reviewed_file_blobs", {}), is_followup_review=data.get("is_followup_review", False), previous_review_id=data.get("previous_review_id"), resolved_findings=data.get("resolved_findings", []), @@ -176,6 +277,8 @@ class MRReviewResult: ), has_posted_findings=data.get("has_posted_findings", False), posted_finding_ids=data.get("posted_finding_ids", []), + ci_status=data.get("ci_status"), + ci_pipeline_id=data.get("ci_pipeline_id"), ) def save(self, gitlab_dir: Path) -> None: diff --git a/apps/backend/runners/gitlab/orchestrator.py b/apps/backend/runners/gitlab/orchestrator.py index 088ecca8..0333112e 100644 --- a/apps/backend/runners/gitlab/orchestrator.py +++ b/apps/backend/runners/gitlab/orchestrator.py @@ -3,8 +3,10 @@ GitLab Automation Orchestrator ============================== Main coordinator for GitLab automation workflows: -- MR Review: AI-powered merge request review +- MR Review: AI-powered merge request review with multi-pass analysis - Follow-up Review: Review changes since last review +- Bot Detection: Prevents infinite review loops +- CI/CD Checking: Pipeline status validation """ from __future__ import annotations @@ -17,6 +19,7 @@ from dataclasses import dataclass from pathlib import Path try: + from .bot_detection import BotDetector from .glab_client import GitLabClient, GitLabConfig from .models import ( GitLabRunnerConfig, @@ -25,8 +28,11 @@ try: MRReviewResult, ) from .services import MRReviewEngine + from .services.ci_checker import CIChecker + from .services.context_gatherer import MRContextGatherer except ImportError: # Fallback for direct script execution (not as a module) + from bot_detection import BotDetector from glab_client import GitLabClient, GitLabConfig from models import ( GitLabRunnerConfig, @@ -35,6 +41,8 @@ except ImportError: MRReviewResult, ) from services import MRReviewEngine + from services.ci_checker import CIChecker + from services.context_gatherer import MRContextGatherer # Import safe_print for BrokenPipeError handling try: @@ -77,10 +85,15 @@ class GitLabOrchestrator: project_dir: Path, config: GitLabRunnerConfig, progress_callback: Callable[[ProgressCallback], None] | None = None, + enable_bot_detection: bool = True, + enable_ci_checking: bool = True, + bot_username: str | None = None, ): self.project_dir = Path(project_dir) self.config = config self.progress_callback = progress_callback + self.enable_bot_detection = enable_bot_detection + self.enable_ci_checking = enable_ci_checking # GitLab directory for storing state self.gitlab_dir = self.project_dir / ".auto-claude" / "gitlab" @@ -107,6 +120,25 @@ class GitLabOrchestrator: progress_callback=self._forward_progress, ) + # Initialize bot detector + if enable_bot_detection: + self.bot_detector = BotDetector( + state_dir=self.gitlab_dir, + bot_username=bot_username, + review_own_mrs=False, + ) + else: + self.bot_detector = None + + # Initialize CI checker + if enable_ci_checking: + self.ci_checker = CIChecker( + project_dir=self.project_dir, + config=self.gitlab_config, + ) + else: + self.ci_checker = None + def _report_progress( self, phase: str, @@ -192,6 +224,8 @@ class GitLabOrchestrator: """ Perform AI-powered review of a merge request. + Includes bot detection and CI/CD status checking. + Args: mr_iid: The MR IID to review @@ -208,15 +242,79 @@ class GitLabOrchestrator: ) try: - # Gather MR context - context = await self._gather_mr_context(mr_iid) + # Get MR data first for bot detection + mr_data = await self.client.get_mr_async(mr_iid) + commits = await self.client.get_mr_commits_async(mr_iid) + + # Bot detection check + if self.bot_detector: + should_skip, skip_reason = self.bot_detector.should_skip_mr_review( + mr_iid=mr_iid, + mr_data=mr_data, + commits=commits, + ) + + if should_skip: + safe_print(f"[GitLab] Skipping MR !{mr_iid}: {skip_reason}") + result = MRReviewResult( + mr_iid=mr_iid, + project=self.config.project, + success=False, + error=f"Skipped: {skip_reason}", + ) + result.save(self.gitlab_dir) + return result + + # CI/CD status check + ci_status = None + ci_pipeline_id = None + ci_blocking_reason = "" + + if self.ci_checker: + self._report_progress( + "checking_ci", + 20, + "Checking CI/CD pipeline status...", + mr_iid=mr_iid, + ) + + pipeline_info = await self.ci_checker.check_mr_pipeline(mr_iid) + + if pipeline_info: + ci_status = pipeline_info.status.value + ci_pipeline_id = pipeline_info.pipeline_id + + if pipeline_info.is_blocking: + ci_blocking_reason = self.ci_checker.get_blocking_reason( + pipeline_info + ) + safe_print(f"[GitLab] CI blocking: {ci_blocking_reason}") + + # For failed pipelines, still do review but note CI failure + if pipeline_info.status == "success": + pass # Continue normally + elif pipeline_info.status == "failed": + # Continue review but note the failure + pass + else: + # For running/pending, we can still review + pass + + # Gather MR context using the context gatherer + context_gatherer = MRContextGatherer( + project_dir=self.project_dir, + mr_iid=mr_iid, + config=self.gitlab_config, + ) + + context = await context_gatherer.gather() safe_print( f"[GitLab] Context gathered: {context.title} " f"({len(context.changed_files)} files, {context.total_additions}+/{context.total_deletions}-)" ) self._report_progress( - "analyzing", 30, "Running AI review...", mr_iid=mr_iid + "analyzing", 40, "Running AI review...", mr_iid=mr_iid ) # Run review @@ -225,6 +323,15 @@ class GitLabOrchestrator: ) safe_print(f"[GitLab] Review complete: {len(findings)} findings") + # Adjust verdict based on CI status + if ci_status == "failed" and ci_blocking_reason: + # CI failure is a blocker + blockers.insert(0, f"CI/CD Pipeline Failed: {ci_blocking_reason}") + if verdict == MergeVerdict.READY_TO_MERGE: + verdict = MergeVerdict.BLOCKED + elif verdict == MergeVerdict.MERGE_WITH_CHANGES: + verdict = MergeVerdict.BLOCKED + # Map verdict to overall_status if verdict == MergeVerdict.BLOCKED: overall_status = "request_changes" @@ -243,6 +350,13 @@ class GitLabOrchestrator: blockers=blockers, ) + # Add CI section if CI was checked + if ci_status and self.ci_checker: + pipeline_info = await self.ci_checker.check_mr_pipeline(mr_iid) + if pipeline_info: + ci_section = self.ci_checker.format_pipeline_summary(pipeline_info) + full_summary = f"{ci_section}\n\n---\n\n{full_summary}" + # Create result result = MRReviewResult( mr_iid=mr_iid, @@ -255,11 +369,17 @@ class GitLabOrchestrator: verdict_reasoning=summary, blockers=blockers, reviewed_commit_sha=context.head_sha, + ci_status=ci_status, + ci_pipeline_id=ci_pipeline_id, ) # Save result result.save(self.gitlab_dir) + # Mark as reviewed in bot detector + if self.bot_detector and context.head_sha: + self.bot_detector.mark_reviewed(mr_iid, context.head_sha) + self._report_progress("complete", 100, "Review complete!", mr_iid=mr_iid) return result diff --git a/apps/backend/runners/gitlab/providers/__init__.py b/apps/backend/runners/gitlab/providers/__init__.py new file mode 100644 index 00000000..4f17b6d2 --- /dev/null +++ b/apps/backend/runners/gitlab/providers/__init__.py @@ -0,0 +1,10 @@ +""" +GitLab Provider Package +======================= + +GitProvider protocol implementation for GitLab. +""" + +from .gitlab_provider import GitLabProvider + +__all__ = ["GitLabProvider"] diff --git a/apps/backend/runners/gitlab/providers/gitlab_provider.py b/apps/backend/runners/gitlab/providers/gitlab_provider.py new file mode 100644 index 00000000..a13693da --- /dev/null +++ b/apps/backend/runners/gitlab/providers/gitlab_provider.py @@ -0,0 +1,816 @@ +""" +GitLab Provider Implementation +============================== + +Implements the GitProvider protocol for GitLab using the GitLab REST API. +Wraps the existing GitLabClient functionality and converts to provider-agnostic models. +""" + +from __future__ import annotations + +import urllib.parse +import urllib.request +from dataclasses import dataclass +from datetime import datetime, timezone +from pathlib import Path +from typing import Any + +# Import from parent package or direct import +try: + from ..glab_client import GitLabClient, GitLabConfig, encode_project_path +except (ImportError, ValueError, SystemError): + from glab_client import GitLabClient, GitLabConfig, encode_project_path + +# Import the protocol and data models from GitHub's protocol definition +# This ensures compatibility across providers +try: + from ...github.providers.protocol import ( + IssueData, + IssueFilters, + LabelData, + PRData, + PRFilters, + ProviderType, + ReviewData, + ) +except (ImportError, ValueError, SystemError): + from runners.github.providers.protocol import ( + IssueData, + IssueFilters, + LabelData, + PRData, + PRFilters, + ProviderType, + ReviewData, + ) + + +@dataclass +class GitLabProvider: + """ + GitLab implementation of the GitProvider protocol. + + Uses the GitLab REST API for all operations. + + Usage: + provider = GitLabProvider( + repo="group/project", + token="glpat-...", + instance_url="https://gitlab.com" + ) + mr = await provider.fetch_pr(123) + await provider.post_review(123, review) + """ + + _repo: str + _token: str + _instance_url: str = "https://gitlab.com" + _project_dir: Path | None = None + _glab_client: GitLabClient | None = None + enable_rate_limiting: bool = True + + def __post_init__(self): + if self._glab_client is None: + project_dir = Path(self._project_dir) if self._project_dir else Path.cwd() + config = GitLabConfig( + token=self._token, + project=self._repo, + instance_url=self._instance_url, + ) + self._glab_client = GitLabClient( + project_dir=project_dir, + config=config, + ) + + @property + def provider_type(self) -> ProviderType: + return ProviderType.GITLAB + + @property + def repo(self) -> str: + return self._repo + + @property + def glab_client(self) -> GitLabClient: + """Get the underlying GitLabClient.""" + return self._glab_client + + # ------------------------------------------------------------------------- + # Pull Request Operations (GitLab calls them Merge Requests) + # ------------------------------------------------------------------------- + + async def fetch_pr(self, number: int) -> PRData: + """ + Fetch a merge request by IID. + + Args: + number: MR IID (GitLab uses IID, not global ID) + + Returns: + PRData with full MR details including diff + """ + # Get MR details + mr_data = self._glab_client.get_mr(number) + + # Get MR changes (includes diff) + changes_data = self._glab_client.get_mr_changes(number) + + # Build diff from changes + diffs = [] + for change in changes_data.get("changes", []): + diff = change.get("diff", "") + if diff: + diffs.append(diff) + diff = "\n".join(diffs) + + return self._parse_mr_data(mr_data, diff, changes_data) + + async def fetch_prs(self, filters: PRFilters | None = None) -> list[PRData]: + """ + Fetch merge requests with optional filters. + + Args: + filters: Optional filters (state, labels, etc.) + + Returns: + List of PRData + """ + filters = filters or PRFilters() + + # Build query parameters for GitLab API + params = {} + if filters.state == "open": + params["state"] = "opened" + elif filters.state == "closed": + params["state"] = "closed" + elif filters.state == "merged": + params["state"] = "merged" + + if filters.labels: + params["labels"] = ",".join(filters.labels) + + if filters.limit: + params["per_page"] = min(filters.limit, 100) # GitLab max is 100 + + # Use direct API call for listing MRs + encoded_project = encode_project_path(self._repo) + endpoint = f"/projects/{encoded_project}/merge_requests" + + mrs_data = self._glab_client._fetch(endpoint, params=params) + + result = [] + for mr_data in mrs_data: + # Apply additional filters that aren't supported by GitLab API + if filters.author: + mr_author = mr_data.get("author", {}).get("username") + if mr_author != filters.author: + continue + + if filters.base_branch: + if mr_data.get("target_branch") != filters.base_branch: + continue + + if filters.head_branch: + if mr_data.get("source_branch") != filters.head_branch: + continue + + # Parse to PRData (lightweight, no diff) + result.append(self._parse_mr_data(mr_data, "", {})) + + return result + + async def fetch_pr_diff(self, number: int) -> str: + """ + Fetch the diff for a merge request. + + Args: + number: MR IID + + Returns: + Unified diff string + """ + return self._glab_client.get_mr_diff(number) + + async def post_review(self, pr_number: int, review: ReviewData) -> int: + """ + Post a review to a merge request. + + GitLab doesn't have the same review concept as GitHub. + We implement this as: + - approve → Approve MR + post note + - request_changes → Post note with request changes + - comment → Post note only + + Args: + pr_number: MR IID + review: Review data with findings and comments + + Returns: + Note ID (or 0 if not available) + """ + # Post the review body as a note + note_data = self._glab_client.post_mr_note(pr_number, review.body) + + # If approving, also approve the MR + if review.event == "approve": + self._glab_client.approve_mr(pr_number) + + # Return note ID + return note_data.get("id", 0) + + async def merge_pr( + self, + pr_number: int, + merge_method: str = "merge", + commit_title: str | None = None, + ) -> bool: + """ + Merge a merge request. + + Args: + pr_number: MR IID + merge_method: merge, squash, or rebase (GitLab supports merge and squash) + commit_title: Optional commit title + + Returns: + True if merged successfully + """ + # Map merge method to GitLab parameters + squash = merge_method == "squash" + + try: + result = self._glab_client.merge_mr(pr_number, squash=squash) + # Check if merge was successful + return result.get("status") != "failed" + except Exception: + return False + + async def close_pr( + self, + pr_number: int, + comment: str | None = None, + ) -> bool: + """ + Close a merge request without merging. + + Args: + pr_number: MR IID + comment: Optional closing comment + + Returns: + True if closed successfully + """ + try: + # Post closing comment if provided + if comment: + self._glab_client.post_mr_note(pr_number, comment) + + # GitLab doesn't have a direct "close" endpoint for MRs + # We need to use the API to set the state event to close + encoded_project = encode_project_path(self._repo) + data = {"state_event": "close"} + self._glab_client._fetch( + f"/projects/{encoded_project}/merge_requests/{pr_number}", + method="PUT", + data=data, + ) + return True + except Exception: + return False + + # ------------------------------------------------------------------------- + # Issue Operations + # ------------------------------------------------------------------------- + + async def fetch_issue(self, number: int) -> IssueData: + """ + Fetch an issue by IID. + + Args: + number: Issue IID + + Returns: + IssueData with full issue details + """ + encoded_project = encode_project_path(self._repo) + issue_data = self._glab_client._fetch( + f"/projects/{encoded_project}/issues/{number}" + ) + return self._parse_issue_data(issue_data) + + async def fetch_issues( + self, filters: IssueFilters | None = None + ) -> list[IssueData]: + """ + Fetch issues with optional filters. + + Args: + filters: Optional filters + + Returns: + List of IssueData + """ + filters = filters or IssueFilters() + + # Build query parameters + params = {} + if filters.state: + params["state"] = filters.state + if filters.labels: + params["labels"] = ",".join(filters.labels) + if filters.limit: + params["per_page"] = min(filters.limit, 100) + + encoded_project = encode_project_path(self._repo) + endpoint = f"/projects/{encoded_project}/issues" + + issues_data = self._glab_client._fetch(endpoint, params=params) + + result = [] + for issue_data in issues_data: + # Filter out MRs if requested + # In GitLab, MRs are separate from issues, so this check is less relevant + # But we check for the "merge_request" label or type + if not filters.include_prs: + # GitLab doesn't mix MRs with issues in the issues endpoint + pass + + # Apply author filter + if filters.author: + author = issue_data.get("author", {}).get("username") + if author != filters.author: + continue + + result.append(self._parse_issue_data(issue_data)) + + return result + + async def create_issue( + self, + title: str, + body: str, + labels: list[str] | None = None, + assignees: list[str] | None = None, + ) -> IssueData: + """ + Create a new issue. + + Args: + title: Issue title + body: Issue body + labels: Optional labels + assignees: Optional assignees (usernames) + + Returns: + Created IssueData + """ + encoded_project = encode_project_path(self._repo) + + data = { + "title": title, + "description": body, + } + + if labels: + data["labels"] = ",".join(labels) + + # GitLab uses assignee IDs, not usernames + # We need to look up user IDs first + if assignees: + assignee_ids = [] + for username in assignees: + try: + user_data = self._glab_client._fetch(f"/users?username={username}") + if user_data: + assignee_ids.append(user_data[0]["id"]) + except Exception: + pass # Skip invalid users + if assignee_ids: + data["assignee_ids"] = assignee_ids + + result = self._glab_client._fetch( + f"/projects/{encoded_project}/issues", + method="POST", + data=data, + ) + + # Return the created issue + return await self.fetch_issue(result["iid"]) + + async def close_issue( + self, + number: int, + comment: str | None = None, + ) -> bool: + """ + Close an issue. + + Args: + number: Issue IID + comment: Optional closing comment + + Returns: + True if closed successfully + """ + try: + # Post closing comment if provided + if comment: + encoded_project = encode_project_path(self._repo) + self._glab_client._fetch( + f"/projects/{encoded_project}/issues/{number}/notes", + method="POST", + data={"body": comment}, + ) + + # Close the issue + encoded_project = encode_project_path(self._repo) + self._glab_client._fetch( + f"/projects/{encoded_project}/issues/{number}", + method="PUT", + data={"state_event": "close"}, + ) + return True + except Exception: + return False + + async def add_comment( + self, + issue_or_pr_number: int, + body: str, + ) -> int: + """ + Add a comment to an issue or MR. + + Args: + issue_or_pr_number: Issue/MR IID + body: Comment body + + Returns: + Note ID + """ + # Try MR first, then issue + try: + note_data = self._glab_client.post_mr_note(issue_or_pr_number, body) + return note_data.get("id", 0) + except Exception: + try: + encoded_project = encode_project_path(self._repo) + note_data = self._glab_client._fetch( + f"/projects/{encoded_project}/issues/{issue_or_pr_number}/notes", + method="POST", + data={"body": body}, + ) + return note_data.get("id", 0) + except Exception: + return 0 + + # ------------------------------------------------------------------------- + # Label Operations + # ------------------------------------------------------------------------- + + async def apply_labels( + self, + issue_or_pr_number: int, + labels: list[str], + ) -> None: + """ + Apply labels to an issue or MR. + + Args: + issue_or_pr_number: Issue/MR IID + labels: Labels to apply + """ + encoded_project = encode_project_path(self._repo) + + # Try MR first + try: + current_data = self._glab_client._fetch( + f"/projects/{encoded_project}/merge_requests/{issue_or_pr_number}" + ) + current_labels = current_data.get("labels", []) + new_labels = list(set(current_labels + labels)) + + self._glab_client._fetch( + f"/projects/{encoded_project}/merge_requests/{issue_or_pr_number}", + method="PUT", + data={"labels": ",".join(new_labels)}, + ) + return + except Exception: + pass + + # Try issue + try: + current_data = self._glab_client._fetch( + f"/projects/{encoded_project}/issues/{issue_or_pr_number}" + ) + current_labels = current_data.get("labels", []) + new_labels = list(set(current_labels + labels)) + + self._glab_client._fetch( + f"/projects/{encoded_project}/issues/{issue_or_pr_number}", + method="PUT", + data={"labels": ",".join(new_labels)}, + ) + except Exception: + pass + + async def remove_labels( + self, + issue_or_pr_number: int, + labels: list[str], + ) -> None: + """ + Remove labels from an issue or MR. + + Args: + issue_or_pr_number: Issue/MR IID + labels: Labels to remove + """ + encoded_project = encode_project_path(self._repo) + + # Try MR first + try: + current_data = self._glab_client._fetch( + f"/projects/{encoded_project}/merge_requests/{issue_or_pr_number}" + ) + current_labels = current_data.get("labels", []) + new_labels = [label for label in current_labels if label not in labels] + + self._glab_client._fetch( + f"/projects/{encoded_project}/merge_requests/{issue_or_pr_number}", + method="PUT", + data={"labels": ",".join(new_labels)}, + ) + return + except Exception: + pass + + # Try issue + try: + current_data = self._glab_client._fetch( + f"/projects/{encoded_project}/issues/{issue_or_pr_number}" + ) + current_labels = current_data.get("labels", []) + new_labels = [label for label in current_labels if label not in labels] + + self._glab_client._fetch( + f"/projects/{encoded_project}/issues/{issue_or_pr_number}", + method="PUT", + data={"labels": ",".join(new_labels)}, + ) + except Exception: + pass + + async def create_label(self, label: LabelData) -> None: + """ + Create a label in the repository. + + Args: + label: Label data + """ + encoded_project = encode_project_path(self._repo) + + data = { + "name": label.name, + "color": label.color.lstrip("#"), # GitLab doesn't want # prefix + } + + if label.description: + data["description"] = label.description + + try: + self._glab_client._fetch( + f"/projects/{encoded_project}/labels", + method="POST", + data=data, + ) + except Exception: + # Label might already exist, try to update + try: + self._glab_client._fetch( + f"/projects/{encoded_project}/labels/{urllib.parse.quote(label.name)}", + method="PUT", + data=data, + ) + except Exception: + pass + + async def list_labels(self) -> list[LabelData]: + """ + List all labels in the repository. + + Returns: + List of LabelData + """ + encoded_project = encode_project_path(self._repo) + + labels_data = self._glab_client._fetch( + f"/projects/{encoded_project}/labels", + params={"per_page": 100}, + ) + + return [ + LabelData( + name=label["name"], + color=f"#{label['color']}", # Add # prefix for consistency + description=label.get("description", ""), + ) + for label in labels_data + ] + + # ------------------------------------------------------------------------- + # Repository Operations + # ------------------------------------------------------------------------- + + async def get_repository_info(self) -> dict[str, Any]: + """ + Get repository information. + + Returns: + Repository metadata + """ + encoded_project = encode_project_path(self._repo) + return self._glab_client._fetch(f"/projects/{encoded_project}") + + async def get_default_branch(self) -> str: + """ + Get the default branch name. + + Returns: + Default branch name (e.g., "main", "master") + """ + repo_info = await self.get_repository_info() + return repo_info.get("default_branch", "main") + + async def check_permissions(self, username: str) -> str: + """ + Check a user's permission level on the repository. + + Args: + username: GitLab username + + Returns: + Permission level (admin, maintain, developer, reporter, guest, none) + """ + try: + encoded_project = encode_project_path(self._repo) + result = self._glab_client._fetch( + f"/projects/{encoded_project}/members/all", + params={"query": username}, + ) + + if result: + # GitLab access levels: 10=guest, 20=reporter, 30=developer, 40=maintainer, 50=owner + access_level = result[0].get("access_level", 0) + + level_map = { + 50: "admin", + 40: "maintain", + 30: "developer", + 20: "reporter", + 10: "guest", + } + + return level_map.get(access_level, "none") + + return "none" + except Exception: + return "none" + + # ------------------------------------------------------------------------- + # API Operations (Low-level) + # ------------------------------------------------------------------------- + + async def api_get( + self, + endpoint: str, + params: dict[str, Any] | None = None, + ) -> Any: + """ + Make a GET request to the GitLab API. + + Args: + endpoint: API endpoint + params: Query parameters + + Returns: + API response data + """ + return self._glab_client._fetch(endpoint, params=params) + + async def api_post( + self, + endpoint: str, + data: dict[str, Any] | None = None, + ) -> Any: + """ + Make a POST request to the GitLab API. + + Args: + endpoint: API endpoint + data: Request body + + Returns: + API response data + """ + return self._glab_client._fetch(endpoint, method="POST", data=data) + + # ------------------------------------------------------------------------- + # Helper Methods + # ------------------------------------------------------------------------- + + def _parse_mr_data( + self, data: dict[str, Any], diff: str, changes_data: dict[str, Any] + ) -> PRData: + """Parse GitLab MR data into PRData.""" + author_data = data.get("author", {}) + author = author_data.get("username", "unknown") if author_data else "unknown" + + labels = data.get("labels", []) + + # Extract files from changes data + files = [] + if changes_data.get("changes"): + for change in changes_data["changes"]: + new_path = change.get("new_path") + old_path = change.get("old_path") + files.append( + { + "path": new_path or old_path, + "new_path": new_path, + "old_path": old_path, + "status": change.get("new_file") + and "added" + or change.get("deleted_file") + and "deleted" + or change.get("renamed_file") + and "renamed" + or "modified", + } + ) + + return PRData( + number=data.get("iid", 0), + title=data.get("title", ""), + body=data.get("description", "") or "", + author=author, + state=data.get("state", "opened"), + source_branch=data.get("source_branch", ""), + target_branch=data.get("target_branch", ""), + additions=changes_data.get("additions", 0), + deletions=changes_data.get("deletions", 0), + changed_files=changes_data.get("changed_files_count", len(files)), + files=files, + diff=diff, + url=data.get("web_url", ""), + created_at=self._parse_datetime(data.get("created_at")), + updated_at=self._parse_datetime(data.get("updated_at")), + labels=labels, + reviewers=[], # GitLab uses "assignees" not reviewers + is_draft=data.get("draft", False), + mergeable=data.get("merge_status") != "cannot_be_merged", + provider=ProviderType.GITLAB, + raw_data=data, + ) + + def _parse_issue_data(self, data: dict[str, Any]) -> IssueData: + """Parse GitLab issue data into IssueData.""" + author_data = data.get("author", {}) + author = author_data.get("username", "unknown") if author_data else "unknown" + + labels = data.get("labels", []) + + assignees = [] + for assignee in data.get("assignees", []): + if isinstance(assignee, dict): + assignees.append(assignee.get("username", "")) + + milestone = data.get("milestone") + if isinstance(milestone, dict): + milestone = milestone.get("title") + + return IssueData( + number=data.get("iid", 0), + title=data.get("title", ""), + body=data.get("description", "") or "", + author=author, + state=data.get("state", "opened"), + labels=labels, + created_at=self._parse_datetime(data.get("created_at")), + updated_at=self._parse_datetime(data.get("updated_at")), + url=data.get("web_url", ""), + assignees=assignees, + milestone=milestone, + provider=ProviderType.GITLAB, + raw_data=data, + ) + + def _parse_datetime(self, dt_str: str | None) -> datetime: + """Parse ISO datetime string.""" + if not dt_str: + return datetime.now(timezone.utc) + try: + return datetime.fromisoformat(dt_str.replace("Z", "+00:00")) + except (ValueError, AttributeError): + return datetime.now(timezone.utc) diff --git a/apps/backend/runners/gitlab/runner.py b/apps/backend/runners/gitlab/runner.py index dad17680..313496e1 100644 --- a/apps/backend/runners/gitlab/runner.py +++ b/apps/backend/runners/gitlab/runner.py @@ -6,6 +6,9 @@ GitLab Automation Runner CLI interface for GitLab automation features: - MR Review: AI-powered merge request review - Follow-up Review: Review changes since last review +- Triage: Classify and organize issues +- Auto-fix: Automatically create specs from issues +- Batch: Group and analyze similar issues Usage: # Review a specific MR @@ -13,6 +16,15 @@ Usage: # Follow-up review after new commits python runner.py followup-review-mr 123 + + # Triage issues + python runner.py triage --state opened --limit 50 + + # Auto-fix an issue + python runner.py auto-fix 42 + + # Batch similar issues + python runner.py batch-issues --label "bug" --min 3 """ from __future__ import annotations @@ -235,6 +247,277 @@ async def cmd_followup_review_mr(args) -> int: return 1 +async def cmd_triage(args) -> int: + """ + Triage and classify GitLab issues. + + Categorizes issues into: duplicates, spam, feature creep, actionable. + """ + from glab_client import GitLabClient, GitLabConfig + + config = get_config(args) + gitlab_config = GitLabConfig( + token=config.token, + project=config.project, + instance_url=config.instance_url, + ) + + client = GitLabClient( + project_dir=args.project_dir, + config=gitlab_config, + ) + + safe_print(f"[Triage] Fetching issues (state={args.state}, limit={args.limit})...") + + # Fetch issues + issues = client.list_issues( + state=args.state, + labels=args.labels if args.labels else None, + per_page=args.limit, + ) + + if not issues: + safe_print("[Triage] No issues found matching criteria") + return 0 + + safe_print(f"[Triage] Found {len(issues)} issues to triage") + + # Basic triage logic + actionable = [] + duplicates = [] + spam = [] + feature_creep = [] + + for issue in issues: + title = issue.get("title", "").lower() + description = issue.get("description", "").lower() + author = issue.get("author", {}).get("username", "") + + # Check for spam + if any(word in title for word in ["test", "spam", "xxx"]): + spam.append(issue) + continue + + # Check for duplicates (simple heuristic) + if any(word in title for word in ["duplicate", "already", "same"]): + duplicates.append(issue) + continue + + # Check for feature creep + if any(word in title for word in ["also", "while", "additionally", "btw"]): + feature_creep.append(issue) + continue + + actionable.append(issue) + + # Print results + print(f"\n{'=' * 60}") + print("Issue Triage Results") + print(f"{'=' * 60}") + print(f"Total Issues: {len(issues)}") + print(f" Actionable: {len(actionable)}") + print(f" Duplicates: {len(duplicates)}") + print(f" Spam: {len(spam)}") + print(f" Feature Creep: {len(feature_creep)}") + + if args.verbose and actionable[:10]: + print("\nActionable Issues (showing first 10):") + for issue in actionable[:10]: + iid = issue.get("iid") + title = issue.get("title", "No title") + labels = issue.get("labels", []) + print(f" !{iid}: {title}") + print(f" Labels: {', '.join(labels)}") + + return 0 + + +async def cmd_auto_fix(args) -> int: + """ + Auto-fix an issue by creating a spec. + + Analyzes the issue and creates a spec for implementation. + """ + from glab_client import GitLabClient, GitLabConfig + + config = get_config(args) + gitlab_config = GitLabConfig( + token=config.token, + project=config.project, + instance_url=config.instance_url, + ) + + client = GitLabClient( + project_dir=args.project_dir, + config=gitlab_config, + ) + + safe_print(f"[Auto-fix] Fetching issue !{args.issue_iid}...") + + # Fetch issue + issue = client.get_issue(args.issue_iid) + + if not issue: + safe_print(f"[Auto-fix] Issue !{args.issue_iid} not found") + return 1 + + title = issue.get("title", "") + description = issue.get("description", "") + labels = issue.get("labels", []) + author = issue.get("author", {}).get("username", "") + + print(f"\n{'=' * 60}") + print(f"Auto-fix for Issue !{args.issue_iid}") + print(f"{'=' * 60}") + print(f"Title: {title}") + print(f"Author: {author}") + print(f"Labels: {', '.join(labels)}") + print(f"\nDescription:\n{description[:500]}...") + + # Check if already auto-fixable + if any(label in labels for label in ["auto-fix", "spec-created"]): + safe_print("[Auto-fix] Issue already marked for auto-fix or has spec") + return 0 + + # Add auto-fix label + if not args.dry_run: + try: + client.update_issue(args.issue_iid, labels=list(set(labels + ["auto-fix"]))) + safe_print(f"[Auto-fix] Added 'auto-fix' label to issue !{args.issue_iid}") + except Exception as e: + safe_print(f"[Auto-fix] Failed to update issue: {e}") + return 1 + else: + safe_print("[Auto-fix] Dry run - would add 'auto-fix' label") + + # Note: In a full implementation, this would: + # 1. Analyze the issue with AI + # 2. Create a spec in .auto-claude/specs/ + # 3. Run the spec creation pipeline + + safe_print("[Auto-fix] Issue marked for auto-fix (spec creation not implemented)") + safe_print( + "[Auto-fix] Run 'python spec_runner.py --task \"\"' to create spec" + ) + + return 0 + + +async def cmd_batch_issues(args) -> int: + """ + Batch similar issues together for analysis. + + Groups issues by labels, keywords, or patterns. + """ + from collections import defaultdict + + from glab_client import GitLabClient, GitLabConfig + + config = get_config(args) + gitlab_config = GitLabConfig( + token=config.token, + project=config.project, + instance_url=config.instance_url, + ) + + client = GitLabClient( + project_dir=args.project_dir, + config=gitlab_config, + ) + + safe_print(f"[Batch] Fetching issues (label={args.label}, limit={args.limit})...") + + # Fetch issues + issues = client.list_issues( + state=args.state, + labels=[args.label] if args.label else None, + per_page=args.limit, + ) + + if not issues: + safe_print("[Batch] No issues found matching criteria") + return 0 + + safe_print(f"[Batch] Found {len(issues)} issues") + + # Group issues by keywords + groups = defaultdict(list) + keywords = [ + "bug", + "error", + "crash", + "fix", + "feature", + "enhancement", + "add", + "implement", + "refactor", + "cleanup", + "improve", + "docs", + "documentation", + "readme", + "test", + "testing", + "coverage", + "performance", + "slow", + "optimize", + ] + + for issue in issues: + title = issue.get("title", "").lower() + description = issue.get("description", "").lower() + combined = f"{title} {description}" + + matched = False + for keyword in keywords: + if keyword in combined: + groups[keyword].append(issue) + matched = True + break + + if not matched: + groups["other"].append(issue) + + # Filter groups by minimum size + filtered_groups = {k: v for k, v in groups.items() if len(v) >= args.min} + + # Print results + print(f"\n{'=' * 60}") + print("Batch Analysis Results") + print(f"{'=' * 60}") + print(f"Total Issues: {len(issues)}") + print(f"Groups Found: {len(filtered_groups)}") + + # Sort by group size + sorted_groups = sorted( + filtered_groups.items(), key=lambda x: len(x[1]), reverse=True + ) + + for keyword, group_issues in sorted_groups: + print(f"\n[{keyword.upper()}] - {len(group_issues)} issues:") + for issue in group_issues[:5]: # Show first 5 + iid = issue.get("iid") + title = issue.get("title", "No title") + print(f" !{iid}: {title[:60]}...") + if len(group_issues) > 5: + print(f" ... and {len(group_issues) - 5} more") + + # Suggest batch actions + if len(sorted_groups) > 0: + largest_group, largest_issues = sorted_groups[0] + if len(largest_issues) >= args.min: + print("\nSuggested batch action:") + print(f" Group: {largest_group}") + print(f" Size: {len(largest_issues)} issues") + print( + f" Command: python runner.py triage --label {args.label} --limit {len(largest_issues)}" + ) + + return 0 + + def main(): """CLI entry point.""" import argparse @@ -294,6 +577,47 @@ def main(): ) followup_parser.add_argument("mr_iid", type=int, help="MR IID to review") + # triage command + triage_parser = subparsers.add_parser("triage", help="Triage and classify issues") + triage_parser.add_argument( + "--state", type=str, default="opened", help="Issue state to filter" + ) + triage_parser.add_argument( + "--labels", type=str, help="Comma-separated labels to filter" + ) + triage_parser.add_argument( + "--limit", type=int, default=50, help="Maximum issues to process" + ) + triage_parser.add_argument( + "-v", "--verbose", action="store_true", help="Show detailed output" + ) + + # auto-fix command + autofix_parser = subparsers.add_parser( + "auto-fix", help="Auto-fix an issue by creating a spec" + ) + autofix_parser.add_argument("issue_iid", type=int, help="Issue IID to auto-fix") + autofix_parser.add_argument( + "--dry-run", + action="store_true", + help="Show what would be done without making changes", + ) + + # batch-issues command + batch_parser = subparsers.add_parser( + "batch-issues", help="Batch and analyze similar issues" + ) + batch_parser.add_argument("--label", type=str, help="Label to filter issues") + batch_parser.add_argument( + "--state", type=str, default="opened", help="Issue state to filter" + ) + batch_parser.add_argument( + "--limit", type=int, default=100, help="Maximum issues to process" + ) + batch_parser.add_argument( + "--min", type=int, default=3, help="Minimum group size to report" + ) + args = parser.parse_args() if not args.command: @@ -304,6 +628,9 @@ def main(): commands = { "review-mr": cmd_review_mr, "followup-review-mr": cmd_followup_review_mr, + "triage": cmd_triage, + "auto-fix": cmd_auto_fix, + "batch-issues": cmd_batch_issues, } handler = commands.get(args.command) diff --git a/apps/backend/runners/gitlab/services/__init__.py b/apps/backend/runners/gitlab/services/__init__.py index e6ad40be..f1d03732 100644 --- a/apps/backend/runners/gitlab/services/__init__.py +++ b/apps/backend/runners/gitlab/services/__init__.py @@ -5,6 +5,23 @@ GitLab Runner Services Service layer for GitLab automation. """ +from .ci_checker import CIChecker, JobStatus, PipelineInfo, PipelineStatus +from .context_gatherer import ( + AIBotComment, + ChangedFile, + FollowupMRContextGatherer, + MRContextGatherer, +) from .mr_review_engine import MRReviewEngine -__all__ = ["MRReviewEngine"] +__all__ = [ + "MRReviewEngine", + "CIChecker", + "JobStatus", + "PipelineInfo", + "PipelineStatus", + "MRContextGatherer", + "FollowupMRContextGatherer", + "ChangedFile", + "AIBotComment", +] diff --git a/apps/backend/runners/gitlab/services/ci_checker.py b/apps/backend/runners/gitlab/services/ci_checker.py new file mode 100644 index 00000000..98c59d77 --- /dev/null +++ b/apps/backend/runners/gitlab/services/ci_checker.py @@ -0,0 +1,444 @@ +""" +CI/CD Pipeline Checker for GitLab +================================== + +Checks GitLab CI/CD pipeline status for merge requests. + +Features: +- Get pipeline status for an MR +- Check for failed jobs +- Detect security policy violations +- Handle workflow approvals +""" + +from __future__ import annotations + +from dataclasses import dataclass +from enum import Enum +from pathlib import Path + +try: + from ..glab_client import GitLabClient, GitLabConfig + from .io_utils import safe_print +except ImportError: + from core.io_utils import safe_print + from glab_client import GitLabClient, GitLabConfig + + +class PipelineStatus(str, Enum): + """GitLab pipeline status.""" + + PENDING = "pending" + RUNNING = "running" + SUCCESS = "success" + FAILED = "failed" + CANCELED = "canceled" + SKIPPED = "skipped" + MANUAL = "manual" + UNKNOWN = "unknown" + + +@dataclass +class JobStatus: + """Status of a single CI job.""" + + name: str + status: str + stage: str + started_at: str | None = None + finished_at: str | None = None + duration: float | None = None + failure_reason: str | None = None + retry_count: int = 0 + allow_failure: bool = False + + +@dataclass +class PipelineInfo: + """Complete pipeline information.""" + + pipeline_id: int + status: PipelineStatus + ref: str + sha: str + created_at: str + updated_at: str + finished_at: str | None = None + duration: float | None = None + jobs: list[JobStatus] = None + failed_jobs: list[JobStatus] = None + blocked_jobs: list[JobStatus] = None + security_issues: list[dict] = None + + def __post_init__(self): + if self.jobs is None: + self.jobs = [] + if self.failed_jobs is None: + self.failed_jobs = [] + if self.blocked_jobs is None: + self.blocked_jobs = [] + if self.security_issues is None: + self.security_issues = [] + + @property + def has_failures(self) -> bool: + """Check if pipeline has any failed jobs.""" + return len(self.failed_jobs) > 0 + + @property + def has_security_issues(self) -> bool: + """Check if pipeline has security scan failures.""" + return len(self.security_issues) > 0 + + @property + def is_blocking(self) -> bool: + """Check if pipeline status blocks merge.""" + # Only SUCCESS status allows merge + # FAILED, CANCELED, RUNNING (with blocking jobs) block merge + if self.status == PipelineStatus.SUCCESS: + return False + if self.status == PipelineStatus.FAILED: + return True + if self.status == PipelineStatus.CANCELED: + return True + if self.status in (PipelineStatus.RUNNING, PipelineStatus.PENDING): + # Check if any critical jobs are expected to fail + return any( + not job.allow_failure for job in self.jobs if job.status == "failed" + ) + return False + + +class CIChecker: + """ + Checks CI/CD pipeline status for GitLab MRs. + + Usage: + checker = CIChecker( + project_dir=Path("/path/to/project"), + config=gitlab_config + ) + pipeline_info = await checker.check_mr_pipeline(mr_iid=123) + if pipeline_info.is_blocking: + print(f"MR blocked by CI: {pipeline_info.status}") + """ + + def __init__( + self, + project_dir: Path, + config: GitLabConfig | None = None, + ): + """ + Initialize CI checker. + + Args: + project_dir: Path to the project directory + config: GitLab configuration (optional) + """ + self.project_dir = Path(project_dir) + + if config: + self.client = GitLabClient( + project_dir=self.project_dir, + config=config, + ) + else: + # Try to load config from project + from ..glab_client import load_gitlab_config + + config = load_gitlab_config(self.project_dir) + if config: + self.client = GitLabClient( + project_dir=self.project_dir, + config=config, + ) + else: + raise ValueError("GitLab configuration not found") + + def _parse_job_status(self, job_data: dict) -> JobStatus: + """Parse job data from GitLab API.""" + return JobStatus( + name=job_data.get("name", ""), + status=job_data.get("status", "unknown"), + stage=job_data.get("stage", ""), + started_at=job_data.get("started_at"), + finished_at=job_data.get("finished_at"), + duration=job_data.get("duration"), + failure_reason=job_data.get("failure_reason"), + retry_count=job_data.get("retry_count", 0), + allow_failure=job_data.get("allow_failure", False), + ) + + async def check_mr_pipeline(self, mr_iid: int) -> PipelineInfo | None: + """ + Check pipeline status for an MR. + + Args: + mr_iid: The MR IID + + Returns: + PipelineInfo or None if no pipeline found + """ + # Get pipelines for this MR + pipelines = await self.client.get_mr_pipelines_async(mr_iid) + + if not pipelines: + safe_print(f"[CI] No pipelines found for MR !{mr_iid}") + return None + + # Get the most recent pipeline (last in list) + latest_pipeline_data = pipelines[-1] + + pipeline_id = latest_pipeline_data.get("id") + status_str = latest_pipeline_data.get("status", "unknown") + + try: + status = PipelineStatus(status_str) + except ValueError: + status = PipelineStatus.UNKNOWN + + safe_print(f"[CI] MR !{mr_iid} has pipeline #{pipeline_id}: {status.value}") + + # Get detailed pipeline info + try: + pipeline_detail = await self.client.get_pipeline_status_async(pipeline_id) + except Exception as e: + safe_print(f"[CI] Error fetching pipeline details: {e}") + pipeline_detail = latest_pipeline_data + + # Get jobs for this pipeline + jobs_data = [] + try: + jobs_data = await self.client.get_pipeline_jobs_async(pipeline_id) + except Exception as e: + safe_print(f"[CI] Error fetching pipeline jobs: {e}") + + # Parse jobs + jobs = [self._parse_job_status(job) for job in jobs_data] + + # Find failed jobs (excluding allow_failure jobs) + failed_jobs = [ + job for job in jobs if job.status == "failed" and not job.allow_failure + ] + + # Find blocked/failed jobs + blocked_jobs = [job for job in jobs if job.status in ("failed", "canceled")] + + # Check for security scan failures + security_issues = self._check_security_scans(jobs) + + return PipelineInfo( + pipeline_id=pipeline_id, + status=status, + ref=latest_pipeline_data.get("ref", ""), + sha=latest_pipeline_data.get("sha", ""), + created_at=latest_pipeline_data.get("created_at", ""), + updated_at=latest_pipeline_data.get("updated_at", ""), + finished_at=pipeline_detail.get("finished_at"), + duration=pipeline_detail.get("duration"), + jobs=jobs, + failed_jobs=failed_jobs, + blocked_jobs=blocked_jobs, + security_issues=security_issues, + ) + + def _check_security_scans(self, jobs: list[JobStatus]) -> list[dict]: + """ + Check for security scan failures. + + Looks for common GitLab security job patterns: + - sast + - secret_detection + - container_scanning + - dependency_scanning + - license_scanning + """ + issues = [] + + security_patterns = { + "sast": "Static Application Security Testing", + "secret_detection": "Secret Detection", + "container_scanning": "Container Scanning", + "dependency_scanning": "Dependency Scanning", + "license_scanning": "License Scanning", + "api_fuzzing": "API Fuzzing", + "dast": "Dynamic Application Security Testing", + } + + for job in jobs: + job_name_lower = job.name.lower() + + # Check if this is a security job + for pattern, scan_type in security_patterns.items(): + if pattern in job_name_lower: + if job.status == "failed" and not job.allow_failure: + issues.append( + { + "type": scan_type, + "job_name": job.name, + "status": job.status, + "failure_reason": job.failure_reason, + } + ) + break + + return issues + + def get_blocking_reason(self, pipeline: PipelineInfo) -> str: + """ + Get human-readable reason for why pipeline is blocking. + + Args: + pipeline: Pipeline info + + Returns: + Human-readable blocking reason + """ + if pipeline.status == PipelineStatus.SUCCESS: + return "" + + if pipeline.status == PipelineStatus.FAILED: + if pipeline.failed_jobs: + failed_job_names = [job.name for job in pipeline.failed_jobs[:3]] + if len(pipeline.failed_jobs) > 3: + failed_job_names.append( + f"... and {len(pipeline.failed_jobs) - 3} more" + ) + return ( + f"Pipeline failed: {', '.join(failed_job_names)}. " + f"Fix these jobs before merging." + ) + return "Pipeline failed. Check CI for details." + + if pipeline.status == PipelineStatus.CANCELED: + return "Pipeline was canceled." + + if pipeline.status in (PipelineStatus.RUNNING, PipelineStatus.PENDING): + return f"Pipeline is {pipeline.status.value}. Wait for completion." + + if pipeline.has_security_issues: + return ( + f"Security scan failures detected: " + f"{', '.join(i['type'] for i in pipeline.security_issues[:3])}" + ) + + return f"Pipeline status: {pipeline.status.value}" + + def format_pipeline_summary(self, pipeline: PipelineInfo) -> str: + """ + Format pipeline info as a summary string. + + Args: + pipeline: Pipeline info + + Returns: + Formatted summary + """ + status_emoji = { + PipelineStatus.SUCCESS: "✅", + PipelineStatus.FAILED: "❌", + PipelineStatus.RUNNING: "🔄", + PipelineStatus.PENDING: "⏳", + PipelineStatus.CANCELED: "🚫", + PipelineStatus.SKIPPED: "⏭️", + PipelineStatus.UNKNOWN: "❓", + } + + emoji = status_emoji.get(pipeline.status, "⚪") + + lines = [ + f"### CI/CD Pipeline #{pipeline.pipeline_id} {emoji}", + f"**Status:** {pipeline.status.value.upper()}", + f"**Branch:** {pipeline.ref}", + f"**Commit:** {pipeline.sha[:8]}", + "", + ] + + if pipeline.duration: + lines.append( + f"**Duration:** {int(pipeline.duration // 60)}m {int(pipeline.duration % 60)}s" + ) + + if pipeline.jobs: + lines.append(f"**Jobs:** {len(pipeline.jobs)} total") + + # Count by status + status_counts = {} + for job in pipeline.jobs: + status_counts[job.status] = status_counts.get(job.status, 0) + 1 + + if status_counts: + lines.append("**Job Status:**") + for status, count in sorted(status_counts.items()): + lines.append(f" - {status}: {count}") + + # Security issues + if pipeline.security_issues: + lines.append("") + lines.append("### 🚨 Security Issues") + for issue in pipeline.security_issues: + lines.append(f"- **{issue['type']}**: {issue['job_name']}") + + # Failed jobs + if pipeline.failed_jobs: + lines.append("") + lines.append("### Failed Jobs") + for job in pipeline.failed_jobs[:5]: + if job.failure_reason: + lines.append( + f"- **{job.name}** ({job.stage}): {job.failure_reason}" + ) + else: + lines.append(f"- **{job.name}** ({job.stage})") + if len(pipeline.failed_jobs) > 5: + lines.append(f"- ... and {len(pipeline.failed_jobs) - 5} more") + + return "\n".join(lines) + + async def wait_for_pipeline_completion( + self, + mr_iid: int, + timeout_seconds: int = 1800, # 30 minutes default + check_interval: int = 30, + ) -> PipelineInfo | None: + """ + Wait for pipeline to complete (for interactive workflows). + + Args: + mr_iid: MR IID + timeout_seconds: Maximum time to wait + check_interval: Seconds between checks + + Returns: + Final PipelineInfo or None if timeout + """ + import asyncio + + safe_print(f"[CI] Waiting for MR !{mr_iid} pipeline to complete...") + + elapsed = 0 + while elapsed < timeout_seconds: + pipeline = await self.check_mr_pipeline(mr_iid) + + if not pipeline: + safe_print("[CI] No pipeline found") + return None + + if pipeline.status in ( + PipelineStatus.SUCCESS, + PipelineStatus.FAILED, + PipelineStatus.CANCELED, + ): + safe_print(f"[CI] Pipeline completed: {pipeline.status.value}") + return pipeline + + safe_print( + f"[CI] Pipeline still running... ({elapsed}s elapsed, " + f"{timeout_seconds - elapsed}s remaining)" + ) + + await asyncio.sleep(check_interval) + elapsed += check_interval + + safe_print(f"[CI] Timeout waiting for pipeline ({timeout_seconds}s)") + return None diff --git a/apps/backend/runners/gitlab/services/context_gatherer.py b/apps/backend/runners/gitlab/services/context_gatherer.py new file mode 100644 index 00000000..30455fcd --- /dev/null +++ b/apps/backend/runners/gitlab/services/context_gatherer.py @@ -0,0 +1,402 @@ +""" +MR Context Gatherer for GitLab +============================== + +Gathers all necessary context for MR review BEFORE the AI starts. + +Responsibilities: +- Fetch MR metadata (title, author, branches, description) +- Get all changed files with full content +- Detect monorepo structure and project layout +- Find related files (imports, tests, configs) +- Build complete diff with context +""" + +from __future__ import annotations + +import re +from dataclasses import dataclass +from pathlib import Path + +try: + from ..glab_client import GitLabClient, GitLabConfig + from ..models import MRContext + from .io_utils import safe_print +except ImportError: + from core.io_utils import safe_print + from glab_client import GitLabClient, GitLabConfig + from models import MRContext + + +# Validation patterns for git refs and paths +SAFE_REF_PATTERN = re.compile(r"^[a-zA-Z0-9._/\-]+$") +SAFE_PATH_PATTERN = re.compile(r"^[a-zA-Z0-9._/\-@]+$") + + +def _validate_git_ref(ref: str) -> bool: + """Validate git ref (branch name or commit SHA) for safe use in commands.""" + if not ref or len(ref) > 256: + return False + return bool(SAFE_REF_PATTERN.match(ref)) + + +def _validate_file_path(path: str) -> bool: + """Validate file path for safe use in git commands.""" + if not path or len(path) > 1024: + return False + if ".." in path or path.startswith("/"): + return False + return bool(SAFE_PATH_PATTERN.match(path)) + + +# Known GitLab AI bot patterns +GITLAB_AI_BOT_PATTERNS = { + # GitLab official bots + "gitlab-bot": "GitLab Bot", + "gitlab": "GitLab", + # AI code review tools + "coderabbit": "CodeRabbit", + "greptile": "Greptile", + "cursor": "Cursor", + "sweep": "Sweep AI", + "codium": "Qodo", + "dependabot": "Dependabot", + "renovate": "Renovate", +} + + +@dataclass +class ChangedFile: + """A file that was changed in the MR.""" + + path: str + status: str # added, modified, deleted, renamed + additions: int + deletions: int + content: str # Current file content + base_content: str # Content before changes + patch: str # The diff patch for this file + + +@dataclass +class AIBotComment: + """A comment from an AI review tool.""" + + comment_id: int + author: str + tool_name: str + body: str + file: str | None + line: int | None + created_at: str + + +class MRContextGatherer: + """Gathers all context needed for MR review BEFORE the AI starts.""" + + def __init__( + self, + project_dir: Path, + mr_iid: int, + config: GitLabConfig | None = None, + ): + self.project_dir = Path(project_dir) + self.mr_iid = mr_iid + + if config: + self.client = GitLabClient( + project_dir=self.project_dir, + config=config, + ) + else: + # Try to load config from project + from ..glab_client import load_gitlab_config + + config = load_gitlab_config(self.project_dir) + if not config: + raise ValueError("GitLab configuration not found") + + self.client = GitLabClient( + project_dir=self.project_dir, + config=config, + ) + + async def gather(self) -> MRContext: + """ + Gather all context for review. + + Returns: + MRContext with all necessary information for review + """ + safe_print(f"[Context] Gathering context for MR !{self.mr_iid}...") + + # Fetch basic MR metadata + mr_data = await self.client.get_mr_async(self.mr_iid) + safe_print( + f"[Context] MR metadata: {mr_data.get('title', 'Unknown')} " + f"by {mr_data.get('author', {}).get('username', 'unknown')}", + ) + + # Fetch changed files with diff + changes_data = await self.client.get_mr_changes_async(self.mr_iid) + safe_print( + f"[Context] Fetched {len(changes_data.get('changes', []))} changed files" + ) + + # Build diff + diff_parts = [] + for change in changes_data.get("changes", []): + diff = change.get("diff", "") + if diff: + diff_parts.append(diff) + + diff = "\n".join(diff_parts) + safe_print(f"[Context] Gathered diff: {len(diff)} chars") + + # Fetch commits + commits = await self.client.get_mr_commits_async(self.mr_iid) + safe_print(f"[Context] Fetched {len(commits)} commits") + + # Get head commit SHA + head_sha = "" + if commits: + head_sha = commits[-1].get("id") or commits[-1].get("sha", "") + + # Build changed files list + changed_files = [] + total_additions = changes_data.get("additions", 0) + total_deletions = changes_data.get("deletions", 0) + + for change in changes_data.get("changes", []): + new_path = change.get("new_path") + old_path = change.get("old_path") + + # Determine status + if change.get("new_file"): + status = "added" + elif change.get("deleted_file"): + status = "deleted" + elif change.get("renamed_file"): + status = "renamed" + else: + status = "modified" + + changed_files.append( + { + "new_path": new_path or old_path, + "old_path": old_path or new_path, + "status": status, + } + ) + + # Fetch AI bot comments for triage + ai_bot_comments = await self._fetch_ai_bot_comments() + safe_print(f"[Context] Fetched {len(ai_bot_comments)} AI bot comments") + + return MRContext( + mr_iid=self.mr_iid, + title=mr_data.get("title", ""), + description=mr_data.get("description", "") or "", + author=mr_data.get("author", {}).get("username", "unknown"), + source_branch=mr_data.get("source_branch", ""), + target_branch=mr_data.get("target_branch", ""), + state=mr_data.get("state", "opened"), + changed_files=changed_files, + diff=diff, + total_additions=total_additions, + total_deletions=total_deletions, + commits=commits, + head_sha=head_sha, + ) + + async def _fetch_ai_bot_comments(self) -> list[AIBotComment]: + """ + Fetch comments from AI code review tools on this MR. + + Returns comments from known AI tools. + """ + ai_comments: list[AIBotComment] = [] + + try: + # Fetch MR notes (comments) + notes = await self.client.get_mr_notes_async(self.mr_iid) + + for note in notes: + comment = self._parse_ai_comment(note) + if comment: + ai_comments.append(comment) + + except Exception as e: + safe_print(f"[Context] Error fetching AI bot comments: {e}") + + return ai_comments + + def _parse_ai_comment(self, note: dict) -> AIBotComment | None: + """ + Parse a note and return AIBotComment if it's from a known AI tool. + + Args: + note: Raw note data from GitLab API + + Returns: + AIBotComment if author is a known AI bot, None otherwise + """ + author_data = note.get("author") + author = (author_data.get("username") if author_data else "") or "" + if not author: + return None + + # Check if author matches any known AI bot pattern + tool_name = None + author_lower = author.lower() + for pattern, name in GITLAB_AI_BOT_PATTERNS.items(): + if pattern in author_lower: + tool_name = name + break + + if not tool_name: + return None + + return AIBotComment( + comment_id=note.get("id", 0), + author=author, + tool_name=tool_name, + body=note.get("body", ""), + file=None, # GitLab notes don't have file/line in the same way + line=None, + created_at=note.get("created_at", ""), + ) + + +class FollowupMRContextGatherer: + """ + Gathers context specifically for follow-up reviews. + + Unlike the full MRContextGatherer, this only fetches: + - New commits since last review + - Changed files since last review + - New comments since last review + """ + + def __init__( + self, + project_dir: Path, + mr_iid: int, + previous_review, # MRReviewResult + config: GitLabConfig | None = None, + ): + self.project_dir = Path(project_dir) + self.mr_iid = mr_iid + self.previous_review = previous_review + + if config: + self.client = GitLabClient( + project_dir=self.project_dir, + config=config, + ) + else: + # Try to load config from project + from ..glab_client import load_gitlab_config + + config = load_gitlab_config(self.project_dir) + if not config: + raise ValueError("GitLab configuration not found") + + self.client = GitLabClient( + project_dir=self.project_dir, + config=config, + ) + + async def gather(self): + """ + Gather context for a follow-up review. + + Returns: + FollowupMRContext with changes since last review + """ + from ..models import FollowupMRContext + + previous_sha = self.previous_review.reviewed_commit_sha + + if not previous_sha: + safe_print( + "[Followup] No reviewed_commit_sha in previous review, " + "cannot gather incremental context" + ) + return FollowupMRContext( + mr_iid=self.mr_iid, + previous_review=self.previous_review, + previous_commit_sha="", + current_commit_sha="", + ) + + safe_print(f"[Followup] Gathering context since commit {previous_sha[:8]}...") + + # Get current MR data + mr_data = await self.client.get_mr_async(self.mr_iid) + + # Get current commits + commits = await self.client.get_mr_commits_async(self.mr_iid) + + # Find new commits since previous review + new_commits = [] + found_previous = False + for commit in commits: + commit_sha = commit.get("id") or commit.get("sha", "") + if commit_sha == previous_sha: + found_previous = True + break + new_commits.append(commit) + + if not found_previous: + safe_print("[Followup] Previous commit SHA not found in MR history") + + # Get current head SHA + current_sha = "" + if commits: + current_sha = commits[-1].get("id") or commits[-1].get("sha", "") + + if previous_sha == current_sha: + safe_print("[Followup] No new commits since last review") + return FollowupMRContext( + mr_iid=self.mr_iid, + previous_review=self.previous_review, + previous_commit_sha=previous_sha, + current_commit_sha=current_sha, + ) + + safe_print( + f"[Followup] Comparing {previous_sha[:8]}...{current_sha[:8]}, " + f"{len(new_commits)} new commits" + ) + + # Build diff from changes + changes_data = await self.client.get_mr_changes_async(self.mr_iid) + + files_changed = [] + diff_parts = [] + for change in changes_data.get("changes", []): + new_path = change.get("new_path") or change.get("old_path", "") + if new_path: + files_changed.append(new_path) + + diff = change.get("diff", "") + if diff: + diff_parts.append(diff) + + diff_since_review = "\n".join(diff_parts) + + safe_print( + f"[Followup] Found {len(new_commits)} new commits, " + f"{len(files_changed)} changed files" + ) + + return FollowupMRContext( + mr_iid=self.mr_iid, + previous_review=self.previous_review, + previous_commit_sha=previous_sha, + current_commit_sha=current_sha, + commits_since_review=new_commits, + files_changed_since_review=files_changed, + diff_since_review=diff_since_review, + ) diff --git a/apps/backend/runners/gitlab/utils/__init__.py b/apps/backend/runners/gitlab/utils/__init__.py new file mode 100644 index 00000000..056389df --- /dev/null +++ b/apps/backend/runners/gitlab/utils/__init__.py @@ -0,0 +1,48 @@ +""" +GitLab Utilities Package +======================== + +Utility modules for GitLab automation. +""" + +from .file_lock import ( + FileLock, + FileLockError, + FileLockTimeout, + atomic_write, + locked_json_read, + locked_json_update, + locked_json_write, + locked_read, + locked_write, +) +from .rate_limiter import ( + CostLimitExceeded, + CostTracker, + RateLimiter, + RateLimitExceeded, + TokenBucket, + check_rate_limit, + rate_limited, +) + +__all__ = [ + # File locking + "FileLock", + "FileLockError", + "FileLockTimeout", + "atomic_write", + "locked_json_read", + "locked_json_update", + "locked_json_write", + "locked_read", + "locked_write", + # Rate limiting + "CostLimitExceeded", + "CostTracker", + "RateLimitExceeded", + "RateLimiter", + "TokenBucket", + "check_rate_limit", + "rate_limited", +] diff --git a/apps/backend/runners/gitlab/utils/file_lock.py b/apps/backend/runners/gitlab/utils/file_lock.py new file mode 100644 index 00000000..065d2028 --- /dev/null +++ b/apps/backend/runners/gitlab/utils/file_lock.py @@ -0,0 +1,481 @@ +""" +File Locking for Concurrent Operations +===================================== + +Thread-safe and process-safe file locking utilities for GitHub automation. +Uses fcntl.flock() on Unix systems and msvcrt.locking() on Windows for proper +cross-process locking. + +Example Usage: + # Simple file locking + async with FileLock("path/to/file.json", timeout=5.0): + # Do work with locked file + pass + + # Atomic write with locking + async with locked_write("path/to/file.json", timeout=5.0) as f: + json.dump(data, f) + +""" + +from __future__ import annotations + +import asyncio +import json +import os +import tempfile +import time +import warnings +from collections.abc import Callable +from contextlib import asynccontextmanager, contextmanager +from pathlib import Path +from typing import Any + +_IS_WINDOWS = os.name == "nt" +_WINDOWS_LOCK_SIZE = 1024 * 1024 + +try: + import fcntl # type: ignore +except ImportError: # pragma: no cover + fcntl = None + +try: + import msvcrt # type: ignore +except ImportError: # pragma: no cover + msvcrt = None + + +def _try_lock(fd: int, exclusive: bool) -> None: + if _IS_WINDOWS: + if msvcrt is None: + raise FileLockError("msvcrt is required for file locking on Windows") + if not exclusive: + warnings.warn( + "Shared file locks are not supported on Windows; using exclusive lock", + RuntimeWarning, + stacklevel=3, + ) + msvcrt.locking(fd, msvcrt.LK_NBLCK, _WINDOWS_LOCK_SIZE) + return + + if fcntl is None: + raise FileLockError( + "fcntl is required for file locking on non-Windows platforms" + ) + + lock_mode = fcntl.LOCK_EX if exclusive else fcntl.LOCK_SH + fcntl.flock(fd, lock_mode | fcntl.LOCK_NB) + + +def _unlock(fd: int) -> None: + if _IS_WINDOWS: + if msvcrt is None: + warnings.warn( + "msvcrt unavailable; cannot unlock file descriptor", + RuntimeWarning, + stacklevel=3, + ) + return + msvcrt.locking(fd, msvcrt.LK_UNLCK, _WINDOWS_LOCK_SIZE) + return + + if fcntl is None: + warnings.warn( + "fcntl unavailable; cannot unlock file descriptor", + RuntimeWarning, + stacklevel=3, + ) + return + fcntl.flock(fd, fcntl.LOCK_UN) + + +class FileLockError(Exception): + """Raised when file locking operations fail.""" + + pass + + +class FileLockTimeout(FileLockError): + """Raised when lock acquisition times out.""" + + pass + + +class FileLock: + """ + Cross-process file lock using platform-specific locking (fcntl.flock on Unix, + msvcrt.locking on Windows). + + Supports both sync and async context managers for flexible usage. + + Args: + filepath: Path to file to lock (will be created if needed) + timeout: Maximum seconds to wait for lock (default: 5.0) + exclusive: Whether to use exclusive lock (default: True) + + Example: + # Synchronous usage + with FileLock("/path/to/file.json"): + # File is locked + pass + + # Asynchronous usage + async with FileLock("/path/to/file.json"): + # File is locked + pass + """ + + def __init__( + self, + filepath: str | Path, + timeout: float = 5.0, + exclusive: bool = True, + ): + self.filepath = Path(filepath) + self.timeout = timeout + self.exclusive = exclusive + self._lock_file: Path | None = None + self._fd: int | None = None + + def _get_lock_file(self) -> Path: + """Get lock file path (separate .lock file).""" + return self.filepath.parent / f"{self.filepath.name}.lock" + + def _acquire_lock(self) -> None: + """Acquire the file lock (blocking with timeout).""" + self._lock_file = self._get_lock_file() + self._lock_file.parent.mkdir(parents=True, exist_ok=True) + + # Open lock file + self._fd = os.open(str(self._lock_file), os.O_CREAT | os.O_RDWR) + + # Try to acquire lock with timeout + start_time = time.time() + + while True: + try: + # Non-blocking lock attempt + _try_lock(self._fd, self.exclusive) + return # Lock acquired + except (BlockingIOError, OSError): + # Lock held by another process + elapsed = time.time() - start_time + if elapsed >= self.timeout: + os.close(self._fd) + self._fd = None + raise FileLockTimeout( + f"Failed to acquire lock on {self.filepath} within " + f"{self.timeout}s" + ) + + # Wait a bit before retrying + time.sleep(0.01) + + def _release_lock(self) -> None: + """Release the file lock.""" + if self._fd is not None: + try: + _unlock(self._fd) + os.close(self._fd) + except Exception: + pass # Best effort cleanup + finally: + self._fd = None + + # Clean up lock file + if self._lock_file and self._lock_file.exists(): + try: + self._lock_file.unlink() + except Exception: + pass # Best effort cleanup + + def __enter__(self): + """Synchronous context manager entry.""" + self._acquire_lock() + return self + + def __exit__(self, exc_type, exc_val, exc_tb): + """Synchronous context manager exit.""" + self._release_lock() + return False + + async def __aenter__(self): + """Async context manager entry.""" + # Run blocking lock acquisition in thread pool + await asyncio.get_running_loop().run_in_executor(None, self._acquire_lock) + return self + + async def __aexit__(self, exc_type, exc_val, exc_tb): + """Async context manager exit.""" + await asyncio.get_running_loop().run_in_executor(None, self._release_lock) + return False + + +@contextmanager +def atomic_write(filepath: str | Path, mode: str = "w"): + """ + Atomic file write using temp file and rename. + + Writes to .tmp file first, then atomically replaces target file + using os.replace() which is atomic on POSIX systems. + + Args: + filepath: Target file path + mode: File open mode (default: "w") + + Example: + with atomic_write("/path/to/file.json") as f: + json.dump(data, f) + """ + filepath = Path(filepath) + filepath.parent.mkdir(parents=True, exist_ok=True) + + # Create temp file in same directory for atomic rename + fd, tmp_path = tempfile.mkstemp( + dir=filepath.parent, prefix=f".{filepath.name}.tmp.", suffix="" + ) + + try: + # Open temp file with requested mode + with os.fdopen(fd, mode) as f: + yield f + + # Atomic replace - succeeds or fails completely + os.replace(tmp_path, filepath) + + except Exception: + # Clean up temp file on error + try: + os.unlink(tmp_path) + except Exception: + pass + raise + + +@asynccontextmanager +async def locked_write( + filepath: str | Path, timeout: float = 5.0, mode: str = "w" +) -> Any: + """ + Async context manager combining file locking and atomic writes. + + Acquires exclusive lock, writes to temp file, atomically replaces target. + This is the recommended way to safely write shared state files. + + Args: + filepath: Target file path + timeout: Lock timeout in seconds (default: 5.0) + mode: File open mode (default: "w") + + Example: + async with locked_write("/path/to/file.json", timeout=5.0) as f: + json.dump(data, f, indent=2) + + Raises: + FileLockTimeout: If lock cannot be acquired within timeout + """ + filepath = Path(filepath) + + # Acquire lock + lock = FileLock(filepath, timeout=timeout, exclusive=True) + await lock.__aenter__() + + try: + # Atomic write in thread pool (since it uses sync file I/O) + fd, tmp_path = await asyncio.get_running_loop().run_in_executor( + None, + lambda: tempfile.mkstemp( + dir=filepath.parent, prefix=f".{filepath.name}.tmp.", suffix="" + ), + ) + + try: + # Open temp file and yield to caller + f = os.fdopen(fd, mode) + try: + yield f + finally: + f.close() + + # Atomic replace + await asyncio.get_running_loop().run_in_executor( + None, os.replace, tmp_path, filepath + ) + + except Exception: + # Clean up temp file on error + try: + await asyncio.get_running_loop().run_in_executor( + None, os.unlink, tmp_path + ) + except Exception: + pass + raise + + finally: + # Release lock + await lock.__aexit__(None, None, None) + + +@asynccontextmanager +async def locked_read(filepath: str | Path, timeout: float = 5.0) -> Any: + """ + Async context manager for locked file reading. + + Acquires shared lock for reading, allowing multiple concurrent readers + but blocking writers. + + Args: + filepath: File path to read + timeout: Lock timeout in seconds (default: 5.0) + + Example: + async with locked_read("/path/to/file.json", timeout=5.0) as f: + data = json.load(f) + + Raises: + FileLockTimeout: If lock cannot be acquired within timeout + FileNotFoundError: If file doesn't exist + """ + filepath = Path(filepath) + + if not filepath.exists(): + raise FileNotFoundError(f"File not found: {filepath}") + + # Acquire shared lock (allows multiple readers) + lock = FileLock(filepath, timeout=timeout, exclusive=False) + await lock.__aenter__() + + try: + # Open file for reading + with open(filepath) as f: + yield f + finally: + # Release lock + await lock.__aexit__(None, None, None) + + +async def locked_json_write( + filepath: str | Path, data: Any, timeout: float = 5.0, indent: int = 2 +) -> None: + """ + Helper function for writing JSON with locking and atomicity. + + Args: + filepath: Target file path + data: Data to serialize as JSON + timeout: Lock timeout in seconds (default: 5.0) + indent: JSON indentation (default: 2) + + Example: + await locked_json_write("/path/to/file.json", {"key": "value"}) + + Raises: + FileLockTimeout: If lock cannot be acquired within timeout + """ + async with locked_write(filepath, timeout=timeout) as f: + json.dump(data, f, indent=indent) + + +async def locked_json_read(filepath: str | Path, timeout: float = 5.0) -> Any: + """ + Helper function for reading JSON with locking. + + Args: + filepath: File path to read + timeout: Lock timeout in seconds (default: 5.0) + + Returns: + Parsed JSON data + + Example: + data = await locked_json_read("/path/to/file.json") + + Raises: + FileLockTimeout: If lock cannot be acquired within timeout + FileNotFoundError: If file doesn't exist + json.JSONDecodeError: If file contains invalid JSON + """ + async with locked_read(filepath, timeout=timeout) as f: + return json.load(f) + + +async def locked_json_update( + filepath: str | Path, + updater: Callable[[Any], Any], + timeout: float = 5.0, + indent: int = 2, +) -> Any: + """ + Helper for atomic read-modify-write of JSON files. + + Acquires exclusive lock, reads current data, applies updater function, + writes updated data atomically. + + Args: + filepath: File path to update + updater: Function that takes current data and returns updated data + timeout: Lock timeout in seconds (default: 5.0) + indent: JSON indentation (default: 2) + + Returns: + Updated data + + Example: + def add_item(data): + data["items"].append({"new": "item"}) + return data + + updated = await locked_json_update("/path/to/file.json", add_item) + + Raises: + FileLockTimeout: If lock cannot be acquired within timeout + """ + filepath = Path(filepath) + + # Acquire exclusive lock + lock = FileLock(filepath, timeout=timeout, exclusive=True) + await lock.__aenter__() + + try: + # Read current data + def _read_json(): + if filepath.exists(): + with open(filepath) as f: + return json.load(f) + return None + + data = await asyncio.get_running_loop().run_in_executor(None, _read_json) + + # Apply update function + updated_data = updater(data) + + # Write atomically + fd, tmp_path = await asyncio.get_running_loop().run_in_executor( + None, + lambda: tempfile.mkstemp( + dir=filepath.parent, prefix=f".{filepath.name}.tmp.", suffix="" + ), + ) + + try: + with os.fdopen(fd, "w") as f: + json.dump(updated_data, f, indent=indent) + + await asyncio.get_running_loop().run_in_executor( + None, os.replace, tmp_path, filepath + ) + + except Exception: + try: + await asyncio.get_running_loop().run_in_executor( + None, os.unlink, tmp_path + ) + except Exception: + pass + raise + + return updated_data + + finally: + await lock.__aexit__(None, None, None) diff --git a/apps/backend/runners/gitlab/utils/rate_limiter.py b/apps/backend/runners/gitlab/utils/rate_limiter.py new file mode 100644 index 00000000..b92d77c8 --- /dev/null +++ b/apps/backend/runners/gitlab/utils/rate_limiter.py @@ -0,0 +1,698 @@ +""" +Rate Limiting Protection for GitHub Automation +=============================================== + +Comprehensive rate limiting system that protects against: +1. GitHub API rate limits (5000 req/hour for authenticated users) +2. AI API cost overruns (configurable budget per run) +3. Thundering herd problems (exponential backoff) + +Components: +- TokenBucket: Classic token bucket algorithm for rate limiting +- RateLimiter: Singleton managing GitHub and AI cost limits +- @rate_limited decorator: Automatic pre-flight checks with retry logic +- Cost tracking: Per-model AI API cost calculation and budgeting + +Usage: + # Singleton instance + limiter = RateLimiter.get_instance( + github_limit=5000, + github_refill_rate=1.4, # tokens per second + cost_limit=10.0, # $10 per run + ) + + # Decorate GitHub operations + @rate_limited(operation_type="github") + async def fetch_pr_data(pr_number: int): + result = subprocess.run(["gh", "pr", "view", str(pr_number)]) + return result + + # Track AI costs + limiter.track_ai_cost( + input_tokens=1000, + output_tokens=500, + model="claude-sonnet-4-20250514" + ) + + # Manual rate check + if not await limiter.acquire_github(): + raise RateLimitExceeded("GitHub API rate limit reached") +""" + +from __future__ import annotations + +import asyncio +import functools +import time +from collections.abc import Callable +from dataclasses import dataclass, field +from datetime import datetime, timedelta +from typing import Any, TypeVar + +# Type for decorated functions +F = TypeVar("F", bound=Callable[..., Any]) + + +class RateLimitExceeded(Exception): + """Raised when rate limit is exceeded and cannot proceed.""" + + pass + + +class CostLimitExceeded(Exception): + """Raised when AI cost budget is exceeded.""" + + pass + + +@dataclass +class TokenBucket: + """ + Token bucket algorithm for rate limiting. + + The bucket has a maximum capacity and refills at a constant rate. + Each operation consumes one token. If bucket is empty, operations + must wait for refill or be rejected. + + Args: + capacity: Maximum number of tokens (e.g., 5000 for GitHub) + refill_rate: Tokens added per second (e.g., 1.4 for 5000/hour) + """ + + capacity: int + refill_rate: float # tokens per second + tokens: float = field(init=False) + last_refill: float = field(init=False) + + def __post_init__(self): + """Initialize bucket as full.""" + self.tokens = float(self.capacity) + self.last_refill = time.monotonic() + + def _refill(self) -> None: + """Refill bucket based on elapsed time.""" + now = time.monotonic() + elapsed = now - self.last_refill + tokens_to_add = elapsed * self.refill_rate + self.tokens = min(self.capacity, self.tokens + tokens_to_add) + self.last_refill = now + + def try_acquire(self, tokens: int = 1) -> bool: + """ + Try to acquire tokens from bucket. + + Returns: + True if tokens acquired, False if insufficient tokens + """ + self._refill() + if self.tokens >= tokens: + self.tokens -= tokens + return True + return False + + async def acquire(self, tokens: int = 1, timeout: float | None = None) -> bool: + """ + Acquire tokens from bucket, waiting if necessary. + + Args: + tokens: Number of tokens to acquire + timeout: Maximum time to wait in seconds + + Returns: + True if tokens acquired, False if timeout reached + """ + start_time = time.monotonic() + + while True: + if self.try_acquire(tokens): + return True + + # Check timeout + if timeout is not None: + elapsed = time.monotonic() - start_time + if elapsed >= timeout: + return False + + # Wait for next refill + # Calculate time until we have enough tokens + tokens_needed = tokens - self.tokens + wait_time = min(tokens_needed / self.refill_rate, 1.0) # Max 1 second wait + await asyncio.sleep(wait_time) + + def available(self) -> int: + """Get number of available tokens.""" + self._refill() + return int(self.tokens) + + def time_until_available(self, tokens: int = 1) -> float: + """ + Calculate seconds until requested tokens available. + + Returns: + 0 if tokens immediately available, otherwise seconds to wait + """ + self._refill() + if self.tokens >= tokens: + return 0.0 + tokens_needed = tokens - self.tokens + return tokens_needed / self.refill_rate + + +# AI model pricing (per 1M tokens) +AI_PRICING = { + # Claude models (as of 2025) + "claude-sonnet-4-20250514": {"input": 3.00, "output": 15.00}, + "claude-opus-4-20250514": {"input": 15.00, "output": 75.00}, + "claude-sonnet-3-5-20241022": {"input": 3.00, "output": 15.00}, + "claude-haiku-3-5-20241022": {"input": 0.80, "output": 4.00}, + # Extended thinking models (higher output costs) + "claude-sonnet-4-20250514-thinking": {"input": 3.00, "output": 15.00}, + # Default fallback + "default": {"input": 3.00, "output": 15.00}, +} + + +@dataclass +class CostTracker: + """Track AI API costs.""" + + total_cost: float = 0.0 + cost_limit: float = 10.0 + operations: list[dict] = field(default_factory=list) + + def add_operation( + self, + input_tokens: int, + output_tokens: int, + model: str, + operation_name: str = "unknown", + ) -> float: + """ + Track cost of an AI operation. + + Args: + input_tokens: Number of input tokens + output_tokens: Number of output tokens + model: Model identifier + operation_name: Name of operation for tracking + + Returns: + Cost of this operation in dollars + + Raises: + CostLimitExceeded: If operation would exceed budget + """ + cost = self.calculate_cost(input_tokens, output_tokens, model) + + # Check if this would exceed limit + if self.total_cost + cost > self.cost_limit: + raise CostLimitExceeded( + f"Operation would exceed cost limit: " + f"${self.total_cost + cost:.2f} > ${self.cost_limit:.2f}" + ) + + self.total_cost += cost + self.operations.append( + { + "timestamp": datetime.now().isoformat(), + "operation": operation_name, + "model": model, + "input_tokens": input_tokens, + "output_tokens": output_tokens, + "cost": cost, + } + ) + + return cost + + @staticmethod + def calculate_cost(input_tokens: int, output_tokens: int, model: str) -> float: + """ + Calculate cost for model usage. + + Args: + input_tokens: Number of input tokens + output_tokens: Number of output tokens + model: Model identifier + + Returns: + Cost in dollars + """ + # Get pricing for model (fallback to default) + pricing = AI_PRICING.get(model, AI_PRICING["default"]) + + input_cost = (input_tokens / 1_000_000) * pricing["input"] + output_cost = (output_tokens / 1_000_000) * pricing["output"] + + return input_cost + output_cost + + def remaining_budget(self) -> float: + """Get remaining budget in dollars.""" + return max(0.0, self.cost_limit - self.total_cost) + + def usage_report(self) -> str: + """Generate cost usage report.""" + lines = [ + "Cost Usage Report", + "=" * 50, + f"Total Cost: ${self.total_cost:.4f}", + f"Budget: ${self.cost_limit:.2f}", + f"Remaining: ${self.remaining_budget():.4f}", + f"Usage: {(self.total_cost / self.cost_limit * 100):.1f}%", + "", + f"Operations: {len(self.operations)}", + ] + + if self.operations: + lines.append("") + lines.append("Top 5 Most Expensive Operations:") + sorted_ops = sorted(self.operations, key=lambda x: x["cost"], reverse=True) + for op in sorted_ops[:5]: + lines.append( + f" ${op['cost']:.4f} - {op['operation']} " + f"({op['input_tokens']} in, {op['output_tokens']} out)" + ) + + return "\n".join(lines) + + +class RateLimiter: + """ + Singleton rate limiter for GitHub automation. + + Manages: + - GitHub API rate limits (token bucket) + - AI cost limits (budget tracking) + - Request queuing and backoff + """ + + _instance: RateLimiter | None = None + _initialized: bool = False + + def __init__( + self, + github_limit: int = 5000, + github_refill_rate: float = 1.4, # ~5000/hour + cost_limit: float = 10.0, + max_retry_delay: float = 300.0, # 5 minutes + ): + """ + Initialize rate limiter. + + Args: + github_limit: Maximum GitHub API calls (default: 5000/hour) + github_refill_rate: Tokens per second refill rate + cost_limit: Maximum AI cost in dollars per run + max_retry_delay: Maximum exponential backoff delay + """ + if RateLimiter._initialized: + return + + self.github_bucket = TokenBucket( + capacity=github_limit, + refill_rate=github_refill_rate, + ) + self.cost_tracker = CostTracker(cost_limit=cost_limit) + self.max_retry_delay = max_retry_delay + + # Request statistics + self.github_requests = 0 + self.github_rate_limited = 0 + self.github_errors = 0 + self.start_time = datetime.now() + + RateLimiter._initialized = True + + @classmethod + def get_instance( + cls, + github_limit: int = 5000, + github_refill_rate: float = 1.4, + cost_limit: float = 10.0, + max_retry_delay: float = 300.0, + ) -> RateLimiter: + """ + Get or create singleton instance. + + Args: + github_limit: Maximum GitHub API calls + github_refill_rate: Tokens per second refill rate + cost_limit: Maximum AI cost in dollars + max_retry_delay: Maximum retry delay + + Returns: + RateLimiter singleton instance + """ + if cls._instance is None: + cls._instance = RateLimiter( + github_limit=github_limit, + github_refill_rate=github_refill_rate, + cost_limit=cost_limit, + max_retry_delay=max_retry_delay, + ) + return cls._instance + + @classmethod + def reset_instance(cls) -> None: + """Reset singleton (for testing).""" + cls._instance = None + cls._initialized = False + + async def acquire_github(self, timeout: float | None = None) -> bool: + """ + Acquire permission for GitHub API call. + + Args: + timeout: Maximum time to wait (None = wait forever) + + Returns: + True if permission granted, False if timeout + """ + self.github_requests += 1 + success = await self.github_bucket.acquire(tokens=1, timeout=timeout) + if not success: + self.github_rate_limited += 1 + return success + + def check_github_available(self) -> tuple[bool, str]: + """ + Check if GitHub API is available without consuming token. + + Returns: + (available, message) tuple + """ + available = self.github_bucket.available() + + if available > 0: + return True, f"{available} requests available" + + wait_time = self.github_bucket.time_until_available() + return False, f"Rate limited. Wait {wait_time:.1f}s for next request" + + def track_ai_cost( + self, + input_tokens: int, + output_tokens: int, + model: str, + operation_name: str = "unknown", + ) -> float: + """ + Track AI API cost. + + Args: + input_tokens: Number of input tokens + output_tokens: Number of output tokens + model: Model identifier + operation_name: Operation name for tracking + + Returns: + Cost of operation + + Raises: + CostLimitExceeded: If budget exceeded + """ + return self.cost_tracker.add_operation( + input_tokens=input_tokens, + output_tokens=output_tokens, + model=model, + operation_name=operation_name, + ) + + def check_cost_available(self) -> tuple[bool, str]: + """ + Check if cost budget is available. + + Returns: + (available, message) tuple + """ + remaining = self.cost_tracker.remaining_budget() + + if remaining > 0: + return True, f"${remaining:.2f} budget remaining" + + return False, f"Cost budget exceeded (${self.cost_tracker.total_cost:.2f})" + + def record_github_error(self) -> None: + """Record a GitHub API error.""" + self.github_errors += 1 + + def statistics(self) -> dict: + """ + Get rate limiter statistics. + + Returns: + Dictionary of statistics + """ + runtime = (datetime.now() - self.start_time).total_seconds() + + return { + "runtime_seconds": runtime, + "github": { + "total_requests": self.github_requests, + "rate_limited": self.github_rate_limited, + "errors": self.github_errors, + "available_tokens": self.github_bucket.available(), + "requests_per_second": self.github_requests / max(runtime, 1), + }, + "cost": { + "total_cost": self.cost_tracker.total_cost, + "budget": self.cost_tracker.cost_limit, + "remaining": self.cost_tracker.remaining_budget(), + "operations": len(self.cost_tracker.operations), + }, + } + + def report(self) -> str: + """Generate comprehensive usage report.""" + stats = self.statistics() + runtime = timedelta(seconds=int(stats["runtime_seconds"])) + + lines = [ + "Rate Limiter Report", + "=" * 60, + f"Runtime: {runtime}", + "", + "GitHub API:", + f" Total Requests: {stats['github']['total_requests']}", + f" Rate Limited: {stats['github']['rate_limited']}", + f" Errors: {stats['github']['errors']}", + f" Available Tokens: {stats['github']['available_tokens']}", + f" Rate: {stats['github']['requests_per_second']:.2f} req/s", + "", + "AI Cost:", + f" Total: ${stats['cost']['total_cost']:.4f}", + f" Budget: ${stats['cost']['budget']:.2f}", + f" Remaining: ${stats['cost']['remaining']:.4f}", + f" Operations: {stats['cost']['operations']}", + "", + self.cost_tracker.usage_report(), + ] + + return "\n".join(lines) + + +def rate_limited( + operation_type: str = "github", + max_retries: int = 3, + base_delay: float = 1.0, +) -> Callable[[F], F]: + """ + Decorator to add rate limiting to functions. + + Features: + - Pre-flight rate check + - Automatic retry with exponential backoff + - Error handling for 403/429 responses + + Args: + operation_type: Type of operation ("github" or "ai") + max_retries: Maximum number of retries + base_delay: Base delay for exponential backoff + + Usage: + @rate_limited(operation_type="github") + async def fetch_pr_data(pr_number: int): + result = subprocess.run(["gh", "pr", "view", str(pr_number)]) + return result + """ + + def decorator(func: F) -> F: + @functools.wraps(func) + async def async_wrapper(*args, **kwargs): + limiter = RateLimiter.get_instance() + + for attempt in range(max_retries + 1): + try: + # Pre-flight check + if operation_type == "github": + available, msg = limiter.check_github_available() + if not available and attempt == 0: + # Try to acquire (will wait if needed) + if not await limiter.acquire_github(timeout=30.0): + raise RateLimitExceeded( + f"GitHub API rate limit exceeded: {msg}" + ) + elif not available: + # On retry, wait for token + await limiter.acquire_github( + timeout=limiter.max_retry_delay + ) + + # Execute function + result = await func(*args, **kwargs) + return result + + except CostLimitExceeded: + # Cost limit is hard stop - no retry + raise + + except RateLimitExceeded as e: + if attempt >= max_retries: + raise + + # Exponential backoff + delay = min( + base_delay * (2**attempt), + limiter.max_retry_delay, + ) + print( + f"[RateLimit] Retry {attempt + 1}/{max_retries} " + f"after {delay:.1f}s: {e}", + flush=True, + ) + await asyncio.sleep(delay) + + except Exception as e: + # Check if it's a rate limit error (403/429) + error_str = str(e).lower() + if ( + "403" in error_str + or "429" in error_str + or "rate limit" in error_str + ): + limiter.record_github_error() + + if attempt >= max_retries: + raise RateLimitExceeded( + f"GitHub API rate limit (HTTP 403/429): {e}" + ) + + # Exponential backoff + delay = min( + base_delay * (2**attempt), + limiter.max_retry_delay, + ) + print( + f"[RateLimit] HTTP 403/429 detected. " + f"Retry {attempt + 1}/{max_retries} after {delay:.1f}s", + flush=True, + ) + await asyncio.sleep(delay) + else: + # Not a rate limit error - propagate immediately + raise + + @functools.wraps(func) + def sync_wrapper(*args, **kwargs): + # For sync functions, run in event loop + return asyncio.run(async_wrapper(*args, **kwargs)) + + # Return appropriate wrapper + if asyncio.iscoroutinefunction(func): + return async_wrapper # type: ignore + else: + return sync_wrapper # type: ignore + + return decorator + + +# Convenience function for pre-flight checks +async def check_rate_limit(operation_type: str = "github") -> None: + """ + Pre-flight rate limit check. + + Args: + operation_type: Type of operation to check + + Raises: + RateLimitExceeded: If rate limit would be exceeded + CostLimitExceeded: If cost budget would be exceeded + """ + limiter = RateLimiter.get_instance() + + if operation_type == "github": + available, msg = limiter.check_github_available() + if not available: + raise RateLimitExceeded(f"GitHub API not available: {msg}") + + elif operation_type == "cost": + available, msg = limiter.check_cost_available() + if not available: + raise CostLimitExceeded(f"Cost budget exceeded: {msg}") + + +# Example usage and testing +if __name__ == "__main__": + + async def example_usage(): + """Example of using the rate limiter.""" + + # Initialize with custom limits + limiter = RateLimiter.get_instance( + github_limit=5000, + github_refill_rate=1.4, + cost_limit=10.0, + ) + + print("Rate Limiter Example") + print("=" * 60) + + # Example 1: Manual rate check + print("\n1. Manual rate check:") + available, msg = limiter.check_github_available() + print(f" GitHub API: {msg}") + + # Example 2: Acquire token + print("\n2. Acquire GitHub token:") + if await limiter.acquire_github(): + print(" ✓ Token acquired") + else: + print(" ✗ Rate limited") + + # Example 3: Track AI cost + print("\n3. Track AI cost:") + try: + cost = limiter.track_ai_cost( + input_tokens=1000, + output_tokens=500, + model="claude-sonnet-4-20250514", + operation_name="PR review", + ) + print(f" Cost: ${cost:.4f}") + print( + f" Remaining budget: ${limiter.cost_tracker.remaining_budget():.2f}" + ) + except CostLimitExceeded as e: + print(f" ✗ {e}") + + # Example 4: Decorated function + print("\n4. Using @rate_limited decorator:") + + @rate_limited(operation_type="github") + async def fetch_github_data(resource: str): + print(f" Fetching: {resource}") + # Simulate GitHub API call + await asyncio.sleep(0.1) + return {"data": "example"} + + try: + result = await fetch_github_data("pr/123") + print(f" Result: {result}") + except RateLimitExceeded as e: + print(f" ✗ {e}") + + # Final report + print("\n" + limiter.report()) + + # Run example + asyncio.run(example_usage())