diff --git a/ariadne/services/hermes_code_repair.py b/ariadne/services/hermes_code_repair.py index 418c22e..545dc97 100644 --- a/ariadne/services/hermes_code_repair.py +++ b/ariadne/services/hermes_code_repair.py @@ -7,6 +7,7 @@ from typing import Any import httpx from ..utils.logging import get_logger +from . import hermes_sonar_client logger = get_logger(__name__) @@ -286,6 +287,13 @@ def _pr_body(incident_id: str, patch: Any, analysis: str, run_id: str, cfg: dict "", f"**Incident:** {incident_id}", f"**File:** `{patch.path}`", + ] + finding = hermes_sonar_client.issue_url(incident_id, cfg.get("sonar_ui_url")) + if finding: + # A sweep proposal exists because of one finding; the reviewer's first + # question is what it said, and an issue key is not an answer. + lines.append(f"**SonarQube finding:** {finding}") + lines += [ f"**Analysis:** {analysis}", f"**Rationale:** {patch.rationale}", ] diff --git a/ariadne/services/hermes_code_repos.py b/ariadne/services/hermes_code_repos.py index a4cfcbb..050958c 100644 --- a/ariadne/services/hermes_code_repos.py +++ b/ariadne/services/hermes_code_repos.py @@ -55,6 +55,8 @@ def build_config(config: Any) -> dict[str, Any]: "repos": _parse_repo_map(getattr(config, "hermes_code_repos", "")), # Shown in the pull request so a reviewer can read the run that wrote it. "hermes_ui_url": str(getattr(config, "hermes_ui_url", "") or ""), + # Shown in sweep proposals so the finding itself is one click away. + "sonar_ui_url": str(getattr(config, "hermes_sonar_ui_url", "") or ""), "job_prefixes": _parse_list_map(getattr(config, "hermes_code_prefixes", "")), "job_suffixes": _parse_list_map(getattr(config, "hermes_code_suffixes", "")), "job_base_branches": dict(_parse_pairs(getattr(config, "hermes_code_base_branches", ""))), diff --git a/ariadne/services/hermes_sonar_client.py b/ariadne/services/hermes_sonar_client.py index fc0b743..981398e 100644 --- a/ariadne/services/hermes_sonar_client.py +++ b/ariadne/services/hermes_sonar_client.py @@ -149,6 +149,34 @@ def component_path(component: Any) -> str: return path.strip() if separator else "" +INCIDENT_PREFIX = "sonar/" +# The console opens one finding in place, with the rule, the effort estimate +# and the offending lines highlighted. +_ISSUE_PATH = "/project/issues?resolved=false&id={project}&open={key}" + + +def issue_url(incident_id: str, ui_url: str) -> str: + """Build the console link for the finding an incident came from. + + Inputs: an incident id shaped `sonar//`, and the SonarQube + base url. Outputs: the deep link, or "" for any incident that did not come + from a sweep - a build-driven repair has no finding to point at. + + Parsed from the incident id rather than threaded through the flow: the id + already carries both halves, and inventing a second path for them to travel + is a second thing that can disagree with the first. + """ + + base = str(ui_url or "").rstrip("/") + text = str(incident_id or "") + if not base or not text.startswith(INCIDENT_PREFIX): + return "" + parts = text[len(INCIDENT_PREFIX) :].split("/", 1) + if len(parts) != 2 or not parts[0].strip() or not parts[1].strip(): + return "" + return base + _ISSUE_PATH.format(project=parts[0].strip(), key=parts[1].strip()) + + def _types(cfg: dict) -> tuple[str, ...]: """Return the finding types this deployment allows, defaulting to all.""" diff --git a/ariadne/settings.py b/ariadne/settings.py index 45411b1..05b340a 100644 --- a/ariadne/settings.py +++ b/ariadne/settings.py @@ -319,6 +319,7 @@ class Settings: hermes_sonar_cron: str hermes_sonar_enabled: bool hermes_sonar_url: str + hermes_sonar_ui_url: str hermes_sonar_token: str hermes_sonar_projects: dict[str, str] hermes_sonar_types: list[str] diff --git a/ariadne/settings_hermes.py b/ariadne/settings_hermes.py index dedd69c..fe5c772 100644 --- a/ariadne/settings_hermes.py +++ b/ariadne/settings_hermes.py @@ -60,6 +60,7 @@ def _hermes_autotriage_config() -> dict[str, Any]: "ARIADNE_HERMES_SONAR_URL", "http://sonarqube.quality.svc.cluster.local:9000" ).rstrip("/"), "hermes_sonar_token": _env("ARIADNE_HERMES_SONAR_TOKEN", ""), + "hermes_sonar_ui_url": _env("ARIADNE_HERMES_SONAR_UI_URL", ""), "hermes_sonar_projects": _pair_map(_env("ARIADNE_HERMES_SONAR_PROJECTS", "")), "hermes_sonar_types": [ item.strip() diff --git a/tests/test_hermes_code_flow.py b/tests/test_hermes_code_flow.py index 3f61b9e..e4156a6 100644 --- a/tests/test_hermes_code_flow.py +++ b/tests/test_hermes_code_flow.py @@ -37,6 +37,7 @@ def _hermes_cfg() -> dict: def _code_cfg(**overrides) -> dict: # type: ignore[no-untyped-def] base = { "hermes_ui_url": "", + "sonar_ui_url": "", "candidate_path": "src/discount.py", "allowed_path_prefixes": ["src/"], "allowed_suffixes": [".py"], diff --git a/tests/test_hermes_code_repair.py b/tests/test_hermes_code_repair.py index b304307..2a662b4 100644 --- a/tests/test_hermes_code_repair.py +++ b/tests/test_hermes_code_repair.py @@ -398,3 +398,35 @@ def test_the_run_link_reopens_the_run_in_the_hermes_console() -> None: ) assert module.run_url("", "run_x") == "" assert module.run_url("https://agent.bstein.dev", " ") == "" + + +def test_a_sweep_proposal_links_to_the_finding_that_caused_it(monkeypatch) -> None: + """The reviewer's first question is what the finding said.""" + + calls = _install_http(monkeypatch, [FakeResponse(201, {"number": 6, "html_url": "u"})]) + module.open_pull_request( + {**_cfg(), "sonar_ui_url": "https://quality.bstein.dev"}, + "sonar/ariadne/AZ2y0FYFKy9i4pkIpNlV", + "run_x", + BRANCH, + _patch(), + "analysis", + ) + + body = calls["requests"][0][2]["json"]["body"] + assert "**SonarQube finding:** https://quality.bstein.dev/project/issues" in body + assert "open=AZ2y0FYFKy9i4pkIpNlV" in body + + +def test_a_build_driven_proposal_has_no_finding_line(monkeypatch) -> None: + calls = _install_http(monkeypatch, [FakeResponse(201, {"number": 6, "html_url": "u"})]) + module.open_pull_request( + {**_cfg(), "sonar_ui_url": "https://quality.bstein.dev"}, + INCIDENT_ID, + "run_x", + BRANCH, + _patch(), + "analysis", + ) + + assert "SonarQube finding" not in calls["requests"][0][2]["json"]["body"] diff --git a/tests/test_hermes_sonar_client.py b/tests/test_hermes_sonar_client.py index 4bab7ab..749b925 100644 --- a/tests/test_hermes_sonar_client.py +++ b/tests/test_hermes_sonar_client.py @@ -248,3 +248,27 @@ def test_the_timeout_falls_back_when_unparseable() -> None: ) def test_the_component_key_yields_the_repository_path(component, expected) -> None: assert module.component_path(component) == expected + + +@pytest.mark.parametrize( + ("incident", "expected"), + [ + ( + "sonar/ariadne/AZ2y0FYFKy9i4pkIpNlV", + "https://quality.bstein.dev/project/issues" + "?resolved=false&id=ariadne&open=AZ2y0FYFKy9i4pkIpNlV", + ), + # A build-driven repair has no finding to point at. + ("ariadne/408", ""), + ("sonar/ariadne", ""), + ("sonar//key", ""), + ("sonar/proj/ ", ""), + ("", ""), + ], +) +def test_the_finding_link_is_derived_from_the_incident_id(incident, expected) -> None: + assert module.issue_url(incident, "https://quality.bstein.dev/") == expected + + +def test_no_finding_link_without_a_configured_console() -> None: + assert module.issue_url("sonar/ariadne/AZ1", "") == ""