diff --git a/ariadne/services/hermes_autotriage.py b/ariadne/services/hermes_autotriage.py index 8579b76..513eb87 100644 --- a/ariadne/services/hermes_autotriage.py +++ b/ariadne/services/hermes_autotriage.py @@ -11,6 +11,8 @@ from ..utils.logging import get_logger from . import hermes_agent_client, hermes_autotriage_repair, hermes_code_flow, hermes_incident_issue from . import hermes_triage_prompt from . import hermes_hung_builds +from . import hermes_jenkins_client +from . import hermes_multibranch from . import hermes_infra_signals from . import hermes_autotriage_decision as hermes_decision from . import hermes_autotriage_events as hermes_events @@ -36,7 +38,6 @@ RETRY_ACTION = "retry_transient_infra" REBUILD_FAILED_REASON = "repair rebuild failed" CODE_FIX_PROPOSED_REASON = "code_fix_proposed" -_LAST_BUILD_TREE = "lastBuild[number,result,building,timestamp,duration,url]" _RUN_COMPLETED = "completed" _UNKNOWN_ACTION_LABEL = "unknown" @@ -57,6 +58,8 @@ def run_hermes_autotriage(storage: Any) -> dict[str, Any]: tick_state: dict[str, Any] = {} for job in settings.hermes_autotriage_job_allowlist: jobs[job] = _process_job(storage, job, incidents, tick_state) + for branch in jobs[job].get("branches") or []: + jobs[branch] = _process_job(storage, branch, incidents, tick_state) HERMES_TRIAGE_DURATION_SECONDS.labels(phase="total").set(time.time() - started) logger.info( "hermes autotriage tick finished", @@ -70,7 +73,11 @@ def _process_job( ) -> dict[str, Any]: """Inspect one allowlisted job's last build and advance its incidents.""" - last_build = _fetch_last_build(job) + payload = hermes_jenkins_client.fetch_job_payload(job) + if hermes_multibranch.is_folder(payload): + branches = hermes_multibranch.branch_job_paths(payload, job, settings.hermes_max_branches) + return {"status": "folder", "branches": branches} + last_build = hermes_jenkins_client.last_build(payload) if last_build is None or last_build.get("number") is None: return {"status": "skipped"} if last_build.get("building"): @@ -109,41 +116,6 @@ def _handle_running_build( return {"status": "human_required", "incident_id": incident_id, "reason": reason} -def _fetch_last_build(job: str) -> dict[str, Any] | None: - """Fetch the job's lastBuild summary from Jenkins, or None on failure.""" - - base_url = settings.jenkins_base_url.strip().rstrip("/") - if not base_url: - return None - try: - with httpx.Client(**_jenkins_client_kwargs()) as client: - response = client.get(f"{base_url}/job/{job}/api/json", params={"tree": _LAST_BUILD_TREE}) - response.raise_for_status() - payload = response.json() - except Exception as exc: - logger.info( - "hermes autotriage jenkins fetch failed", - extra={"event": "hermes_autotriage", "status": "jenkins_error", "job": job, "detail": str(exc)}, - ) - return None - last_build = payload.get("lastBuild") if isinstance(payload, dict) else None - return last_build if isinstance(last_build, dict) else None - - -def _jenkins_client_kwargs() -> dict[str, Any]: - """Build httpx client kwargs with Ariadne's Jenkins read credential.""" - - kwargs: dict[str, Any] = { - "timeout": settings.jenkins_api_timeout_sec, - "follow_redirects": True, - } - username = settings.jenkins_api_user.strip() - token = settings.jenkins_api_token.strip() - if username and token: - kwargs["auth"] = (username, token) - return kwargs - - def _resolve_on_success( storage: Any, job: str, last_build: dict[str, Any], incidents: dict[str, dict[str, Any]] ) -> dict[str, Any]: diff --git a/ariadne/services/hermes_jenkins_client.py b/ariadne/services/hermes_jenkins_client.py new file mode 100644 index 0000000..657d59a --- /dev/null +++ b/ariadne/services/hermes_jenkins_client.py @@ -0,0 +1,78 @@ +"""Read-only Jenkins access for the auto-triage tick. + +Kept apart from the orchestrator so the HTTP concerns - credentials, the field +selector, and failure handling - stay in one place, and so the orchestrator +reads as decision logic rather than transport. +""" + +from __future__ import annotations + +from typing import Any + +import httpx + +from ..settings import settings +from ..utils.logging import get_logger + + +logger = get_logger(__name__) + +# `jobs[name]` is requested alongside the build fields so a multibranch folder, +# which has no lastBuild of its own, can be recognised from the same call +# rather than costing a second request per job per tick. +JOB_TREE = "lastBuild[number,result,building,timestamp,duration,url],jobs[name]" + + +def base_url() -> str: + """Return the configured Jenkins base URL without a trailing slash.""" + + return settings.jenkins_base_url.strip().rstrip("/") + + +def client_kwargs() -> dict[str, Any]: + """Build httpx client kwargs carrying Ariadne's Jenkins read credential.""" + + kwargs: dict[str, Any] = { + "timeout": settings.jenkins_api_timeout_sec, + "follow_redirects": True, + } + username = settings.jenkins_api_user.strip() + token = settings.jenkins_api_token.strip() + if username and token: + kwargs["auth"] = (username, token) + return kwargs + + +def fetch_job_payload(job: str) -> dict[str, Any] | None: + """Fetch one job's build summary, or None when Jenkins cannot be read. + + Inputs: a job name, which may be a nested path such as `parent/job/branch` + because Jenkins addresses branch jobs that way. Outputs: the parsed + payload, or None on any transport, status, or parse failure - a tick must + survive Jenkins being briefly unavailable. + """ + + root = base_url() + if not root: + return None + try: + with httpx.Client(**client_kwargs()) as client: + response = client.get(f"{root}/job/{job}/api/json", params={"tree": JOB_TREE}) + response.raise_for_status() + payload = response.json() + except Exception as exc: + logger.info( + "hermes autotriage jenkins fetch failed", + extra={"event": "hermes_autotriage", "status": "jenkins_error", "job": job, "detail": str(exc)}, + ) + return None + return payload if isinstance(payload, dict) else None + + +def last_build(payload: Any) -> dict[str, Any] | None: + """Return the lastBuild object from a job payload, if it has one.""" + + if not isinstance(payload, dict): + return None + build = payload.get("lastBuild") + return build if isinstance(build, dict) else None diff --git a/ariadne/services/hermes_multibranch.py b/ariadne/services/hermes_multibranch.py new file mode 100644 index 0000000..3d386fc --- /dev/null +++ b/ariadne/services/hermes_multibranch.py @@ -0,0 +1,65 @@ +"""Expand Jenkins multibranch folders into the branch jobs they contain. + +A multibranch project is a folder, not a job: its `/api/json` carries no +`lastBuild` at all, only a `jobs` array with one child per branch. Detection +reads `lastBuild`, so such a job was silently skipped on every tick and no +branch could ever be triaged. + +Expansion is capped rather than unbounded. A repository with many active +branches would otherwise multiply the watched job count without limit and +could open one incident per failing branch in a single tick. +""" + +from __future__ import annotations + +from typing import Any + + +FOLDER_CLASS_MARKERS = ("MultiBranchProject", "Folder") +DEFAULT_MAX_BRANCHES = 5 + +_JOBS_KEY = "jobs" + + +def is_folder(payload: Any) -> bool: + """Report whether a job payload is a folder rather than a buildable job. + + Inputs: the parsed `/api/json` payload for one job. Outputs: True when it + exposes child jobs and no `lastBuild` of its own. Both conditions are + required so a buildable job that merely reports children is never mistaken + for a folder. Never raises. + """ + + if not isinstance(payload, dict): + return False + if payload.get("lastBuild") is not None: + return False + return isinstance(payload.get(_JOBS_KEY), list) + + +def branch_job_paths(payload: Any, parent: str, max_branches: int = DEFAULT_MAX_BRANCHES) -> list[str]: + """Return the API paths of the branches inside a multibranch folder. + + Inputs: the folder's `/api/json` payload, the parent job name, and the cap + on how many branches to expand. Outputs: paths shaped `parent/job/branch`, + which is what Jenkins' nested job URLs expect and what the existing + last-build fetch already builds correctly. + + Branch names arrive URL-encoded (`hermes-repair%2F4`) and are passed + through untouched: re-encoding or decoding them yields a 404. Never + raises; returns [] for anything that is not a folder. + """ + + if not is_folder(payload) or max_branches <= 0: + return [] + paths: list[str] = [] + for child in payload.get(_JOBS_KEY) or []: + if not isinstance(child, dict): + continue + name = str(child.get("name") or "").strip() + if not name or "/" in name: + continue + paths.append(f"{parent}/job/{name}") + if len(paths) >= max_branches: + break + return paths diff --git a/ariadne/settings.py b/ariadne/settings.py index cdd6b3d..ce6d0ce 100644 --- a/ariadne/settings.py +++ b/ariadne/settings.py @@ -190,6 +190,7 @@ class Settings: hermes_parameterized_jobs: list[str] hermes_min_confidence: float hermes_hung_build_minutes: float + hermes_max_branches: int hermes_max_actions_per_incident: int hermes_api_url: str hermes_api_key: str diff --git a/ariadne/settings_sections.py b/ariadne/settings_sections.py index ab30ac7..25264ba 100644 --- a/ariadne/settings_sections.py +++ b/ariadne/settings_sections.py @@ -290,6 +290,7 @@ def _hermes_autotriage_config() -> dict[str, Any]: ], "hermes_min_confidence": _env_float("ARIADNE_HERMES_MIN_CONFIDENCE", 0.85), "hermes_hung_build_minutes": _env_float("ARIADNE_HERMES_HUNG_BUILD_MINUTES", 45.0), + "hermes_max_branches": _env_int("ARIADNE_HERMES_MAX_BRANCHES", 5), "hermes_max_actions_per_incident": _env_int("ARIADNE_HERMES_MAX_ACTIONS_PER_INCIDENT", 1), "hermes_api_url": _env( "ARIADNE_HERMES_API_URL", diff --git a/tests/hermes_autotriage_harness.py b/tests/hermes_autotriage_harness.py index 6f4e667..7ffbc5d 100644 --- a/tests/hermes_autotriage_harness.py +++ b/tests/hermes_autotriage_harness.py @@ -59,6 +59,7 @@ def _settings(**overrides) -> SimpleNamespace: # type: ignore[no-untyped-def] "hermes_parameterized_jobs": [JOB], "hermes_min_confidence": 0.85, "hermes_hung_build_minutes": 45.0, + "hermes_max_branches": 5, "hermes_max_actions_per_incident": 1, "hermes_api_url": "http://hermes:8642", "hermes_api_key": "key", @@ -168,7 +169,7 @@ def _install_jenkins(monkeypatch, calls, last_build, exc) -> None: # type: igno raise exc return FakeResponse({"lastBuild": last_build}) - monkeypatch.setattr(module.httpx, "Client", FakeClient) + monkeypatch.setattr(module.hermes_jenkins_client.httpx, "Client", FakeClient) def _prepare( # type: ignore[no-untyped-def] # noqa: PLR0913 @@ -187,7 +188,11 @@ def _prepare( # type: ignore[no-untyped-def] # noqa: PLR0913 ): storage = storage if storage is not None else FakeStorage() calls: dict = {"gets": [], "triage": [], "repairs": [], "rebuilds": [], "retries": []} - monkeypatch.setattr(module, "settings", cfg if cfg is not None else _settings()) + resolved = cfg if cfg is not None else _settings() + monkeypatch.setattr(module, "settings", resolved) + # The Jenkins transport holds its own settings reference, so patching + # only the orchestrator leaves it reading the real configuration. + monkeypatch.setattr(module.hermes_jenkins_client, "settings", resolved) _install_jenkins( monkeypatch, calls, last_build if last_build is not None else _build(12, "FAILURE"), jenkins_exc ) diff --git a/tests/test_hermes_autotriage.py b/tests/test_hermes_autotriage.py index f4e2e16..671f4e3 100644 --- a/tests/test_hermes_autotriage.py +++ b/tests/test_hermes_autotriage.py @@ -341,7 +341,11 @@ def test_jenkins_request_uses_basic_auth_and_tree(monkeypatch) -> None: assert env.calls["client_kwargs"]["auth"] == ("user", "token") url, params = env.calls["gets"][0] assert url == f"https://ci.example/job/{JOB}/api/json" - assert params == {"tree": "lastBuild[number,result,building,timestamp,duration,url]"} + tree = params["tree"] + assert "lastBuild[number,result,building,timestamp,duration,url]" in tree + # jobs[name] rides along on the same request so a multibranch folder, which + # carries no lastBuild, is recognisable without a second call per tick. + assert "jobs[name]" in tree def _issue_settings(**overrides): # type: ignore[no-untyped-def] diff --git a/tests/test_hermes_code_flow.py b/tests/test_hermes_code_flow.py index 5d020fe..8790bce 100644 --- a/tests/test_hermes_code_flow.py +++ b/tests/test_hermes_code_flow.py @@ -369,9 +369,16 @@ def _prepare_orchestrator(monkeypatch, flow_result, **setting_overrides): # typ calls: list = [] monkeypatch.setattr(autotriage, "settings", _orchestrator_settings(**setting_overrides)) monkeypatch.setattr( - autotriage, - "_fetch_last_build", - lambda job: {"number": 7, "result": "FAILURE", "building": False, "url": "https://ci.example/7/"}, + autotriage.hermes_jenkins_client, + "fetch_job_payload", + lambda job: { + "lastBuild": { + "number": 7, + "result": "FAILURE", + "building": False, + "url": "https://ci.example/7/", + } + }, ) monkeypatch.setattr(autotriage.hermes_evidence, "collect_evidence", lambda i, j, b: dict(BUNDLE)) diff --git a/tests/test_hermes_multibranch.py b/tests/test_hermes_multibranch.py new file mode 100644 index 0000000..9375339 --- /dev/null +++ b/tests/test_hermes_multibranch.py @@ -0,0 +1,91 @@ +"""Tests for expanding Jenkins multibranch folders into branch jobs.""" + +from __future__ import annotations + +from ariadne.services import hermes_multibranch as module + + +def _folder(*names: str) -> dict: + return {"jobs": [{"name": n} for n in names]} + + +def test_a_folder_has_child_jobs_and_no_last_build() -> None: + assert module.is_folder(_folder("master")) is True + + +def test_a_buildable_job_is_never_treated_as_a_folder() -> None: + """Both conditions are required, or a real job could be expanded away.""" + + assert module.is_folder({"lastBuild": {"number": 1}, "jobs": [{"name": "x"}]}) is False + assert module.is_folder({"lastBuild": {"number": 1}}) is False + assert module.is_folder({}) is False + assert module.is_folder(None) is False + assert module.is_folder("not-a-dict") is False + + +def test_branches_expand_to_nested_jenkins_paths() -> None: + """Jenkins addresses a branch as parent/job/branch.""" + + paths = module.branch_job_paths(_folder("master", "develop"), "demo-branches") + assert paths == ["demo-branches/job/master", "demo-branches/job/develop"] + + +def test_encoded_branch_names_are_passed_through_untouched() -> None: + """Re-encoding or decoding a branch name yields a 404.""" + + paths = module.branch_job_paths(_folder("hermes-repair%2F4"), "demo") + assert paths == ["demo/job/hermes-repair%2F4"] + + +def test_expansion_is_capped() -> None: + """A busy repository must not multiply the watched job count without limit.""" + + folder = _folder(*[f"b{i}" for i in range(20)]) + assert len(module.branch_job_paths(folder, "demo")) == module.DEFAULT_MAX_BRANCHES + assert len(module.branch_job_paths(folder, "demo", max_branches=2)) == 2 + assert module.branch_job_paths(folder, "demo", max_branches=0) == [] + assert module.branch_job_paths(folder, "demo", max_branches=-1) == [] + + +def test_malformed_children_are_skipped_not_fatal() -> None: + folder = {"jobs": [None, {"name": ""}, {"name": " "}, {"nope": 1}, {"name": "ok"}]} + assert module.branch_job_paths(folder, "demo") == ["demo/job/ok"] + + +def test_a_name_containing_a_slash_is_refused() -> None: + """A raw slash would escape the parent and address an unrelated job.""" + + assert module.branch_job_paths(_folder("a/b"), "demo") == [] + + +def test_non_folders_expand_to_nothing() -> None: + assert module.branch_job_paths({"lastBuild": {"number": 3}}, "demo") == [] + assert module.branch_job_paths(None, "demo") == [] + + +def test_tick_expands_a_folder_into_its_branches(monkeypatch) -> None: + """End to end: a multibranch job must stop being silently skipped.""" + + from ariadne.services import hermes_autotriage as autotriage + from tests.hermes_autotriage_harness import _prepare + + env = _prepare(monkeypatch) + + payloads = { + "hermes-triage-demo": {"jobs": [{"name": "master"}, {"name": "hermes-repair%2F4"}]}, + "hermes-triage-demo/job/master": { + "lastBuild": {"number": 3, "result": "SUCCESS", "building": False, "url": "u"} + }, + "hermes-triage-demo/job/hermes-repair%2F4": { + "lastBuild": {"number": 4, "result": "SUCCESS", "building": False, "url": "u"} + }, + } + monkeypatch.setattr( + autotriage.hermes_jenkins_client, "fetch_job_payload", lambda job: payloads.get(job) + ) + + jobs = autotriage.run_hermes_autotriage(env.storage)["jobs"] + + assert jobs["hermes-triage-demo"]["status"] == "folder" + assert jobs["hermes-triage-demo/job/master"]["status"] == "healthy" + assert jobs["hermes-triage-demo/job/hermes-repair%2F4"]["status"] == "healthy"