mirror of
https://github.com/Sea-Haven-Industries/open-swe.git
synced 2026-09-30 06:53:14 +00:00
fix: block open-swe from approving PRs (#1177)
* fix: block open-swe from approving PRs * linting * fix: add case-insensitive APPROVE guard and unit tests
This commit is contained in:
parent
4d4f5fbfc7
commit
67c782c295
2 changed files with 55 additions and 3 deletions
|
|
@ -93,8 +93,8 @@ def create_pr_review(
|
|||
|
||||
Args:
|
||||
pull_number: The PR number to review.
|
||||
body: The review body text (required for APPROVE/REQUEST_CHANGES, optional for COMMENT).
|
||||
event: The review action - one of APPROVE, REQUEST_CHANGES, or COMMENT.
|
||||
body: The review body text (required for REQUEST_CHANGES, optional for COMMENT).
|
||||
event: The review action - one of REQUEST_CHANGES or COMMENT. APPROVE is not allowed.
|
||||
comments: Optional list of review comments. Each comment dict should have:
|
||||
- path (str): The relative file path to comment on.
|
||||
- body (str): The comment text.
|
||||
|
|
@ -107,6 +107,12 @@ def create_pr_review(
|
|||
Returns:
|
||||
Dictionary with success status and the created review data.
|
||||
"""
|
||||
if event.upper() == "APPROVE":
|
||||
return {
|
||||
"success": False,
|
||||
"error": "APPROVE is not allowed. Use COMMENT or REQUEST_CHANGES.",
|
||||
}
|
||||
|
||||
repo_config = _get_repo_config()
|
||||
if not repo_config:
|
||||
return {"success": False, "error": "No repo config found"}
|
||||
|
|
@ -229,11 +235,17 @@ def submit_pr_review(
|
|||
pull_number: The PR number.
|
||||
review_id: The ID of the pending review to submit.
|
||||
body: Optional body text for the review submission.
|
||||
event: The review action - one of APPROVE, REQUEST_CHANGES, or COMMENT.
|
||||
event: The review action - one of REQUEST_CHANGES or COMMENT. APPROVE is not allowed.
|
||||
|
||||
Returns:
|
||||
Dictionary with success status and the submitted review data.
|
||||
"""
|
||||
if event.upper() == "APPROVE":
|
||||
return {
|
||||
"success": False,
|
||||
"error": "APPROVE is not allowed. Use COMMENT or REQUEST_CHANGES.",
|
||||
}
|
||||
|
||||
repo_config = _get_repo_config()
|
||||
if not repo_config:
|
||||
return {"success": False, "error": "No repo config found"}
|
||||
|
|
|
|||
40
tests/test_github_review.py
Normal file
40
tests/test_github_review.py
Normal file
|
|
@ -0,0 +1,40 @@
|
|||
"""Tests for github_review tool guards."""
|
||||
|
||||
from unittest.mock import patch
|
||||
|
||||
from agent.tools.github_review import create_pr_review, submit_pr_review
|
||||
|
||||
|
||||
class TestApproveBlocked:
|
||||
"""APPROVE event must be rejected in both create and submit."""
|
||||
|
||||
def test_create_pr_review_blocks_approve(self):
|
||||
result = create_pr_review(pull_number=1, body="lgtm", event="APPROVE")
|
||||
assert result["success"] is False
|
||||
assert "APPROVE is not allowed" in result["error"]
|
||||
|
||||
def test_submit_pr_review_blocks_approve(self):
|
||||
result = submit_pr_review(pull_number=1, review_id=1, event="APPROVE")
|
||||
assert result["success"] is False
|
||||
assert "APPROVE is not allowed" in result["error"]
|
||||
|
||||
def test_create_pr_review_blocks_approve_lowercase(self):
|
||||
result = create_pr_review(pull_number=1, body="lgtm", event="approve")
|
||||
assert result["success"] is False
|
||||
assert "APPROVE is not allowed" in result["error"]
|
||||
|
||||
def test_submit_pr_review_blocks_approve_mixed_case(self):
|
||||
result = submit_pr_review(pull_number=1, review_id=1, event="Approve")
|
||||
assert result["success"] is False
|
||||
assert "APPROVE is not allowed" in result["error"]
|
||||
|
||||
@patch("agent.tools.github_review._get_repo_config", return_value=None)
|
||||
def test_create_pr_review_allows_comment(self, _mock):
|
||||
result = create_pr_review(pull_number=1, body="looks good", event="COMMENT")
|
||||
# Will fail with "No repo config found" but NOT with the approve error
|
||||
assert "APPROVE is not allowed" not in result.get("error", "")
|
||||
|
||||
@patch("agent.tools.github_review._get_repo_config", return_value=None)
|
||||
def test_create_pr_review_allows_request_changes(self, _mock):
|
||||
result = create_pr_review(pull_number=1, body="fix this", event="REQUEST_CHANGES")
|
||||
assert "APPROVE is not allowed" not in result.get("error", "")
|
||||
Loading…
Add table
Reference in a new issue