feat(hermes): one open proposal per rule, and link every issue to its run
All checks were successful
Tests / Declarative: Post Actions passed: 1413

One SonarQube rule is usually one root cause spread across many files. S2208
appears in three Ariadne modules and the cognitive-complexity rule in dozens,
and a sweep with no memory of what it already proposed would open a
near-identical pull request for every instance. Thirty of those get read as
none, which costs more than proposing nothing.

The sweep now skips any rule that already has an open proposal for that
project. The rules under review are read back from the open pull requests'
own titles rather than from a stored index: the pull requests are the thing
that actually exists, an index could disagree with them, and disagreeing is
the one failure mode that matters here. Once the open one is dealt with, the
next instance of that rule becomes eligible again.

This is not the root-cause collapse - it does not make one pull request fix
every instance of a rule, it just stops proposing the same rule repeatedly.
The collapse needs multi-file patch sets, which the frozen patch contract
cannot express yet.

Fails open like every other duplicate check here: an unreadable list yields no
known rules, so a lookup failure costs one extra proposal rather than
silently dropping a whole rule.

Issues now link their run id into the Hermes console, matching what pull
requests already do. Both artifacts claim a model made the call; both should
let a reader open the page where that call is visible.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
codex 2026-08-07 00:39:05 -03:00
parent eebc9b16c4
commit 8bc8940d48
9 changed files with 257 additions and 18 deletions

View File

@ -26,6 +26,9 @@ _COMMIT_IDENTITY = {"name": "Hermes Agent", "email": "hermes@bstein.dev"}
_COMMIT_OK_STATUSES = {HTTP_OK, HTTP_CREATED} _COMMIT_OK_STATUSES = {HTTP_OK, HTTP_CREATED}
# The Hermes console reopens a finished run from its id at this route. # The Hermes console reopens a finished run from its id at this route.
RUN_PATH = "/chat?resume=" RUN_PATH = "/chat?resume="
# Every proposal title is built from this, so the incident an open pull
# request belongs to can be read back without storing a second index.
PR_TITLE_PREFIX = "fix(hermes): repair "
_BRANCH_RETRY_STATUSES = {HTTP_NOT_FOUND, HTTP_UNPROCESSABLE} _BRANCH_RETRY_STATUSES = {HTTP_NOT_FOUND, HTTP_UNPROCESSABLE}
_BRANCH_UNSAFE = re.compile(r"[^A-Za-z0-9._-]+") _BRANCH_UNSAFE = re.compile(r"[^A-Za-z0-9._-]+")
@ -86,6 +89,45 @@ def find_open_proposal(cfg: dict) -> dict[str, Any]:
return _oldest_repair_pull(response, _base_branch(cfg)) return _oldest_repair_pull(response, _base_branch(cfg))
def open_proposal_incidents(cfg: dict) -> tuple[list[str], str | None]:
"""List the incidents that already have an open repair pull request.
Inputs: `cfg` as for `fetch_file`. Outputs: (incident_ids, error).
Read back from the pull request titles rather than from a stored index,
because the pull requests are the thing that actually exists - an index
could disagree with them, and the whole point of the check is to not
propose something a reviewer is already looking at.
Fails open: on any error the list is empty, so a lookup failure produces a
duplicate rather than silently suppressing real work.
"""
base_url = _base_url(cfg)
if not base_url:
return [], "gitea base url is empty"
try:
with httpx.Client(timeout=_timeout(cfg)) as client:
response = client.get(
f"{base_url}/api/v1/repos/{_owner(cfg)}/{_repo(cfg)}/pulls",
headers=_headers(cfg),
params={"state": "open", "limit": _OPEN_PULLS_LIMIT},
)
if response.status_code != HTTP_OK:
return [], f"open proposal lookup http {response.status_code}"
payload = response.json()
except Exception as exc:
return [], f"open proposal lookup failed: {exc}"
if not isinstance(payload, list):
return [], "open proposal payload is not a list"
found = []
for pull in payload:
title = str(pull.get("title") or "") if isinstance(pull, dict) else ""
if title.startswith(PR_TITLE_PREFIX):
found.append(title[len(PR_TITLE_PREFIX) :].strip())
return found, None
def push_branch( def push_branch(
cfg: dict, incident_id: str, ref: Any, patch: Any, patched_contents: str cfg: dict, incident_id: str, ref: Any, patch: Any, patched_contents: str
) -> dict[str, Any]: ) -> dict[str, Any]:
@ -137,7 +179,7 @@ def open_pull_request( # noqa: PLR0913 - the body needs the full proposal prove
payload = { payload = {
"head": branch, "head": branch,
"base": _base_branch(cfg), "base": _base_branch(cfg),
"title": f"fix(hermes): repair {incident_id}", "title": f"{PR_TITLE_PREFIX}{incident_id}",
"body": _pr_body(incident_id, patch, analysis, run_id, cfg), "body": _pr_body(incident_id, patch, analysis, run_id, cfg),
} }
try: try:

View File

@ -38,7 +38,7 @@ _AUDIT_NOTE = (
"event types `hermes_autotriage_incident` and `hermes_autotriage_diagnosis`." "event types `hermes_autotriage_incident` and `hermes_autotriage_diagnosis`."
) )
_FOOTER_TEMPLATE = ( _FOOTER_TEMPLATE = (
"Filed automatically by Ariadne from a Hermes Agent diagnosis (run `{run_id}`). " "Filed automatically by Ariadne from a Hermes Agent diagnosis ({run}). "
"Hermes has no write access to this repository; no files or infrastructure were changed." "Hermes has no write access to this repository; no files or infrastructure were changed."
) )
# Some escalations never reach a model at all - a build still running has no # Some escalations never reach a model at all - a build still running has no
@ -168,7 +168,11 @@ def _footer(context: dict) -> str:
run_id = str(context.get("run_id") or "") run_id = str(context.get("run_id") or "")
if not run_id or run_id == "unknown": if not run_id or run_id == "unknown":
return _NO_RUN_FOOTER return _NO_RUN_FOOTER
return _FOOTER_TEMPLATE.format(run_id=run_id) # A bare id is something to copy; a link is a page to open, and that page
# is the whole answer to who decided this.
run_url = str(context.get("run_url") or "")
run = f"run [{run_id}]({run_url})" if run_url else f"run `{run_id}`"
return _FOOTER_TEMPLATE.format(run=run)
def _human_section(context: dict) -> str: def _human_section(context: dict) -> str:

