diff --git a/CLAUDE.md b/CLAUDE.md index 460417e2..ac5b70ca 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -682,3 +682,26 @@ AI-powered enhancements controlled by `ai-features` config (global or per-repo). **On AI CLI failure:** Error is logged, flow continues without suggestion **Module:** `webhook_server/libs/ai_cli.py` - shared AI CLI wrapper + +### AI Code Review Integration + +External AI service integration for PR code review via [pr-ai-reviewer](https://github.com/myk-org/pr-ai-reviewer). + +**Schema:** `webhook_server/config/schema.yaml` (`ai-review`), configurable globally or per-repo + +**Triggers:** `pr-opened`, `pr-synchronized` (skips clean rebases). Default: both enabled. + +**Module:** `webhook_server/libs/ai_review.py` - `call_ai_reviewer()` thin HTTP client + +**Features:** + +- 3 specialized review agents (quality, guidelines, security) from [pi-config](https://github.com/myk-org/pi-config) +- Single provider: standard AI review +- Multiple providers: peer review with consensus loop +- Posts inline review comments on PR diff lines via GitHub Pull Request Review API + +**Error handling:** + +- Health check failure: PR comment posted, continue flow +- Review errors: log only, no PR comment +- Never breaks webhook processing diff --git a/examples/config.yaml b/examples/config.yaml index 4be978e6..b936b177 100644 --- a/examples/config.yaml +++ b/examples/config.yaml @@ -123,6 +123,23 @@ test-oracle: # - pr-opened # Run when PR is opened # - pr-synchronized # Run when new commits pushed +# AI Code Review integration +# Reviews PR diffs using specialized AI review agents +# See: https://github.com/myk-org/pr-ai-reviewer +ai-review: + server-url: "http://localhost:8001" + providers: + - ai-provider: "claude" + ai-model: "claude-opus-4-6[1m]" + # Add more providers for peer review: + # - ai-provider: "gemini" + # ai-model: "gemini-2.5-pro" + max-rounds: 3 # Max peer review rounds (default: 3) + timeout-minutes: 30 # Timeout per AI CLI call (default: 30) + triggers: # Default: [pr-opened, pr-synchronized] + - pr-opened + - pr-synchronized + # AI Features configuration # Enables AI-powered enhancements (e.g., conventional title suggestions) ai-features: @@ -130,11 +147,11 @@ ai-features: ai-model: "claude-opus-4-6[1m]" conventional-title: enabled: true - mode: suggest # suggest: show in checkrun | fix: auto-update PR title + mode: suggest # suggest: show in checkrun | fix: auto-update PR title timeout-minutes: 10 resolve-cherry-pick-conflicts-with-ai: enabled: true - timeout-minutes: 10 # Timeout in minutes for AI CLI (default: 10) + timeout-minutes: 10 # Timeout in minutes for AI CLI (default: 10) repositories: my-repository: diff --git a/webhook_server/config/schema.yaml b/webhook_server/config/schema.yaml index 78565127..3b0ca19e 100644 --- a/webhook_server/config/schema.yaml +++ b/webhook_server/config/schema.yaml @@ -68,6 +68,73 @@ $defs: - ai-provider - ai-model additionalProperties: false + ai-review: + type: object + description: | + AI-powered code review for pull requests. + See https://github.com/myk-org/pr-ai-reviewer for server setup. + Each provider uses 3 specialized review agents (quality, guidelines, security). + Single provider: standard review. + Multiple providers: peer review (first = main, rest = peers). + They debate until consensus, then final comments are posted. + properties: + server-url: + type: string + format: uri + description: URL of the pr-ai-reviewer server (e.g., http://localhost:8001) + providers: + type: array + description: | + List of AI providers for code review. + Single provider: standard review. + Multiple providers: peer review (first = main reviewer, rest = peers). + items: + type: object + properties: + ai-provider: + type: string + enum: + - claude + - gemini + - cursor + description: AI CLI provider to use for review + ai-model: + type: string + description: AI model identifier + required: + - ai-provider + - ai-model + additionalProperties: false + minItems: 1 + max-rounds: + type: integer + minimum: 1 + maximum: 5 + default: 3 + description: Maximum peer review debate rounds before accepting main AI result + timeout-minutes: + type: integer + minimum: 1 + default: 30 + description: Timeout in minutes for each AI CLI call + triggers: + type: array + default: + - pr-opened + - pr-synchronized + items: + type: string + enum: + - pr-opened + - pr-synchronized + description: | + When to automatically run AI code review. Default: [pr-opened, pr-synchronized]. + - pr-opened: Run when a new PR is opened + - pr-synchronized: Run when new commits are pushed to a PR (skips clean rebases) + required: + - server-url + - providers + additionalProperties: false type: object properties: log-level: @@ -213,6 +280,8 @@ properties: additionalProperties: false ai-features: $ref: '#/$defs/ai-features' + ai-review: + $ref: '#/$defs/ai-review' labels: type: object description: Configure which labels are enabled and their colors @@ -577,6 +646,8 @@ properties: additionalProperties: false ai-features: $ref: '#/$defs/ai-features' + ai-review: + $ref: '#/$defs/ai-review' custom-check-runs: type: array description: | diff --git a/webhook_server/libs/ai_review.py b/webhook_server/libs/ai_review.py new file mode 100644 index 00000000..4a7b1a7a --- /dev/null +++ b/webhook_server/libs/ai_review.py @@ -0,0 +1,160 @@ +from __future__ import annotations + +import asyncio +from typing import TYPE_CHECKING, Any + +import httpx + +from webhook_server.utils.constants import AI_REVIEW_STR + +if TYPE_CHECKING: + from github.PullRequest import PullRequest + + from webhook_server.libs.github_api import GithubWebhook + from webhook_server.libs.handlers.check_run_handler import CheckRunHandler + +DEFAULT_TRIGGERS: list[str] = ["pr-opened", "pr-synchronized"] + + +async def call_ai_reviewer( + github_webhook: GithubWebhook, + pull_request: PullRequest, + check_run_handler: CheckRunHandler, + trigger: str | None = None, +) -> None: + """Call the pr-ai-reviewer service to review a PR. + + Args: + github_webhook: The GithubWebhook instance with config and token. + pull_request: The PyGithub PullRequest object. + check_run_handler: CheckRunHandler for updating check run status. + trigger: The event trigger (e.g., "pr-opened", "pr-synchronized"). + None means command-triggered (always runs if configured). + Command trigger is reserved for a future /ai-review comment command. + """ + config = github_webhook.ai_review_config + if not config: + return + + # trigger=None means command-triggered (e.g., future /ai-review command). + # Currently only webhook-triggered via pr-opened/pr-synchronized. + + if trigger is not None: + triggers: list[str] = config.get("triggers", DEFAULT_TRIGGERS) + if trigger not in triggers: + github_webhook.logger.debug( + f"{github_webhook.log_prefix} AI reviewer trigger '{trigger}' not in configured triggers {triggers}" + ) + return + + server_url: str = config["server-url"] + log_prefix: str = github_webhook.log_prefix + + check_in_progress = False + try: + await check_run_handler.set_check_in_progress(name=AI_REVIEW_STR) + check_in_progress = True + + async with httpx.AsyncClient(base_url=server_url) as client: + # Health check + try: + health_response = await client.get("/health", timeout=5.0) + health_response.raise_for_status() + except httpx.HTTPError as e: + status_info = "" + if isinstance(e, httpx.HTTPStatusError): + status_info = f" (status {e.response.status_code})" + + msg = f"AI Reviewer server at {server_url} is not responding{status_info}" + github_webhook.logger.warning(f"{log_prefix} {msg}") + await check_run_handler.set_check_failure( + name=AI_REVIEW_STR, + output={"title": "AI Review Failed", "summary": msg}, + ) + return + + # Build review payload + pr_url: str = pull_request.html_url + + # Convert provider configs from YAML format to API format + providers_config: list[dict[str, str]] = config.get("providers", []) + providers_payload: list[dict[str, str]] = [ + {"ai_provider": p["ai-provider"], "ai_model": p["ai-model"]} for p in providers_config + ] + + payload: dict[str, Any] = { + "pr_url": pr_url, + "providers": providers_payload, + "github_token": github_webhook.token, + } + + if "max-rounds" in config: + payload["max_rounds"] = config["max-rounds"] + + if "timeout-minutes" in config: + payload["timeout_minutes"] = config["timeout-minutes"] + + # Call review endpoint + try: + github_webhook.logger.info(f"{log_prefix} Calling AI Reviewer for {pr_url}") + # Long timeout: AI review with peer consensus can take up to timeout_minutes * max_rounds + timeout_minutes = config.get("timeout-minutes", 30) + max_rounds = config.get("max-rounds", 3) + # Worst-case: each round takes timeout_minutes (sequential AI calls) + 60s buffer + request_timeout = float(timeout_minutes * max_rounds * 60 + 60) + + response = await client.post("/review", json=payload, timeout=request_timeout) + response.raise_for_status() + + result = response.json() + review_posted = result.get("review_posted", False) + comments_count = len(result.get("comments", [])) + summary = result.get("summary", "no summary") + + github_webhook.logger.info( + f"{log_prefix} AI Reviewer complete: {comments_count} comment(s), " + f"review_posted={review_posted}, summary={summary}" + ) + + await check_run_handler.set_check_success( + name=AI_REVIEW_STR, + output={ + "title": "AI Review Complete", + "summary": f"{comments_count} comment(s) posted" if comments_count else "No issues found", + }, + ) + except httpx.HTTPError as e: + err_detail = "" + if isinstance(e, httpx.HTTPStatusError): + err_detail = f": {e.response.text[:2000]}" + error_msg = f"AI Reviewer request failed{err_detail}" + github_webhook.logger.error(f"{log_prefix} {error_msg}") + await check_run_handler.set_check_failure( + name=AI_REVIEW_STR, + output={"title": "AI Review Failed", "summary": error_msg}, + ) + except ValueError: + github_webhook.logger.error(f"{log_prefix} AI Reviewer returned invalid JSON response") + await check_run_handler.set_check_failure( + name=AI_REVIEW_STR, + output={"title": "AI Review Failed", "summary": "Invalid JSON response from AI Reviewer"}, + ) + except asyncio.CancelledError: + if check_in_progress: + try: + await check_run_handler.set_check_failure( + name=AI_REVIEW_STR, + output={"title": "AI Review Cancelled", "summary": "Review was cancelled"}, + ) + except Exception: + github_webhook.logger.exception(f"{log_prefix} Failed to set check run failure on cancellation") + raise + except Exception: + github_webhook.logger.exception(f"{log_prefix} AI Reviewer call failed unexpectedly") + try: + await check_run_handler.set_check_failure( + name=AI_REVIEW_STR, + output={"title": "AI Review Failed", "summary": "Unexpected error"}, + ) + except Exception: + github_webhook.logger.exception(f"{log_prefix} Failed to set check run failure") diff --git a/webhook_server/libs/github_api.py b/webhook_server/libs/github_api.py index 2aa76044..ccf1a34e 100644 --- a/webhook_server/libs/github_api.py +++ b/webhook_server/libs/github_api.py @@ -808,6 +808,9 @@ def _repo_data_from_config(self, repository_config: dict[str, Any]) -> None: self.ai_features: dict[str, Any] | None = self.config.get_value( value="ai-features", return_on_none=None, extra_dict=repository_config ) + self.ai_review_config: dict[str, Any] | None = self.config.get_value( + value="ai-review", return_on_none=None, extra_dict=repository_config + ) _auto_merge_prs = self.config.get_value( value="set-auto-merge-prs", return_on_none=[], extra_dict=repository_config ) diff --git a/webhook_server/libs/handlers/pull_request_handler.py b/webhook_server/libs/handlers/pull_request_handler.py index fd844642..3d557781 100644 --- a/webhook_server/libs/handlers/pull_request_handler.py +++ b/webhook_server/libs/handlers/pull_request_handler.py @@ -12,6 +12,7 @@ from github.Repository import Repository from timeout_sampler import TimeoutExpiredError, TimeoutSampler +from webhook_server.libs.ai_review import call_ai_reviewer from webhook_server.libs.handlers.check_run_handler import CheckRunHandler, CheckRunOutput from webhook_server.libs.handlers.labels_handler import LabelsHandler from webhook_server.libs.handlers.owners_files_handler import OwnersFileHandler @@ -19,6 +20,7 @@ from webhook_server.libs.test_oracle import call_test_oracle from webhook_server.utils.constants import ( AI_RESOLVED_CONFLICTS_LABEL, + AI_REVIEW_STR, APPROVED_BY_LABEL_PREFIX, AUTOMERGE_LABEL_STR, BRANCH_LABEL_PREFIX, @@ -275,6 +277,18 @@ async def process_pull_request_webhook_data(self, pull_request: PullRequest) -> _background_tasks.add(task) task.add_done_callback(_background_tasks.discard) + if hook_action == "opened" and self.github_webhook.ai_review_config: + ai_review_task = asyncio.create_task( + call_ai_reviewer( + github_webhook=self.github_webhook, + pull_request=pull_request, + check_run_handler=self.check_run_handler, + trigger="pr-opened", + ) + ) + _background_tasks.add(ai_review_task) + ai_review_task.add_done_callback(_background_tasks.discard) + if self.ctx: self.ctx.complete_step("pr_handler", action=hook_action) return @@ -315,6 +329,18 @@ async def process_pull_request_webhook_data(self, pull_request: PullRequest) -> _background_tasks.add(task) task.add_done_callback(_background_tasks.discard) + if not clean_rebase and self.github_webhook.ai_review_config: + ai_review_task = asyncio.create_task( + call_ai_reviewer( + github_webhook=self.github_webhook, + pull_request=pull_request, + check_run_handler=self.check_run_handler, + trigger="pr-synchronized", + ) + ) + _background_tasks.add(ai_review_task) + ai_review_task.add_done_callback(_background_tasks.discard) + if self.ctx: self.ctx.complete_step("pr_handler", action=hook_action) return @@ -1042,6 +1068,9 @@ async def process_opened_or_synchronize_pull_request( if self.github_webhook.build_and_push_container: setup_tasks.append(self.check_run_handler.set_check_queued(name=BUILD_CONTAINER_STR)) + if self.github_webhook.ai_review_config: + setup_tasks.append(self.check_run_handler.set_check_queued(name=AI_REVIEW_STR)) + if is_clean_rebase: # label_names is guaranteed non-None when is_clean_rebase=True (caller always provides it) setup_tasks.append( diff --git a/webhook_server/tests/test_ai_review.py b/webhook_server/tests/test_ai_review.py new file mode 100644 index 00000000..bd7086ea --- /dev/null +++ b/webhook_server/tests/test_ai_review.py @@ -0,0 +1,327 @@ +"""Tests for webhook_server.libs.ai_review module.""" + +import asyncio +from unittest.mock import AsyncMock, MagicMock, Mock, patch + +import httpx +import pytest + +from webhook_server.libs.ai_review import DEFAULT_TRIGGERS, call_ai_reviewer +from webhook_server.utils.constants import AI_REVIEW_STR + + +@pytest.fixture +def mock_github_webhook() -> Mock: + """Create a mock GithubWebhook for AI review tests.""" + mock = Mock() + mock.ai_review_config = { + "server-url": "http://localhost:8001", + "providers": [ + {"ai-provider": "claude", "ai-model": "sonnet"}, + ], + "triggers": ["pr-opened", "pr-synchronized"], + } + mock.logger = MagicMock() + mock.log_prefix = "[TEST]" + mock.token = "ghp_test_token" # pragma: allowlist secret + return mock + + +@pytest.fixture +def mock_pull_request() -> Mock: + """Create a mock PullRequest.""" + mock = Mock() + mock.html_url = "https://github.com/test-org/test-repo/pull/42" + return mock + + +@pytest.fixture +def mock_check_run_handler() -> AsyncMock: + """Create a mock CheckRunHandler.""" + mock = AsyncMock() + mock.set_check_in_progress = AsyncMock() + mock.set_check_success = AsyncMock() + mock.set_check_failure = AsyncMock() + return mock + + +class TestCallAiReviewer: + @pytest.mark.asyncio + async def test_returns_early_when_config_is_none( + self, mock_pull_request: Mock, mock_check_run_handler: AsyncMock + ) -> None: + mock_webhook = Mock() + mock_webhook.ai_review_config = None + await call_ai_reviewer(mock_webhook, mock_pull_request, mock_check_run_handler) + mock_check_run_handler.set_check_in_progress.assert_not_called() + + @pytest.mark.asyncio + async def test_returns_early_when_trigger_not_configured( + self, mock_github_webhook: Mock, mock_pull_request: Mock, mock_check_run_handler: AsyncMock + ) -> None: + mock_github_webhook.ai_review_config["triggers"] = ["pr-opened"] + await call_ai_reviewer( + mock_github_webhook, mock_pull_request, mock_check_run_handler, trigger="pr-synchronized" + ) + mock_github_webhook.logger.debug.assert_called_once() + mock_check_run_handler.set_check_in_progress.assert_not_called() + + @pytest.mark.asyncio + async def test_command_trigger_always_runs( + self, mock_github_webhook: Mock, mock_pull_request: Mock, mock_check_run_handler: AsyncMock + ) -> None: + """When trigger is None (command-triggered), always run regardless of configured triggers.""" + mock_response = Mock() + mock_response.raise_for_status = Mock() + mock_response.json.return_value = {"status": "healthy"} + + review_response = Mock() + review_response.raise_for_status = Mock() + review_response.json.return_value = { + "review_posted": True, + "comments": [{"path": "a.py", "line": 1, "body": "issue"}], + "summary": "1 issue found", + } + + with patch("webhook_server.libs.ai_review.httpx.AsyncClient") as mock_client_cls: + mock_client = AsyncMock() + mock_client.__aenter__ = AsyncMock(return_value=mock_client) + mock_client.__aexit__ = AsyncMock(return_value=None) + mock_client.get = AsyncMock(return_value=mock_response) + mock_client.post = AsyncMock(return_value=review_response) + mock_client_cls.return_value = mock_client + + with patch("asyncio.to_thread", side_effect=lambda f, *a, **kw: f(*a, **kw) if callable(f) else f): + await call_ai_reviewer(mock_github_webhook, mock_pull_request, mock_check_run_handler, trigger=None) + + mock_client.post.assert_called_once() + mock_check_run_handler.set_check_in_progress.assert_called_once() + mock_check_run_handler.set_check_success.assert_called_once() + + @pytest.mark.asyncio + async def test_health_check_failure_sets_check_failure( + self, mock_github_webhook: Mock, mock_pull_request: Mock, mock_check_run_handler: AsyncMock + ) -> None: + with patch("webhook_server.libs.ai_review.httpx.AsyncClient") as mock_client_cls: + mock_client = AsyncMock() + mock_client.__aenter__ = AsyncMock(return_value=mock_client) + mock_client.__aexit__ = AsyncMock(return_value=None) + mock_client.get = AsyncMock(side_effect=httpx.ConnectError("Connection refused")) + mock_client_cls.return_value = mock_client + + await call_ai_reviewer(mock_github_webhook, mock_pull_request, mock_check_run_handler, trigger="pr-opened") + + mock_check_run_handler.set_check_in_progress.assert_called_once() + mock_check_run_handler.set_check_failure.assert_called_once() + call_args = mock_check_run_handler.set_check_failure.call_args + assert call_args.kwargs["name"] == AI_REVIEW_STR + assert "not responding" in call_args.kwargs["output"]["summary"] + + @pytest.mark.asyncio + async def test_successful_review( + self, mock_github_webhook: Mock, mock_pull_request: Mock, mock_check_run_handler: AsyncMock + ) -> None: + health_response = Mock() + health_response.raise_for_status = Mock() + + review_response = Mock() + review_response.raise_for_status = Mock() + review_response.json.return_value = { + "review_posted": True, + "comments": [{"path": "a.py", "line": 1, "body": "bug"}], + "summary": "1 issue", + } + + with patch("webhook_server.libs.ai_review.httpx.AsyncClient") as mock_client_cls: + mock_client = AsyncMock() + mock_client.__aenter__ = AsyncMock(return_value=mock_client) + mock_client.__aexit__ = AsyncMock(return_value=None) + mock_client.get = AsyncMock(return_value=health_response) + mock_client.post = AsyncMock(return_value=review_response) + mock_client_cls.return_value = mock_client + + with patch("asyncio.to_thread", side_effect=lambda f, *a, **kw: f(*a, **kw) if callable(f) else f): + await call_ai_reviewer( + mock_github_webhook, mock_pull_request, mock_check_run_handler, trigger="pr-opened" + ) + + mock_check_run_handler.set_check_success.assert_called_once() + call_args = mock_check_run_handler.set_check_success.call_args + assert "1 comment(s) posted" in call_args.kwargs["output"]["summary"] + + @pytest.mark.asyncio + async def test_successful_review_no_issues( + self, mock_github_webhook: Mock, mock_pull_request: Mock, mock_check_run_handler: AsyncMock + ) -> None: + health_response = Mock() + health_response.raise_for_status = Mock() + + review_response = Mock() + review_response.raise_for_status = Mock() + review_response.json.return_value = { + "review_posted": False, + "comments": [], + "summary": "No issues found", + } + + with patch("webhook_server.libs.ai_review.httpx.AsyncClient") as mock_client_cls: + mock_client = AsyncMock() + mock_client.__aenter__ = AsyncMock(return_value=mock_client) + mock_client.__aexit__ = AsyncMock(return_value=None) + mock_client.get = AsyncMock(return_value=health_response) + mock_client.post = AsyncMock(return_value=review_response) + mock_client_cls.return_value = mock_client + + with patch("asyncio.to_thread", side_effect=lambda f, *a, **kw: f(*a, **kw) if callable(f) else f): + await call_ai_reviewer( + mock_github_webhook, mock_pull_request, mock_check_run_handler, trigger="pr-opened" + ) + + mock_check_run_handler.set_check_success.assert_called_once() + call_args = mock_check_run_handler.set_check_success.call_args + assert "No issues found" in call_args.kwargs["output"]["summary"] + + @pytest.mark.asyncio + async def test_review_http_error_sets_check_failure( + self, mock_github_webhook: Mock, mock_pull_request: Mock, mock_check_run_handler: AsyncMock + ) -> None: + health_response = Mock() + health_response.raise_for_status = Mock() + + error_response = Mock() + error_response.status_code = 502 + error_response.text = "Bad Gateway" + review_error = httpx.HTTPStatusError("error", request=Mock(), response=error_response) + + with patch("webhook_server.libs.ai_review.httpx.AsyncClient") as mock_client_cls: + mock_client = AsyncMock() + mock_client.__aenter__ = AsyncMock(return_value=mock_client) + mock_client.__aexit__ = AsyncMock(return_value=None) + mock_client.get = AsyncMock(return_value=health_response) + mock_client.post = AsyncMock(side_effect=review_error) + mock_client_cls.return_value = mock_client + + with patch("asyncio.to_thread", side_effect=lambda f, *a, **kw: f(*a, **kw) if callable(f) else f): + await call_ai_reviewer( + mock_github_webhook, mock_pull_request, mock_check_run_handler, trigger="pr-opened" + ) + + mock_check_run_handler.set_check_failure.assert_called_once() + call_args = mock_check_run_handler.set_check_failure.call_args + assert "failed" in call_args.kwargs["output"]["summary"].lower() + + @pytest.mark.asyncio + async def test_review_invalid_json_sets_check_failure( + self, mock_github_webhook: Mock, mock_pull_request: Mock, mock_check_run_handler: AsyncMock + ) -> None: + health_response = Mock() + health_response.raise_for_status = Mock() + + review_response = Mock() + review_response.raise_for_status = Mock() + review_response.json.side_effect = ValueError("Invalid JSON") + + with patch("webhook_server.libs.ai_review.httpx.AsyncClient") as mock_client_cls: + mock_client = AsyncMock() + mock_client.__aenter__ = AsyncMock(return_value=mock_client) + mock_client.__aexit__ = AsyncMock(return_value=None) + mock_client.get = AsyncMock(return_value=health_response) + mock_client.post = AsyncMock(return_value=review_response) + mock_client_cls.return_value = mock_client + + with patch("asyncio.to_thread", side_effect=lambda f, *a, **kw: f(*a, **kw) if callable(f) else f): + await call_ai_reviewer( + mock_github_webhook, mock_pull_request, mock_check_run_handler, trigger="pr-opened" + ) + + mock_check_run_handler.set_check_failure.assert_called_once() + call_args = mock_check_run_handler.set_check_failure.call_args + assert "Invalid JSON" in call_args.kwargs["output"]["summary"] + + @pytest.mark.asyncio + async def test_unexpected_exception_sets_check_failure( + self, mock_github_webhook: Mock, mock_pull_request: Mock, mock_check_run_handler: AsyncMock + ) -> None: + with patch("webhook_server.libs.ai_review.httpx.AsyncClient", side_effect=RuntimeError("boom")): + await call_ai_reviewer(mock_github_webhook, mock_pull_request, mock_check_run_handler, trigger="pr-opened") + mock_github_webhook.logger.exception.assert_called_once() + mock_check_run_handler.set_check_failure.assert_called_once() + + @pytest.mark.asyncio + async def test_cancelled_error_reraised( + self, mock_github_webhook: Mock, mock_pull_request: Mock, mock_check_run_handler: AsyncMock + ) -> None: + with ( + patch("webhook_server.libs.ai_review.httpx.AsyncClient", side_effect=asyncio.CancelledError), + pytest.raises(asyncio.CancelledError), + ): + await call_ai_reviewer(mock_github_webhook, mock_pull_request, mock_check_run_handler, trigger="pr-opened") + + @pytest.mark.asyncio + async def test_cancelled_error_sets_check_failure_when_in_progress( + self, mock_github_webhook: Mock, mock_pull_request: Mock, mock_check_run_handler: AsyncMock + ) -> None: + """CancelledError after check is in_progress should mark it as failed before re-raising.""" + with patch("webhook_server.libs.ai_review.httpx.AsyncClient") as mock_client_cls: + mock_client = AsyncMock() + mock_client.__aenter__ = AsyncMock(return_value=mock_client) + mock_client.__aexit__ = AsyncMock(return_value=None) + # Health check succeeds, but review call raises CancelledError + mock_client.get = AsyncMock(return_value=Mock(raise_for_status=Mock())) + mock_client.post = AsyncMock(side_effect=asyncio.CancelledError) + mock_client_cls.return_value = mock_client + + with patch("asyncio.to_thread", side_effect=lambda f, *a, **kw: f(*a, **kw) if callable(f) else f): + with pytest.raises(asyncio.CancelledError): + await call_ai_reviewer( + mock_github_webhook, mock_pull_request, mock_check_run_handler, trigger="pr-opened" + ) + + # Verify check was marked in_progress then failed on cancellation + mock_check_run_handler.set_check_in_progress.assert_called_once() + mock_check_run_handler.set_check_failure.assert_called_once() + call_args = mock_check_run_handler.set_check_failure.call_args + assert call_args.kwargs["output"]["title"] == "AI Review Cancelled" + assert call_args.kwargs["output"]["summary"] == "Review was cancelled" + + @pytest.mark.asyncio + async def test_provider_config_conversion( + self, mock_github_webhook: Mock, mock_pull_request: Mock, mock_check_run_handler: AsyncMock + ) -> None: + """Verify YAML-format providers are converted to API format.""" + mock_github_webhook.ai_review_config["providers"] = [ + {"ai-provider": "claude", "ai-model": "sonnet"}, + {"ai-provider": "gemini", "ai-model": "pro"}, + ] + + health_response = Mock() + health_response.raise_for_status = Mock() + + review_response = Mock() + review_response.raise_for_status = Mock() + review_response.json.return_value = {"review_posted": False, "comments": [], "summary": "ok"} + + with patch("webhook_server.libs.ai_review.httpx.AsyncClient") as mock_client_cls: + mock_client = AsyncMock() + mock_client.__aenter__ = AsyncMock(return_value=mock_client) + mock_client.__aexit__ = AsyncMock(return_value=None) + mock_client.get = AsyncMock(return_value=health_response) + mock_client.post = AsyncMock(return_value=review_response) + mock_client_cls.return_value = mock_client + + with patch("asyncio.to_thread", side_effect=lambda f, *a, **kw: f(*a, **kw) if callable(f) else f): + await call_ai_reviewer( + mock_github_webhook, mock_pull_request, mock_check_run_handler, trigger="pr-opened" + ) + + call_args = mock_client.post.call_args + payload = call_args.kwargs.get("json") or call_args[1].get("json") + assert payload["providers"] == [ + {"ai_provider": "claude", "ai_model": "sonnet"}, + {"ai_provider": "gemini", "ai_model": "pro"}, + ] + + +class TestDefaultTriggers: + def test_default_triggers(self) -> None: + assert DEFAULT_TRIGGERS == ["pr-opened", "pr-synchronized"] diff --git a/webhook_server/tests/test_clean_rebase_detection.py b/webhook_server/tests/test_clean_rebase_detection.py index 51cb9d03..520fb45a 100644 --- a/webhook_server/tests/test_clean_rebase_detection.py +++ b/webhook_server/tests/test_clean_rebase_detection.py @@ -73,6 +73,7 @@ def mock_github_webhook() -> Mock: mock_webhook.enabled_labels = None mock_webhook.custom_check_runs = [] mock_webhook.ai_features = None + mock_webhook.ai_review_config = None mock_webhook.required_conversation_resolution = False mock_webhook.config = Mock() mock_webhook.config.get_value = Mock(return_value=None) diff --git a/webhook_server/tests/test_pull_request_handler.py b/webhook_server/tests/test_pull_request_handler.py index 0f2263d0..cadf5d1e 100644 --- a/webhook_server/tests/test_pull_request_handler.py +++ b/webhook_server/tests/test_pull_request_handler.py @@ -93,6 +93,7 @@ def mock_github_webhook(self) -> Mock: mock_webhook.enabled_labels = None # Default: all labels enabled mock_webhook.custom_check_runs = [] mock_webhook.ai_features = None + mock_webhook.ai_review_config = None mock_webhook.required_conversation_resolution = False mock_webhook.config = Mock() mock_webhook.config.get_value = Mock(return_value=None) @@ -476,6 +477,7 @@ def test_prepare_welcome_comment_issue_creation_disabled(self, pull_request_hand def test_prepare_ai_features_section_no_features(self, pull_request_handler: PullRequestHandler) -> None: """Test AI features section is empty when no features are configured.""" pull_request_handler.github_webhook.ai_features = None + pull_request_handler.github_webhook.ai_review_config = None result = pull_request_handler._prepare_ai_features_welcome_section assert result == "" @@ -3285,3 +3287,134 @@ async def test_process_reopened_action_does_not_call_test_oracle( await pull_request_handler.process_pull_request_webhook_data(mock_pull_request) mock_test_oracle.assert_not_called() mock_create_task.assert_not_called() + + @pytest.mark.asyncio + async def test_ai_review_triggered_on_opened( + self, pull_request_handler: PullRequestHandler, mock_pull_request: Mock + ) -> None: + """Test that call_ai_reviewer is fired as a background task with trigger='pr-opened' when a PR is opened.""" + pull_request_handler.hook_data["action"] = "opened" + pull_request_handler.github_webhook.ai_review_config = { + "server-url": "http://localhost:8001", + "providers": [{"ai-provider": "claude", "ai-model": "sonnet"}], + } + + with ( + patch.object(pull_request_handler, "create_issue_for_new_pull_request"), + patch.object(pull_request_handler, "set_wip_label_based_on_title"), + patch.object(pull_request_handler, "process_opened_or_synchronize_pull_request"), + patch.object(pull_request_handler, "set_pull_request_automerge"), + patch( + "webhook_server.libs.handlers.pull_request_handler.call_test_oracle", + new_callable=AsyncMock, + ), + patch( + "webhook_server.libs.handlers.pull_request_handler.call_ai_reviewer", + new_callable=AsyncMock, + ) as mock_ai_reviewer, + patch("asyncio.create_task") as mock_create_task, + ): + await pull_request_handler.process_pull_request_webhook_data(mock_pull_request) + mock_ai_reviewer.assert_called_once_with( + github_webhook=pull_request_handler.github_webhook, + pull_request=mock_pull_request, + check_run_handler=pull_request_handler.check_run_handler, + trigger="pr-opened", + ) + assert mock_create_task.call_count == 2 + + @pytest.mark.asyncio + async def test_ai_review_not_triggered_when_config_none( + self, pull_request_handler: PullRequestHandler, mock_pull_request: Mock + ) -> None: + """Test that call_ai_reviewer is NOT called when ai_review_config is None.""" + pull_request_handler.hook_data["action"] = "opened" + pull_request_handler.github_webhook.ai_review_config = None + + with ( + patch.object(pull_request_handler, "create_issue_for_new_pull_request"), + patch.object(pull_request_handler, "set_wip_label_based_on_title"), + patch.object(pull_request_handler, "process_opened_or_synchronize_pull_request"), + patch.object(pull_request_handler, "set_pull_request_automerge"), + patch( + "webhook_server.libs.handlers.pull_request_handler.call_test_oracle", + new_callable=AsyncMock, + ), + patch( + "webhook_server.libs.handlers.pull_request_handler.call_ai_reviewer", + new_callable=AsyncMock, + ) as mock_ai_reviewer, + patch("asyncio.create_task") as mock_create_task, + ): + await pull_request_handler.process_pull_request_webhook_data(mock_pull_request) + mock_ai_reviewer.assert_not_called() + # Only test_oracle should create a task, not ai_reviewer + mock_create_task.assert_called_once() + + @pytest.mark.asyncio + async def test_ai_review_triggered_on_synchronize_non_clean( + self, pull_request_handler: PullRequestHandler, mock_pull_request: Mock + ) -> None: + """Test that call_ai_reviewer is fired with trigger='pr-synchronized' on non-clean-rebase synchronize.""" + pull_request_handler.hook_data["action"] = "synchronize" + pull_request_handler.hook_data["before"] = "aaa1111111111111111111111111111111111111" + pull_request_handler.hook_data["after"] = "bbb2222222222222222222222222222222222222" + pull_request_handler.github_webhook.ai_review_config = { + "server-url": "http://localhost:8001", + "providers": [{"ai-provider": "claude", "ai-model": "sonnet"}], + } + + with ( + patch.object(pull_request_handler, "_is_clean_rebase", new_callable=AsyncMock, return_value=False), + patch.object(pull_request_handler, "process_opened_or_synchronize_pull_request"), + patch.object(pull_request_handler, "remove_labels_when_pull_request_sync"), + patch( + "webhook_server.libs.handlers.pull_request_handler.call_test_oracle", + new_callable=AsyncMock, + ), + patch( + "webhook_server.libs.handlers.pull_request_handler.call_ai_reviewer", + new_callable=AsyncMock, + ) as mock_ai_reviewer, + patch("asyncio.create_task") as mock_create_task, + ): + await pull_request_handler.process_pull_request_webhook_data(mock_pull_request) + mock_ai_reviewer.assert_called_once_with( + github_webhook=pull_request_handler.github_webhook, + pull_request=mock_pull_request, + check_run_handler=pull_request_handler.check_run_handler, + trigger="pr-synchronized", + ) + assert mock_create_task.call_count == 2 + + @pytest.mark.asyncio + async def test_ai_review_not_triggered_on_clean_rebase( + self, pull_request_handler: PullRequestHandler, mock_pull_request: Mock + ) -> None: + """Test that call_ai_reviewer is NOT called when synchronize is a clean rebase.""" + pull_request_handler.hook_data["action"] = "synchronize" + pull_request_handler.hook_data["before"] = "aaa1111111111111111111111111111111111111" + pull_request_handler.hook_data["after"] = "bbb2222222222222222222222222222222222222" + pull_request_handler.github_webhook.ai_review_config = { + "server-url": "http://localhost:8001", + "providers": [{"ai-provider": "claude", "ai-model": "sonnet"}], + } + + with ( + patch.object(pull_request_handler, "_is_clean_rebase", new_callable=AsyncMock, return_value=True), + patch.object(pull_request_handler, "_post_clean_rebase_comment", new_callable=AsyncMock), + patch.object(pull_request_handler, "process_opened_or_synchronize_pull_request"), + patch( + "webhook_server.libs.handlers.pull_request_handler.call_test_oracle", + new_callable=AsyncMock, + ), + patch( + "webhook_server.libs.handlers.pull_request_handler.call_ai_reviewer", + new_callable=AsyncMock, + ) as mock_ai_reviewer, + patch("asyncio.create_task") as mock_create_task, + ): + await pull_request_handler.process_pull_request_webhook_data(mock_pull_request) + mock_ai_reviewer.assert_not_called() + # Only test_oracle should create a task, not ai_reviewer + mock_create_task.assert_called_once() diff --git a/webhook_server/tests/test_runner_handler.py b/webhook_server/tests/test_runner_handler.py index 203ede24..09736785 100644 --- a/webhook_server/tests/test_runner_handler.py +++ b/webhook_server/tests/test_runner_handler.py @@ -57,6 +57,7 @@ def mock_github_webhook(self) -> Mock: mock_webhook.ctx = None mock_webhook.custom_check_runs = [] mock_webhook.ai_features = None + mock_webhook.ai_review_config = None mock_webhook.config = Mock() mock_webhook.config.get_value = Mock(return_value=None) return mock_webhook @@ -754,6 +755,7 @@ async def test_conventional_title_failure_without_ai_features( runner_handler.github_webhook.conventional_title = "feat,fix" mock_pull_request.title = "bad title" runner_handler.github_webhook.ai_features = None + runner_handler.github_webhook.ai_review_config = None with patch.object( runner_handler.check_run_handler, "is_check_run_in_progress", new=AsyncMock(return_value=False) @@ -1811,6 +1813,7 @@ async def test_cherry_pick_ai_not_configured_fallback( ) -> None: """Cherry-pick conflicts + AI not configured — manual fallback, call_ai_cli not called.""" runner_handler.github_webhook.ai_features = None + runner_handler.github_webhook.ai_review_config = None async def run_command_side_effect(command: str, **kwargs: Any) -> tuple[bool, str, str]: if "cherry-pick" in command and "--continue" not in command: diff --git a/webhook_server/utils/constants.py b/webhook_server/utils/constants.py index 8c2911e8..aaba22c2 100644 --- a/webhook_server/utils/constants.py +++ b/webhook_server/utils/constants.py @@ -11,6 +11,7 @@ IN_PROGRESS_STR: str = "in_progress" QUEUED_STR: str = "queued" ADD_STR: str = "add" +AI_REVIEW_STR: str = "AI-Review" DELETE_STR: str = "delete" CAN_BE_MERGED_STR: str = "can-be-merged" BUILD_CONTAINER_STR: str = "build-container"