mirror of
https://github.com/Sea-Haven-Industries/open-swe.git
synced 2026-09-30 04:33:12 +00:00
test(github): harden review command routing
This commit is contained in:
parent
882f776809
commit
5f6d756e0d
3 changed files with 135 additions and 25 deletions
|
|
@ -1151,7 +1151,7 @@ _MIXED_REVIEW_ACTION_RE = re.compile(
|
|||
r"(?:^(?:please\s+)?|(?:^|[\s,;])(?:and|then|also)\s+(?:please\s+)?)"
|
||||
r"(?:add|address|change|commit|create|delete|deploy|fix|implement|merge|modify|"
|
||||
r"push|refactor|remove|resolve|run|update|write)\b",
|
||||
re.IGNORECASE,
|
||||
re.IGNORECASE | re.MULTILINE,
|
||||
)
|
||||
|
||||
|
||||
|
|
|
|||
|
|
@ -56,11 +56,13 @@ def test_gate_blocks_non_member_on_public_pr_comment(monkeypatch) -> None:
|
|||
_common_setup(monkeypatch)
|
||||
seen = _install_membership_stub(monkeypatch, members={"insider"})
|
||||
|
||||
async def fake_process_github_pr_comment(*_args, **_kwargs) -> None:
|
||||
async def fake_process_github_review_command(*_args, **_kwargs) -> None:
|
||||
raise AssertionError("should not be called")
|
||||
|
||||
monkeypatch.setattr(
|
||||
github_webhooks, "process_github_pr_comment", fake_process_github_pr_comment
|
||||
github_webhooks,
|
||||
"process_github_review_command",
|
||||
fake_process_github_review_command,
|
||||
)
|
||||
|
||||
client = TestClient(api_app.app)
|
||||
|
|
|
|||
|
|
@ -20,15 +20,22 @@ def _sign_body(body: bytes) -> str:
|
|||
return f"sha256={digest}"
|
||||
|
||||
|
||||
def _post_issue_comment(client: TestClient, body_text: str) -> object:
|
||||
payload = {
|
||||
"action": "created",
|
||||
"issue": {
|
||||
"number": 245,
|
||||
"html_url": "https://github.com/acme/widgets/pull/245",
|
||||
"pull_request": {"html_url": "https://github.com/acme/widgets/pull/245"},
|
||||
def _post_webhook(client: TestClient, event_type: str, payload: dict[str, object]) -> object:
|
||||
body = json.dumps(payload, separators=(",", ":")).encode()
|
||||
return client.post(
|
||||
"/webhooks/github",
|
||||
content=body,
|
||||
headers={
|
||||
"X-GitHub-Event": event_type,
|
||||
"X-Hub-Signature-256": _sign_body(body),
|
||||
"Content-Type": "application/json",
|
||||
},
|
||||
"comment": {"id": 91, "node_id": "IC_91", "body": body_text},
|
||||
)
|
||||
|
||||
|
||||
def _comment_payload(event_type: str, body_text: str) -> dict[str, object]:
|
||||
payload: dict[str, object] = {
|
||||
"action": "submitted" if event_type == "pull_request_review" else "created",
|
||||
"repository": {
|
||||
"owner": {"login": "acme"},
|
||||
"name": "widgets",
|
||||
|
|
@ -36,16 +43,25 @@ def _post_issue_comment(client: TestClient, body_text: str) -> object:
|
|||
},
|
||||
"sender": {"login": "octocat", "id": 123},
|
||||
}
|
||||
body = json.dumps(payload, separators=(",", ":")).encode()
|
||||
return client.post(
|
||||
"/webhooks/github",
|
||||
content=body,
|
||||
headers={
|
||||
"X-GitHub-Event": "issue_comment",
|
||||
"X-Hub-Signature-256": _sign_body(body),
|
||||
"Content-Type": "application/json",
|
||||
},
|
||||
)
|
||||
if event_type == "issue_comment":
|
||||
payload["issue"] = {
|
||||
"number": 245,
|
||||
"html_url": "https://github.com/acme/widgets/pull/245",
|
||||
"pull_request": {"html_url": "https://github.com/acme/widgets/pull/245"},
|
||||
}
|
||||
payload["comment"] = {"id": 91, "node_id": "IC_91", "body": body_text}
|
||||
else:
|
||||
payload["pull_request"] = {
|
||||
"number": 245,
|
||||
"html_url": "https://github.com/acme/widgets/pull/245",
|
||||
}
|
||||
key = "review" if event_type == "pull_request_review" else "comment"
|
||||
payload[key] = {"id": 91, "node_id": "IC_91", "body": body_text}
|
||||
return payload
|
||||
|
||||
|
||||
def _post_issue_comment(client: TestClient, body_text: str) -> object:
|
||||
return _post_webhook(client, "issue_comment", _comment_payload("issue_comment", body_text))
|
||||
|
||||
|
||||
@pytest.mark.parametrize(
|
||||
|
|
@ -65,6 +81,10 @@ def _post_issue_comment(client: TestClient, body_text: str) -> object:
|
|||
"@openswe review\nPrioritize correctness over style.",
|
||||
"Prioritize correctness over style.",
|
||||
),
|
||||
(
|
||||
"@openswe review\nCheck authentication.\nValidate error handling.",
|
||||
"Check authentication.\nValidate error handling.",
|
||||
),
|
||||
],
|
||||
)
|
||||
def test_parse_review_command(body: str, instructions: str) -> None:
|
||||
|
|
@ -77,6 +97,7 @@ def test_parse_review_command(body: str, instructions: str) -> None:
|
|||
"@openswe review and fix the failing test",
|
||||
"@open-swe review focus on auth, then update the tests",
|
||||
"@openswe review fix the failing test",
|
||||
"@openswe review Check authentication.\nFix the failing tests.",
|
||||
"Please @openswe review",
|
||||
"@openswe review this\n@openswe fix it",
|
||||
"@openswe reviewer",
|
||||
|
|
@ -130,11 +151,100 @@ def test_review_command_route_bypasses_auto_review_and_coding_agent(
|
|||
assert captured["instructions"] == "Focus on the exact authorization boundary."
|
||||
|
||||
|
||||
def test_mixed_review_request_falls_through_to_coding_agent(
|
||||
@pytest.mark.parametrize(
|
||||
"event_type",
|
||||
["pull_request_review", "pull_request_review_comment"],
|
||||
)
|
||||
def test_review_command_routes_review_event_payloads(
|
||||
monkeypatch: pytest.MonkeyPatch, event_type: str
|
||||
) -> None:
|
||||
captured: dict[str, object] = {}
|
||||
|
||||
async def allow_gate(*_args: object) -> None:
|
||||
return None
|
||||
|
||||
async def process_review(
|
||||
payload: dict[str, object], received_event_type: str, *, instructions: str
|
||||
) -> None:
|
||||
captured.update(
|
||||
payload=payload,
|
||||
event_type=received_event_type,
|
||||
instructions=instructions,
|
||||
)
|
||||
|
||||
async def fail_coding_agent(*_args: object, **_kwargs: object) -> None:
|
||||
raise AssertionError("coding-agent dispatch must not run")
|
||||
|
||||
monkeypatch.setattr(webhook_common, "GITHUB_WEBHOOK_SECRET", _TEST_WEBHOOK_SECRET)
|
||||
monkeypatch.setattr(webhook_common, "_is_repo_allowed", lambda _repo: True)
|
||||
monkeypatch.setattr(webhook_common, "_enforce_public_repo_org_gate", allow_gate)
|
||||
monkeypatch.setattr(github_webhooks, "process_github_review_command", process_review)
|
||||
monkeypatch.setattr(github_webhooks, "process_github_pr_comment", fail_coding_agent)
|
||||
|
||||
response = _post_webhook(
|
||||
TestClient(api_app.app),
|
||||
event_type,
|
||||
_comment_payload(event_type, "@open-swe review Focus on authorization."),
|
||||
)
|
||||
|
||||
assert response.status_code == 200
|
||||
assert response.json() == {
|
||||
"status": "accepted",
|
||||
"message": "Processing on-demand PR review",
|
||||
}
|
||||
assert captured["event_type"] == event_type
|
||||
assert captured["instructions"] == "Focus on authorization."
|
||||
|
||||
|
||||
def test_finding_reply_takes_precedence_over_review_command(
|
||||
monkeypatch: pytest.MonkeyPatch,
|
||||
) -> None:
|
||||
captured: dict[str, object] = {}
|
||||
|
||||
async def allow_gate(*_args: object) -> None:
|
||||
return None
|
||||
|
||||
async def process_finding_reply(payload: dict[str, object]) -> None:
|
||||
captured["payload"] = payload
|
||||
|
||||
async def fail_review(*_args: object, **_kwargs: object) -> None:
|
||||
raise AssertionError("review command must not steal finding replies")
|
||||
|
||||
payload = _comment_payload("pull_request_review_comment", "@openswe review")
|
||||
comment = payload["comment"]
|
||||
assert isinstance(comment, dict)
|
||||
comment["in_reply_to_id"] = 90
|
||||
|
||||
monkeypatch.setattr(webhook_common, "GITHUB_WEBHOOK_SECRET", _TEST_WEBHOOK_SECRET)
|
||||
monkeypatch.setattr(webhook_common, "_is_repo_allowed", lambda _repo: True)
|
||||
monkeypatch.setattr(webhook_common, "_enforce_public_repo_org_gate", allow_gate)
|
||||
monkeypatch.setattr(
|
||||
github_webhooks, "process_github_review_finding_reply", process_finding_reply
|
||||
)
|
||||
monkeypatch.setattr(github_webhooks, "process_github_review_command", fail_review)
|
||||
|
||||
response = _post_webhook(TestClient(api_app.app), "pull_request_review_comment", payload)
|
||||
|
||||
assert response.status_code == 200
|
||||
assert response.json() == {
|
||||
"status": "accepted",
|
||||
"message": "Processing review finding reply",
|
||||
}
|
||||
assert captured["payload"] == payload
|
||||
|
||||
|
||||
@pytest.mark.parametrize(
|
||||
"body",
|
||||
[
|
||||
"@openswe review and fix the failing test",
|
||||
"@openswe review Check authentication.\nFix the failing tests.",
|
||||
],
|
||||
)
|
||||
def test_mixed_review_request_falls_through_to_coding_agent(
|
||||
monkeypatch: pytest.MonkeyPatch, body: str
|
||||
) -> None:
|
||||
captured: dict[str, object] = {}
|
||||
|
||||
async def allow_gate(*_args: object) -> None:
|
||||
return None
|
||||
|
||||
|
|
@ -150,9 +260,7 @@ def test_mixed_review_request_falls_through_to_coding_agent(
|
|||
monkeypatch.setattr(github_webhooks, "process_github_review_command", fail_review)
|
||||
monkeypatch.setattr(github_webhooks, "process_github_pr_comment", process_coding_agent)
|
||||
|
||||
response = _post_issue_comment(
|
||||
TestClient(api_app.app), "@openswe review and fix the failing test"
|
||||
)
|
||||
response = _post_issue_comment(TestClient(api_app.app), body)
|
||||
|
||||
assert response.status_code == 200
|
||||
assert response.json()["status"] == "accepted"
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue