diff --git a/services/webhook/successful_check_suite_handler.py b/services/webhook/successful_check_suite_handler.py index b28c5e454..b49e69fb1 100644 --- a/services/webhook/successful_check_suite_handler.py +++ b/services/webhook/successful_check_suite_handler.py @@ -1,5 +1,6 @@ from typing import cast +from config import GITHUB_APP_USER_NAME from services.github.comments.create_comment import create_comment from services.github.commits.check_commit_has_skip_ci import check_commit_has_skip_ci from services.github.commits.create_empty_commit import create_empty_commit @@ -27,6 +28,7 @@ def handle_successful_check_suite(payload: CheckSuiteCompletedPayload): pull_request = pull_requests[0] pr_number = pull_request["number"] + pr_author = pull_request["user"]["login"] # Get repository info repo = payload["repository"] @@ -62,6 +64,12 @@ def handle_successful_check_suite(payload: CheckSuiteCompletedPayload): print(msg) return + # Only auto-merge PRs created by GitAuto + if pr_author != GITHUB_APP_USER_NAME: + msg = f"Auto-merge skipped: PR #{pr_number} not created by GitAuto (author: {pr_author})" + print(msg) + return + # Get installation token installation_id = payload["installation"]["id"] token = get_installation_access_token(installation_id=installation_id) diff --git a/services/webhook/test_successful_check_suite_handler.py b/services/webhook/test_successful_check_suite_handler.py index 97f0d746b..303f10149 100644 --- a/services/webhook/test_successful_check_suite_handler.py +++ b/services/webhook/test_successful_check_suite_handler.py @@ -18,6 +18,7 @@ def test_handle_successful_check_suite_with_pr(): "url": "https://api.github.com/repos/test-owner/test-repo/pulls/123", "id": 2131041354, "number": 123, + "user": {"login": "test-user", "id": 12345}, } ], }, @@ -112,6 +113,7 @@ def test_handle_successful_check_suite_no_usage_record_found(): "url": "https://api.github.com/repos/test-owner/test-repo/pulls/123", "id": 2131041354, "number": 123, + "user": {"login": "test-user", "id": 12345}, } ], }, @@ -165,6 +167,7 @@ def test_handle_successful_check_suite_with_exception(): "url": "https://api.github.com/repos/test-owner/test-repo/pulls/123", "id": 2131041354, "number": 123, + "user": {"login": "test-user", "id": 12345}, } ], }, @@ -193,6 +196,10 @@ def test_handle_successful_check_suite_with_exception(): assert result is None +@patch( + "services.webhook.successful_check_suite_handler.GITHUB_APP_USER_NAME", + "gitauto[bot]", +) @patch("services.webhook.successful_check_suite_handler.check_commit_has_skip_ci") @patch("services.webhook.successful_check_suite_handler.merge_pull_request") @patch("services.webhook.successful_check_suite_handler.get_pull_request_files") @@ -221,7 +228,7 @@ def test_auto_merge_success( "number": 123, "title": "Test PR", "body": "Test PR body", - "user": {"login": "test-user", "id": 12345}, + "user": {"login": "gitauto[bot]", "id": 12345}, "head": {"ref": "feature-branch", "sha": "abc123"}, "base": {"ref": "main", "sha": "def456"}, "mergeable": True, @@ -292,17 +299,9 @@ def test_auto_merge_success( ) -@patch("services.webhook.successful_check_suite_handler.check_commit_has_skip_ci") -@patch("services.webhook.successful_check_suite_handler.merge_pull_request") -@patch("services.webhook.successful_check_suite_handler.get_pull_request_files") -@patch("services.webhook.successful_check_suite_handler.get_installation_access_token") @patch("services.webhook.successful_check_suite_handler.get_repository_features") def test_auto_merge_disabled( mock_get_repo_features, - mock_get_token, - mock_get_files, - mock_merge_pr, - mock_check_skip_ci, ): payload = { "check_suite": { @@ -316,6 +315,7 @@ def test_auto_merge_disabled( "url": "https://api.github.com/repos/test-owner/test-repo/pulls/123", "id": 2131041354, "number": 123, + "user": {"login": "test-user", "id": 12345}, } ], }, @@ -359,9 +359,11 @@ def test_auto_merge_disabled( handle_successful_check_suite(cast(CheckSuiteCompletedPayload, payload)) - mock_merge_pr.assert_not_called() - +@patch( + "services.webhook.successful_check_suite_handler.GITHUB_APP_USER_NAME", + "gitauto[bot]", +) @patch("services.webhook.successful_check_suite_handler.check_commit_has_skip_ci") @patch("services.webhook.successful_check_suite_handler.merge_pull_request") @patch("services.webhook.successful_check_suite_handler.get_pull_request_files") @@ -390,7 +392,7 @@ def test_auto_merge_multiple_test_files_changed( "number": 123, "title": "Test PR", "body": "Test PR body", - "user": {"login": "test-user", "id": 12345}, + "user": {"login": "gitauto[bot]", "id": 12345}, "head": {"ref": "feature-branch", "sha": "abc123"}, "base": {"ref": "main", "sha": "def456"}, "mergeable": True, @@ -456,6 +458,10 @@ def test_auto_merge_multiple_test_files_changed( mock_merge_pr.assert_called_once() +@patch( + "services.webhook.successful_check_suite_handler.GITHUB_APP_USER_NAME", + "gitauto[bot]", +) @patch("services.webhook.successful_check_suite_handler.merge_pull_request") @patch("services.webhook.successful_check_suite_handler.get_pull_request_files") @patch("services.webhook.successful_check_suite_handler.get_installation_access_token") @@ -478,6 +484,7 @@ def test_auto_merge_mixed_test_and_non_test_files( "url": "https://api.github.com/repos/test-owner/test-repo/pulls/123", "id": 2131041354, "number": 123, + "user": {"login": "gitauto[bot]", "id": 12345}, } ], }, @@ -538,6 +545,10 @@ def is_test_side_effect(filename): mock_merge_pr.assert_not_called() +@patch( + "services.webhook.successful_check_suite_handler.GITHUB_APP_USER_NAME", + "gitauto[bot]", +) @patch("services.webhook.successful_check_suite_handler.check_commit_has_skip_ci") @patch("services.webhook.successful_check_suite_handler.merge_pull_request") @patch("services.webhook.successful_check_suite_handler.get_pull_request_files") @@ -566,7 +577,7 @@ def test_auto_merge_with_non_test_files_allowed( "number": 123, "title": "Test PR", "body": "Test PR body", - "user": {"login": "test-user", "id": 12345}, + "user": {"login": "gitauto[bot]", "id": 12345}, "head": {"ref": "feature-branch", "sha": "abc123"}, "base": {"ref": "main", "sha": "def456"}, "mergeable": True, @@ -632,6 +643,91 @@ def test_auto_merge_with_non_test_files_allowed( mock_merge_pr.assert_called_once() +@patch("services.webhook.successful_check_suite_handler.merge_pull_request") +@patch("services.webhook.successful_check_suite_handler.get_installation_access_token") +@patch("services.webhook.successful_check_suite_handler.get_repository_features") +def test_auto_merge_skipped_for_human_pr( + mock_get_repo_features, + mock_get_token, + mock_merge_pr, +): + payload = { + "check_suite": { + "id": 31710113401, + "name": "test-job", + "conclusion": "success", + "head_sha": "abc123", + "head_branch": "feature-branch", + "pull_requests": [ + { + "url": "https://api.github.com/repos/test-owner/test-repo/pulls/123", + "id": 2131041354, + "number": 123, + "title": "Human PR", + "body": "This is a human-created PR", + "user": {"login": "human-developer", "id": 98765}, + "head": {"ref": "feature-branch", "sha": "abc123"}, + "base": {"ref": "main", "sha": "def456"}, + "mergeable": True, + "mergeable_state": "clean", + } + ], + }, + "repository": { + "id": 871345449, + "name": "test-repo", + "owner": { + "id": 4620828, + "login": "test-owner", + "type": "Organization", + }, + "clone_url": "https://github.com/test-owner/test-repo.git", + "fork": False, + }, + "installation": {"id": 12345}, + "sender": {"id": 98765, "login": "human-developer"}, + } + + mock_get_repo_features.return_value = { + "auto_merge": True, + "merge_method": "squash", + } + mock_get_token.return_value = "test-token" + + with patch( + "services.webhook.successful_check_suite_handler.supabase" + ) as mock_supabase: + mock_table = MagicMock() + mock_select = MagicMock() + mock_eq1 = MagicMock() + mock_eq2 = MagicMock() + mock_eq3 = MagicMock() + mock_order = MagicMock() + mock_limit = MagicMock() + + mock_supabase.table.return_value = mock_table + mock_table.select.return_value = mock_select + mock_select.eq.return_value = mock_eq1 + mock_eq1.eq.return_value = mock_eq2 + mock_eq2.eq.return_value = mock_eq3 + mock_eq3.order.return_value = mock_order + mock_order.limit.return_value = mock_limit + mock_limit.execute.return_value = MagicMock(data=[{"id": 100}]) + + mock_update = MagicMock() + mock_update_eq = MagicMock() + mock_table.update.return_value = mock_update + mock_update.eq.return_value = mock_update_eq + + handle_successful_check_suite(cast(CheckSuiteCompletedPayload, payload)) + + mock_merge_pr.assert_not_called() + + +@patch( + "services.webhook.successful_check_suite_handler.GITHUB_APP_USER_NAME", + "gitauto[bot]", +) @patch("services.webhook.successful_check_suite_handler.create_empty_commit") @patch("services.webhook.successful_check_suite_handler.create_comment") @patch("services.webhook.successful_check_suite_handler.check_commit_has_skip_ci") @@ -656,6 +752,7 @@ def test_auto_merge_blocked_skip_ci( "url": "https://api.github.com/repos/test-owner/test-repo/pulls/123", "id": 2131041354, "number": 123, + "user": {"login": "gitauto[bot]", "id": 12345}, } ], },