From cb59de278f03776c818bc7275a8136fcdb39d5cc Mon Sep 17 00:00:00 2001 From: Meni Yakove Date: Sat, 28 Feb 2026 12:25:45 +0200 Subject: [PATCH 01/14] feat: add test-oracle constant and config schema Co-Authored-By: Claude Opus 4.6 (1M context) --- webhook_server/config/schema.yaml | 88 +++++++++++++++++++++++++++++++ webhook_server/utils/constants.py | 1 + 2 files changed, 89 insertions(+) diff --git a/webhook_server/config/schema.yaml b/webhook_server/config/schema.yaml index e3825cc2..3b2330fe 100644 --- a/webhook_server/config/schema.yaml +++ b/webhook_server/config/schema.yaml @@ -92,6 +92,50 @@ properties: - Not set (default): commands blocked on draft PRs - Empty list []: all commands allowed on draft PRs - List with commands: only specified commands allowed (e.g., ["build-and-push-container", "retest"]) + test-oracle: + type: object + description: | + PR Test Oracle integration. Sends PR data to an external AI service + that analyzes diffs and recommends which tests to run. + The /test-oracle comment command always works when configured (no trigger needed). + properties: + server-url: + type: string + format: uri + description: URL of the pr-test-oracle server (e.g., http://localhost:8000) + ai-provider: + type: string + enum: + - claude + - gemini + - cursor + description: AI provider for test analysis + ai-model: + type: string + description: AI model identifier (e.g., sonnet, gemini-2.5-pro) + test-patterns: + type: array + items: + type: string + description: Glob patterns for test files (e.g., ["tests/**/*.py"]). Optional - oracle has defaults. + triggers: + type: array + items: + type: string + enum: + - approved + - pr-opened + - pr-synchronized + description: | + When to automatically run test oracle analysis. Default: [approved]. + - approved: Run when a PR review is approved + - pr-opened: Run when a new PR is opened + - pr-synchronized: Run when new commits are pushed to a PR + required: + - server-url + - ai-provider + - ai-model + additionalProperties: false labels: type: object description: Configure which labels are enabled and their colors @@ -402,6 +446,50 @@ properties: required: - threshold additionalProperties: false + test-oracle: + type: object + description: | + PR Test Oracle integration. Sends PR data to an external AI service + that analyzes diffs and recommends which tests to run. + The /test-oracle comment command always works when configured (no trigger needed). + properties: + server-url: + type: string + format: uri + description: URL of the pr-test-oracle server (e.g., http://localhost:8000) + ai-provider: + type: string + enum: + - claude + - gemini + - cursor + description: AI provider for test analysis + ai-model: + type: string + description: AI model identifier (e.g., sonnet, gemini-2.5-pro) + test-patterns: + type: array + items: + type: string + description: Glob patterns for test files (e.g., ["tests/**/*.py"]). Optional - oracle has defaults. + triggers: + type: array + items: + type: string + enum: + - approved + - pr-opened + - pr-synchronized + description: | + When to automatically run test oracle analysis. Default: [approved]. + - approved: Run when a PR review is approved + - pr-opened: Run when a new PR is opened + - pr-synchronized: Run when new commits are pushed to a PR + required: + - server-url + - ai-provider + - ai-model + additionalProperties: false custom-check-runs: type: array description: | diff --git a/webhook_server/utils/constants.py b/webhook_server/utils/constants.py index e526da55..289552b1 100644 --- a/webhook_server/utils/constants.py +++ b/webhook_server/utils/constants.py @@ -41,6 +41,7 @@ COMMAND_ADD_ALLOWED_USER_STR: str = "add-allowed-user" COMMAND_AUTOMERGE_STR: str = "automerge" COMMAND_REGENERATE_WELCOME_STR: str = "regenerate-welcome" +COMMAND_TEST_ORACLE_STR: str = "test-oracle" AUTOMERGE_LABEL_STR: str = "automerge" ROOT_APPROVERS_KEY: str = "root-approvers" From 810a9958c810fe19e817d39108253a0ecf242313 Mon Sep 17 00:00:00 2001 From: Meni Yakove Date: Sat, 28 Feb 2026 12:30:05 +0200 Subject: [PATCH 02/14] feat: add test_oracle module with call_test_oracle helper Co-Authored-By: Claude Opus 4.6 (1M context) --- webhook_server/libs/test_oracle.py | 89 ++++++++ webhook_server/tests/test_test_oracle.py | 260 +++++++++++++++++++++++ 2 files changed, 349 insertions(+) create mode 100644 webhook_server/libs/test_oracle.py create mode 100644 webhook_server/tests/test_test_oracle.py diff --git a/webhook_server/libs/test_oracle.py b/webhook_server/libs/test_oracle.py new file mode 100644 index 00000000..6c083cd1 --- /dev/null +++ b/webhook_server/libs/test_oracle.py @@ -0,0 +1,89 @@ +from __future__ import annotations + +import asyncio +from typing import TYPE_CHECKING, Any + +import httpx + +if TYPE_CHECKING: + from github.PullRequest import PullRequest + + from webhook_server.libs.github_api import GithubWebhook + +DEFAULT_TRIGGERS: list[str] = ["approved"] + + +async def call_test_oracle( + github_webhook: GithubWebhook, + pull_request: PullRequest, + trigger: str | None = None, +) -> None: + """Call the pr-test-oracle service to analyze a PR for test recommendations. + + Args: + github_webhook: The GithubWebhook instance with config and token. + pull_request: The PyGithub PullRequest object. + trigger: The event trigger (e.g., "approved", "pr-opened"). None means + command-triggered (always runs if configured). + """ + config: dict[str, Any] | None = github_webhook.config.get_value("test-oracle") + if not config: + return + + 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} Test oracle trigger '{trigger}' not in configured triggers {triggers}" + ) + return + + server_url: str = config["server-url"] + log_prefix: str = github_webhook.log_prefix + + async with httpx.AsyncClient(base_url=server_url) as client: + # Health check + try: + health_response = await client.get("/health", timeout=5.0) + if health_response.status_code != 200: + msg = ( + f"Test Oracle server at {server_url} is not responding" + f" (status {health_response.status_code}), skipping test analysis" + ) + github_webhook.logger.warning(f"{log_prefix} {msg}") + await asyncio.to_thread(pull_request.create_issue_comment, msg) + return + except httpx.HTTPError: + msg = f"Test Oracle server at {server_url} is not responding, skipping test analysis" + github_webhook.logger.warning(f"{log_prefix} {msg}") + await asyncio.to_thread(pull_request.create_issue_comment, msg) + return + + # Build analyze payload + payload: dict[str, Any] = { + "pr_url": pull_request.html_url, + "ai_provider": config["ai-provider"], + "ai_model": config["ai-model"], + "github_token": github_webhook.token, + } + + if "test-patterns" in config: + payload["test_patterns"] = config["test-patterns"] + + # Call analyze + try: + github_webhook.logger.info(f"{log_prefix} Calling Test Oracle for {pull_request.html_url}") + response = await client.post("/analyze", json=payload, timeout=300.0) + + if response.status_code != 200: + github_webhook.logger.error( + f"{log_prefix} Test Oracle analyze failed with status {response.status_code}: {response.text}" + ) + return + + result = response.json() + github_webhook.logger.info( + f"{log_prefix} Test Oracle analysis complete: {result.get('summary', 'no summary')}" + ) + except httpx.HTTPError: + github_webhook.logger.error(f"{log_prefix} Test Oracle analyze request failed") diff --git a/webhook_server/tests/test_test_oracle.py b/webhook_server/tests/test_test_oracle.py new file mode 100644 index 00000000..c71ce908 --- /dev/null +++ b/webhook_server/tests/test_test_oracle.py @@ -0,0 +1,260 @@ +"""Tests for webhook_server.libs.test_oracle module.""" + +from __future__ import annotations + +from unittest.mock import AsyncMock, Mock, patch + +import httpx +import pytest + +from webhook_server.libs.test_oracle import call_test_oracle +from webhook_server.tests.conftest import TEST_GITHUB_TOKEN + + +class TestCallTestOracle: + """Test suite for call_test_oracle function.""" + + @pytest.fixture + def mock_github_webhook(self) -> Mock: + """Create a mock GithubWebhook with test-oracle config.""" + mock_webhook = Mock() + mock_webhook.logger = Mock() + mock_webhook.log_prefix = "[TEST]" + mock_webhook.token = TEST_GITHUB_TOKEN + mock_webhook.config = Mock() + mock_webhook.config.get_value = Mock( + return_value={ + "server-url": "http://localhost:8000", + "ai-provider": "claude", + "ai-model": "sonnet", + "test-patterns": ["tests/**/*.py"], + "triggers": ["approved"], + } + ) + return mock_webhook + + @pytest.fixture + def mock_pull_request(self) -> Mock: + """Create a mock PullRequest.""" + mock_pr = Mock() + mock_pr.html_url = "https://github.com/test-org/test-repo/pull/42" + return mock_pr + + @pytest.mark.asyncio + async def test_not_configured_skips_silently(self, mock_github_webhook: Mock, mock_pull_request: Mock) -> None: + """Test that call_test_oracle skips silently when not configured.""" + mock_github_webhook.config.get_value = Mock(return_value=None) + + with patch("webhook_server.libs.test_oracle.httpx.AsyncClient") as mock_client_cls: + await call_test_oracle(github_webhook=mock_github_webhook, pull_request=mock_pull_request) + mock_client_cls.assert_not_called() + + @pytest.mark.asyncio + async def test_health_check_failure_posts_comment(self, mock_github_webhook: Mock, mock_pull_request: Mock) -> None: + """Test that health check failure posts a PR comment and returns.""" + with patch("webhook_server.libs.test_oracle.httpx.AsyncClient") as mock_client_cls: + mock_client = AsyncMock() + mock_client_cls.return_value.__aenter__ = AsyncMock(return_value=mock_client) + mock_client_cls.return_value.__aexit__ = AsyncMock(return_value=False) + mock_client.get.side_effect = httpx.ConnectError("Connection refused") + + with patch("asyncio.to_thread", new_callable=AsyncMock) as mock_to_thread: + await call_test_oracle(github_webhook=mock_github_webhook, pull_request=mock_pull_request) + + mock_to_thread.assert_called_once() + call_args = mock_to_thread.call_args + assert call_args[0][0] == mock_pull_request.create_issue_comment + assert "not responding" in call_args[0][1] + + @pytest.mark.asyncio + async def test_health_check_non_200_posts_comment(self, mock_github_webhook: Mock, mock_pull_request: Mock) -> None: + """Test that health check returning non-200 posts a PR comment.""" + with patch("webhook_server.libs.test_oracle.httpx.AsyncClient") as mock_client_cls: + mock_client = AsyncMock() + mock_client_cls.return_value.__aenter__ = AsyncMock(return_value=mock_client) + mock_client_cls.return_value.__aexit__ = AsyncMock(return_value=False) + mock_client.get.return_value = Mock(status_code=503) + + with patch("asyncio.to_thread", new_callable=AsyncMock) as mock_to_thread: + await call_test_oracle(github_webhook=mock_github_webhook, pull_request=mock_pull_request) + + mock_to_thread.assert_called_once() + call_args = mock_to_thread.call_args + assert call_args[0][0] == mock_pull_request.create_issue_comment + assert "not responding" in call_args[0][1] + + @pytest.mark.asyncio + async def test_successful_analyze_call(self, mock_github_webhook: Mock, mock_pull_request: Mock) -> None: + """Test successful health check + analyze call.""" + with patch("webhook_server.libs.test_oracle.httpx.AsyncClient") as mock_client_cls: + mock_client = AsyncMock() + mock_client_cls.return_value.__aenter__ = AsyncMock(return_value=mock_client) + mock_client_cls.return_value.__aexit__ = AsyncMock(return_value=False) + + mock_client.get.return_value = Mock(status_code=200) + + mock_analyze_response = Mock() + mock_analyze_response.status_code = 200 + mock_analyze_response.json.return_value = { + "pr_url": "https://github.com/test-org/test-repo/pull/42", + "summary": "2 test files recommended", + "review_posted": True, + } + mock_client.post.return_value = mock_analyze_response + + await call_test_oracle(github_webhook=mock_github_webhook, pull_request=mock_pull_request) + + mock_client.get.assert_called_once_with("/health", timeout=5.0) + + mock_client.post.assert_called_once() + call_args = mock_client.post.call_args + assert call_args[0][0] == "/analyze" + payload = call_args[1]["json"] + assert payload["pr_url"] == "https://github.com/test-org/test-repo/pull/42" + assert payload["ai_provider"] == "claude" + assert payload["ai_model"] == "sonnet" + assert payload["github_token"] == TEST_GITHUB_TOKEN + assert payload["test_patterns"] == ["tests/**/*.py"] + + @pytest.mark.asyncio + async def test_analyze_error_logs_only(self, mock_github_webhook: Mock, mock_pull_request: Mock) -> None: + """Test that analyze errors are logged but no PR comment is posted.""" + with patch("webhook_server.libs.test_oracle.httpx.AsyncClient") as mock_client_cls: + mock_client = AsyncMock() + mock_client_cls.return_value.__aenter__ = AsyncMock(return_value=mock_client) + mock_client_cls.return_value.__aexit__ = AsyncMock(return_value=False) + + mock_client.get.return_value = Mock(status_code=200) + mock_client.post.return_value = Mock(status_code=500, text="Internal Server Error") + + with patch("asyncio.to_thread", new_callable=AsyncMock) as mock_to_thread: + await call_test_oracle(github_webhook=mock_github_webhook, pull_request=mock_pull_request) + + mock_to_thread.assert_not_called() + mock_github_webhook.logger.error.assert_called() + + @pytest.mark.asyncio + async def test_analyze_network_error_logs_only(self, mock_github_webhook: Mock, mock_pull_request: Mock) -> None: + """Test that network errors during analyze are logged but no PR comment is posted.""" + with patch("webhook_server.libs.test_oracle.httpx.AsyncClient") as mock_client_cls: + mock_client = AsyncMock() + mock_client_cls.return_value.__aenter__ = AsyncMock(return_value=mock_client) + mock_client_cls.return_value.__aexit__ = AsyncMock(return_value=False) + + mock_client.get.return_value = Mock(status_code=200) + mock_client.post.side_effect = httpx.ConnectError("Connection lost") + + with patch("asyncio.to_thread", new_callable=AsyncMock) as mock_to_thread: + await call_test_oracle(github_webhook=mock_github_webhook, pull_request=mock_pull_request) + + mock_to_thread.assert_not_called() + mock_github_webhook.logger.error.assert_called() + + @pytest.mark.asyncio + async def test_no_test_patterns_in_config(self, mock_github_webhook: Mock, mock_pull_request: Mock) -> None: + """Test that test_patterns is omitted from payload when not in config.""" + mock_github_webhook.config.get_value = Mock( + return_value={ + "server-url": "http://localhost:8000", + "ai-provider": "claude", + "ai-model": "sonnet", + } + ) + + with patch("webhook_server.libs.test_oracle.httpx.AsyncClient") as mock_client_cls: + mock_client = AsyncMock() + mock_client_cls.return_value.__aenter__ = AsyncMock(return_value=mock_client) + mock_client_cls.return_value.__aexit__ = AsyncMock(return_value=False) + + mock_client.get.return_value = Mock(status_code=200) + mock_client.post.return_value = Mock( + status_code=200, + json=Mock(return_value={"summary": "ok", "review_posted": True}), + ) + + await call_test_oracle(github_webhook=mock_github_webhook, pull_request=mock_pull_request) + + payload = mock_client.post.call_args[1]["json"] + assert "test_patterns" not in payload + + @pytest.mark.asyncio + async def test_trigger_check_approved(self, mock_github_webhook: Mock, mock_pull_request: Mock) -> None: + """Test that trigger check works correctly for approved trigger.""" + with patch("webhook_server.libs.test_oracle.httpx.AsyncClient") as mock_client_cls: + mock_client = AsyncMock() + mock_client_cls.return_value.__aenter__ = AsyncMock(return_value=mock_client) + mock_client_cls.return_value.__aexit__ = AsyncMock(return_value=False) + mock_client.get.return_value = Mock(status_code=200) + mock_client.post.return_value = Mock( + status_code=200, + json=Mock(return_value={"summary": "ok", "review_posted": True}), + ) + + await call_test_oracle( + github_webhook=mock_github_webhook, + pull_request=mock_pull_request, + trigger="approved", + ) + mock_client.post.assert_called_once() + + @pytest.mark.asyncio + async def test_trigger_not_in_config_skips(self, mock_github_webhook: Mock, mock_pull_request: Mock) -> None: + """Test that call is skipped when trigger is not in config triggers list.""" + with patch("webhook_server.libs.test_oracle.httpx.AsyncClient") as mock_client_cls: + await call_test_oracle( + github_webhook=mock_github_webhook, + pull_request=mock_pull_request, + trigger="pr-opened", + ) + mock_client_cls.assert_not_called() + + @pytest.mark.asyncio + async def test_default_triggers_when_not_specified( + self, mock_github_webhook: Mock, mock_pull_request: Mock + ) -> None: + """Test that default trigger is 'approved' when triggers not in config.""" + mock_github_webhook.config.get_value = Mock( + return_value={ + "server-url": "http://localhost:8000", + "ai-provider": "claude", + "ai-model": "sonnet", + } + ) + + with patch("webhook_server.libs.test_oracle.httpx.AsyncClient") as mock_client_cls: + mock_client = AsyncMock() + mock_client_cls.return_value.__aenter__ = AsyncMock(return_value=mock_client) + mock_client_cls.return_value.__aexit__ = AsyncMock(return_value=False) + mock_client.get.return_value = Mock(status_code=200) + mock_client.post.return_value = Mock( + status_code=200, + json=Mock(return_value={"summary": "ok", "review_posted": True}), + ) + + await call_test_oracle( + github_webhook=mock_github_webhook, + pull_request=mock_pull_request, + trigger="approved", + ) + mock_client.post.assert_called_once() + + @pytest.mark.asyncio + async def test_no_trigger_param_skips_trigger_check( + self, mock_github_webhook: Mock, mock_pull_request: Mock + ) -> None: + """Test that when trigger param is None (comment command), trigger check is skipped.""" + with patch("webhook_server.libs.test_oracle.httpx.AsyncClient") as mock_client_cls: + mock_client = AsyncMock() + mock_client_cls.return_value.__aenter__ = AsyncMock(return_value=mock_client) + mock_client_cls.return_value.__aexit__ = AsyncMock(return_value=False) + mock_client.get.return_value = Mock(status_code=200) + mock_client.post.return_value = Mock( + status_code=200, + json=Mock(return_value={"summary": "ok", "review_posted": True}), + ) + + await call_test_oracle( + github_webhook=mock_github_webhook, + pull_request=mock_pull_request, + ) + mock_client.post.assert_called_once() From 9e3e7f252c65f60e3fa50338b3c6ca8b9a45d822 Mon Sep 17 00:00:00 2001 From: Meni Yakove Date: Sat, 28 Feb 2026 12:33:21 +0200 Subject: [PATCH 03/14] feat: add /test-oracle comment command Co-Authored-By: Claude Opus 4.6 (1M context) --- .../libs/handlers/issue_comment_handler.py | 9 +++++++ .../tests/test_issue_comment_handler.py | 25 +++++++++++++++++++ 2 files changed, 34 insertions(+) diff --git a/webhook_server/libs/handlers/issue_comment_handler.py b/webhook_server/libs/handlers/issue_comment_handler.py index 1708e15a..e1d5b72d 100644 --- a/webhook_server/libs/handlers/issue_comment_handler.py +++ b/webhook_server/libs/handlers/issue_comment_handler.py @@ -14,6 +14,7 @@ from webhook_server.libs.handlers.owners_files_handler import OwnersFileHandler from webhook_server.libs.handlers.pull_request_handler import PullRequestHandler from webhook_server.libs.handlers.runner_handler import RunnerHandler +from webhook_server.libs.test_oracle import call_test_oracle from webhook_server.utils.constants import ( AUTOMERGE_LABEL_STR, BUILD_AND_PUSH_CONTAINER_STR, @@ -26,6 +27,7 @@ COMMAND_REGENERATE_WELCOME_STR, COMMAND_REPROCESS_STR, COMMAND_RETEST_STR, + COMMAND_TEST_ORACLE_STR, HOLD_LABEL_STR, REACTIONS, USER_LABELS_DICT, @@ -159,6 +161,7 @@ async def user_commands( COMMAND_ASSIGN_REVIEWER_STR, COMMAND_ADD_ALLOWED_USER_STR, COMMAND_REGENERATE_WELCOME_STR, + COMMAND_TEST_ORACLE_STR, ] command_and_args: list[str] = command.split(" ", 1) @@ -272,6 +275,12 @@ async def user_commands( self.logger.info(f"{self.log_prefix} Regenerating welcome message") await self.pull_request_handler.regenerate_welcome_message(pull_request=pull_request) + elif _command == COMMAND_TEST_ORACLE_STR: + await call_test_oracle( + github_webhook=self.github_webhook, + pull_request=pull_request, + ) + elif _command == BUILD_AND_PUSH_CONTAINER_STR: if self.github_webhook.build_and_push_container: await self.runner_handler.run_build_container( diff --git a/webhook_server/tests/test_issue_comment_handler.py b/webhook_server/tests/test_issue_comment_handler.py index 16187677..0d3215c8 100644 --- a/webhook_server/tests/test_issue_comment_handler.py +++ b/webhook_server/tests/test_issue_comment_handler.py @@ -14,6 +14,7 @@ COMMAND_REGENERATE_WELCOME_STR, COMMAND_REPROCESS_STR, COMMAND_RETEST_STR, + COMMAND_TEST_ORACLE_STR, HOLD_LABEL_STR, REACTIONS, TOX_STR, @@ -1425,3 +1426,27 @@ async def test_user_commands_non_draft_pr_ignores_config(self, issue_comment_han # Command should proceed because PR is not a draft mock_check.assert_called_once_with(pull_request=mock_pull_request) mock_reaction.assert_awaited_once() + + @pytest.mark.asyncio + async def test_test_oracle_command(self, issue_comment_handler: IssueCommentHandler) -> None: + """Test that /test-oracle command calls call_test_oracle.""" + mock_pull_request = Mock() + mock_pull_request.draft = False + + with patch("asyncio.to_thread", new_callable=AsyncMock, side_effect=lambda f, *a, **k: f(*a, **k)): + with patch.object(issue_comment_handler, "create_comment_reaction", new_callable=AsyncMock): + with patch( + "webhook_server.libs.handlers.issue_comment_handler.call_test_oracle", + new_callable=AsyncMock, + ) as mock_oracle: + await issue_comment_handler.user_commands( + pull_request=mock_pull_request, + command=COMMAND_TEST_ORACLE_STR, + reviewed_user="test-user", + issue_comment_id=456, + is_draft=False, + ) + mock_oracle.assert_called_once_with( + github_webhook=issue_comment_handler.github_webhook, + pull_request=mock_pull_request, + ) From da53b1a1fa2b72efbd7b13393b04adb84ebedca5 Mon Sep 17 00:00:00 2001 From: Meni Yakove Date: Sat, 28 Feb 2026 12:33:57 +0200 Subject: [PATCH 04/14] feat: call test oracle on PR opened/synchronized Co-Authored-By: Claude Opus 4.6 (1M context) --- .../libs/handlers/pull_request_handler.py | 16 +++++ .../tests/test_pull_request_handler.py | 68 +++++++++++++++++++ 2 files changed, 84 insertions(+) diff --git a/webhook_server/libs/handlers/pull_request_handler.py b/webhook_server/libs/handlers/pull_request_handler.py index a26eb4e7..cc61ff16 100644 --- a/webhook_server/libs/handlers/pull_request_handler.py +++ b/webhook_server/libs/handlers/pull_request_handler.py @@ -13,6 +13,7 @@ from webhook_server.libs.handlers.labels_handler import LabelsHandler from webhook_server.libs.handlers.owners_files_handler import OwnersFileHandler from webhook_server.libs.handlers.runner_handler import RunnerHandler +from webhook_server.libs.test_oracle import call_test_oracle from webhook_server.utils.constants import ( APPROVED_BY_LABEL_PREFIX, AUTOMERGE_LABEL_STR, @@ -100,6 +101,14 @@ async def process_pull_request_webhook_data(self, pull_request: PullRequest) -> # Set auto merge only after all initialization of a new PR is done. await self.set_pull_request_automerge(pull_request=pull_request) + + if hook_action == "opened": + await call_test_oracle( + github_webhook=self.github_webhook, + pull_request=pull_request, + trigger="pr-opened", + ) + if self.ctx: self.ctx.complete_step("pr_handler", action=hook_action) return @@ -115,6 +124,13 @@ async def process_pull_request_webhook_data(self, pull_request: PullRequest) -> for result in results: if isinstance(result, Exception): self.logger.error(f"{self.log_prefix} Async task failed: {result}") + + await call_test_oracle( + github_webhook=self.github_webhook, + pull_request=pull_request, + trigger="pr-synchronized", + ) + if self.ctx: self.ctx.complete_step("pr_handler", action=hook_action) return diff --git a/webhook_server/tests/test_pull_request_handler.py b/webhook_server/tests/test_pull_request_handler.py index 7c34d1e1..587017fa 100644 --- a/webhook_server/tests/test_pull_request_handler.py +++ b/webhook_server/tests/test_pull_request_handler.py @@ -90,6 +90,8 @@ def mock_github_webhook(self) -> Mock: mock_webhook.enabled_labels = None # Default: all labels enabled mock_webhook.custom_check_runs = [] mock_webhook.required_conversation_resolution = False + mock_webhook.config = Mock() + mock_webhook.config.get_value = Mock(return_value=None) return mock_webhook @pytest.fixture @@ -2646,3 +2648,69 @@ async def test_regenerate_welcome_message_finds_correct_comment_among_many( mock_comment3.edit.assert_not_called() # Verify no new comment was created mock_pull_request.create_issue_comment.assert_not_called() + + @pytest.mark.asyncio + async def test_process_opened_action_calls_test_oracle_with_pr_opened_trigger( + self, pull_request_handler: PullRequestHandler, mock_pull_request: Mock + ) -> None: + """Test that call_test_oracle is called with trigger='pr-opened' when a PR is opened.""" + pull_request_handler.hook_data["action"] = "opened" + + 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, + ) as mock_test_oracle, + ): + await pull_request_handler.process_pull_request_webhook_data(mock_pull_request) + mock_test_oracle.assert_called_once_with( + github_webhook=pull_request_handler.github_webhook, + pull_request=mock_pull_request, + trigger="pr-opened", + ) + + @pytest.mark.asyncio + async def test_process_synchronize_action_calls_test_oracle_with_pr_synchronized_trigger( + self, pull_request_handler: PullRequestHandler, mock_pull_request: Mock + ) -> None: + """Test that call_test_oracle is called with trigger='pr-synchronized' when a PR is synchronized.""" + pull_request_handler.hook_data["action"] = "synchronize" + + with ( + 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, + ) as mock_test_oracle, + ): + await pull_request_handler.process_pull_request_webhook_data(mock_pull_request) + mock_test_oracle.assert_called_once_with( + github_webhook=pull_request_handler.github_webhook, + pull_request=mock_pull_request, + trigger="pr-synchronized", + ) + + @pytest.mark.asyncio + async def test_process_reopened_action_does_not_call_test_oracle( + self, pull_request_handler: PullRequestHandler, mock_pull_request: Mock + ) -> None: + """Test that call_test_oracle is NOT called when a PR is reopened.""" + pull_request_handler.hook_data["action"] = "reopened" + + 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, + ) as mock_test_oracle, + ): + await pull_request_handler.process_pull_request_webhook_data(mock_pull_request) + mock_test_oracle.assert_not_called() From 4dd390fb2f7290dca2d3b94d56d36484fe77476e Mon Sep 17 00:00:00 2001 From: Meni Yakove Date: Sat, 28 Feb 2026 12:34:44 +0200 Subject: [PATCH 05/14] feat: call test oracle on PR approval Co-Authored-By: Claude Opus 4.6 (1M context) --- .../handlers/pull_request_review_handler.py | 8 + .../tests/test_pull_request_review_handler.py | 240 ++++++++++++------ 2 files changed, 165 insertions(+), 83 deletions(-) diff --git a/webhook_server/libs/handlers/pull_request_review_handler.py b/webhook_server/libs/handlers/pull_request_review_handler.py index eac1720f..db754504 100644 --- a/webhook_server/libs/handlers/pull_request_review_handler.py +++ b/webhook_server/libs/handlers/pull_request_review_handler.py @@ -6,6 +6,7 @@ from webhook_server.libs.handlers.labels_handler import LabelsHandler from webhook_server.libs.handlers.owners_files_handler import OwnersFileHandler +from webhook_server.libs.test_oracle import call_test_oracle from webhook_server.utils.constants import ADD_STR, APPROVE_STR if TYPE_CHECKING: @@ -60,6 +61,13 @@ async def process_pull_request_review_webhook_data(self, pull_request: PullReque remove=False, reviewed_user=reviewed_user, ) + + if review_state == "approved": + await call_test_oracle( + github_webhook=self.github_webhook, + pull_request=pull_request, + trigger="approved", + ) finally: if self.ctx: self.ctx.complete_step("pr_review_handler") diff --git a/webhook_server/tests/test_pull_request_review_handler.py b/webhook_server/tests/test_pull_request_review_handler.py index 74675b5a..1f4861e9 100644 --- a/webhook_server/tests/test_pull_request_review_handler.py +++ b/webhook_server/tests/test_pull_request_review_handler.py @@ -53,20 +53,24 @@ async def test_process_pull_request_review_webhook_data_submitted_action( with patch.object( pull_request_review_handler.labels_handler, "label_by_user_comment" ) as mock_label_comment: - await pull_request_review_handler.process_pull_request_review_webhook_data(mock_pull_request) + with patch( + "webhook_server.libs.handlers.pull_request_review_handler.call_test_oracle", + new_callable=AsyncMock, + ): + await pull_request_review_handler.process_pull_request_review_webhook_data(mock_pull_request) - mock_manage_label.assert_called_once_with( - pull_request=mock_pull_request, - review_state="approved", - action=ADD_STR, - reviewed_user="test-reviewer", - ) - mock_label_comment.assert_called_once_with( - pull_request=mock_pull_request, - user_requested_label=APPROVE_STR, - remove=False, - reviewed_user="test-reviewer", - ) + mock_manage_label.assert_called_once_with( + pull_request=mock_pull_request, + review_state="approved", + action=ADD_STR, + reviewed_user="test-reviewer", + ) + mock_label_comment.assert_called_once_with( + pull_request=mock_pull_request, + user_requested_label=APPROVE_STR, + remove=False, + reviewed_user="test-reviewer", + ) @pytest.mark.asyncio async def test_process_pull_request_review_webhook_data_non_submitted_action( @@ -101,15 +105,19 @@ async def test_process_pull_request_review_webhook_data_no_body( with patch.object( pull_request_review_handler.labels_handler, "label_by_user_comment" ) as mock_label_comment: - await pull_request_review_handler.process_pull_request_review_webhook_data(mock_pull_request) + with patch( + "webhook_server.libs.handlers.pull_request_review_handler.call_test_oracle", + new_callable=AsyncMock, + ): + await pull_request_review_handler.process_pull_request_review_webhook_data(mock_pull_request) - mock_manage_label.assert_called_once_with( - pull_request=mock_pull_request, - review_state="approved", - action=ADD_STR, - reviewed_user="test-reviewer", - ) - mock_label_comment.assert_not_called() + mock_manage_label.assert_called_once_with( + pull_request=mock_pull_request, + review_state="approved", + action=ADD_STR, + reviewed_user="test-reviewer", + ) + mock_label_comment.assert_not_called() @pytest.mark.asyncio async def test_process_pull_request_review_webhook_data_empty_body( @@ -125,15 +133,19 @@ async def test_process_pull_request_review_webhook_data_empty_body( with patch.object( pull_request_review_handler.labels_handler, "label_by_user_comment" ) as mock_label_comment: - await pull_request_review_handler.process_pull_request_review_webhook_data(mock_pull_request) + with patch( + "webhook_server.libs.handlers.pull_request_review_handler.call_test_oracle", + new_callable=AsyncMock, + ): + await pull_request_review_handler.process_pull_request_review_webhook_data(mock_pull_request) - mock_manage_label.assert_called_once_with( - pull_request=mock_pull_request, - review_state="approved", - action=ADD_STR, - reviewed_user="test-reviewer", - ) - mock_label_comment.assert_not_called() + mock_manage_label.assert_called_once_with( + pull_request=mock_pull_request, + review_state="approved", + action=ADD_STR, + reviewed_user="test-reviewer", + ) + mock_label_comment.assert_not_called() @pytest.mark.asyncio async def test_process_pull_request_review_webhook_data_body_without_approve( @@ -149,15 +161,19 @@ async def test_process_pull_request_review_webhook_data_body_without_approve( with patch.object( pull_request_review_handler.labels_handler, "label_by_user_comment" ) as mock_label_comment: - await pull_request_review_handler.process_pull_request_review_webhook_data(mock_pull_request) + with patch( + "webhook_server.libs.handlers.pull_request_review_handler.call_test_oracle", + new_callable=AsyncMock, + ): + await pull_request_review_handler.process_pull_request_review_webhook_data(mock_pull_request) - mock_manage_label.assert_called_once_with( - pull_request=mock_pull_request, - review_state="approved", - action=ADD_STR, - reviewed_user="test-reviewer", - ) - mock_label_comment.assert_not_called() + mock_manage_label.assert_called_once_with( + pull_request=mock_pull_request, + review_state="approved", + action=ADD_STR, + reviewed_user="test-reviewer", + ) + mock_label_comment.assert_not_called() @pytest.mark.asyncio async def test_process_pull_request_review_webhook_data_different_review_states( @@ -177,24 +193,28 @@ async def test_process_pull_request_review_webhook_data_different_review_states( with patch.object( pull_request_review_handler.labels_handler, "label_by_user_comment" ) as mock_label_comment: - await pull_request_review_handler.process_pull_request_review_webhook_data(mock_pull_request) - - mock_manage_label.assert_called_once_with( - pull_request=mock_pull_request, - review_state=state, - action=ADD_STR, - reviewed_user="test-reviewer", - ) - mock_label_comment.assert_called_once_with( - pull_request=mock_pull_request, - user_requested_label=APPROVE_STR, - remove=False, - reviewed_user="test-reviewer", - ) - - # Reset mocks for next iteration - mock_manage_label.reset_mock() - mock_label_comment.reset_mock() + with patch( + "webhook_server.libs.handlers.pull_request_review_handler.call_test_oracle", + new_callable=AsyncMock, + ): + await pull_request_review_handler.process_pull_request_review_webhook_data(mock_pull_request) + + mock_manage_label.assert_called_once_with( + pull_request=mock_pull_request, + review_state=state, + action=ADD_STR, + reviewed_user="test-reviewer", + ) + mock_label_comment.assert_called_once_with( + pull_request=mock_pull_request, + user_requested_label=APPROVE_STR, + remove=False, + reviewed_user="test-reviewer", + ) + + # Reset mocks for next iteration + mock_manage_label.reset_mock() + mock_label_comment.reset_mock() @pytest.mark.asyncio async def test_process_pull_request_review_webhook_data_different_users( @@ -214,21 +234,25 @@ async def test_process_pull_request_review_webhook_data_different_users( with patch.object( pull_request_review_handler.labels_handler, "label_by_user_comment" ) as mock_label_comment: - await pull_request_review_handler.process_pull_request_review_webhook_data(mock_pull_request) - - mock_manage_label.assert_called_once_with( - pull_request=mock_pull_request, review_state="approved", action=ADD_STR, reviewed_user=user - ) - mock_label_comment.assert_called_once_with( - pull_request=mock_pull_request, - user_requested_label=APPROVE_STR, - remove=False, - reviewed_user=user, - ) - - # Reset mocks for next iteration - mock_manage_label.reset_mock() - mock_label_comment.reset_mock() + with patch( + "webhook_server.libs.handlers.pull_request_review_handler.call_test_oracle", + new_callable=AsyncMock, + ): + await pull_request_review_handler.process_pull_request_review_webhook_data(mock_pull_request) + + mock_manage_label.assert_called_once_with( + pull_request=mock_pull_request, review_state="approved", action=ADD_STR, reviewed_user=user + ) + mock_label_comment.assert_called_once_with( + pull_request=mock_pull_request, + user_requested_label=APPROVE_STR, + remove=False, + reviewed_user=user, + ) + + # Reset mocks for next iteration + mock_manage_label.reset_mock() + mock_label_comment.reset_mock() @pytest.mark.asyncio async def test_process_pull_request_review_webhook_data_exact_approve_match( @@ -248,21 +272,71 @@ async def test_process_pull_request_review_webhook_data_exact_approve_match( with patch.object( pull_request_review_handler.labels_handler, "label_by_user_comment" ) as mock_label_comment: + with patch( + "webhook_server.libs.handlers.pull_request_review_handler.call_test_oracle", + new_callable=AsyncMock, + ): + await pull_request_review_handler.process_pull_request_review_webhook_data(mock_pull_request) + + mock_manage_label.assert_called_once_with( + pull_request=mock_pull_request, + review_state="approved", + action=ADD_STR, + reviewed_user="test-reviewer", + ) + mock_label_comment.assert_called_once_with( + pull_request=mock_pull_request, + user_requested_label=APPROVE_STR, + remove=False, + reviewed_user="test-reviewer", + ) + + # Reset mocks for next iteration + mock_manage_label.reset_mock() + mock_label_comment.reset_mock() + + @pytest.mark.asyncio + async def test_calls_test_oracle_on_approval(self, pull_request_review_handler: PullRequestReviewHandler) -> None: + """Test that test oracle is called when PR review is approved.""" + mock_pull_request = Mock(spec=PullRequest) + + with patch.object( + pull_request_review_handler.labels_handler, "manage_reviewed_by_label", new_callable=AsyncMock + ): + with patch.object( + pull_request_review_handler.labels_handler, "label_by_user_comment", new_callable=AsyncMock + ): + with patch( + "webhook_server.libs.handlers.pull_request_review_handler.call_test_oracle", + new_callable=AsyncMock, + ) as mock_oracle: await pull_request_review_handler.process_pull_request_review_webhook_data(mock_pull_request) - mock_manage_label.assert_called_once_with( + mock_oracle.assert_called_once_with( + github_webhook=pull_request_review_handler.github_webhook, pull_request=mock_pull_request, - review_state="approved", - action=ADD_STR, - reviewed_user="test-reviewer", - ) - mock_label_comment.assert_called_once_with( - pull_request=mock_pull_request, - user_requested_label=APPROVE_STR, - remove=False, - reviewed_user="test-reviewer", + trigger="approved", ) - # Reset mocks for next iteration - mock_manage_label.reset_mock() - mock_label_comment.reset_mock() + @pytest.mark.asyncio + async def test_does_not_call_test_oracle_on_non_approval( + self, pull_request_review_handler: PullRequestReviewHandler + ) -> None: + """Test that test oracle is NOT called for non-approval reviews.""" + mock_pull_request = Mock(spec=PullRequest) + pull_request_review_handler.hook_data["review"]["state"] = "commented" + pull_request_review_handler.hook_data["review"]["body"] = "Looks good" + + with patch.object( + pull_request_review_handler.labels_handler, "manage_reviewed_by_label", new_callable=AsyncMock + ): + with patch.object( + pull_request_review_handler.labels_handler, "label_by_user_comment", new_callable=AsyncMock + ): + with patch( + "webhook_server.libs.handlers.pull_request_review_handler.call_test_oracle", + new_callable=AsyncMock, + ) as mock_oracle: + await pull_request_review_handler.process_pull_request_review_webhook_data(mock_pull_request) + + mock_oracle.assert_not_called() From de28d55e8deef5e180b3fd264e6a2c1f4478077b Mon Sep 17 00:00:00 2001 From: Meni Yakove Date: Sat, 28 Feb 2026 12:36:51 +0200 Subject: [PATCH 06/14] docs: add test-oracle example config --- examples/.github-webhook-server.yaml | 12 ++++++++++++ examples/config.yaml | 24 ++++++++++++++++++++++++ 2 files changed, 36 insertions(+) diff --git a/examples/.github-webhook-server.yaml b/examples/.github-webhook-server.yaml index 288b36ed..2b60141c 100644 --- a/examples/.github-webhook-server.yaml +++ b/examples/.github-webhook-server.yaml @@ -143,3 +143,15 @@ pr-size-thresholds: Extreme: threshold: inf # PRs with 1000+ lines changed (unbounded largest category) color: black # 'inf' means no upper limit - catches all PRs above 1000 lines + +# PR Test Oracle integration (overrides global config) +# Analyzes PR diffs with AI and recommends which tests to run +# See: https://github.com/myk-org/pr-test-oracle +# test-oracle: +# server-url: "http://localhost:8000" +# ai-provider: "claude" # claude | gemini | cursor +# ai-model: "sonnet" +# test-patterns: +# - "tests/**/*.py" +# triggers: +# - approved diff --git a/examples/config.yaml b/examples/config.yaml index 60556d27..96ae4caa 100644 --- a/examples/config.yaml +++ b/examples/config.yaml @@ -107,6 +107,20 @@ branch-protection: required_linear_history: True required_conversation_resolution: True +# PR Test Oracle integration +# Analyzes PR diffs with AI and recommends which tests to run +# See: https://github.com/myk-org/pr-test-oracle +# test-oracle: +# server-url: "http://localhost:8000" +# ai-provider: "claude" # claude | gemini | cursor +# ai-model: "sonnet" +# test-patterns: +# - "tests/**/*.py" +# triggers: # Default: [approved] +# - approved # Run when PR gets approved +# # - pr-opened # Run when PR is opened +# # - pr-synchronized # Run when new commits pushed + repositories: my-repository: name: my-org/my-repository @@ -222,3 +236,13 @@ repositories: set-auto-merge-prs: - main + + # PR Test Oracle (overrides global) + # test-oracle: + # server-url: "http://localhost:8000" + # ai-provider: "claude" + # ai-model: "sonnet" + # test-patterns: + # - "tests/**/*.py" + # triggers: + # - approved From 1fe866b35f6ab4dad097df7b983b807860fd213b Mon Sep 17 00:00:00 2001 From: Meni Yakove Date: Sat, 28 Feb 2026 12:37:25 +0200 Subject: [PATCH 07/14] docs: add test-oracle documentation to CLAUDE.md and README Co-Authored-By: Claude Opus 4.6 (1M context) --- CLAUDE.md | 18 ++++++++++++++++++ README.md | 4 ++++ 2 files changed, 22 insertions(+) diff --git a/CLAUDE.md b/CLAUDE.md index 37bfdd16..2a2d6842 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -654,3 +654,21 @@ def mock_github_api(): 2. Run `uv run pytest webhook_server/tests/test_config_schema.py -v` 3. Update examples in `examples/config.yaml` 4. Test with `uv run webhook_server/tests/test_schema_validator.py examples/config.yaml` + +### PR Test Oracle Integration + +External AI service integration for test recommendations. Configured via `test-oracle` in config (global or per-repo). + +**Config keys:** `server-url` (required), `ai-provider` (required: claude/gemini/cursor), `ai-model` (required), `test-patterns` (optional), `triggers` (optional, default: [approved]) + +**Trigger events:** `approved`, `pr-opened`, `pr-synchronized` + +**Comment command:** `/test-oracle` (always works when configured, no trigger needed) + +**Module:** `webhook_server/libs/test_oracle.py` - `call_test_oracle()` shared helper + +**Error handling:** + +- Health check failure: PR comment posted, continue flow +- Analyze errors: log only, no PR comment +- Never breaks webhook processing diff --git a/README.md b/README.md index c26f9788..b2bf0474 100644 --- a/README.md +++ b/README.md @@ -85,6 +85,7 @@ GitHub Events → Webhook Server → Repository Management - **PyPI package publishing** for Python projects - **Tox testing integration** with configurable test environments - **Pre-commit hook validation** for code quality assurance +- **PR Test Oracle** - AI-powered test recommendations based on PR diff analysis ### 👥 User Commands @@ -593,6 +594,7 @@ uv run pytest webhook_server/tests/test_config_schema.py::TestConfigSchema::test | **Features** | `verified-job`, `pre-commit`, `pypi`, `tox`, `container` | | **Pull Requests** | `minimum-lgtm`, `conventional-title`, `can-be-merged-required-labels`, `create-issue-for-new-pr` | | **Automation** | `set-auto-merge-prs`, `auto-verified-and-merged-users` | +| **AI** | `test-oracle` (`server-url`, `ai-provider`, `ai-model`, `test-patterns`, `triggers`) | | **Protection** | `protected-branches`, `branch-protection` | ## Deployment @@ -1237,6 +1239,7 @@ Users can interact with the webhook server through GitHub comments on pull reque | `/assign-reviewers` | Assign OWNERS-based reviewers | `/assign-reviewers` | | `/check-can-merge` | Check merge readiness | `/check-can-merge` | | `/reprocess` | Trigger complete PR workflow reprocessing (OWNERS only) | `/reprocess` | +| `/test-oracle` | Request AI-powered test recommendations for PR changes | `/test-oracle` | ### Workflow Management @@ -1322,6 +1325,7 @@ auto-verify-cherry-picked-prs: true # Default: true (auto-verify). Set to false | Command | Description | Example | | --------------------- | --------------------------------------------- | --------------------- | | `/retest ` | Run specific tests like `tox` or `pre-commit` | `/retest ` | +| `/test-oracle` | Request AI-powered test recommendations | `/test-oracle` | ## OWNERS File Format From 61eb85d15a5ff41fdcf781a1f1176cf35aaadbaf Mon Sep 17 00:00:00 2001 From: Meni Yakove Date: Sat, 28 Feb 2026 12:49:02 +0200 Subject: [PATCH 08/14] fix: uncomment test-oracle example config to match existing style Co-Authored-By: Claude Opus 4.6 (1M context) --- examples/.github-webhook-server.yaml | 16 ++++++------- examples/config.yaml | 36 ++++++++++++++-------------- 2 files changed, 26 insertions(+), 26 deletions(-) diff --git a/examples/.github-webhook-server.yaml b/examples/.github-webhook-server.yaml index 2b60141c..5843d0e0 100644 --- a/examples/.github-webhook-server.yaml +++ b/examples/.github-webhook-server.yaml @@ -147,11 +147,11 @@ pr-size-thresholds: # PR Test Oracle integration (overrides global config) # Analyzes PR diffs with AI and recommends which tests to run # See: https://github.com/myk-org/pr-test-oracle -# test-oracle: -# server-url: "http://localhost:8000" -# ai-provider: "claude" # claude | gemini | cursor -# ai-model: "sonnet" -# test-patterns: -# - "tests/**/*.py" -# triggers: -# - approved +test-oracle: + server-url: "http://localhost:8000" + ai-provider: "claude" # claude | gemini | cursor + ai-model: "sonnet" + test-patterns: + - "tests/**/*.py" + triggers: + - approved diff --git a/examples/config.yaml b/examples/config.yaml index 96ae4caa..6e7b49dc 100644 --- a/examples/config.yaml +++ b/examples/config.yaml @@ -110,16 +110,16 @@ branch-protection: # PR Test Oracle integration # Analyzes PR diffs with AI and recommends which tests to run # See: https://github.com/myk-org/pr-test-oracle -# test-oracle: -# server-url: "http://localhost:8000" -# ai-provider: "claude" # claude | gemini | cursor -# ai-model: "sonnet" -# test-patterns: -# - "tests/**/*.py" -# triggers: # Default: [approved] -# - approved # Run when PR gets approved -# # - pr-opened # Run when PR is opened -# # - pr-synchronized # Run when new commits pushed +test-oracle: + server-url: "http://localhost:8000" + ai-provider: "claude" # claude | gemini | cursor + ai-model: "sonnet" + test-patterns: + - "tests/**/*.py" + triggers: # Default: [approved] + - approved # Run when PR gets approved + # - pr-opened # Run when PR is opened + # - pr-synchronized # Run when new commits pushed repositories: my-repository: @@ -238,11 +238,11 @@ repositories: - main # PR Test Oracle (overrides global) - # test-oracle: - # server-url: "http://localhost:8000" - # ai-provider: "claude" - # ai-model: "sonnet" - # test-patterns: - # - "tests/**/*.py" - # triggers: - # - approved + test-oracle: + server-url: "http://localhost:8000" + ai-provider: "claude" + ai-model: "sonnet" + test-patterns: + - "tests/**/*.py" + triggers: + - approved From d00482d12a8dbbcd181bcb2e9cbbf696e93072ea Mon Sep 17 00:00:00 2001 From: Meni Yakove Date: Sat, 28 Feb 2026 12:55:13 +0200 Subject: [PATCH 09/14] docs: add pr-test-oracle repo link to all documentation Co-Authored-By: Claude Opus 4.6 (1M context) --- CLAUDE.md | 2 +- README.md | 6 +++--- examples/config.yaml | 1 + webhook_server/config/schema.yaml | 2 ++ 4 files changed, 7 insertions(+), 4 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index 2a2d6842..8ae14469 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -657,7 +657,7 @@ def mock_github_api(): ### PR Test Oracle Integration -External AI service integration for test recommendations. Configured via `test-oracle` in config (global or per-repo). +External AI service integration for test recommendations via [pr-test-oracle](https://github.com/myk-org/pr-test-oracle). Configured via `test-oracle` in config (global or per-repo). **Config keys:** `server-url` (required), `ai-provider` (required: claude/gemini/cursor), `ai-model` (required), `test-patterns` (optional), `triggers` (optional, default: [approved]) diff --git a/README.md b/README.md index b2bf0474..2f95e891 100644 --- a/README.md +++ b/README.md @@ -594,7 +594,7 @@ uv run pytest webhook_server/tests/test_config_schema.py::TestConfigSchema::test | **Features** | `verified-job`, `pre-commit`, `pypi`, `tox`, `container` | | **Pull Requests** | `minimum-lgtm`, `conventional-title`, `can-be-merged-required-labels`, `create-issue-for-new-pr` | | **Automation** | `set-auto-merge-prs`, `auto-verified-and-merged-users` | -| **AI** | `test-oracle` (`server-url`, `ai-provider`, `ai-model`, `test-patterns`, `triggers`) | +| **AI** | [`test-oracle`](https://github.com/myk-org/pr-test-oracle) (`server-url`, `ai-provider`, `ai-model`, `test-patterns`, `triggers`) | | **Protection** | `protected-branches`, `branch-protection` | ## Deployment @@ -1239,7 +1239,7 @@ Users can interact with the webhook server through GitHub comments on pull reque | `/assign-reviewers` | Assign OWNERS-based reviewers | `/assign-reviewers` | | `/check-can-merge` | Check merge readiness | `/check-can-merge` | | `/reprocess` | Trigger complete PR workflow reprocessing (OWNERS only) | `/reprocess` | -| `/test-oracle` | Request AI-powered test recommendations for PR changes | `/test-oracle` | +| `/test-oracle` | Request AI-powered test recommendations for PR changes ([pr-test-oracle](https://github.com/myk-org/pr-test-oracle)) | `/test-oracle` | ### Workflow Management @@ -1325,7 +1325,7 @@ auto-verify-cherry-picked-prs: true # Default: true (auto-verify). Set to false | Command | Description | Example | | --------------------- | --------------------------------------------- | --------------------- | | `/retest ` | Run specific tests like `tox` or `pre-commit` | `/retest ` | -| `/test-oracle` | Request AI-powered test recommendations | `/test-oracle` | +| `/test-oracle` | Request AI-powered test recommendations ([pr-test-oracle](https://github.com/myk-org/pr-test-oracle)) | `/test-oracle` | ## OWNERS File Format diff --git a/examples/config.yaml b/examples/config.yaml index 6e7b49dc..53b011c7 100644 --- a/examples/config.yaml +++ b/examples/config.yaml @@ -238,6 +238,7 @@ repositories: - main # PR Test Oracle (overrides global) + # See: https://github.com/myk-org/pr-test-oracle test-oracle: server-url: "http://localhost:8000" ai-provider: "claude" diff --git a/webhook_server/config/schema.yaml b/webhook_server/config/schema.yaml index 3b2330fe..8e47af10 100644 --- a/webhook_server/config/schema.yaml +++ b/webhook_server/config/schema.yaml @@ -97,6 +97,7 @@ properties: description: | PR Test Oracle integration. Sends PR data to an external AI service that analyzes diffs and recommends which tests to run. + See https://github.com/myk-org/pr-test-oracle for server setup. The /test-oracle comment command always works when configured (no trigger needed). properties: server-url: @@ -451,6 +452,7 @@ properties: description: | PR Test Oracle integration. Sends PR data to an external AI service that analyzes diffs and recommends which tests to run. + See https://github.com/myk-org/pr-test-oracle for server setup. The /test-oracle comment command always works when configured (no trigger needed). properties: server-url: From eb3d0edcf71e1990b11109dd765bcfb0c66e77de Mon Sep 17 00:00:00 2001 From: Meni Yakove Date: Sat, 28 Feb 2026 13:21:09 +0200 Subject: [PATCH 10/14] fix: address review feedback for test-oracle integration - Consolidate error handling with raise_for_status() in health check and analyze - Add secure transport validation (reject remote http://, allow localhost) - Guard create_issue_comment calls against PyGithub failures - Catch ValueError from invalid JSON responses - Add outer try/except with CancelledError re-raise - Bypass draft PR gate for /test-oracle command - Add default: [approved] to triggers schema - Add 8 new regression tests (19 total, 100% coverage) --- webhook_server/config/schema.yaml | 4 + .../libs/handlers/issue_comment_handler.py | 10 +- .../libs/handlers/pull_request_handler.py | 20 ++- .../handlers/pull_request_review_handler.py | 11 +- webhook_server/libs/test_oracle.py | 97 +++++----- .../tests/test_issue_comment_handler.py | 27 +-- .../tests/test_pull_request_handler.py | 16 +- .../tests/test_pull_request_review_handler.py | 17 +- webhook_server/tests/test_test_oracle.py | 166 +++++++++++++++++- 9 files changed, 281 insertions(+), 87 deletions(-) diff --git a/webhook_server/config/schema.yaml b/webhook_server/config/schema.yaml index 8e47af10..0f640de2 100644 --- a/webhook_server/config/schema.yaml +++ b/webhook_server/config/schema.yaml @@ -121,6 +121,8 @@ properties: description: Glob patterns for test files (e.g., ["tests/**/*.py"]). Optional - oracle has defaults. triggers: type: array + default: + - approved items: type: string enum: @@ -476,6 +478,8 @@ properties: description: Glob patterns for test files (e.g., ["tests/**/*.py"]). Optional - oracle has defaults. triggers: type: array + default: + - approved items: type: string enum: diff --git a/webhook_server/libs/handlers/issue_comment_handler.py b/webhook_server/libs/handlers/issue_comment_handler.py index e1d5b72d..390a268e 100644 --- a/webhook_server/libs/handlers/issue_comment_handler.py +++ b/webhook_server/libs/handlers/issue_comment_handler.py @@ -169,7 +169,7 @@ async def user_commands( _args: str = command_and_args[1] if len(command_and_args) > 1 else "" # Check if command is allowed on draft PRs - if is_draft: + if is_draft and _command != COMMAND_TEST_ORACLE_STR: allow_commands_on_draft = self.github_webhook.config.get_value("allow-commands-on-draft-prs") if not isinstance(allow_commands_on_draft, list): self.logger.debug( @@ -276,9 +276,11 @@ async def user_commands( await self.pull_request_handler.regenerate_welcome_message(pull_request=pull_request) elif _command == COMMAND_TEST_ORACLE_STR: - await call_test_oracle( - github_webhook=self.github_webhook, - pull_request=pull_request, + asyncio.create_task( + call_test_oracle( + github_webhook=self.github_webhook, + pull_request=pull_request, + ) ) elif _command == BUILD_AND_PUSH_CONTAINER_STR: diff --git a/webhook_server/libs/handlers/pull_request_handler.py b/webhook_server/libs/handlers/pull_request_handler.py index cc61ff16..a2400b86 100644 --- a/webhook_server/libs/handlers/pull_request_handler.py +++ b/webhook_server/libs/handlers/pull_request_handler.py @@ -103,10 +103,12 @@ async def process_pull_request_webhook_data(self, pull_request: PullRequest) -> await self.set_pull_request_automerge(pull_request=pull_request) if hook_action == "opened": - await call_test_oracle( - github_webhook=self.github_webhook, - pull_request=pull_request, - trigger="pr-opened", + asyncio.create_task( + call_test_oracle( + github_webhook=self.github_webhook, + pull_request=pull_request, + trigger="pr-opened", + ) ) if self.ctx: @@ -125,10 +127,12 @@ async def process_pull_request_webhook_data(self, pull_request: PullRequest) -> if isinstance(result, Exception): self.logger.error(f"{self.log_prefix} Async task failed: {result}") - await call_test_oracle( - github_webhook=self.github_webhook, - pull_request=pull_request, - trigger="pr-synchronized", + asyncio.create_task( + call_test_oracle( + github_webhook=self.github_webhook, + pull_request=pull_request, + trigger="pr-synchronized", + ) ) if self.ctx: diff --git a/webhook_server/libs/handlers/pull_request_review_handler.py b/webhook_server/libs/handlers/pull_request_review_handler.py index db754504..1ac02094 100644 --- a/webhook_server/libs/handlers/pull_request_review_handler.py +++ b/webhook_server/libs/handlers/pull_request_review_handler.py @@ -1,5 +1,6 @@ from __future__ import annotations +import asyncio from typing import TYPE_CHECKING from github.PullRequest import PullRequest @@ -63,10 +64,12 @@ async def process_pull_request_review_webhook_data(self, pull_request: PullReque ) if review_state == "approved": - await call_test_oracle( - github_webhook=self.github_webhook, - pull_request=pull_request, - trigger="approved", + asyncio.create_task( + call_test_oracle( + github_webhook=self.github_webhook, + pull_request=pull_request, + trigger="approved", + ) ) finally: if self.ctx: diff --git a/webhook_server/libs/test_oracle.py b/webhook_server/libs/test_oracle.py index 6c083cd1..bc0d6149 100644 --- a/webhook_server/libs/test_oracle.py +++ b/webhook_server/libs/test_oracle.py @@ -2,6 +2,7 @@ import asyncio from typing import TYPE_CHECKING, Any +from urllib.parse import urlparse import httpx @@ -41,49 +42,61 @@ async def call_test_oracle( server_url: str = config["server-url"] log_prefix: str = github_webhook.log_prefix - async with httpx.AsyncClient(base_url=server_url) as client: - # Health check - try: - health_response = await client.get("/health", timeout=5.0) - if health_response.status_code != 200: - msg = ( - f"Test Oracle server at {server_url} is not responding" - f" (status {health_response.status_code}), skipping test analysis" - ) + # Secure transport check + parsed_url = urlparse(server_url) + is_local = parsed_url.hostname in {"localhost", "127.0.0.1", "::1"} + if parsed_url.scheme != "https" and not is_local: + github_webhook.logger.error( + f"{log_prefix} Insecure test-oracle server-url '{server_url}'. Use https or localhost http." + ) + return + + try: + 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"Test Oracle server at {server_url} is not responding{status_info}, skipping test analysis" github_webhook.logger.warning(f"{log_prefix} {msg}") - await asyncio.to_thread(pull_request.create_issue_comment, msg) + try: + await asyncio.to_thread(pull_request.create_issue_comment, msg) + except Exception: + github_webhook.logger.exception(f"{log_prefix} Failed to post health check comment") return - except httpx.HTTPError: - msg = f"Test Oracle server at {server_url} is not responding, skipping test analysis" - github_webhook.logger.warning(f"{log_prefix} {msg}") - await asyncio.to_thread(pull_request.create_issue_comment, msg) - return - # Build analyze payload - payload: dict[str, Any] = { - "pr_url": pull_request.html_url, - "ai_provider": config["ai-provider"], - "ai_model": config["ai-model"], - "github_token": github_webhook.token, - } - - if "test-patterns" in config: - payload["test_patterns"] = config["test-patterns"] - - # Call analyze - try: - github_webhook.logger.info(f"{log_prefix} Calling Test Oracle for {pull_request.html_url}") - response = await client.post("/analyze", json=payload, timeout=300.0) - - if response.status_code != 200: - github_webhook.logger.error( - f"{log_prefix} Test Oracle analyze failed with status {response.status_code}: {response.text}" + # Build analyze payload + payload: dict[str, Any] = { + "pr_url": pull_request.html_url, + "ai_provider": config["ai-provider"], + "ai_model": config["ai-model"], + "github_token": github_webhook.token, + } + + if "test-patterns" in config: + payload["test_patterns"] = config["test-patterns"] + + # Call analyze + try: + github_webhook.logger.info(f"{log_prefix} Calling Test Oracle for {pull_request.html_url}") + response = await client.post("/analyze", json=payload, timeout=300.0) + response.raise_for_status() + + result = response.json() + github_webhook.logger.info( + f"{log_prefix} Test Oracle analysis complete: {result.get('summary', 'no summary')}" ) - return - - result = response.json() - github_webhook.logger.info( - f"{log_prefix} Test Oracle analysis complete: {result.get('summary', 'no summary')}" - ) - except httpx.HTTPError: - github_webhook.logger.error(f"{log_prefix} Test Oracle analyze request failed") + except httpx.HTTPError as e: + err_detail = f": {e.response.text}" if isinstance(e, httpx.HTTPStatusError) else "" + github_webhook.logger.error(f"{log_prefix} Test Oracle analyze request failed{err_detail}") + except ValueError: + github_webhook.logger.error(f"{log_prefix} Test Oracle returned invalid JSON response") + except asyncio.CancelledError: + raise + except Exception: + github_webhook.logger.exception(f"{log_prefix} Test Oracle call failed unexpectedly") diff --git a/webhook_server/tests/test_issue_comment_handler.py b/webhook_server/tests/test_issue_comment_handler.py index 0d3215c8..39ff80cc 100644 --- a/webhook_server/tests/test_issue_comment_handler.py +++ b/webhook_server/tests/test_issue_comment_handler.py @@ -1429,7 +1429,7 @@ async def test_user_commands_non_draft_pr_ignores_config(self, issue_comment_han @pytest.mark.asyncio async def test_test_oracle_command(self, issue_comment_handler: IssueCommentHandler) -> None: - """Test that /test-oracle command calls call_test_oracle.""" + """Test that /test-oracle command fires call_test_oracle as a background task.""" mock_pull_request = Mock() mock_pull_request.draft = False @@ -1437,16 +1437,17 @@ async def test_test_oracle_command(self, issue_comment_handler: IssueCommentHand with patch.object(issue_comment_handler, "create_comment_reaction", new_callable=AsyncMock): with patch( "webhook_server.libs.handlers.issue_comment_handler.call_test_oracle", - new_callable=AsyncMock, ) as mock_oracle: - await issue_comment_handler.user_commands( - pull_request=mock_pull_request, - command=COMMAND_TEST_ORACLE_STR, - reviewed_user="test-user", - issue_comment_id=456, - is_draft=False, - ) - mock_oracle.assert_called_once_with( - github_webhook=issue_comment_handler.github_webhook, - pull_request=mock_pull_request, - ) + with patch("asyncio.create_task") as mock_create_task: + await issue_comment_handler.user_commands( + pull_request=mock_pull_request, + command=COMMAND_TEST_ORACLE_STR, + reviewed_user="test-user", + issue_comment_id=456, + is_draft=False, + ) + mock_oracle.assert_called_once_with( + github_webhook=issue_comment_handler.github_webhook, + pull_request=mock_pull_request, + ) + mock_create_task.assert_called_once() diff --git a/webhook_server/tests/test_pull_request_handler.py b/webhook_server/tests/test_pull_request_handler.py index 587017fa..e56b7b4e 100644 --- a/webhook_server/tests/test_pull_request_handler.py +++ b/webhook_server/tests/test_pull_request_handler.py @@ -2653,7 +2653,7 @@ async def test_regenerate_welcome_message_finds_correct_comment_among_many( async def test_process_opened_action_calls_test_oracle_with_pr_opened_trigger( self, pull_request_handler: PullRequestHandler, mock_pull_request: Mock ) -> None: - """Test that call_test_oracle is called with trigger='pr-opened' when a PR is opened.""" + """Test that call_test_oracle is fired as a background task with trigger='pr-opened' when a PR is opened.""" pull_request_handler.hook_data["action"] = "opened" with ( @@ -2663,8 +2663,8 @@ async def test_process_opened_action_calls_test_oracle_with_pr_opened_trigger( patch.object(pull_request_handler, "set_pull_request_automerge"), patch( "webhook_server.libs.handlers.pull_request_handler.call_test_oracle", - new_callable=AsyncMock, ) as mock_test_oracle, + patch("asyncio.create_task") as mock_create_task, ): await pull_request_handler.process_pull_request_webhook_data(mock_pull_request) mock_test_oracle.assert_called_once_with( @@ -2672,12 +2672,16 @@ async def test_process_opened_action_calls_test_oracle_with_pr_opened_trigger( pull_request=mock_pull_request, trigger="pr-opened", ) + mock_create_task.assert_called_once() @pytest.mark.asyncio async def test_process_synchronize_action_calls_test_oracle_with_pr_synchronized_trigger( self, pull_request_handler: PullRequestHandler, mock_pull_request: Mock ) -> None: - """Test that call_test_oracle is called with trigger='pr-synchronized' when a PR is synchronized.""" + """Test that call_test_oracle is fired as a background task. + + Verifies trigger='pr-synchronized' when a PR is synchronized. + """ pull_request_handler.hook_data["action"] = "synchronize" with ( @@ -2685,8 +2689,8 @@ async def test_process_synchronize_action_calls_test_oracle_with_pr_synchronized 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, ) as mock_test_oracle, + patch("asyncio.create_task") as mock_create_task, ): await pull_request_handler.process_pull_request_webhook_data(mock_pull_request) mock_test_oracle.assert_called_once_with( @@ -2694,6 +2698,7 @@ async def test_process_synchronize_action_calls_test_oracle_with_pr_synchronized pull_request=mock_pull_request, trigger="pr-synchronized", ) + mock_create_task.assert_called_once() @pytest.mark.asyncio async def test_process_reopened_action_does_not_call_test_oracle( @@ -2709,8 +2714,9 @@ async def test_process_reopened_action_does_not_call_test_oracle( patch.object(pull_request_handler, "set_pull_request_automerge"), patch( "webhook_server.libs.handlers.pull_request_handler.call_test_oracle", - new_callable=AsyncMock, ) as mock_test_oracle, + patch("asyncio.create_task") as mock_create_task, ): await pull_request_handler.process_pull_request_webhook_data(mock_pull_request) mock_test_oracle.assert_not_called() + mock_create_task.assert_not_called() diff --git a/webhook_server/tests/test_pull_request_review_handler.py b/webhook_server/tests/test_pull_request_review_handler.py index 1f4861e9..8858f92d 100644 --- a/webhook_server/tests/test_pull_request_review_handler.py +++ b/webhook_server/tests/test_pull_request_review_handler.py @@ -297,7 +297,7 @@ async def test_process_pull_request_review_webhook_data_exact_approve_match( @pytest.mark.asyncio async def test_calls_test_oracle_on_approval(self, pull_request_review_handler: PullRequestReviewHandler) -> None: - """Test that test oracle is called when PR review is approved.""" + """Test that test oracle is fired as a background task when PR review is approved.""" mock_pull_request = Mock(spec=PullRequest) with patch.object( @@ -308,15 +308,16 @@ async def test_calls_test_oracle_on_approval(self, pull_request_review_handler: ): with patch( "webhook_server.libs.handlers.pull_request_review_handler.call_test_oracle", - new_callable=AsyncMock, ) as mock_oracle: - await pull_request_review_handler.process_pull_request_review_webhook_data(mock_pull_request) + with patch("asyncio.create_task") as mock_create_task: + await pull_request_review_handler.process_pull_request_review_webhook_data(mock_pull_request) - mock_oracle.assert_called_once_with( - github_webhook=pull_request_review_handler.github_webhook, - pull_request=mock_pull_request, - trigger="approved", - ) + mock_oracle.assert_called_once_with( + github_webhook=pull_request_review_handler.github_webhook, + pull_request=mock_pull_request, + trigger="approved", + ) + mock_create_task.assert_called_once() @pytest.mark.asyncio async def test_does_not_call_test_oracle_on_non_approval( diff --git a/webhook_server/tests/test_test_oracle.py b/webhook_server/tests/test_test_oracle.py index c71ce908..e3157310 100644 --- a/webhook_server/tests/test_test_oracle.py +++ b/webhook_server/tests/test_test_oracle.py @@ -2,6 +2,7 @@ from __future__ import annotations +import asyncio from unittest.mock import AsyncMock, Mock, patch import httpx @@ -73,7 +74,12 @@ async def test_health_check_non_200_posts_comment(self, mock_github_webhook: Moc mock_client = AsyncMock() mock_client_cls.return_value.__aenter__ = AsyncMock(return_value=mock_client) mock_client_cls.return_value.__aexit__ = AsyncMock(return_value=False) - mock_client.get.return_value = Mock(status_code=503) + + mock_response = Mock(status_code=503) + mock_response.raise_for_status.side_effect = httpx.HTTPStatusError( + "Server Error", request=Mock(), response=Mock(status_code=503) + ) + mock_client.get.return_value = mock_response with patch("asyncio.to_thread", new_callable=AsyncMock) as mock_to_thread: await call_test_oracle(github_webhook=mock_github_webhook, pull_request=mock_pull_request) @@ -82,6 +88,7 @@ async def test_health_check_non_200_posts_comment(self, mock_github_webhook: Moc call_args = mock_to_thread.call_args assert call_args[0][0] == mock_pull_request.create_issue_comment assert "not responding" in call_args[0][1] + assert "(status 503)" in call_args[0][1] @pytest.mark.asyncio async def test_successful_analyze_call(self, mock_github_webhook: Mock, mock_pull_request: Mock) -> None: @@ -124,8 +131,15 @@ async def test_analyze_error_logs_only(self, mock_github_webhook: Mock, mock_pul mock_client_cls.return_value.__aenter__ = AsyncMock(return_value=mock_client) mock_client_cls.return_value.__aexit__ = AsyncMock(return_value=False) - mock_client.get.return_value = Mock(status_code=200) - mock_client.post.return_value = Mock(status_code=500, text="Internal Server Error") + mock_health = Mock() + mock_health.raise_for_status = Mock() + mock_client.get.return_value = mock_health + + mock_analyze_response = Mock(status_code=500, text="Internal Server Error") + mock_analyze_response.raise_for_status.side_effect = httpx.HTTPStatusError( + "Server Error", request=Mock(), response=Mock(status_code=500, text="Internal Server Error") + ) + mock_client.post.return_value = mock_analyze_response with patch("asyncio.to_thread", new_callable=AsyncMock) as mock_to_thread: await call_test_oracle(github_webhook=mock_github_webhook, pull_request=mock_pull_request) @@ -258,3 +272,149 @@ async def test_no_trigger_param_skips_trigger_check( pull_request=mock_pull_request, ) mock_client.post.assert_called_once() + + @pytest.mark.asyncio + async def test_insecure_url_rejected(self, mock_github_webhook: Mock, mock_pull_request: Mock) -> None: + """Test that non-localhost http URLs are rejected.""" + mock_github_webhook.config.get_value = Mock( + return_value={ + "server-url": "http://remote-server.example.com:8000", + "ai-provider": "claude", + "ai-model": "sonnet", + } + ) + + with patch("webhook_server.libs.test_oracle.httpx.AsyncClient") as mock_client_cls: + await call_test_oracle(github_webhook=mock_github_webhook, pull_request=mock_pull_request) + + mock_client_cls.assert_not_called() + mock_github_webhook.logger.error.assert_called_once() + error_msg = mock_github_webhook.logger.error.call_args[0][0] + assert "Insecure" in error_msg + + @pytest.mark.asyncio + async def test_https_url_allowed(self, mock_github_webhook: Mock, mock_pull_request: Mock) -> None: + """Test that https URLs are always allowed.""" + mock_github_webhook.config.get_value = Mock( + return_value={ + "server-url": "https://remote-server.example.com:8000", + "ai-provider": "claude", + "ai-model": "sonnet", + } + ) + + with patch("webhook_server.libs.test_oracle.httpx.AsyncClient") as mock_client_cls: + mock_client = AsyncMock() + mock_client_cls.return_value.__aenter__ = AsyncMock(return_value=mock_client) + mock_client_cls.return_value.__aexit__ = AsyncMock(return_value=False) + mock_client.get.return_value = Mock(status_code=200) + mock_client.post.return_value = Mock( + status_code=200, + json=Mock(return_value={"summary": "ok", "review_posted": True}), + ) + + await call_test_oracle(github_webhook=mock_github_webhook, pull_request=mock_pull_request) + + mock_client_cls.assert_called_once() + + @pytest.mark.asyncio + async def test_localhost_http_allowed(self, mock_github_webhook: Mock, mock_pull_request: Mock) -> None: + """Test that http://localhost URLs are allowed.""" + with patch("webhook_server.libs.test_oracle.httpx.AsyncClient") as mock_client_cls: + mock_client = AsyncMock() + mock_client_cls.return_value.__aenter__ = AsyncMock(return_value=mock_client) + mock_client_cls.return_value.__aexit__ = AsyncMock(return_value=False) + mock_client.get.return_value = Mock(status_code=200) + mock_client.post.return_value = Mock( + status_code=200, + json=Mock(return_value={"summary": "ok", "review_posted": True}), + ) + + await call_test_oracle(github_webhook=mock_github_webhook, pull_request=mock_pull_request) + + mock_client_cls.assert_called_once() + + @pytest.mark.asyncio + async def test_127_0_0_1_http_allowed(self, mock_github_webhook: Mock, mock_pull_request: Mock) -> None: + """Test that http://127.0.0.1 URLs are allowed.""" + mock_github_webhook.config.get_value = Mock( + return_value={ + "server-url": "http://127.0.0.1:8000", + "ai-provider": "claude", + "ai-model": "sonnet", + } + ) + + with patch("webhook_server.libs.test_oracle.httpx.AsyncClient") as mock_client_cls: + mock_client = AsyncMock() + mock_client_cls.return_value.__aenter__ = AsyncMock(return_value=mock_client) + mock_client_cls.return_value.__aexit__ = AsyncMock(return_value=False) + mock_client.get.return_value = Mock(status_code=200) + mock_client.post.return_value = Mock( + status_code=200, + json=Mock(return_value={"summary": "ok", "review_posted": True}), + ) + + await call_test_oracle(github_webhook=mock_github_webhook, pull_request=mock_pull_request) + + mock_client_cls.assert_called_once() + + @pytest.mark.asyncio + async def test_outer_exception_caught_and_logged(self, mock_github_webhook: Mock, mock_pull_request: Mock) -> None: + """Test that unexpected exceptions are caught by the outer try/except.""" + with patch("webhook_server.libs.test_oracle.httpx.AsyncClient") as mock_client_cls: + mock_client_cls.side_effect = RuntimeError("Unexpected failure") + + await call_test_oracle(github_webhook=mock_github_webhook, pull_request=mock_pull_request) + + mock_github_webhook.logger.exception.assert_called_once() + exc_msg = mock_github_webhook.logger.exception.call_args[0][0] + assert "failed unexpectedly" in exc_msg + + @pytest.mark.asyncio + async def test_cancelled_error_reraised(self, mock_github_webhook: Mock, mock_pull_request: Mock) -> None: + """Test that asyncio.CancelledError is re-raised, not swallowed.""" + with patch("webhook_server.libs.test_oracle.httpx.AsyncClient") as mock_client_cls: + mock_client_cls.side_effect = asyncio.CancelledError() + + with pytest.raises(asyncio.CancelledError): + await call_test_oracle(github_webhook=mock_github_webhook, pull_request=mock_pull_request) + + @pytest.mark.asyncio + async def test_health_comment_failure_logged(self, mock_github_webhook: Mock, mock_pull_request: Mock) -> None: + """Test that failure to post health check PR comment is logged.""" + with patch("webhook_server.libs.test_oracle.httpx.AsyncClient") as mock_client_cls: + mock_client = AsyncMock() + mock_client_cls.return_value.__aenter__ = AsyncMock(return_value=mock_client) + mock_client_cls.return_value.__aexit__ = AsyncMock(return_value=False) + mock_client.get.side_effect = httpx.ConnectError("Connection refused") + + with patch("asyncio.to_thread", new_callable=AsyncMock) as mock_to_thread: + mock_to_thread.side_effect = RuntimeError("GitHub API error") + + await call_test_oracle(github_webhook=mock_github_webhook, pull_request=mock_pull_request) + + mock_github_webhook.logger.exception.assert_called_once() + exc_msg = mock_github_webhook.logger.exception.call_args[0][0] + assert "Failed to post health check comment" in exc_msg + + @pytest.mark.asyncio + async def test_analyze_invalid_json_logged(self, mock_github_webhook: Mock, mock_pull_request: Mock) -> None: + """Test that invalid JSON response from analyze is logged.""" + with patch("webhook_server.libs.test_oracle.httpx.AsyncClient") as mock_client_cls: + mock_client = AsyncMock() + mock_client_cls.return_value.__aenter__ = AsyncMock(return_value=mock_client) + mock_client_cls.return_value.__aexit__ = AsyncMock(return_value=False) + + mock_client.get.return_value = Mock(status_code=200) + + mock_analyze_response = Mock() + mock_analyze_response.raise_for_status = Mock() + mock_analyze_response.json.side_effect = ValueError("Invalid JSON") + mock_client.post.return_value = mock_analyze_response + + await call_test_oracle(github_webhook=mock_github_webhook, pull_request=mock_pull_request) + + mock_github_webhook.logger.error.assert_called_once() + error_msg = mock_github_webhook.logger.error.call_args[0][0] + assert "invalid JSON" in error_msg From 338774a173a461e8dd531724473d2480045748c4 Mon Sep 17 00:00:00 2001 From: Meni Yakove Date: Sat, 28 Feb 2026 13:43:06 +0200 Subject: [PATCH 11/14] fix: address round 2 review feedback for test-oracle - Store asyncio.create_task references with _background_tasks set to prevent GC - Add test-oracle to global config reference table in README - Strengthen create_task test assertions to verify coroutine wiring - Fix missing APPROVE_STR import in issue_comment_handler --- README.md | 1 + .../libs/handlers/issue_comment_handler.py | 17 +++++++++- .../libs/handlers/pull_request_handler.py | 10 ++++-- .../handlers/pull_request_review_handler.py | 18 +++++----- .../tests/test_issue_comment_handler.py | 34 ++++++++++++++++++- .../tests/test_pull_request_handler.py | 6 ++-- .../tests/test_pull_request_review_handler.py | 3 +- 7 files changed, 74 insertions(+), 15 deletions(-) diff --git a/README.md b/README.md index 2f95e891..70e9415d 100644 --- a/README.md +++ b/README.md @@ -585,6 +585,7 @@ uv run pytest webhook_server/tests/test_config_schema.py::TestConfigSchema::test | **Security** | `webhook-secret`, `verify-github-ips`, `verify-cloudflare-ips`, `disable-ssl-warnings` | | **GitHub** | `github-app-id`, `github-tokens`, `webhook-ip` | | **Defaults** | `docker`, `default-status-checks`, `auto-verified-and-merged-users`, `branch-protection`, `create-issue-for-new-pr` | +| **AI** | [`test-oracle`](https://github.com/myk-org/pr-test-oracle) | #### Repository Level Options diff --git a/webhook_server/libs/handlers/issue_comment_handler.py b/webhook_server/libs/handlers/issue_comment_handler.py index 390a268e..34e94185 100644 --- a/webhook_server/libs/handlers/issue_comment_handler.py +++ b/webhook_server/libs/handlers/issue_comment_handler.py @@ -16,6 +16,7 @@ from webhook_server.libs.handlers.runner_handler import RunnerHandler from webhook_server.libs.test_oracle import call_test_oracle from webhook_server.utils.constants import ( + APPROVE_STR, AUTOMERGE_LABEL_STR, BUILD_AND_PUSH_CONTAINER_STR, CHERRY_PICK_LABEL_PREFIX, @@ -39,6 +40,8 @@ from webhook_server.libs.github_api import GithubWebhook from webhook_server.utils.context import WebhookContext +_background_tasks: set[asyncio.Task[None]] = set() + class IssueCommentHandler: def __init__(self, github_webhook: GithubWebhook, owners_file_handler: OwnersFileHandler): @@ -276,12 +279,14 @@ async def user_commands( await self.pull_request_handler.regenerate_welcome_message(pull_request=pull_request) elif _command == COMMAND_TEST_ORACLE_STR: - asyncio.create_task( + task = asyncio.create_task( call_test_oracle( github_webhook=self.github_webhook, pull_request=pull_request, ) ) + _background_tasks.add(task) + task.add_done_callback(_background_tasks.discard) elif _command == BUILD_AND_PUSH_CONTAINER_STR: if self.github_webhook.build_and_push_container: @@ -350,6 +355,16 @@ async def user_commands( remove=remove, reviewed_user=reviewed_user, ) + if _command == APPROVE_STR: + task = asyncio.create_task( + call_test_oracle( + github_webhook=self.github_webhook, + pull_request=pull_request, + trigger="approved", + ) + ) + _background_tasks.add(task) + task.add_done_callback(_background_tasks.discard) async def create_comment_reaction(self, pull_request: PullRequest, issue_comment_id: int, reaction: str) -> None: _comment = await asyncio.to_thread(pull_request.get_issue_comment, issue_comment_id) diff --git a/webhook_server/libs/handlers/pull_request_handler.py b/webhook_server/libs/handlers/pull_request_handler.py index a2400b86..512a504a 100644 --- a/webhook_server/libs/handlers/pull_request_handler.py +++ b/webhook_server/libs/handlers/pull_request_handler.py @@ -43,6 +43,8 @@ from webhook_server.libs.github_api import GithubWebhook from webhook_server.utils.context import WebhookContext +_background_tasks: set[asyncio.Task[None]] = set() + class PullRequestHandler: def __init__(self, github_webhook: GithubWebhook, owners_file_handler: OwnersFileHandler): @@ -103,13 +105,15 @@ async def process_pull_request_webhook_data(self, pull_request: PullRequest) -> await self.set_pull_request_automerge(pull_request=pull_request) if hook_action == "opened": - asyncio.create_task( + task = asyncio.create_task( call_test_oracle( github_webhook=self.github_webhook, pull_request=pull_request, trigger="pr-opened", ) ) + _background_tasks.add(task) + task.add_done_callback(_background_tasks.discard) if self.ctx: self.ctx.complete_step("pr_handler", action=hook_action) @@ -127,13 +131,15 @@ async def process_pull_request_webhook_data(self, pull_request: PullRequest) -> if isinstance(result, Exception): self.logger.error(f"{self.log_prefix} Async task failed: {result}") - asyncio.create_task( + task = asyncio.create_task( call_test_oracle( github_webhook=self.github_webhook, pull_request=pull_request, trigger="pr-synchronized", ) ) + _background_tasks.add(task) + task.add_done_callback(_background_tasks.discard) if self.ctx: self.ctx.complete_step("pr_handler", action=hook_action) diff --git a/webhook_server/libs/handlers/pull_request_review_handler.py b/webhook_server/libs/handlers/pull_request_review_handler.py index 1ac02094..fa30556e 100644 --- a/webhook_server/libs/handlers/pull_request_review_handler.py +++ b/webhook_server/libs/handlers/pull_request_review_handler.py @@ -14,6 +14,8 @@ from webhook_server.libs.github_api import GithubWebhook from webhook_server.utils.context import WebhookContext +_background_tasks: set[asyncio.Task[None]] = set() + class PullRequestReviewHandler: def __init__(self, github_webhook: GithubWebhook, owners_file_handler: OwnersFileHandler) -> None: @@ -62,15 +64,15 @@ async def process_pull_request_review_webhook_data(self, pull_request: PullReque remove=False, reviewed_user=reviewed_user, ) - - if review_state == "approved": - asyncio.create_task( - call_test_oracle( - github_webhook=self.github_webhook, - pull_request=pull_request, - trigger="approved", + task = asyncio.create_task( + call_test_oracle( + github_webhook=self.github_webhook, + pull_request=pull_request, + trigger="approved", + ) ) - ) + _background_tasks.add(task) + task.add_done_callback(_background_tasks.discard) finally: if self.ctx: self.ctx.complete_step("pr_review_handler") diff --git a/webhook_server/tests/test_issue_comment_handler.py b/webhook_server/tests/test_issue_comment_handler.py index 39ff80cc..4ca6efb1 100644 --- a/webhook_server/tests/test_issue_comment_handler.py +++ b/webhook_server/tests/test_issue_comment_handler.py @@ -6,6 +6,7 @@ from webhook_server.libs.handlers.issue_comment_handler import IssueCommentHandler from webhook_server.utils.constants import ( + APPROVE_STR, BUILD_AND_PUSH_CONTAINER_STR, COMMAND_ASSIGN_REVIEWER_STR, COMMAND_ASSIGN_REVIEWERS_STR, @@ -1437,6 +1438,7 @@ async def test_test_oracle_command(self, issue_comment_handler: IssueCommentHand with patch.object(issue_comment_handler, "create_comment_reaction", new_callable=AsyncMock): with patch( "webhook_server.libs.handlers.issue_comment_handler.call_test_oracle", + new_callable=Mock, ) as mock_oracle: with patch("asyncio.create_task") as mock_create_task: await issue_comment_handler.user_commands( @@ -1450,4 +1452,34 @@ async def test_test_oracle_command(self, issue_comment_handler: IssueCommentHand github_webhook=issue_comment_handler.github_webhook, pull_request=mock_pull_request, ) - mock_create_task.assert_called_once() + mock_create_task.assert_called_once_with(mock_oracle.return_value) + + @pytest.mark.asyncio + async def test_approve_command_calls_test_oracle(self, issue_comment_handler: IssueCommentHandler) -> None: + """Test that /approve command fires call_test_oracle with trigger='approved' as a background task.""" + mock_pull_request = Mock() + mock_pull_request.draft = False + + with patch("asyncio.to_thread", new_callable=AsyncMock, side_effect=lambda f, *a, **k: f(*a, **k)): + with patch.object(issue_comment_handler, "create_comment_reaction", new_callable=AsyncMock): + with patch.object( + issue_comment_handler.labels_handler, "label_by_user_comment", new_callable=AsyncMock + ): + with patch( + "webhook_server.libs.handlers.issue_comment_handler.call_test_oracle", + new_callable=Mock, + ) as mock_oracle: + with patch("asyncio.create_task") as mock_create_task: + await issue_comment_handler.user_commands( + pull_request=mock_pull_request, + command=APPROVE_STR, + reviewed_user="test-user", + issue_comment_id=456, + is_draft=False, + ) + mock_oracle.assert_called_once_with( + github_webhook=issue_comment_handler.github_webhook, + pull_request=mock_pull_request, + trigger="approved", + ) + mock_create_task.assert_called_once_with(mock_oracle.return_value) diff --git a/webhook_server/tests/test_pull_request_handler.py b/webhook_server/tests/test_pull_request_handler.py index e56b7b4e..52436815 100644 --- a/webhook_server/tests/test_pull_request_handler.py +++ b/webhook_server/tests/test_pull_request_handler.py @@ -2663,6 +2663,7 @@ async def test_process_opened_action_calls_test_oracle_with_pr_opened_trigger( patch.object(pull_request_handler, "set_pull_request_automerge"), patch( "webhook_server.libs.handlers.pull_request_handler.call_test_oracle", + new_callable=Mock, ) as mock_test_oracle, patch("asyncio.create_task") as mock_create_task, ): @@ -2672,7 +2673,7 @@ async def test_process_opened_action_calls_test_oracle_with_pr_opened_trigger( pull_request=mock_pull_request, trigger="pr-opened", ) - mock_create_task.assert_called_once() + mock_create_task.assert_called_once_with(mock_test_oracle.return_value) @pytest.mark.asyncio async def test_process_synchronize_action_calls_test_oracle_with_pr_synchronized_trigger( @@ -2689,6 +2690,7 @@ async def test_process_synchronize_action_calls_test_oracle_with_pr_synchronized patch.object(pull_request_handler, "remove_labels_when_pull_request_sync"), patch( "webhook_server.libs.handlers.pull_request_handler.call_test_oracle", + new_callable=Mock, ) as mock_test_oracle, patch("asyncio.create_task") as mock_create_task, ): @@ -2698,7 +2700,7 @@ async def test_process_synchronize_action_calls_test_oracle_with_pr_synchronized pull_request=mock_pull_request, trigger="pr-synchronized", ) - mock_create_task.assert_called_once() + mock_create_task.assert_called_once_with(mock_test_oracle.return_value) @pytest.mark.asyncio async def test_process_reopened_action_does_not_call_test_oracle( diff --git a/webhook_server/tests/test_pull_request_review_handler.py b/webhook_server/tests/test_pull_request_review_handler.py index 8858f92d..8c48c247 100644 --- a/webhook_server/tests/test_pull_request_review_handler.py +++ b/webhook_server/tests/test_pull_request_review_handler.py @@ -308,6 +308,7 @@ async def test_calls_test_oracle_on_approval(self, pull_request_review_handler: ): with patch( "webhook_server.libs.handlers.pull_request_review_handler.call_test_oracle", + new_callable=Mock, ) as mock_oracle: with patch("asyncio.create_task") as mock_create_task: await pull_request_review_handler.process_pull_request_review_webhook_data(mock_pull_request) @@ -317,7 +318,7 @@ async def test_calls_test_oracle_on_approval(self, pull_request_review_handler: pull_request=mock_pull_request, trigger="approved", ) - mock_create_task.assert_called_once() + mock_create_task.assert_called_once_with(mock_oracle.return_value) @pytest.mark.asyncio async def test_does_not_call_test_oracle_on_non_approval( From 742b424bd2c8d612230f3be7b92f123c4a53f9cc Mon Sep 17 00:00:00 2001 From: Meni Yakove Date: Sat, 28 Feb 2026 13:56:32 +0200 Subject: [PATCH 12/14] fix: remove URL scheme validation from test oracle The user controls the server URL via config - no need to second-guess their network setup. --- webhook_server/libs/test_oracle.py | 10 --- webhook_server/tests/test_test_oracle.py | 86 ------------------------ 2 files changed, 96 deletions(-) diff --git a/webhook_server/libs/test_oracle.py b/webhook_server/libs/test_oracle.py index bc0d6149..9fb2ed80 100644 --- a/webhook_server/libs/test_oracle.py +++ b/webhook_server/libs/test_oracle.py @@ -2,7 +2,6 @@ import asyncio from typing import TYPE_CHECKING, Any -from urllib.parse import urlparse import httpx @@ -42,15 +41,6 @@ async def call_test_oracle( server_url: str = config["server-url"] log_prefix: str = github_webhook.log_prefix - # Secure transport check - parsed_url = urlparse(server_url) - is_local = parsed_url.hostname in {"localhost", "127.0.0.1", "::1"} - if parsed_url.scheme != "https" and not is_local: - github_webhook.logger.error( - f"{log_prefix} Insecure test-oracle server-url '{server_url}'. Use https or localhost http." - ) - return - try: async with httpx.AsyncClient(base_url=server_url) as client: # Health check diff --git a/webhook_server/tests/test_test_oracle.py b/webhook_server/tests/test_test_oracle.py index e3157310..cffc7961 100644 --- a/webhook_server/tests/test_test_oracle.py +++ b/webhook_server/tests/test_test_oracle.py @@ -273,92 +273,6 @@ async def test_no_trigger_param_skips_trigger_check( ) mock_client.post.assert_called_once() - @pytest.mark.asyncio - async def test_insecure_url_rejected(self, mock_github_webhook: Mock, mock_pull_request: Mock) -> None: - """Test that non-localhost http URLs are rejected.""" - mock_github_webhook.config.get_value = Mock( - return_value={ - "server-url": "http://remote-server.example.com:8000", - "ai-provider": "claude", - "ai-model": "sonnet", - } - ) - - with patch("webhook_server.libs.test_oracle.httpx.AsyncClient") as mock_client_cls: - await call_test_oracle(github_webhook=mock_github_webhook, pull_request=mock_pull_request) - - mock_client_cls.assert_not_called() - mock_github_webhook.logger.error.assert_called_once() - error_msg = mock_github_webhook.logger.error.call_args[0][0] - assert "Insecure" in error_msg - - @pytest.mark.asyncio - async def test_https_url_allowed(self, mock_github_webhook: Mock, mock_pull_request: Mock) -> None: - """Test that https URLs are always allowed.""" - mock_github_webhook.config.get_value = Mock( - return_value={ - "server-url": "https://remote-server.example.com:8000", - "ai-provider": "claude", - "ai-model": "sonnet", - } - ) - - with patch("webhook_server.libs.test_oracle.httpx.AsyncClient") as mock_client_cls: - mock_client = AsyncMock() - mock_client_cls.return_value.__aenter__ = AsyncMock(return_value=mock_client) - mock_client_cls.return_value.__aexit__ = AsyncMock(return_value=False) - mock_client.get.return_value = Mock(status_code=200) - mock_client.post.return_value = Mock( - status_code=200, - json=Mock(return_value={"summary": "ok", "review_posted": True}), - ) - - await call_test_oracle(github_webhook=mock_github_webhook, pull_request=mock_pull_request) - - mock_client_cls.assert_called_once() - - @pytest.mark.asyncio - async def test_localhost_http_allowed(self, mock_github_webhook: Mock, mock_pull_request: Mock) -> None: - """Test that http://localhost URLs are allowed.""" - with patch("webhook_server.libs.test_oracle.httpx.AsyncClient") as mock_client_cls: - mock_client = AsyncMock() - mock_client_cls.return_value.__aenter__ = AsyncMock(return_value=mock_client) - mock_client_cls.return_value.__aexit__ = AsyncMock(return_value=False) - mock_client.get.return_value = Mock(status_code=200) - mock_client.post.return_value = Mock( - status_code=200, - json=Mock(return_value={"summary": "ok", "review_posted": True}), - ) - - await call_test_oracle(github_webhook=mock_github_webhook, pull_request=mock_pull_request) - - mock_client_cls.assert_called_once() - - @pytest.mark.asyncio - async def test_127_0_0_1_http_allowed(self, mock_github_webhook: Mock, mock_pull_request: Mock) -> None: - """Test that http://127.0.0.1 URLs are allowed.""" - mock_github_webhook.config.get_value = Mock( - return_value={ - "server-url": "http://127.0.0.1:8000", - "ai-provider": "claude", - "ai-model": "sonnet", - } - ) - - with patch("webhook_server.libs.test_oracle.httpx.AsyncClient") as mock_client_cls: - mock_client = AsyncMock() - mock_client_cls.return_value.__aenter__ = AsyncMock(return_value=mock_client) - mock_client_cls.return_value.__aexit__ = AsyncMock(return_value=False) - mock_client.get.return_value = Mock(status_code=200) - mock_client.post.return_value = Mock( - status_code=200, - json=Mock(return_value={"summary": "ok", "review_posted": True}), - ) - - await call_test_oracle(github_webhook=mock_github_webhook, pull_request=mock_pull_request) - - mock_client_cls.assert_called_once() - @pytest.mark.asyncio async def test_outer_exception_caught_and_logged(self, mock_github_webhook: Mock, mock_pull_request: Mock) -> None: """Test that unexpected exceptions are caught by the outer try/except.""" From 58231ef123e03b9715740aec8fc994b07ee45a2b Mon Sep 17 00:00:00 2001 From: Meni Yakove Date: Sat, 28 Feb 2026 14:06:52 +0200 Subject: [PATCH 13/14] fix: address round 3 review feedback for test-oracle - Use AsyncMock for call_test_oracle patches in all test files - Add test verifying GitHub approval without /approve does NOT trigger oracle - Add inline comments explaining /approve flow and token usage --- .../handlers/pull_request_review_handler.py | 3 ++ webhook_server/libs/test_oracle.py | 2 ++ .../tests/test_issue_comment_handler.py | 10 ++++--- .../tests/test_pull_request_handler.py | 10 ++++--- .../tests/test_pull_request_review_handler.py | 28 +++++++++++++++++-- 5 files changed, 43 insertions(+), 10 deletions(-) diff --git a/webhook_server/libs/handlers/pull_request_review_handler.py b/webhook_server/libs/handlers/pull_request_review_handler.py index fa30556e..a316c02c 100644 --- a/webhook_server/libs/handlers/pull_request_review_handler.py +++ b/webhook_server/libs/handlers/pull_request_review_handler.py @@ -57,6 +57,9 @@ async def process_pull_request_review_webhook_data(self, pull_request: PullReque if body := self.hook_data["review"]["body"]: self.github_webhook.logger.debug(f"{self.github_webhook.log_prefix} Found review body: {body}") + # In this project, "approved" means a maintainer uses the /approve command + # (which adds an approved- label), NOT GitHub's review approval state. + # The oracle trigger fires only when /approve is found in the review body. if f"/{APPROVE_STR}" in body: await self.labels_handler.label_by_user_comment( pull_request=pull_request, diff --git a/webhook_server/libs/test_oracle.py b/webhook_server/libs/test_oracle.py index 9fb2ed80..96fd9658 100644 --- a/webhook_server/libs/test_oracle.py +++ b/webhook_server/libs/test_oracle.py @@ -65,6 +65,8 @@ async def call_test_oracle( "pr_url": pull_request.html_url, "ai_provider": config["ai-provider"], "ai_model": config["ai-model"], + # Token is required by the oracle server to fetch PR data and post reviews. + # Server URL is configured by the admin - they control the network setup. "github_token": github_webhook.token, } diff --git a/webhook_server/tests/test_issue_comment_handler.py b/webhook_server/tests/test_issue_comment_handler.py index 4ca6efb1..45ccd992 100644 --- a/webhook_server/tests/test_issue_comment_handler.py +++ b/webhook_server/tests/test_issue_comment_handler.py @@ -1438,7 +1438,7 @@ async def test_test_oracle_command(self, issue_comment_handler: IssueCommentHand with patch.object(issue_comment_handler, "create_comment_reaction", new_callable=AsyncMock): with patch( "webhook_server.libs.handlers.issue_comment_handler.call_test_oracle", - new_callable=Mock, + new_callable=AsyncMock, ) as mock_oracle: with patch("asyncio.create_task") as mock_create_task: await issue_comment_handler.user_commands( @@ -1452,7 +1452,8 @@ async def test_test_oracle_command(self, issue_comment_handler: IssueCommentHand github_webhook=issue_comment_handler.github_webhook, pull_request=mock_pull_request, ) - mock_create_task.assert_called_once_with(mock_oracle.return_value) + mock_create_task.assert_called_once() + assert asyncio.iscoroutine(mock_create_task.call_args.args[0]) @pytest.mark.asyncio async def test_approve_command_calls_test_oracle(self, issue_comment_handler: IssueCommentHandler) -> None: @@ -1467,7 +1468,7 @@ async def test_approve_command_calls_test_oracle(self, issue_comment_handler: Is ): with patch( "webhook_server.libs.handlers.issue_comment_handler.call_test_oracle", - new_callable=Mock, + new_callable=AsyncMock, ) as mock_oracle: with patch("asyncio.create_task") as mock_create_task: await issue_comment_handler.user_commands( @@ -1482,4 +1483,5 @@ async def test_approve_command_calls_test_oracle(self, issue_comment_handler: Is pull_request=mock_pull_request, trigger="approved", ) - mock_create_task.assert_called_once_with(mock_oracle.return_value) + mock_create_task.assert_called_once() + assert asyncio.iscoroutine(mock_create_task.call_args.args[0]) diff --git a/webhook_server/tests/test_pull_request_handler.py b/webhook_server/tests/test_pull_request_handler.py index 52436815..8be5eef1 100644 --- a/webhook_server/tests/test_pull_request_handler.py +++ b/webhook_server/tests/test_pull_request_handler.py @@ -2663,7 +2663,7 @@ async def test_process_opened_action_calls_test_oracle_with_pr_opened_trigger( patch.object(pull_request_handler, "set_pull_request_automerge"), patch( "webhook_server.libs.handlers.pull_request_handler.call_test_oracle", - new_callable=Mock, + new_callable=AsyncMock, ) as mock_test_oracle, patch("asyncio.create_task") as mock_create_task, ): @@ -2673,7 +2673,8 @@ async def test_process_opened_action_calls_test_oracle_with_pr_opened_trigger( pull_request=mock_pull_request, trigger="pr-opened", ) - mock_create_task.assert_called_once_with(mock_test_oracle.return_value) + mock_create_task.assert_called_once() + assert asyncio.iscoroutine(mock_create_task.call_args.args[0]) @pytest.mark.asyncio async def test_process_synchronize_action_calls_test_oracle_with_pr_synchronized_trigger( @@ -2690,7 +2691,7 @@ async def test_process_synchronize_action_calls_test_oracle_with_pr_synchronized patch.object(pull_request_handler, "remove_labels_when_pull_request_sync"), patch( "webhook_server.libs.handlers.pull_request_handler.call_test_oracle", - new_callable=Mock, + new_callable=AsyncMock, ) as mock_test_oracle, patch("asyncio.create_task") as mock_create_task, ): @@ -2700,7 +2701,8 @@ async def test_process_synchronize_action_calls_test_oracle_with_pr_synchronized pull_request=mock_pull_request, trigger="pr-synchronized", ) - mock_create_task.assert_called_once_with(mock_test_oracle.return_value) + mock_create_task.assert_called_once() + assert asyncio.iscoroutine(mock_create_task.call_args.args[0]) @pytest.mark.asyncio async def test_process_reopened_action_does_not_call_test_oracle( diff --git a/webhook_server/tests/test_pull_request_review_handler.py b/webhook_server/tests/test_pull_request_review_handler.py index 8c48c247..f8b47756 100644 --- a/webhook_server/tests/test_pull_request_review_handler.py +++ b/webhook_server/tests/test_pull_request_review_handler.py @@ -1,5 +1,6 @@ """Tests for webhook_server.libs.handlers.pull_request_review_handler module.""" +import asyncio from unittest.mock import AsyncMock, Mock, patch import pytest @@ -308,7 +309,7 @@ async def test_calls_test_oracle_on_approval(self, pull_request_review_handler: ): with patch( "webhook_server.libs.handlers.pull_request_review_handler.call_test_oracle", - new_callable=Mock, + new_callable=AsyncMock, ) as mock_oracle: with patch("asyncio.create_task") as mock_create_task: await pull_request_review_handler.process_pull_request_review_webhook_data(mock_pull_request) @@ -318,7 +319,30 @@ async def test_calls_test_oracle_on_approval(self, pull_request_review_handler: pull_request=mock_pull_request, trigger="approved", ) - mock_create_task.assert_called_once_with(mock_oracle.return_value) + mock_create_task.assert_called_once() + assert asyncio.iscoroutine(mock_create_task.call_args.args[0]) + + @pytest.mark.asyncio + async def test_does_not_call_test_oracle_on_github_approval_without_approve_command( + self, pull_request_review_handler: PullRequestReviewHandler + ) -> None: + """Test that GitHub review approval without /approve command does NOT trigger oracle.""" + mock_pull_request = Mock(spec=PullRequest) + pull_request_review_handler.hook_data["review"]["state"] = "approved" + pull_request_review_handler.hook_data["review"]["body"] = "" + + with patch.object( + pull_request_review_handler.labels_handler, "manage_reviewed_by_label", new_callable=AsyncMock + ): + with patch.object( + pull_request_review_handler.labels_handler, "label_by_user_comment", new_callable=AsyncMock + ): + with patch( + "webhook_server.libs.handlers.pull_request_review_handler.call_test_oracle", + new_callable=AsyncMock, + ) as mock_oracle: + await pull_request_review_handler.process_pull_request_review_webhook_data(mock_pull_request) + mock_oracle.assert_not_called() @pytest.mark.asyncio async def test_does_not_call_test_oracle_on_non_approval( From 22627265a07191d8bd456d48f4bcf8a5fd7729e0 Mon Sep 17 00:00:00 2001 From: Meni Yakove Date: Sat, 28 Feb 2026 14:20:27 +0200 Subject: [PATCH 14/14] fix: address round 4 review feedback for test-oracle - Use exact line-based matching for /approve in review body - Wrap pull_request.html_url with asyncio.to_thread per async rules - Clarify DEFAULT_TRIGGERS and docstring: "approved" means /approve command --- .../libs/handlers/pull_request_review_handler.py | 2 +- webhook_server/libs/test_oracle.py | 11 +++++++---- .../tests/test_pull_request_review_handler.py | 4 ++-- webhook_server/tests/test_test_oracle.py | 6 ++++-- 4 files changed, 14 insertions(+), 9 deletions(-) diff --git a/webhook_server/libs/handlers/pull_request_review_handler.py b/webhook_server/libs/handlers/pull_request_review_handler.py index a316c02c..8870b5e2 100644 --- a/webhook_server/libs/handlers/pull_request_review_handler.py +++ b/webhook_server/libs/handlers/pull_request_review_handler.py @@ -60,7 +60,7 @@ async def process_pull_request_review_webhook_data(self, pull_request: PullReque # In this project, "approved" means a maintainer uses the /approve command # (which adds an approved- label), NOT GitHub's review approval state. # The oracle trigger fires only when /approve is found in the review body. - if f"/{APPROVE_STR}" in body: + if any(line.strip() == f"/{APPROVE_STR}" for line in body.splitlines()): await self.labels_handler.label_by_user_comment( pull_request=pull_request, user_requested_label=APPROVE_STR, diff --git a/webhook_server/libs/test_oracle.py b/webhook_server/libs/test_oracle.py index 96fd9658..be88c8d9 100644 --- a/webhook_server/libs/test_oracle.py +++ b/webhook_server/libs/test_oracle.py @@ -10,6 +10,7 @@ from webhook_server.libs.github_api import GithubWebhook +# "approved" refers to the /approve command trigger, not GitHub's review approval state. DEFAULT_TRIGGERS: list[str] = ["approved"] @@ -23,8 +24,9 @@ async def call_test_oracle( Args: github_webhook: The GithubWebhook instance with config and token. pull_request: The PyGithub PullRequest object. - trigger: The event trigger (e.g., "approved", "pr-opened"). None means - command-triggered (always runs if configured). + trigger: The event trigger (e.g., "approved", "pr-opened"). + "approved" means the /approve command, not GitHub review state. + None means command-triggered (always runs if configured). """ config: dict[str, Any] | None = github_webhook.config.get_value("test-oracle") if not config: @@ -61,8 +63,9 @@ async def call_test_oracle( return # Build analyze payload + pr_url: str = await asyncio.to_thread(lambda: pull_request.html_url) payload: dict[str, Any] = { - "pr_url": pull_request.html_url, + "pr_url": pr_url, "ai_provider": config["ai-provider"], "ai_model": config["ai-model"], # Token is required by the oracle server to fetch PR data and post reviews. @@ -75,7 +78,7 @@ async def call_test_oracle( # Call analyze try: - github_webhook.logger.info(f"{log_prefix} Calling Test Oracle for {pull_request.html_url}") + github_webhook.logger.info(f"{log_prefix} Calling Test Oracle for {pr_url}") response = await client.post("/analyze", json=payload, timeout=300.0) response.raise_for_status() diff --git a/webhook_server/tests/test_pull_request_review_handler.py b/webhook_server/tests/test_pull_request_review_handler.py index f8b47756..a6d95e33 100644 --- a/webhook_server/tests/test_pull_request_review_handler.py +++ b/webhook_server/tests/test_pull_request_review_handler.py @@ -19,7 +19,7 @@ def mock_github_webhook(self) -> Mock: mock_webhook = Mock() mock_webhook.hook_data = { "action": "submitted", - "review": {"user": {"login": "test-reviewer"}, "state": "approved", "body": "Great work! /approve"}, + "review": {"user": {"login": "test-reviewer"}, "state": "approved", "body": "Great work!\n/approve"}, } mock_webhook.logger = Mock() mock_webhook.log_prefix = "[TEST]" @@ -262,7 +262,7 @@ async def test_process_pull_request_review_webhook_data_exact_approve_match( """Test processing pull request review webhook data with exact /approve match.""" mock_pull_request = Mock(spec=PullRequest) - test_bodies = ["/approve", "Great work! /approve", "LGTM /approve thanks", "/approve this looks good"] + test_bodies = ["/approve", "Great work!\n/approve", "LGTM\n/approve\nthanks", " /approve "] for body in test_bodies: pull_request_review_handler.hook_data["review"]["body"] = body diff --git a/webhook_server/tests/test_test_oracle.py b/webhook_server/tests/test_test_oracle.py index cffc7961..b4d9599a 100644 --- a/webhook_server/tests/test_test_oracle.py +++ b/webhook_server/tests/test_test_oracle.py @@ -144,7 +144,8 @@ async def test_analyze_error_logs_only(self, mock_github_webhook: Mock, mock_pul with patch("asyncio.to_thread", new_callable=AsyncMock) as mock_to_thread: await call_test_oracle(github_webhook=mock_github_webhook, pull_request=mock_pull_request) - mock_to_thread.assert_not_called() + # asyncio.to_thread is called once for pull_request.html_url, but not for posting a comment + assert mock_to_thread.call_count == 1 mock_github_webhook.logger.error.assert_called() @pytest.mark.asyncio @@ -161,7 +162,8 @@ async def test_analyze_network_error_logs_only(self, mock_github_webhook: Mock, with patch("asyncio.to_thread", new_callable=AsyncMock) as mock_to_thread: await call_test_oracle(github_webhook=mock_github_webhook, pull_request=mock_pull_request) - mock_to_thread.assert_not_called() + # asyncio.to_thread is called once for pull_request.html_url, but not for posting a comment + assert mock_to_thread.call_count == 1 mock_github_webhook.logger.error.assert_called() @pytest.mark.asyncio