View File

@ -22,7 +22,7 @@ from typing import Any
import httpx import httpx
from ..utils.logging import get_logger from ..utils.logging import get_logger
from . import hermes_incident_body as body from . import hermes_code_repair, hermes_incident_body as body
logger = get_logger(__name__) logger = get_logger(__name__)
@ -171,7 +171,9 @@ def issue_config(config: Any) -> dict[str, Any]:
} }
def issue_context(base: dict[str, Any], diagnosis: dict[str, Any]) -> dict[str, Any]: def issue_context(
base: dict[str, Any], diagnosis: dict[str, Any], hermes_ui_url: str = ""
) -> dict[str, Any]:
"""Flatten the incident, evidence bundle, and outcome into body inputs. """Flatten the incident, evidence bundle, and outcome into body inputs.
Inputs: the incident identity fields and the diagnosis dict passed to Inputs: the incident identity fields and the diagnosis dict passed to
@ -212,6 +214,7 @@ def issue_context(base: dict[str, Any], diagnosis: dict[str, Any]) -> dict[str,
# console text is the whole explanation and must not be dropped. # console text is the whole explanation and must not be dropped.
"observation": _observation(jenkins) if decision is None else "", "observation": _observation(jenkins) if decision is None else "",
"run_id": diagnosis.get("run_id"), "run_id": diagnosis.get("run_id"),
"run_url": hermes_code_repair.run_url(hermes_ui_url, str(diagnosis.get("run_id") or "")),
"code_proposal_url": str(proposal.get("url") or "") if isinstance(proposal, dict) else "", "code_proposal_url": str(proposal.get("url") or "") if isinstance(proposal, dict) else "",
} }
@ -241,7 +244,7 @@ def _file_issue(
if not cfg["enabled"] or target is None or _filed_count(tick_state) >= _max_per_tick(cfg): if not cfg["enabled"] or target is None or _filed_count(tick_state) >= _max_per_tick(cfg):
return None return None
repo_cfg = {**cfg, "owner": target[0], "repo": target[1]} repo_cfg = {**cfg, "owner": target[0], "repo": target[1]}
context = issue_context(base, diagnosis) context = issue_context(base, diagnosis, str(getattr(config, "hermes_ui_url", "") or ""))
existing = find_open_incident_issue( existing = find_open_incident_issue(
repo_cfg, job, str(context["classification"]), str(context["incident_id"]) repo_cfg, job, str(context["classification"]), str(context["incident_id"])
) )

View File

@ -171,7 +171,11 @@ def issue_url(incident_id: str, ui_url: str) -> str:
text = str(incident_id or "") text = str(incident_id or "")
if not base or not text.startswith(INCIDENT_PREFIX): if not base or not text.startswith(INCIDENT_PREFIX):
return "" return ""
project, separator, key = text[len(INCIDENT_PREFIX) :].partition("/") # `sonar/<project>/<rule>/<key>`; the rule sits between them so the
# project is read from the front and the key from the back.
remainder = text[len(INCIDENT_PREFIX) :]
project, separator, rest = remainder.partition("/")
_, _, key = rest.rpartition("/") if "/" in rest else ("", "", rest)
if not separator or not project.strip() or not key.strip(): if not separator or not project.strip() or not key.strip():
return "" return ""
return base + _ISSUE_PATH.format(project=project.strip(), key=key.strip()) return base + _ISSUE_PATH.format(project=project.strip(), key=key.strip())

View File

@ -27,7 +27,7 @@ from typing import Any
from ..settings import settings from ..settings import settings
from ..utils.logging import get_logger from ..utils.logging import get_logger
from . import hermes_code_flow, hermes_sonar_client from . import hermes_code_flow, hermes_code_repair, hermes_sonar_client
logger = get_logger(__name__) logger = get_logger(__name__)
@ -131,9 +131,10 @@ def _propose_for_project( # noqa: PLR0913 - one project needs every config the
issues, error = hermes_sonar_client.fetch_issues(sonar_cfg, project) issues, error = hermes_sonar_client.fetch_issues(sonar_cfg, project)
if error: if error:
return {"proposed": 0, "skipped": [f"{project}: {error}"]} return {"proposed": 0, "skipped": [f"{project}: {error}"]}
chosen = select_issue(issues, repo_cfg, config) open_rules = proposed_rules(repo_cfg, project)
chosen = select_issue(issues, repo_cfg, config, open_rules)
if chosen is None: if chosen is None:
return {"proposed": 0, "skipped": [f"{project}: no mechanically fixable finding"]} return {"proposed": 0, "skipped": [f"{project}: no new mechanically fixable finding"]}
result = hermes_code_flow.propose_code_fix( result = hermes_code_flow.propose_code_fix(
storage, storage,
incident_id=incident_id(project, chosen), incident_id=incident_id(project, chosen),
@ -148,11 +149,43 @@ def _propose_for_project( # noqa: PLR0913 - one project needs every config the
return {"proposed": 0, "skipped": [f"{project}: {result.get('reason')}"]} return {"proposed": 0, "skipped": [f"{project}: {result.get('reason')}"]}
def select_issue(issues: list[dict], repo_cfg: dict, config: Any) -> dict[str, Any] | None: def proposed_rules(repo_cfg: dict, project: str) -> set[str]:
"""Return the rules this project already has an open proposal for.
Inputs: the resolved per-repo cfg and the SonarQube project key. Outputs:
the rule ids, read back from the open pull requests' incident ids.
One rule is usually one root cause spread across many files: S2208 appears
in three Ariadne modules, and the cognitive-complexity rule in dozens. One
finding at a time with no memory would open a near-identical pull request
for every instance, and a reviewer facing thirty of those reads none of
them. Until the open one is dealt with, the rest of that rule waits.
Fails open, like every other duplicate check here: an unreadable list
yields an empty set, so the cost of a lookup failure is one extra proposal
rather than a whole rule silently going unreported.
"""
incidents, _error = hermes_code_repair.open_proposal_incidents(repo_cfg)
prefix = f"{hermes_sonar_client.INCIDENT_PREFIX}{project}/"
rules = set()
for incident in incidents:
if not incident.startswith(prefix):
continue
rule, separator, _key = incident[len(prefix) :].rpartition("/")
if separator and rule:
rules.add(rule)
return rules
def select_issue(
issues: list[dict], repo_cfg: dict, config: Any, open_rules: set[str] | None = None
) -> dict[str, Any] | None:
"""Pick the single most mechanically fixable finding for one project. """Pick the single most mechanically fixable finding for one project.
Inputs: the normalized findings, the resolved per-repo cfg (whose write Inputs: the normalized findings, the resolved per-repo cfg (whose write
allowlist decides what is patchable at all), and the settings object. allowlist decides what is patchable at all), the settings object, and the
rules that already have an open proposal.
Outputs: one finding, or None when none qualify. Outputs: one finding, or None when none qualify.
Ordered by effort, then severity, then key. Effort leads because it is the Ordered by effort, then severity, then key. Effort leads because it is the
@ -163,10 +196,12 @@ def select_issue(issues: list[dict], repo_cfg: dict, config: Any) -> dict[str, A
""" """
ceiling = _max_effort_minutes(config) ceiling = _max_effort_minutes(config)
seen = open_rules or set()
eligible = [ eligible = [
issue issue
for issue in issues for issue in issues
if _is_writable(str(issue.get("path") or ""), repo_cfg) if str(issue.get("rule") or "") not in seen
and _is_writable(str(issue.get("path") or ""), repo_cfg)
and effort_minutes(issue.get("effort")) is not None and effort_minutes(issue.get("effort")) is not None
and (effort_minutes(issue.get("effort")) or 0) <= ceiling and (effort_minutes(issue.get("effort")) or 0) <= ceiling
] ]
@ -236,9 +271,14 @@ def bundle_for(project: str, issue: dict[str, Any]) -> dict[str, Any]:
def incident_id(project: str, issue: dict[str, Any]) -> str: def incident_id(project: str, issue: dict[str, Any]) -> str:
"""Name the incident for one finding, stable across sweeps.""" """Name the incident for one finding, stable across sweeps.
return f"sonar/{project}/{issue.get('key')}" Carries the rule as well as the key so an open proposal announces which
root cause is already being reviewed. The alternative was a second index
of proposed rules, which could disagree with the pull requests themselves.
"""
return f"sonar/{project}/{issue.get('rule')}/{issue.get('key')}"
def client_config(config: Any) -> dict[str, Any]: def client_config(config: Any) -> dict[str, Any]:

View File

@ -3,6 +3,8 @@ from __future__ import annotations
import base64 import base64
import json import json
import httpx
from ariadne.services import hermes_code_repair as module from ariadne.services import hermes_code_repair as module
from ariadne.services.hermes_code_patch import ProposedPatch from ariadne.services.hermes_code_patch import ProposedPatch
@ -430,3 +432,48 @@ def test_a_build_driven_proposal_has_no_finding_line(monkeypatch) -> None:
) )
assert "SonarQube finding" not in calls["requests"][0][2]["json"]["body"] assert "SonarQube finding" not in calls["requests"][0][2]["json"]["body"]
def test_open_proposal_incidents_reads_them_from_the_titles(monkeypatch) -> None:
"""The pull requests are the thing that exists; an index could disagree."""
calls = _install_http(
monkeypatch,
[
FakeResponse(
200,
[
{"title": "fix(hermes): repair sonar/ariadne/python:S2208/AZ1"},
{"title": "fix(hermes): repair ariadne/408"},
{"title": "chore: something a person opened"},
{"title": ""},
"not-a-dict",
],
)
],
)
incidents, error = module.open_proposal_incidents(_cfg())
assert error is None
assert incidents == ["sonar/ariadne/python:S2208/AZ1", "ariadne/408"]
assert calls["requests"][0][2]["params"]["state"] == "open"
def test_open_proposal_incidents_fails_open(monkeypatch) -> None:
"""A duplicate is noise; a suppressed rule is lost work."""
_install_http(monkeypatch, [FakeResponse(500, None)])
assert module.open_proposal_incidents(_cfg()) == ([], "open proposal lookup http 500")
_install_http(monkeypatch, [FakeResponse(200, {"not": "a list"})])
assert module.open_proposal_incidents(_cfg()) == ([], "open proposal payload is not a list")
_install_http(monkeypatch, [httpx.ConnectError("refused")])
incidents, error = module.open_proposal_incidents(_cfg())
assert incidents == []
assert "open proposal lookup failed" in error
assert module.open_proposal_incidents({**_cfg(), "gitea_base_url": ""}) == (
[],
"gitea base url is empty",
)

View File

@ -7,6 +7,23 @@ from types import SimpleNamespace
from ariadne.services import hermes_incident_body as body from ariadne.services import hermes_incident_body as body
from ariadne.services import hermes_incident_issue as module from ariadne.services import hermes_incident_issue as module
_BASE = {"incident_id": "ariadne/408", "job": "ariadne", "build_number": 408}
def _diagnosis(**overrides):
"""A completed diagnosis, as maybe_file_issue receives it."""
diagnosis = {
"bundle": {"jenkins": {"url": "https://ci.example/job/ariadne/408/"}},
"outcome": None,
"authorize_reason": "human_required",
"run_id": "run_a5af87af",
}
diagnosis.update(overrides)
return diagnosis
def test_body_does_not_claim_a_diagnosis_that_never_happened() -> None: def test_body_does_not_claim_a_diagnosis_that_never_happened() -> None:
"""A hung build is escalated without any model call. """A hung build is escalated without any model call.
@ -99,3 +116,21 @@ def test_explicit_classification_is_used_only_without_a_decision() -> None:
"classification": "build_exceeded_time_cap", "run_id": "run_x"}, "classification": "build_exceeded_time_cap", "run_id": "run_x"},
) )
assert ctx["classification"] == "pytest_test_failure" assert ctx["classification"] == "pytest_test_failure"
def test_the_issue_footer_links_to_the_hermes_run() -> None:
"""A bare id is something to copy; a link is a page to open."""
context = module.issue_context(_BASE, _diagnosis(), "https://agent.bstein.dev")
rendered = body.issue_body(context)
assert context["run_url"].startswith("https://agent.bstein.dev/chat?resume=")
assert f"run [{context['run_id']}]({context['run_url']})" in rendered
def test_the_footer_falls_back_to_a_bare_id_without_a_console() -> None:
context = module.issue_context(_BASE, _diagnosis())
rendered = body.issue_body(context)
assert context["run_url"] == ""
assert f"run `{context['run_id']}`" in rendered

