From 281f06dccd80efa55b81b5be6f81289731763945 Mon Sep 17 00:00:00 2001 From: codex Date: Thu, 6 Aug 2026 11:01:57 -0300 Subject: [PATCH] feat(hermes): triage the branches inside multibranch folders 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 read lastBuild, so hermes-code-demo-branches was returned as skipped on every tick since it was created and no branch could ever be triaged. Expand a folder into its branch jobs and process each. Jenkins addresses them as parent/job/branch, which the existing fetch already builds correctly, and branch names stay URL-encoded because re-encoding them yields a 404. Expansion is capped by ARIADNE_HERMES_MAX_BRANCHES (default 5): 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. A name containing a slash is refused outright, since it would escape the parent and address an unrelated job. The Jenkins transport moves to its own module so the orchestrator reads as decision logic, and jobs[name] rides along on the existing request rather than costing a second call per job per tick. Co-Authored-By: Claude Opus 5 --- ariadne/services/hermes_autotriage.py | 46 +++--------- ariadne/services/hermes_jenkins_client.py | 78 +++++++++++++++++++ ariadne/services/hermes_multibranch.py | 65 ++++++++++++++++ ariadne/settings.py | 1 + ariadne/settings_sections.py | 1 + tests/hermes_autotriage_harness.py | 9 ++- tests/test_hermes_autotriage.py | 6 +- tests/test_hermes_code_flow.py | 13 +++- tests/test_hermes_multibranch.py | 91 +++++++++++++++++++++++ 9 files changed, 267 insertions(+), 43 deletions(-) create mode 100644 ariadne/services/hermes_jenkins_client.py create mode 100644 ariadne/services/hermes_multibranch.py create mode 100644 tests/test_hermes_multibranch.py 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"