diff --git a/hermes_cli/managed_uv.py b/hermes_cli/managed_uv.py index f924601140..77a89b0933 100644 --- a/hermes_cli/managed_uv.py +++ b/hermes_cli/managed_uv.py @@ -197,21 +197,32 @@ def _uv_version(uv_bin: str) -> str: ).stdout.strip() +def _record_runtime_repair(repair: RuntimeRepairResult) -> None: + """Put the repair outcome into the update receipt (no-op outside ``hermes update``). + + Receipts are built only from explicit ``record_step``/``record_skip`` calls, so without this + a failed repair left ``outcome: partial`` with no step naming the reason or the SQLite + versions. A deferred or not-applicable repair is a skip WITH its reason, not a failed step: + every pip/non-venv install would otherwise carry a red step in every receipt. + """ + from hermes_cli.update_receipt import record_skip, record_step + + detail = ( + f"{repair.status}: {repair.detail}" if repair.detail else repair.status + ) + f" (sqlite {repair.sqlite_before or 'unknown'} → {repair.sqlite_after or 'unknown'})" + if repair.status in {"skipped", "not-applicable"}: + record_skip("sqlite_runtime_repair", detail) + else: + record_step("sqlite_runtime_repair", repair.status in {"safe", "repaired"}, detail) + + def _run_runtime_repair( uv_bin: str, repair_observer: Callable[[RuntimeRepairResult], None] | None, *, print_skip: bool = False) -> None: """Run the vulnerable-runtime repair hook; never raises (repair is non-fatal).""" try: repair = repair_vulnerable_runtime(uv_bin) - from hermes_cli.update_receipt import record_skip, record_step - - detail = ( - f"{repair.status}: {repair.detail} " - f"(sqlite {repair.sqlite_before or 'unknown'} → {repair.sqlite_after or 'unknown'})" - ) - record_step("sqlite_runtime_repair", repair.status in {"safe", "repaired"}, detail) - if repair.status in {"skipped", "not-applicable"}: - record_skip("sqlite_runtime_repair", detail) + _record_runtime_repair(repair) if repair_observer is not None: repair_observer(repair) if repair.status == "failed": @@ -573,7 +584,7 @@ def _smoke_candidate_venv(venv_dir: Path) -> tuple[bool, str, SQLiteRuntimeInfo # A failed ``uv sync`` prints its diagnosis last, so the tail is the actionable part. Kept -# short: the reason travels into a one-line log entry and a one-line console warning. +# short: the reason travels into a one-line log entry, the failure report and the receipt step. _SYNC_TAIL_LINES = 6 _SYNC_REASON_CHARS = 600 @@ -601,11 +612,9 @@ def _stream_sync(argv: list[str], *, cwd: Path, env: dict[str, str]) -> tuple[in child's stdout while it runs, so a full stderr pipe blocks uv forever — stderr is merged into stdout and forwarded line by line instead of being captured and reprinted at the end. - The tail is kept anyway. Inherited stdout lands the child's diagnosis in console scrollback - ONLY: the rejection line the logger records carried the bare exit code, and the generic - "did not pass dependency and import smoke tests" detail the repair returns carries no reason - either (receipts quote explicitly recorded steps — none records this repair). Seen in the - field, that reads as "hermes update says the SQLite repair failed and never says why". + The tail is kept anyway: with inherited stdout the child's diagnosis survived in console + scrollback only, and the rejection carried a bare exit code — "hermes update says the SQLite + repair failed and never says why". """ proc = subprocess.Popen( list(argv), cwd=cwd, env=env, stdout=subprocess.PIPE, stderr=subprocess.STDOUT, @@ -666,10 +675,8 @@ def _stage_candidate_venv( [uv_bin, "sync", "--extra", "all", "--locked", "--python", str(_venv_python(candidate))], cwd=project_root, env=sync_env) if status != 0: - # `_repair_under_lock`'s failure detail is generic ("did not pass dependency and import - # smoke tests"), so the reason is announced here; without it only console scrollback has - # the text — the rejection line the log records carries the bare exit code. - print(f" ⚠ candidate dependency sync failed (rc={status}): {reason}") + # The reason travels with the rejection into RuntimeRepairResult.detail, which the + # failure report prints and the update receipt records. return reject("candidate dependency sync failed (rc=%d): %s", status, reason) healthy, detail, _ = _smoke_candidate_venv(candidate) if not healthy: diff --git a/tests/hermes_cli/test_managed_uv.py b/tests/hermes_cli/test_managed_uv.py index fc42fe92f4..e00bd84746 100644 --- a/tests/hermes_cli/test_managed_uv.py +++ b/tests/hermes_cli/test_managed_uv.py @@ -667,8 +667,11 @@ class TestStageCandidateVenvCrossPlatform: assert "UV_NO_CONFIG" not in sync_kwargs["env"] assert sync_kwargs["stderr"] == subprocess.STDOUT - def test_sync_failure_reports_the_child_reason(self, tmp_path, capsys, caplog): - """A rejected candidate must say WHY — the child's own diagnosis, not a bare rc.""" + def test_sync_failure_reports_the_child_reason(self, tmp_path, caplog): + """A rejected candidate must say WHY — the child's own diagnosis, not a bare rc. + + The reason rides the rejection into ``RuntimeRepairResult.detail`` (#111417, #111497). + """ import logging from hermes_cli.managed_uv import _stage_candidate_venv @@ -690,25 +693,25 @@ class TestStageCandidateVenvCrossPlatform: patch( "hermes_cli.managed_uv._smoke_candidate_venv", return_value=(True, "", None), - ): - candidate = _stage_candidate_venv( + ), \ + pytest.raises(_CandidateStageError) as rejected: + _stage_candidate_venv( "uv", project_root=root, generation=generation, python=python, ) - assert candidate is None - console = capsys.readouterr().out # The reason is the child's own diagnosis: the error + hint, without the progress noise - # that precedes them. It reaches the console AND the rejection the updater logs. - reason_line = next( - line for line in console.splitlines() if "dependency sync failed" in line) - assert "error: The lockfile at `uv.lock` needs to be updated" in reason_line - assert "hint: To update the lockfile, run `uv lock`." in reason_line - assert "Resolving despite existing lockfile" not in reason_line + # that precedes them. It reaches the rejection the repair returns AND the log. + reason = str(rejected.value) + assert reason.startswith("candidate dependency sync failed (rc=1): ") + assert "error: The lockfile at `uv.lock` needs to be updated" in reason + assert "hint: To update the lockfile, run `uv lock`." in reason + assert "Resolving despite existing lockfile" not in reason assert "candidate dependency sync failed (rc=1)" in caplog.text assert "needs to be updated" in caplog.text + assert not list((root / ".hermes-runtime").glob("venv-candidate-*")) class TestRuntimeCutover: diff --git a/tests/hermes_cli/test_runtime_repair_receipt.py b/tests/hermes_cli/test_runtime_repair_receipt.py index b17423fc55..48306bd345 100644 --- a/tests/hermes_cli/test_runtime_repair_receipt.py +++ b/tests/hermes_cli/test_runtime_repair_receipt.py @@ -1,7 +1,10 @@ -"""Runtime repair diagnostics must reach persisted update receipts (#111497).""" +"""The SQLite runtime repair outcome must reach the persisted update receipt (#111497). + +``record_step``/``record_skip`` are the only way into a receipt; before this a failed repair left +``outcome: partial`` with no step naming the reason or the SQLite versions involved. +""" import json -from types import SimpleNamespace import pytest @@ -9,76 +12,37 @@ from hermes_cli import managed_uv as uv from hermes_cli import update_receipt as receipts -@pytest.mark.parametrize("status", ["safe", "repaired", "failed", "skipped", "not-applicable"]) -@pytest.mark.parametrize("entry", ["update", "bootstrap"]) -def test_runtime_result_reaches_persisted_receipt(tmp_path, monkeypatch, status, entry): +@pytest.mark.parametrize("status, ok, bucket", [ + ("failed", False, "steps"), + ("repaired", True, "steps"), + ("skipped", None, "skips"), +]) +def test_runtime_repair_outcome_reaches_persisted_receipt(tmp_path, monkeypatch, status, ok, bucket): monkeypatch.setenv("HERMES_HOME", str(tmp_path)) monkeypatch.setattr(receipts, "_current", None) - result = uv.RuntimeRepairResult(status, "repair diagnostic", "3.50.4", "3.53.1") + result = uv.RuntimeRepairResult( + status, "candidate dependency sync failed (rc=1): error: lockfile stale", "3.50.4", "3.53.1") monkeypatch.setattr(uv, "repair_vulnerable_runtime", lambda _: result) - if entry == "update": - monkeypatch.setattr(uv, "resolve_uv", lambda: "uv") - monkeypatch.setattr(uv, "_uv_self_update_is_fresh", lambda: True) - invoke = uv.update_managed_uv - else: - paths = iter([None, "uv"]) - monkeypatch.setattr(uv, "resolve_uv", lambda: next(paths)) - monkeypatch.setattr(uv, "_install_uv", lambda _: None) - monkeypatch.setattr(uv, "_uv_version", lambda _: "test") - invoke = uv.ensure_uv + monkeypatch.setattr(uv, "resolve_uv", lambda: "uv") + monkeypatch.setattr(uv, "_uv_self_update_is_fresh", lambda: True) observed = [] + receipts.begin_update_receipt() - invoke(repair_observer=observed.append) - path = receipts.finalize_update_receipt("partial") - data = json.loads(path.read_text()) - step, = data["steps"] - assert step["name"] == "sqlite_runtime_repair" - assert step["ok"] == (status in {"safe", "repaired"}) - assert all(value in step["detail"] for value in ( - result.status, result.detail, result.sqlite_before, result.sqlite_after)) - assert bool(data["skips"]) == (status in {"skipped", "not-applicable"}) + uv.update_managed_uv(repair_observer=observed.append) + data = json.loads(receipts.finalize_update_receipt("partial").read_text(encoding="utf-8")) + assert observed == [result] - # The same repair hook is also used outside an update, without an active receipt. + entry, = data[bucket] + other = "skips" if bucket == "steps" else "steps" + assert data[other] == [] + assert entry["name"] == "sqlite_runtime_repair" + text = entry["detail"] if bucket == "steps" else entry["reason"] + if bucket == "steps": + assert entry["ok"] is ok + assert text.startswith(f"{status}: ") + assert result.detail in text + assert "sqlite 3.50.4 → 3.53.1" in text + # Outside `hermes update` (setup/bootstrap) there is no receipt; the hook must stay a no-op. uv._run_runtime_repair("uv", observed.append) assert receipts._current is None assert observed == [result, result] - - -@pytest.mark.parametrize("stage, reason", [ - ("create", "candidate venv creation failed (rc=1): permission denied"), - ("lock", "candidate dependency sync refused: uv.lock is missing"), - ("sync", "candidate dependency sync failed (rc=1)"), - ("smoke", "candidate venv smoke failed: missing module"), -]) -def test_stage_rejection_preserves_reason_and_live_environment(tmp_path, monkeypatch, stage, reason): - root = tmp_path / "checkout" - live = root / "venv" - live.mkdir(parents=True) - sentinel = live / "sentinel" - sentinel.write_text("unchanged") - generation = root / ".hermes-runtime" / "python" / "generation" - generation.mkdir(parents=True) - if stage != "lock": - (root / "uv.lock").write_text("lock") - current = SimpleNamespace(wal_reset_vulnerable=True, sqlite_version_string="3.50.4") - fixed = SimpleNamespace(sqlite_version_string="3.53.1") - monkeypatch.setattr(uv, "probe_sqlite_runtime", lambda _: current) - monkeypatch.setattr(uv, "_install_safe_python_generation", lambda *a, **kw: ( - generation, generation / "python", fixed)) - - def run(argv, **kwargs): - if argv[1] == "venv": - from pathlib import Path - Path(argv[2]).mkdir(parents=True) - failed = (argv[1] == "venv" and stage == "create") or (argv[1] == "sync" and stage == "sync") - return SimpleNamespace(returncode=int(failed), stderr="permission denied", stdout="") - - monkeypatch.setattr(uv.subprocess, "run", run) - monkeypatch.setattr(uv, "_smoke_candidate_venv", lambda _: (False, "missing module", None)) - result = uv._repair_under_lock("uv", root=root, live=live, live_python=live / "python", - runtime_root=root / ".hermes-runtime") - assert result.status == "failed" - assert result.detail == reason - assert sentinel.read_text() == "unchanged" - assert not generation.exists() - assert not list((root / ".hermes-runtime").glob("venv-candidate-*"))