mirror of
https://github.com/rennf93/roboco.git
synced 2026-08-03 07:23:24 +02:00
* [a1bde3b9] Add CI-status guard to pr_pass + update pr_reviewer prompt (#417) (#420) * [a1bde3b9] feat(gateway): CI-status guard on pr_pass + reviewer prompt update * [a1bde3b9] docs(pr-gate-review, worksession-git): document CI-status guard on pr_pass Updated two architecture documentation files to reflect the new CI-status guard: **pr-gate-review.md:** - Documented _ci_status_guard method: blocks pr_pass on failing/pending/unscheduled/error CI with reviewer-aware pr_fail remediation - Documented _resolve_ci_status: best-effort GitHub check-runs lookup with fail-open behavior - Updated _pr_pass_blocked description: now returns (rejection_envelope, ci_note) tuple - Updated _record_gate_verdict_for/verdict to note ci_status field stamping on pr_pass - Added ci_note parameter documentation for evidence tracking when no CI is configured - Updated Logical Tree to show new methods - Added Config Flags note: CI guard is always armed, fails open on config gaps - Added two regression risks: check-runs-only limitation, fail-open design **worksession-git.md:** - Documented GitService.get_pr_ci_status(project_slug, pr_number): CI status lookup with state classification - Documented supporting methods: _ci_status_prereqs, _fetch_check_runs, _classify_check_runs, _classify_zero_check_runs - Each method notes its fail-open behavior and configuration gap handling --------- Co-authored-by: Backend Developer 1 <be-dev-1@roboco.tech> Co-authored-by: Backend Documenter <be-doc@roboco.tech> * [e8f275d7] test(gateway): lock the 7-AC-to-test map + assert pr_reviewer prompt content (#425) (#426) Co-authored-by: Backend Developer 1 <be-dev-1@roboco.tech> * [24b4237e] Fix reflow-check, CI-status classification, and noqa suppression (#440) (#443) * [24b4237e] fix(gateway): classify unreachable/nonexistent CI-status repo as no_ci_configured, remove test noqa, reflow pr_reviewer.md Split GitService.get_pr_ci_status's PR-head-sha lookup into a dedicated helper so a config gap (missing project/git_url/token) or an unreachable/ nonexistent repo/PR (network error or 404) classifies as no_ci_configured (pr_pass passes through and stamps the evidence note) while a genuine GitHub API failure on a real, reachable repo (any other non-2xx, or an unparseable body) stays the fail-closed error state. Replaced the `# noqa: PLR2004` in test_git_pr_ci_status.py with a named HTTP-status range constant, updated the config-gap tests to assert the new classification, and added tests for the unreachable-repo and real-repo- API-error branches. Reflowed agents/prompts/roles/pr_reviewer.md's one hard-wrapped continuation line so it passes make reflow-check. * [24b4237e] docs(gateway): update pr-gate-review.md for CI-status classification refactor Updated the internal architectural map to reflect the new CI-status classification scheme introduced in PR #440. Configuration gaps (missing project/git_url/token) and unreachable/nonexistent repos (404 or network error) now explicitly classify as no_ci_configured and pass through with evidence stamps. Genuine GitHub API failures on reachable repos classify as error and stay fail-closed (retryable). - Clarified _ci_status_guard behavior: config gaps/unreachable repos pass through with distinct classification; only real API failures stay fail-closed - Updated Config Flags section to describe the new three-way classification - Updated Regression Risks section to document the new explicit classification scheme - Noted that _resolve_ci_status now wraps git.get_pr_ci_status and interprets its result dict --------- Co-authored-by: Backend Developer 1 <be-dev-1@roboco.tech> Co-authored-by: Backend Documenter <be-doc@roboco.tech> * [1f6a06a2] round-3 fixes: pr_gate back to xenon rank A; 404 means no CI, not error Eight extracted helpers bring the module average from B(5.05) to A(4.04) with every external contract untouched (170 gate tests byte-identical). The CI-status guard now classifies a 404 on the check-runs or workflows endpoints as no_ci_configured (pass-through with evidence note) — a repo without Actions is not a transport failure — reserving the fail-closed error state for network/5xx/auth failures, with pinning tests for all four shapes. The e2e fake-GitHub router gains check-runs and workflows routes so the scripted lifecycle exercises the guard's green-CI success branch end to end. * [1f6a06a2] merge master; align gate-diff-base tests with the tuple contract The merged tree is the first integration of the CI-status guard with the preferred-parent diff-base guard: _pr_pass_blocked now returns (rejection, ci_note), so the diff-base tests unpack it instead of asserting on a bare result. Both guards verified live in the merged pr_gate (preferred_parent threading and _ci_status_guard present). --------- Co-authored-by: Backend Developer 1 <be-dev-1@roboco.tech> Co-authored-by: Backend Documenter <be-doc@roboco.tech> Co-authored-by: Renn F <rennf93@users.noreply.github.com>
341 lines
13 KiB
Python
341 lines
13 KiB
Python
"""GitService.get_pr_ci_status — the CI signal behind the pr_pass gate.
|
|
|
|
Reads GitHub check-runs on a PR's head SHA (falling back to list-workflows
|
|
when zero check-runs exist yet) and classifies the result into one of:
|
|
success, failure, pending, pending_not_scheduled, no_ci_configured, error.
|
|
Every unresolvable case is classified explicitly — a missing project/git_url/
|
|
token, or an unreachable/nonexistent repo or PR, is ``no_ci_configured``
|
|
(the guard passes through with an evidence stamp); a genuine GitHub API
|
|
failure on a real, reachable repo is ``error`` (the guard stays fail-closed).
|
|
"""
|
|
|
|
from __future__ import annotations
|
|
|
|
from typing import Any
|
|
from unittest.mock import AsyncMock, MagicMock, patch
|
|
|
|
import httpx
|
|
import pytest
|
|
from roboco.services.git import GitService
|
|
|
|
_PR = 42
|
|
_SHA = "deadbeefcafebabe0000111122223333aaaabbbb"
|
|
_HTTP_SUCCESS_RANGE = range(200, 300)
|
|
|
|
|
|
def _service() -> GitService:
|
|
session = MagicMock()
|
|
session.execute = AsyncMock()
|
|
svc = GitService(session)
|
|
object.__setattr__(svc, "_token_for_project", AsyncMock(return_value="tok"))
|
|
return svc
|
|
|
|
|
|
def _resp(status_code: int, *, json_payload: Any = None) -> MagicMock:
|
|
resp = MagicMock()
|
|
resp.status_code = status_code
|
|
resp.is_success = status_code in _HTTP_SUCCESS_RANGE
|
|
resp.json.return_value = json_payload
|
|
return resp
|
|
|
|
|
|
def _client(*get_responses: MagicMock) -> MagicMock:
|
|
"""A fake httpx.AsyncClient whose ``.get`` serves responses in call order —
|
|
every ``async with httpx.AsyncClient(...) as client`` in the service reuses
|
|
the SAME instance (the patch target returns it unconditionally), so a list
|
|
``side_effect`` lines up with the sequential PR-head / check-runs /
|
|
workflows calls."""
|
|
client = MagicMock()
|
|
client.__aenter__ = AsyncMock(return_value=client)
|
|
client.__aexit__ = AsyncMock(return_value=False)
|
|
client.get = AsyncMock(side_effect=list(get_responses))
|
|
return client
|
|
|
|
|
|
def _patch_project() -> Any:
|
|
fake = MagicMock()
|
|
fake.get_by_slug = AsyncMock(
|
|
return_value=MagicMock(git_url="https://github.com/acme/repo.git")
|
|
)
|
|
return patch("roboco.services.git.get_project_service", return_value=fake)
|
|
|
|
|
|
def _pr_head_resp() -> MagicMock:
|
|
return _resp(200, json_payload={"head": {"sha": _SHA}})
|
|
|
|
|
|
def _check_run(name: str, *, status: str, conclusion: str | None) -> dict[str, Any]:
|
|
return {"name": name, "status": status, "conclusion": conclusion}
|
|
|
|
|
|
# ---------------------------------------------------------------------------
|
|
# Six CI-guard branches
|
|
# ---------------------------------------------------------------------------
|
|
|
|
|
|
@pytest.mark.asyncio
|
|
async def test_all_checks_green_is_success() -> None:
|
|
checks = _resp(
|
|
200,
|
|
json_payload={
|
|
"check_runs": [
|
|
_check_run("lint", status="completed", conclusion="success"),
|
|
_check_run("tests", status="completed", conclusion="neutral"),
|
|
]
|
|
},
|
|
)
|
|
client = _client(_pr_head_resp(), checks)
|
|
with (
|
|
_patch_project(),
|
|
patch("roboco.services.git.httpx.AsyncClient", return_value=client),
|
|
):
|
|
out = await _service().get_pr_ci_status("roboco", _PR)
|
|
assert out == {"state": "success", "head_sha": _SHA}
|
|
|
|
|
|
@pytest.mark.asyncio
|
|
async def test_failing_check_names_it() -> None:
|
|
checks = _resp(
|
|
200,
|
|
json_payload={
|
|
"check_runs": [
|
|
_check_run("lint", status="completed", conclusion="success"),
|
|
_check_run("tests", status="completed", conclusion="failure"),
|
|
]
|
|
},
|
|
)
|
|
client = _client(_pr_head_resp(), checks)
|
|
with (
|
|
_patch_project(),
|
|
patch("roboco.services.git.httpx.AsyncClient", return_value=client),
|
|
):
|
|
out = await _service().get_pr_ci_status("roboco", _PR)
|
|
assert out is not None
|
|
assert out["state"] == "failure"
|
|
assert out["failing_checks"] == ["tests"]
|
|
assert out["head_sha"] == _SHA
|
|
|
|
|
|
@pytest.mark.asyncio
|
|
async def test_still_running_check_is_pending() -> None:
|
|
checks = _resp(
|
|
200,
|
|
json_payload={
|
|
"check_runs": [
|
|
_check_run("lint", status="completed", conclusion="success"),
|
|
_check_run("tests", status="in_progress", conclusion=None),
|
|
]
|
|
},
|
|
)
|
|
client = _client(_pr_head_resp(), checks)
|
|
with (
|
|
_patch_project(),
|
|
patch("roboco.services.git.httpx.AsyncClient", return_value=client),
|
|
):
|
|
out = await _service().get_pr_ci_status("roboco", _PR)
|
|
assert out == {"state": "pending", "head_sha": _SHA}
|
|
|
|
|
|
@pytest.mark.asyncio
|
|
async def test_check_runs_api_error_is_error_state() -> None:
|
|
checks = _resp(500, json_payload={"message": "internal error"})
|
|
client = _client(_pr_head_resp(), checks)
|
|
with (
|
|
_patch_project(),
|
|
patch("roboco.services.git.httpx.AsyncClient", return_value=client),
|
|
):
|
|
out = await _service().get_pr_ci_status("roboco", _PR)
|
|
assert out == {"state": "error", "head_sha": _SHA}
|
|
|
|
|
|
@pytest.mark.asyncio
|
|
async def test_zero_checks_with_no_workflows_is_no_ci_configured() -> None:
|
|
checks = _resp(200, json_payload={"check_runs": []})
|
|
workflows = _resp(200, json_payload={"total_count": 0})
|
|
client = _client(_pr_head_resp(), checks, workflows)
|
|
with (
|
|
_patch_project(),
|
|
patch("roboco.services.git.httpx.AsyncClient", return_value=client),
|
|
):
|
|
out = await _service().get_pr_ci_status("roboco", _PR)
|
|
assert out == {"state": "no_ci_configured", "head_sha": _SHA}
|
|
|
|
|
|
@pytest.mark.asyncio
|
|
async def test_zero_checks_with_workflows_configured_is_pending_not_scheduled() -> None:
|
|
checks = _resp(200, json_payload={"check_runs": []})
|
|
workflows = _resp(200, json_payload={"total_count": 3})
|
|
client = _client(_pr_head_resp(), checks, workflows)
|
|
with (
|
|
_patch_project(),
|
|
patch("roboco.services.git.httpx.AsyncClient", return_value=client),
|
|
):
|
|
out = await _service().get_pr_ci_status("roboco", _PR)
|
|
assert out == {"state": "pending_not_scheduled", "head_sha": _SHA}
|
|
|
|
|
|
# ---------------------------------------------------------------------------
|
|
# Config gaps and an unreachable/nonexistent repo pass through cleanly as
|
|
# no_ci_configured (pr_pass stamps the evidence note, never mistaken for a
|
|
# CI signal)
|
|
# ---------------------------------------------------------------------------
|
|
|
|
|
|
@pytest.mark.asyncio
|
|
async def test_no_ci_configured_on_missing_token() -> None:
|
|
svc = _service()
|
|
object.__setattr__(svc, "_token_for_project", AsyncMock(return_value=None))
|
|
with _patch_project():
|
|
out = await svc.get_pr_ci_status("roboco", _PR)
|
|
assert out == {"state": "no_ci_configured", "head_sha": None}
|
|
|
|
|
|
@pytest.mark.asyncio
|
|
async def test_no_ci_configured_on_missing_project() -> None:
|
|
fake = MagicMock()
|
|
fake.get_by_slug = AsyncMock(return_value=None)
|
|
with patch("roboco.services.git.get_project_service", return_value=fake):
|
|
out = await _service().get_pr_ci_status("roboco", _PR)
|
|
assert out == {"state": "no_ci_configured", "head_sha": None}
|
|
|
|
|
|
@pytest.mark.asyncio
|
|
async def test_no_ci_configured_when_pr_lookup_404s() -> None:
|
|
# The PR lookup itself 404s — the repo/PR doesn't exist or isn't reachable.
|
|
pr_lookup = _resp(404, json_payload={"message": "not found"})
|
|
client = _client(pr_lookup)
|
|
with (
|
|
_patch_project(),
|
|
patch("roboco.services.git.httpx.AsyncClient", return_value=client),
|
|
):
|
|
out = await _service().get_pr_ci_status("roboco", _PR)
|
|
assert out == {"state": "no_ci_configured", "head_sha": None}
|
|
|
|
|
|
@pytest.mark.asyncio
|
|
async def test_no_ci_configured_when_pr_lookup_unreachable() -> None:
|
|
# A connection/network failure resolving the PR's head SHA — the repo is
|
|
# unreachable, not just returning an error response.
|
|
client = MagicMock()
|
|
client.__aenter__ = AsyncMock(return_value=client)
|
|
client.__aexit__ = AsyncMock(return_value=False)
|
|
client.get = AsyncMock(side_effect=httpx.ConnectError("connection refused"))
|
|
with (
|
|
_patch_project(),
|
|
patch("roboco.services.git.httpx.AsyncClient", return_value=client),
|
|
):
|
|
out = await _service().get_pr_ci_status("roboco", _PR)
|
|
assert out == {"state": "no_ci_configured", "head_sha": None}
|
|
|
|
|
|
# ---------------------------------------------------------------------------
|
|
# A real, reachable repo's genuinely failing GitHub API call stays
|
|
# fail-closed (error), never conflated with the no_ci_configured cases above
|
|
# ---------------------------------------------------------------------------
|
|
|
|
|
|
@pytest.mark.asyncio
|
|
async def test_error_when_pr_lookup_api_fails_on_real_repo() -> None:
|
|
# The repo/project/token all resolve fine, but the PR-head-sha lookup
|
|
# itself returns a genuine 5xx — this must stay fail-closed (error), not
|
|
# be conflated with the unreachable/nonexistent-repo no_ci_configured case.
|
|
pr_lookup = _resp(500, json_payload={"message": "internal error"})
|
|
client = _client(pr_lookup)
|
|
with (
|
|
_patch_project(),
|
|
patch("roboco.services.git.httpx.AsyncClient", return_value=client),
|
|
):
|
|
out = await _service().get_pr_ci_status("roboco", _PR)
|
|
assert out == {"state": "error", "head_sha": None}
|
|
|
|
|
|
@pytest.mark.asyncio
|
|
async def test_error_when_pr_lookup_body_unparseable() -> None:
|
|
pr_lookup = _resp(200, json_payload={"head": {}}) # missing "sha" key
|
|
client = _client(pr_lookup)
|
|
with (
|
|
_patch_project(),
|
|
patch("roboco.services.git.httpx.AsyncClient", return_value=client),
|
|
):
|
|
out = await _service().get_pr_ci_status("roboco", _PR)
|
|
assert out == {"state": "error", "head_sha": None}
|
|
|
|
|
|
@pytest.mark.asyncio
|
|
async def test_workflows_api_error_after_zero_checks_is_error_state() -> None:
|
|
checks = _resp(200, json_payload={"check_runs": []})
|
|
workflows = _resp(503, json_payload={"message": "busy"})
|
|
client = _client(_pr_head_resp(), checks, workflows)
|
|
with (
|
|
_patch_project(),
|
|
patch("roboco.services.git.httpx.AsyncClient", return_value=client),
|
|
):
|
|
out = await _service().get_pr_ci_status("roboco", _PR)
|
|
assert out == {"state": "error", "head_sha": _SHA}
|
|
|
|
|
|
# ---------------------------------------------------------------------------
|
|
# A 404 on the check-runs or workflows endpoint means the repo has no CI
|
|
# integration at all (e.g. the e2e harness's fake GitHub with no routes
|
|
# mounted for either) — no_ci_configured, never mistaken for error. 500s and
|
|
# network failures on the same two endpoints stay fail-closed (error).
|
|
# ---------------------------------------------------------------------------
|
|
|
|
|
|
@pytest.mark.asyncio
|
|
async def test_no_ci_configured_when_check_runs_404s() -> None:
|
|
checks = _resp(404, json_payload={"message": "not found"})
|
|
client = _client(_pr_head_resp(), checks)
|
|
with (
|
|
_patch_project(),
|
|
patch("roboco.services.git.httpx.AsyncClient", return_value=client),
|
|
):
|
|
out = await _service().get_pr_ci_status("roboco", _PR)
|
|
assert out == {"state": "no_ci_configured", "head_sha": _SHA}
|
|
|
|
|
|
@pytest.mark.asyncio
|
|
async def test_error_when_check_runs_request_times_out() -> None:
|
|
client = MagicMock()
|
|
client.__aenter__ = AsyncMock(return_value=client)
|
|
client.__aexit__ = AsyncMock(return_value=False)
|
|
client.get = AsyncMock(
|
|
side_effect=[_pr_head_resp(), httpx.ReadTimeout("timed out")]
|
|
)
|
|
with (
|
|
_patch_project(),
|
|
patch("roboco.services.git.httpx.AsyncClient", return_value=client),
|
|
):
|
|
out = await _service().get_pr_ci_status("roboco", _PR)
|
|
assert out == {"state": "error", "head_sha": _SHA}
|
|
|
|
|
|
@pytest.mark.asyncio
|
|
async def test_no_ci_configured_when_workflows_404s() -> None:
|
|
checks = _resp(200, json_payload={"check_runs": []})
|
|
workflows = _resp(404, json_payload={"message": "not found"})
|
|
client = _client(_pr_head_resp(), checks, workflows)
|
|
with (
|
|
_patch_project(),
|
|
patch("roboco.services.git.httpx.AsyncClient", return_value=client),
|
|
):
|
|
out = await _service().get_pr_ci_status("roboco", _PR)
|
|
assert out == {"state": "no_ci_configured", "head_sha": _SHA}
|
|
|
|
|
|
@pytest.mark.asyncio
|
|
async def test_error_when_workflows_request_times_out() -> None:
|
|
checks = _resp(200, json_payload={"check_runs": []})
|
|
client = MagicMock()
|
|
client.__aenter__ = AsyncMock(return_value=client)
|
|
client.__aexit__ = AsyncMock(return_value=False)
|
|
client.get = AsyncMock(
|
|
side_effect=[_pr_head_resp(), checks, httpx.ReadTimeout("timed out")]
|
|
)
|
|
with (
|
|
_patch_project(),
|
|
patch("roboco.services.git.httpx.AsyncClient", return_value=client),
|
|
):
|
|
out = await _service().get_pr_ci_status("roboco", _PR)
|
|
assert out == {"state": "error", "head_sha": _SHA}
|