mirror of
https://github.com/rennf93/roboco.git
synced 2026-08-03 07:23:24 +02:00
fix(self-heal): make the CI regression signal deterministic
The loop read the latest completed Actions run with per_page=1 and an empty default workflow scope, so the conclusion flickered: on RoboCo's 8-workflow repo a green run from an unrelated workflow (or a green run on an older commit) masked a red ci.yml run, and a single transient GitHub error silently skipped the whole cycle — so self-heal sometimes fired on a real regression and sometimes did not. - Default self_heal_ci_workflow to "ci.yml" so the signal is scoped to the gate workflow, not "latest run across all workflows". - Fetch a window of recent completed runs and resolve the conclusion against the branch's current HEAD (newest commit's latest attempt), so a stale/unrelated green run can't mask the HEAD failure and a green re-run supersedes the original failure. - Retry transient network / 429 / 5xx errors within the cycle instead of treating one blip as "all green".
This commit is contained in:
+7
-6
@@ -380,13 +380,14 @@ class Settings(BaseSettings):
|
|||||||
),
|
),
|
||||||
)
|
)
|
||||||
self_heal_ci_workflow: str = Field(
|
self_heal_ci_workflow: str = Field(
|
||||||
default="",
|
default="ci.yml",
|
||||||
description=(
|
description=(
|
||||||
"Optional GitHub Actions workflow file name (e.g. 'ci.yml') to scope "
|
"GitHub Actions workflow file name to scope the CI signal to. "
|
||||||
"the CI signal to. Empty = use the latest completed run across ALL "
|
"Defaults to 'ci.yml' (RoboCo's own gate). Set empty ONLY for a "
|
||||||
"workflows on the default branch, which is imprecise when the repo "
|
"single-workflow repo — an empty value reads the latest completed run "
|
||||||
"has several workflows; set this to the real CI workflow for a "
|
"across ALL workflows on the default branch, which on a "
|
||||||
"reliable signal."
|
"multi-workflow repo lets an unrelated green run mask a red CI run "
|
||||||
|
"and makes the self-heal signal flicker."
|
||||||
),
|
),
|
||||||
)
|
)
|
||||||
self_heal_originate_enabled: bool = Field(
|
self_heal_originate_enabled: bool = Field(
|
||||||
|
|||||||
+73
-22
@@ -136,6 +136,33 @@ _HTTP_NOT_FOUND = 404
|
|||||||
# refs are stale. `pr_merge` re-syncs and retries exactly once on this code.
|
# refs are stale. `pr_merge` re-syncs and retries exactly once on this code.
|
||||||
_HTTP_CONFLICT = 409
|
_HTTP_CONFLICT = 409
|
||||||
|
|
||||||
|
# --- Self-heal CI signal -------------------------------------------------
|
||||||
|
# Pull a WINDOW of recent completed runs (not just the single newest) so the
|
||||||
|
# conclusion can be resolved against the branch's current HEAD rather than
|
||||||
|
# whichever run finished most recently — otherwise a green run on an older
|
||||||
|
# commit, or (on the unscoped all-workflows endpoint) an unrelated green
|
||||||
|
# workflow, masks the HEAD commit's failing run and the signal flickers.
|
||||||
|
_CI_RUN_WINDOW = 20
|
||||||
|
# Transient GitHub failures (network, 429, 5xx) are retried within the cycle so
|
||||||
|
# a single blip does not silently skip a whole self-heal pass.
|
||||||
|
_CI_FETCH_ATTEMPTS = 3
|
||||||
|
_CI_FETCH_BACKOFF_SECONDS = 0.5
|
||||||
|
_CI_RETRYABLE_STATUS = frozenset({429, 500, 502, 503, 504})
|
||||||
|
|
||||||
|
|
||||||
|
def _select_ci_head_run(runs: list[dict[str, Any]]) -> dict[str, Any]:
|
||||||
|
"""Pick the run reflecting the branch's current-HEAD CI conclusion.
|
||||||
|
|
||||||
|
GitHub returns completed runs newest-created first, so ``runs[0]`` belongs to
|
||||||
|
the newest commit. Among all returned runs sharing that ``head_sha`` we take
|
||||||
|
the highest ``run_attempt`` so a green re-run supersedes the original
|
||||||
|
failure. This stops a stale green run on an older commit (or an unrelated
|
||||||
|
workflow on the all-workflows endpoint) from masking the HEAD failure.
|
||||||
|
"""
|
||||||
|
head_sha = runs[0].get("head_sha")
|
||||||
|
same_head = [r for r in runs if r.get("head_sha") == head_sha] or [runs[0]]
|
||||||
|
return max(same_head, key=lambda r: int(r.get("run_attempt") or 0))
|
||||||
|
|
||||||
|
|
||||||
class GitService(BaseService):
|
class GitService(BaseService):
|
||||||
"""
|
"""
|
||||||
@@ -1700,45 +1727,69 @@ class GitService(BaseService):
|
|||||||
git_token: str,
|
git_token: str,
|
||||||
workflow: str | None = None,
|
workflow: str | None = None,
|
||||||
) -> dict[str, Any] | None:
|
) -> dict[str, Any] | None:
|
||||||
"""GET the most recent completed Actions run on ``branch``; None on error.
|
"""Resolve ``branch``'s current-HEAD CI conclusion; None on error.
|
||||||
|
|
||||||
Scopes to ``workflow`` (a workflow file name) when given — the precise
|
Scopes to ``workflow`` (a workflow file name) when given — the precise
|
||||||
signal — otherwise looks across all workflows.
|
signal — otherwise reads across ALL workflows, which on a multi-workflow
|
||||||
|
repo is unreliable (an unrelated green run can mask a red CI run). Pulls a
|
||||||
|
WINDOW of recent completed runs and selects the newest commit's latest
|
||||||
|
attempt (see ``_select_ci_head_run``) rather than the single
|
||||||
|
most-recently-completed run, so a green run on an older commit can't mask
|
||||||
|
the HEAD's failure and a green re-run correctly supersedes it. ``branch``
|
||||||
|
filters by head branch, so only pushes to the default branch (not
|
||||||
|
pull-request runs, whose head is a feature branch) count — exactly the
|
||||||
|
"is the default branch red" signal self-heal needs. Transient network /
|
||||||
|
429 / 5xx errors are retried a few times before giving up so a single
|
||||||
|
blip doesn't silently skip the cycle.
|
||||||
"""
|
"""
|
||||||
owner, repo = owner_repo
|
owner, repo = owner_repo
|
||||||
api_base = settings.github_api_base_url.rstrip("/")
|
api_base = settings.github_api_base_url.rstrip("/")
|
||||||
base = f"{api_base}/repos/{owner}/{repo}/actions"
|
base = f"{api_base}/repos/{owner}/{repo}/actions"
|
||||||
url = f"{base}/workflows/{workflow}/runs" if workflow else f"{base}/runs"
|
url = f"{base}/workflows/{workflow}/runs" if workflow else f"{base}/runs"
|
||||||
try:
|
headers = {
|
||||||
async with httpx.AsyncClient(timeout=_default_git_timeout()) as client:
|
"Authorization": f"Bearer {git_token}",
|
||||||
resp = await client.get(
|
"Accept": "application/vnd.github+json",
|
||||||
url,
|
"X-GitHub-Api-Version": "2022-11-28",
|
||||||
headers={
|
}
|
||||||
"Authorization": f"Bearer {git_token}",
|
params: dict[str, str | int] = {
|
||||||
"Accept": "application/vnd.github+json",
|
"branch": branch,
|
||||||
"X-GitHub-Api-Version": "2022-11-28",
|
"status": "completed",
|
||||||
},
|
"per_page": _CI_RUN_WINDOW,
|
||||||
params={"branch": branch, "status": "completed", "per_page": 1},
|
}
|
||||||
)
|
resp: httpx.Response | None = None
|
||||||
except httpx.HTTPError as e:
|
for attempt in range(_CI_FETCH_ATTEMPTS):
|
||||||
self.log.warning(
|
last = attempt + 1 == _CI_FETCH_ATTEMPTS
|
||||||
"get_latest_ci_conclusion request failed",
|
try:
|
||||||
project=project_slug,
|
async with httpx.AsyncClient(timeout=_default_git_timeout()) as client:
|
||||||
error=str(e),
|
resp = await client.get(url, headers=headers, params=params)
|
||||||
)
|
except httpx.HTTPError as e:
|
||||||
return None
|
if last:
|
||||||
if not resp.is_success:
|
self.log.warning(
|
||||||
|
"get_latest_ci_conclusion request failed",
|
||||||
|
project=project_slug,
|
||||||
|
error=str(e),
|
||||||
|
)
|
||||||
|
return None
|
||||||
|
await asyncio.sleep(_CI_FETCH_BACKOFF_SECONDS * (attempt + 1))
|
||||||
|
continue
|
||||||
|
if resp.is_success:
|
||||||
|
break
|
||||||
|
if resp.status_code in _CI_RETRYABLE_STATUS and not last:
|
||||||
|
await asyncio.sleep(_CI_FETCH_BACKOFF_SECONDS * (attempt + 1))
|
||||||
|
continue
|
||||||
self.log.warning(
|
self.log.warning(
|
||||||
"get_latest_ci_conclusion non-2xx",
|
"get_latest_ci_conclusion non-2xx",
|
||||||
project=project_slug,
|
project=project_slug,
|
||||||
status=resp.status_code,
|
status=resp.status_code,
|
||||||
)
|
)
|
||||||
return None
|
return None
|
||||||
|
if resp is None or not resp.is_success:
|
||||||
|
return None
|
||||||
data = resp.json()
|
data = resp.json()
|
||||||
runs = data.get("workflow_runs") if isinstance(data, dict) else None
|
runs = data.get("workflow_runs") if isinstance(data, dict) else None
|
||||||
if not runs:
|
if not runs:
|
||||||
return None
|
return None
|
||||||
return cast("dict[str, Any]", runs[0])
|
return _select_ci_head_run(runs)
|
||||||
|
|
||||||
async def _post_pr(
|
async def _post_pr(
|
||||||
self,
|
self,
|
||||||
|
|||||||
@@ -13,7 +13,7 @@ from typing import Any
|
|||||||
from unittest.mock import AsyncMock, MagicMock, patch
|
from unittest.mock import AsyncMock, MagicMock, patch
|
||||||
|
|
||||||
import pytest
|
import pytest
|
||||||
from roboco.services.git import GitService
|
from roboco.services.git import _CI_RUN_WINDOW, GitService
|
||||||
|
|
||||||
_PR = 7
|
_PR = 7
|
||||||
|
|
||||||
@@ -195,13 +195,16 @@ def _patch_project_ci() -> Any:
|
|||||||
return patch("roboco.services.git.get_project_service", return_value=fake)
|
return patch("roboco.services.git.get_project_service", return_value=fake)
|
||||||
|
|
||||||
|
|
||||||
def _run(conclusion: str) -> dict[str, Any]:
|
def _run(
|
||||||
|
conclusion: str, *, head_sha: str = "abc123", attempt: int = 1
|
||||||
|
) -> dict[str, Any]:
|
||||||
return {
|
return {
|
||||||
"conclusion": conclusion,
|
"conclusion": conclusion,
|
||||||
"head_sha": "abc123",
|
"head_sha": head_sha,
|
||||||
"html_url": "https://github.com/acme/repo/actions/runs/99",
|
"html_url": "https://github.com/acme/repo/actions/runs/99",
|
||||||
"name": "CI",
|
"name": "CI",
|
||||||
"updated_at": "2026-06-17T00:00:00Z",
|
"updated_at": "2026-06-17T00:00:00Z",
|
||||||
|
"run_attempt": attempt,
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
||||||
@@ -227,7 +230,7 @@ async def test_get_latest_ci_conclusion_normalizes_and_requests_correctly() -> N
|
|||||||
assert call.kwargs["params"] == {
|
assert call.kwargs["params"] == {
|
||||||
"branch": "master",
|
"branch": "master",
|
||||||
"status": "completed",
|
"status": "completed",
|
||||||
"per_page": 1,
|
"per_page": _CI_RUN_WINDOW,
|
||||||
}
|
}
|
||||||
assert call.kwargs["headers"]["Authorization"] == "Bearer tok"
|
assert call.kwargs["headers"]["Authorization"] == "Bearer tok"
|
||||||
|
|
||||||
@@ -276,3 +279,82 @@ async def test_get_latest_ci_conclusion_scopes_to_workflow() -> None:
|
|||||||
assert out is not None
|
assert out is not None
|
||||||
assert out["conclusion"] == "success"
|
assert out["conclusion"] == "success"
|
||||||
assert client.get.await_args.args[0].endswith("/actions/workflows/ci.yml/runs")
|
assert client.get.await_args.args[0].endswith("/actions/workflows/ci.yml/runs")
|
||||||
|
|
||||||
|
|
||||||
|
@pytest.mark.asyncio
|
||||||
|
async def test_get_latest_ci_conclusion_requests_a_window() -> None:
|
||||||
|
# The signal pulls a window of recent runs, not just the single newest, so a
|
||||||
|
# green run on an older commit can't mask the HEAD's failure.
|
||||||
|
svc = _service()
|
||||||
|
client = _client(_resp(200, json_payload={"workflow_runs": [_run("failure")]}))
|
||||||
|
with (
|
||||||
|
_patch_project_ci(),
|
||||||
|
patch("roboco.services.git.httpx.AsyncClient", return_value=client),
|
||||||
|
):
|
||||||
|
await svc.get_latest_ci_conclusion("roboco", workflow="ci.yml")
|
||||||
|
assert client.get.await_args.kwargs["params"]["per_page"] == _CI_RUN_WINDOW
|
||||||
|
|
||||||
|
|
||||||
|
@pytest.mark.asyncio
|
||||||
|
async def test_get_latest_ci_conclusion_red_head_not_masked_by_older_green() -> None:
|
||||||
|
# Newest commit's run FAILED; older commits' runs (different sha) are green.
|
||||||
|
# The signal must follow the newest commit — not whatever completed last — so
|
||||||
|
# a stale/unrelated green run cannot mask the regression.
|
||||||
|
svc = _service()
|
||||||
|
payload = {
|
||||||
|
"workflow_runs": [
|
||||||
|
_run("failure", head_sha="newsha"),
|
||||||
|
_run("success", head_sha="oldsha1"),
|
||||||
|
_run("success", head_sha="oldsha2"),
|
||||||
|
]
|
||||||
|
}
|
||||||
|
client = _client(_resp(200, json_payload=payload))
|
||||||
|
with (
|
||||||
|
_patch_project_ci(),
|
||||||
|
patch("roboco.services.git.httpx.AsyncClient", return_value=client),
|
||||||
|
):
|
||||||
|
out = await svc.get_latest_ci_conclusion("roboco", workflow="ci.yml")
|
||||||
|
assert out is not None
|
||||||
|
assert out["conclusion"] == "failure"
|
||||||
|
assert out["head_sha"] == "newsha"
|
||||||
|
|
||||||
|
|
||||||
|
@pytest.mark.asyncio
|
||||||
|
async def test_get_latest_ci_conclusion_green_rerun_supersedes_failure() -> None:
|
||||||
|
# Same commit re-run green after a red first attempt → reads green.
|
||||||
|
svc = _service()
|
||||||
|
payload = {
|
||||||
|
"workflow_runs": [
|
||||||
|
_run("success", head_sha="sha", attempt=2),
|
||||||
|
_run("failure", head_sha="sha", attempt=1),
|
||||||
|
]
|
||||||
|
}
|
||||||
|
client = _client(_resp(200, json_payload=payload))
|
||||||
|
with (
|
||||||
|
_patch_project_ci(),
|
||||||
|
patch("roboco.services.git.httpx.AsyncClient", return_value=client),
|
||||||
|
):
|
||||||
|
out = await svc.get_latest_ci_conclusion("roboco", workflow="ci.yml")
|
||||||
|
assert out is not None
|
||||||
|
assert out["conclusion"] == "success"
|
||||||
|
|
||||||
|
|
||||||
|
@pytest.mark.asyncio
|
||||||
|
async def test_get_latest_ci_conclusion_retries_transient_5xx() -> None:
|
||||||
|
# A transient 503 must not silently skip the cycle — retry, then succeed.
|
||||||
|
svc = _service()
|
||||||
|
ok = _resp(200, json_payload={"workflow_runs": [_run("failure")]})
|
||||||
|
client = MagicMock()
|
||||||
|
client.__aenter__ = AsyncMock(return_value=client)
|
||||||
|
client.__aexit__ = AsyncMock(return_value=False)
|
||||||
|
client.get = AsyncMock(side_effect=[_resp(503, text="busy"), ok])
|
||||||
|
with (
|
||||||
|
_patch_project_ci(),
|
||||||
|
patch("roboco.services.git.httpx.AsyncClient", return_value=client),
|
||||||
|
patch("roboco.services.git.asyncio.sleep", AsyncMock()),
|
||||||
|
):
|
||||||
|
out = await svc.get_latest_ci_conclusion("roboco", workflow="ci.yml")
|
||||||
|
assert out is not None
|
||||||
|
assert out["conclusion"] == "failure"
|
||||||
|
expected_get_calls = 2
|
||||||
|
assert client.get.await_count == expected_get_calls
|
||||||
|
|||||||
Reference in New Issue
Block a user