diff --git a/ariadne/services/hermes_code_flow.py b/ariadne/services/hermes_code_flow.py index b45eecb..6962bc5 100644 --- a/ariadne/services/hermes_code_flow.py +++ b/ariadne/services/hermes_code_flow.py @@ -412,7 +412,7 @@ def _publish( pull = hermes_code_repair.open_pull_request( code_cfg, incident.incident_id, - incident.build_number, + proposal.run_id, branch, proposal.patch, proposal.analysis, diff --git a/ariadne/services/hermes_code_repair.py b/ariadne/services/hermes_code_repair.py index 0dbb0d2..70abb45 100644 --- a/ariadne/services/hermes_code_repair.py +++ b/ariadne/services/hermes_code_repair.py @@ -115,13 +115,14 @@ def push_branch( return {"branch": branch, "committed": error is None, "error": error} -def open_pull_request( # noqa: PLR0913 - flow contract mirrors push_branch identity fields - cfg: dict, incident_id: str, build_number: int, branch: str, patch: Any, analysis: str +def open_pull_request( # noqa: PLR0913 - the body needs the full proposal provenance + cfg: dict, incident_id: str, run_id: str, branch: str, patch: Any, analysis: str ) -> dict[str, Any]: """Open the human-review pull request for a pushed repair branch. - Inputs: `cfg` as for `fetch_file`; the incident id, failed build number, - pushed branch name, the validated ProposedPatch, and the model analysis. + Inputs: `cfg` as for `fetch_file` plus optional `hermes_ui_url`; the + incident id; the Hermes run that produced the patch; the pushed branch + name; the validated ProposedPatch; and the model analysis. Outputs: {"pr_number", "url", "error"}; an existing PR reported by a 409 counts as success when the payload identifies it. Never raises and never logs the token. @@ -134,7 +135,7 @@ def open_pull_request( # noqa: PLR0913 - flow contract mirrors push_branch iden "head": branch, "base": _base_branch(cfg), "title": f"fix(hermes): repair {incident_id}", - "body": _pr_body(incident_id, patch, analysis), + "body": _pr_body(incident_id, patch, analysis, run_id, cfg), } try: with httpx.Client(timeout=_timeout(cfg)) as client: @@ -255,22 +256,37 @@ def _pr_result(response: Any) -> dict[str, Any]: return {"pr_number": number, "url": url, "error": None} -def _pr_body(incident_id: str, patch: Any, analysis: str) -> str: - """Render the markdown pull-request body for human review.""" +def _pr_body(incident_id: str, patch: Any, analysis: str, run_id: str, cfg: dict) -> str: + """Render the markdown pull-request body for human review. - return "\n".join( - [ - f"## Hermes repair proposal for incident {incident_id}", - "", - f"**Incident:** {incident_id}", - f"**File:** `{patch.path}`", - f"**Analysis:** {analysis}", - f"**Rationale:** {patch.rationale}", - "", - "Proposed by Hermes; validated and pushed by Ariadne; " - "requires human review — no automatic merge.", - ] - ) + Names the Hermes run that produced the patch. "Proposed by Hermes" is a + claim; the run id is the receipt, and without it a reviewer has no way to + read the prompt the model was given or the tool calls it made. The + diagnosis issues have carried this from the start - the pull requests, + which are the more consequential artifact, did not. + """ + + lines = [ + f"## Hermes repair proposal for incident {incident_id}", + "", + f"**Incident:** {incident_id}", + f"**File:** `{patch.path}`", + f"**Analysis:** {analysis}", + f"**Rationale:** {patch.rationale}", + ] + if run_id: + lines.append(f"**Hermes run:** `{run_id}`") + ui_url = str(cfg.get("hermes_ui_url") or "").rstrip("/") + if ui_url: + lines.append( + f"The prompt this run was given and every tool call it made are at {ui_url}." + ) + lines += [ + "", + "Proposed by Hermes; validated and pushed by Ariadne; " + "requires human review — no automatic merge.", + ] + return "\n".join(lines) def _branch_name(ref: Any) -> str: diff --git a/ariadne/services/hermes_code_repos.py b/ariadne/services/hermes_code_repos.py index 0f9be48..a4cfcbb 100644 --- a/ariadne/services/hermes_code_repos.py +++ b/ariadne/services/hermes_code_repos.py @@ -53,6 +53,8 @@ def build_config(config: Any) -> dict[str, Any]: "timeout_seconds": _GITEA_TIMEOUT_SECONDS, "legacy_job": str(getattr(config, "hermes_code_job", "") or ""), "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 ""), "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/settings.py b/ariadne/settings.py index b338ce5..45411b1 100644 --- a/ariadne/settings.py +++ b/ariadne/settings.py @@ -315,6 +315,7 @@ class Settings: jenkins_workspace_cleanup_cron: str testing_triage_cron: str hermes_autotriage_cron: str + hermes_ui_url: str hermes_sonar_cron: str hermes_sonar_enabled: bool hermes_sonar_url: str diff --git a/ariadne/settings_hermes.py b/ariadne/settings_hermes.py index 294a939..dedd69c 100644 --- a/ariadne/settings_hermes.py +++ b/ariadne/settings_hermes.py @@ -50,6 +50,7 @@ def _hermes_autotriage_config() -> dict[str, Any]: "http://hermes.hermes.svc.cluster.local:8642", ).rstrip("/"), "hermes_api_key": _env("ARIADNE_HERMES_API_KEY", ""), + "hermes_ui_url": _env("ARIADNE_HERMES_UI_URL", ""), "hermes_run_timeout_seconds": _env_float("ARIADNE_HERMES_RUN_TIMEOUT_SECONDS", 420.0), # SonarQube sweep. Disabled until projects are named: the map is # sonar-project=jenkins-job, and the job already carries the repository diff --git a/tests/test_hermes_code_flow.py b/tests/test_hermes_code_flow.py index 454768f..3f61b9e 100644 --- a/tests/test_hermes_code_flow.py +++ b/tests/test_hermes_code_flow.py @@ -36,6 +36,7 @@ def _hermes_cfg() -> dict: def _code_cfg(**overrides) -> dict: # type: ignore[no-untyped-def] base = { + "hermes_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 96bc0f2..09e74a7 100644 --- a/tests/test_hermes_code_repair.py +++ b/tests/test_hermes_code_repair.py @@ -295,7 +295,7 @@ def test_open_pull_request_created(monkeypatch) -> None: monkeypatch, [FakeResponse(201, {"number": 5, "html_url": "https://scm.example/pulls/5"})], ) - result = module.open_pull_request(_cfg(), INCIDENT_ID, 7, BRANCH, _patch(), "root cause analysis") + result = module.open_pull_request(_cfg(), INCIDENT_ID, "run_x", BRANCH, _patch(), "root cause analysis") assert result == {"pr_number": 5, "url": "https://scm.example/pulls/5", "error": None} method, url, kwargs = calls["requests"][0] assert (method, url) == ("POST", "https://scm.example/api/v1/repos/bstein/hermes-code-demo/pulls") @@ -314,31 +314,31 @@ def test_open_pull_request_created(monkeypatch) -> None: def test_open_pull_request_conflict_returns_existing(monkeypatch) -> None: _install_http(monkeypatch, [FakeResponse(409, {"number": 9, "html_url": "https://scm.example/pulls/9"})]) - result = module.open_pull_request(_cfg(), INCIDENT_ID, 7, BRANCH, _patch(), "analysis") + result = module.open_pull_request(_cfg(), INCIDENT_ID, "run_x", BRANCH, _patch(), "analysis") assert result == {"pr_number": 9, "url": "https://scm.example/pulls/9", "error": None} def test_open_pull_request_conflict_without_payload(monkeypatch) -> None: _install_http(monkeypatch, [FakeResponse(409)]) - result = module.open_pull_request(_cfg(), INCIDENT_ID, 7, BRANCH, _patch(), "analysis") + result = module.open_pull_request(_cfg(), INCIDENT_ID, "run_x", BRANCH, _patch(), "analysis") assert result == {"pr_number": None, "url": None, "error": "pull request already exists"} def test_open_pull_request_http_error(monkeypatch) -> None: _install_http(monkeypatch, [FakeResponse(500)]) - result = module.open_pull_request(_cfg(), INCIDENT_ID, 7, BRANCH, _patch(), "analysis") + result = module.open_pull_request(_cfg(), INCIDENT_ID, "run_x", BRANCH, _patch(), "analysis") assert result == {"pr_number": None, "url": None, "error": "pull request http 500"} def test_open_pull_request_created_with_bad_payload(monkeypatch) -> None: _install_http(monkeypatch, [FakeResponse(201, {"number": True})]) - result = module.open_pull_request(_cfg(), INCIDENT_ID, 7, BRANCH, _patch(), "analysis") + result = module.open_pull_request(_cfg(), INCIDENT_ID, "run_x", BRANCH, _patch(), "analysis") assert result == {"pr_number": None, "url": None, "error": None} def test_open_pull_request_request_exception(monkeypatch) -> None: _install_http(monkeypatch, [RuntimeError("down")]) - result = module.open_pull_request(_cfg(), INCIDENT_ID, 7, BRANCH, _patch(), "analysis") + result = module.open_pull_request(_cfg(), INCIDENT_ID, "run_x", BRANCH, _patch(), "analysis") assert result == {"pr_number": None, "url": None, "error": "pull request failed: down"} @@ -347,3 +347,42 @@ def test_open_pull_request_without_base_url(monkeypatch) -> None: result = module.open_pull_request(_cfg(gitea_base_url=" "), INCIDENT_ID, 7, BRANCH, _patch(), "a") assert result == {"pr_number": None, "url": None, "error": "gitea base url is empty"} assert calls["requests"] == [] + + +def test_the_pull_request_names_the_hermes_run(monkeypatch) -> None: + """"Proposed by Hermes" is a claim; the run id is the receipt.""" + + calls = _install_http(monkeypatch, [FakeResponse(201, {"number": 5, "html_url": "u"})]) + module.open_pull_request( + {**_cfg(), "hermes_ui_url": "https://agent.bstein.dev/"}, + INCIDENT_ID, + "run_a5af87af", + BRANCH, + _patch(), + "analysis", + ) + + body = calls["requests"][0][2]["json"]["body"] + assert "**Hermes run:** `run_a5af87af`" in body + assert "https://agent.bstein.dev." in body + assert "every tool call it made" in body + + +def test_the_pull_request_omits_the_link_when_no_ui_is_configured(monkeypatch) -> None: + calls = _install_http(monkeypatch, [FakeResponse(201, {"number": 5, "html_url": "u"})]) + module.open_pull_request(_cfg(), INCIDENT_ID, "run_x", BRANCH, _patch(), "analysis") + + body = calls["requests"][0][2]["json"]["body"] + assert "**Hermes run:** `run_x`" in body + assert "http" not in body.split("**Hermes run:**")[1] + + +def test_the_pull_request_stays_readable_without_a_run_id(monkeypatch) -> None: + """A proposal with no run is still worth opening; it just claims less.""" + + calls = _install_http(monkeypatch, [FakeResponse(201, {"number": 5, "html_url": "u"})]) + module.open_pull_request(_cfg(), INCIDENT_ID, "", BRANCH, _patch(), "analysis") + + body = calls["requests"][0][2]["json"]["body"] + assert "Hermes run" not in body + assert "requires human review" in body