View File

@ -493,3 +493,4 @@ def test_broken_repo_map_entries_file_nothing(monkeypatch) -> None:
is None is None
) )
assert calls["requests"] == [] assert calls["requests"] == []

View File

@ -72,6 +72,10 @@ def wiring(monkeypatch):
return calls["result"] return calls["result"]
monkeypatch.setattr(module.hermes_code_flow, "propose_code_fix", _propose) monkeypatch.setattr(module.hermes_code_flow, "propose_code_fix", _propose)
monkeypatch.setattr(
module.hermes_code_repair, "open_proposal_incidents",
lambda cfg: (calls.get("open_incidents", []), None),
)
return calls return calls
@ -146,7 +150,7 @@ def test_a_sweep_opens_one_proposal_for_the_chosen_finding(wiring) -> None:
assert result == {"proposed": 1, "skipped": [], "projects": 1} assert result == {"proposed": 1, "skipped": [], "projects": 1}
proposal = wiring["proposals"][0] proposal = wiring["proposals"][0]
assert proposal["incident_id"] == "sonar/ariadne/AZ-1" assert proposal["incident_id"] == "sonar/ariadne/python:S1172/AZ-1"
assert proposal["job"] == "ariadne" assert proposal["job"] == "ariadne"
assert proposal["build_number"] == "sonar-AZ-1" assert proposal["build_number"] == "sonar-AZ-1"
@ -159,7 +163,7 @@ def test_the_bundle_offers_only_the_file_the_finding_names() -> None:
assert bundle["sonarqube"] == {"project": "ariadne", "issues": [_issue()]} assert bundle["sonarqube"] == {"project": "ariadne", "issues": [_issue()]}
assert bundle["jenkins"]["console_failures"] == [] assert bundle["jenkins"]["console_failures"] == []
assert bundle["jenkins"]["console_tail"] == "" assert bundle["jenkins"]["console_tail"] == ""
assert bundle["incident_id"] == "sonar/ariadne/AZ-1" assert bundle["incident_id"] == "sonar/ariadne/python:S1172/AZ-1"
def test_the_sweep_budget_stops_after_its_quota(wiring) -> None: def test_the_sweep_budget_stops_after_its_quota(wiring) -> None:
@ -216,7 +220,7 @@ def test_a_project_with_nothing_mechanical_is_reported(wiring) -> None:
result = module.sweep(_Storage(), _config(), {}) result = module.sweep(_Storage(), _config(), {})
assert result["skipped"] == ["ariadne: no mechanically fixable finding"] assert result["skipped"] == ["ariadne: no new mechanically fixable finding"]
def test_a_sweep_never_takes_down_the_scheduler(monkeypatch) -> None: def test_a_sweep_never_takes_down_the_scheduler(monkeypatch) -> None:
@ -360,3 +364,62 @@ def test_a_bundles_origin_decides_the_framing(bundle, expected) -> None:
from ariadne.services import hermes_code_prompt from ariadne.services import hermes_code_prompt
assert hermes_code_prompt.is_quality_sweep(bundle) is expected assert hermes_code_prompt.is_quality_sweep(bundle) is expected
def test_a_rule_already_under_review_is_not_proposed_again(wiring) -> None:
"""One rule is one root cause; thirty near-identical PRs get read as none."""
wiring["open_incidents"] = ["sonar/ariadne/python:S1172/AZ-OTHER"]
wiring["issues"] = [
_issue(key="a", rule="python:S1172"),
_issue(key="b", rule="python:S2208", effort="10min"),
]
result = module.sweep(_Storage(), _config(), {})
assert result["proposed"] == 1
assert wiring["proposals"][0]["incident_id"] == "sonar/ariadne/python:S2208/b"
def test_every_rule_under_review_means_nothing_new_to_propose(wiring) -> None:
wiring["open_incidents"] = ["sonar/ariadne/python:S1172/AZ-OTHER"]
wiring["issues"] = [_issue(rule="python:S1172")]
result = module.sweep(_Storage(), _config(), {})
assert result["proposed"] == 0
assert result["skipped"] == ["ariadne: no new mechanically fixable finding"]
def test_another_projects_open_proposal_does_not_block_this_one(wiring) -> None:
wiring["open_incidents"] = ["sonar/metis/python:S1172/AZ-X", "ariadne/408"]
assert module.sweep(_Storage(), _config(), {})["proposed"] == 1
def test_the_rules_under_review_are_read_from_the_open_proposals(monkeypatch) -> None:
monkeypatch.setattr(
module.hermes_code_repair, "open_proposal_incidents",
lambda cfg: (
[
"sonar/ariadne/python:S2208/AZ1",
"sonar/ariadne/python:S3776/AZ2",
"sonar/other/python:S1172/AZ3",
"ariadne/408",
"sonar/ariadne/malformed",
],
None,
),
)
assert module.proposed_rules(REPO_CFG, "ariadne") == {"python:S2208", "python:S3776"}
def test_an_unreadable_proposal_list_costs_a_duplicate_not_a_silent_rule(monkeypatch) -> None:
"""Failing open here means one extra PR; failing closed loses a whole rule."""
monkeypatch.setattr(
module.hermes_code_repair, "open_proposal_incidents", lambda cfg: ([], "http 500")
)
assert module.proposed_rules(REPO_CFG, "ariadne") == set()