fix: one lookup reads a run's archived receipt, with one error contract
The run's own archive was read five ways: _read_terminal_receipt, _running_record and amend_terminal_followup (each globbing by update_id with different error handling), plus the _persist_running stale-snapshot guard and finalize_interrupted_update_receipt (each re-reading the _run_file path inline). update_receipt.read_run_record is now the only reader: unreadable, non-object and other-run files all mean "no record", it never raises, and each caller keeps its outcome on top of that. The divergence was a bug in one place: _read_terminal_receipt and _running_record called .get() on whatever json.loads returned, so a run archive holding valid JSON that is not an object (null, a list) raised AttributeError out of settle_lost_completion -- a post-commit path that must never fail the update. It now settles like a missing archive.
This commit is contained in:
@@ -135,33 +135,23 @@ def _resume_receipt(data: dict) -> None:
|
||||
def _read_terminal_receipt(request: dict) -> dict | None:
|
||||
from hermes_cli import update_receipt
|
||||
|
||||
# The ROOT home's receipts (the run's own file), never latest.json: another
|
||||
# profile/context may have finalized more recently.
|
||||
update_id = request["receipt"]["update_id"]
|
||||
try:
|
||||
for path in update_receipt._receipt_dir().glob(f"update_*_{update_id}.json"):
|
||||
data = json.loads(path.read_text(encoding="utf-8-sig"))
|
||||
if data.get("update_id") == update_id and data.get("finished_at"):
|
||||
return data
|
||||
except (OSError, ValueError):
|
||||
pass
|
||||
found = update_receipt.read_run_record(update_id)
|
||||
if found is not None and found[1].get("finished_at"):
|
||||
return found[1]
|
||||
# The store refused the terminal write after the commit point (loudly): this process still
|
||||
# holds the correlated terminal record it finalized, so the exit status never turns into 1.
|
||||
return update_receipt.finalized_receipt(update_id)
|
||||
|
||||
|
||||
def _running_record(request: dict) -> dict | None:
|
||||
from hermes_cli.update_receipt import _receipt_dir
|
||||
from hermes_cli.update_receipt import read_run_record
|
||||
|
||||
for path in _receipt_dir().glob(f"update_*_{request['receipt']['update_id']}.json"):
|
||||
try:
|
||||
data = json.loads(path.read_text(encoding="utf-8-sig"))
|
||||
except (OSError, ValueError):
|
||||
continue
|
||||
if data.get("update_id") == request["receipt"]["update_id"] and data.get("outcome") == "running":
|
||||
data.pop("writer_pid", None)
|
||||
return data
|
||||
return None
|
||||
found = read_run_record(request["receipt"]["update_id"])
|
||||
if found is None or found[1].get("outcome") != "running":
|
||||
return None
|
||||
found[1].pop("writer_pid", None)
|
||||
return found[1]
|
||||
|
||||
|
||||
def _owed_user_action(request: dict) -> str | None:
|
||||
|
||||
@@ -261,6 +261,23 @@ def _run_file(directory: Path, data: dict[str, Any]) -> Path:
|
||||
return directory / f"update_{stamp[:8]}_{stamp[8:]}_{data.get('pid') or os.getpid()}_{data.get('update_id')}.json"
|
||||
|
||||
|
||||
def read_run_record(update_id: str) -> Optional[tuple[Path, dict[str, Any]]]:
|
||||
"""The run's own archive ``(path, record)`` in the root store, or None. Never raises.
|
||||
|
||||
The ONE lookup of a run's archive (named at begin, so its running and terminal records share
|
||||
one file); never ``latest.json``, which another profile or run may have replaced since. A file
|
||||
that cannot be read, is not a JSON object, or names another run is not this run's record:
|
||||
callers treat all of those exactly like a missing archive.
|
||||
"""
|
||||
with suppress(OSError):
|
||||
for path in _receipt_dir().glob(f"update_*_{update_id}.json"):
|
||||
with suppress(OSError, ValueError):
|
||||
record = json.loads(path.read_text(encoding="utf-8-sig"))
|
||||
if isinstance(record, dict) and record.get("update_id") == update_id:
|
||||
return path, record
|
||||
return None
|
||||
|
||||
|
||||
def _persist_running(data: dict[str, Any]) -> None:
|
||||
"""Write the open run to disk: a killed update still leaves its own record. Never raises."""
|
||||
with suppress(Exception):
|
||||
@@ -271,10 +288,9 @@ def _persist_running(data: dict[str, Any]) -> None:
|
||||
path = _run_file(directory, data)
|
||||
# A completion child can finalize while its parent still holds the pre-child
|
||||
# context. That stale running snapshot must never undo the terminal receipt.
|
||||
with suppress(OSError, ValueError):
|
||||
stored = json.loads(path.read_text(encoding="utf-8-sig"))
|
||||
if stored.get("update_id") == data.get("update_id") and stored.get("finished_at"):
|
||||
return
|
||||
stored = read_run_record(str(data.get("update_id")))
|
||||
if stored is not None and stored[1].get("finished_at"):
|
||||
return
|
||||
from hermes_cli.process_identity import _process_create_time
|
||||
|
||||
payload = (json.dumps({**data, "writer_pid": os.getpid(),
|
||||
@@ -428,19 +444,18 @@ def record_user_action(step: str, reason: str) -> None:
|
||||
|
||||
def amend_terminal_followup(update_id: str, step: str, reason: str) -> None:
|
||||
"""A follow-up that failed after the run's receipt was finalized lands on that receipt. Never raises."""
|
||||
with suppress(Exception):
|
||||
found = read_run_record(update_id)
|
||||
if found is None:
|
||||
return
|
||||
path, record = found
|
||||
record.setdefault("followups", []).append({"step": step, "reason": reason, "at": _utc_now_iso()})
|
||||
payload = (json.dumps(record, indent=2, default=str) + "\n").encode("utf-8")
|
||||
with suppress(Exception): # an unwritable store: the printed follow-up line still names it
|
||||
from hermes_cli.runtime_state import _atomic_bytes
|
||||
|
||||
directory = _receipt_dir()
|
||||
for path in directory.glob(f"update_*_{update_id}.json"):
|
||||
record = json.loads(path.read_text(encoding="utf-8-sig"))
|
||||
if record.get("update_id") != update_id:
|
||||
continue
|
||||
record.setdefault("followups", []).append({"step": step, "reason": reason, "at": _utc_now_iso()})
|
||||
payload = (json.dumps(record, indent=2, default=str) + "\n").encode("utf-8")
|
||||
_atomic_bytes(path, payload)
|
||||
if (read_latest_receipt() or {}).get("update_id") == update_id:
|
||||
_write_latest(payload)
|
||||
_atomic_bytes(path, payload)
|
||||
if (read_latest_receipt() or {}).get("update_id") == update_id:
|
||||
_write_latest(payload)
|
||||
|
||||
|
||||
def owe_followup(update_id: Optional[str], step: str, reason: str, **retry: str) -> None:
|
||||
@@ -710,11 +725,9 @@ def finalize_interrupted_update_receipt(stop_reason: str, *, exit_code: int = 13
|
||||
with suppress(Exception):
|
||||
clone = copy.copy(current)
|
||||
clone.data = copy.deepcopy(current.data)
|
||||
on_disk: dict = {}
|
||||
with suppress(Exception):
|
||||
path = _run_file(_receipt_dir(), current.data)
|
||||
on_disk = json.loads(path.read_text(encoding="utf-8-sig"))
|
||||
if on_disk.get("update_id") == current.data.get("update_id"):
|
||||
found = read_run_record(str(current.data.get("update_id")))
|
||||
if found is not None:
|
||||
path, on_disk = found
|
||||
if on_disk.get("finished_at"): # the completion child already closed the run
|
||||
_current.reset(current.current_token)
|
||||
return path
|
||||
|
||||
@@ -168,6 +168,9 @@ def transition(tmp_path):
|
||||
"def record_user_action(*args): pass\n"
|
||||
"def _receipt_dir():\n"
|
||||
" return pathlib.Path(os.environ['HERMES_HOME']) / 'logs/update_receipts'\n"
|
||||
"def read_run_record(update_id):\n"
|
||||
" for path in _receipt_dir().glob(f'update_*_{update_id}.json'):\n"
|
||||
" return path, json.loads(path.read_text(encoding='utf-8-sig'))\n"
|
||||
"def finalize_pending_update_receipt(code, reason):\n"
|
||||
" r = _current.get()\n"
|
||||
" if r is None: return\n"
|
||||
|
||||
28
tests/hermes_cli/test_update_receipt_run_record.py
Normal file
28
tests/hermes_cli/test_update_receipt_run_record.py
Normal file
@@ -0,0 +1,28 @@
|
||||
"""A run's own archive is read through one lookup: an unreadable record is a missing record."""
|
||||
from __future__ import annotations
|
||||
|
||||
import pytest
|
||||
|
||||
from hermes_cli import update_completion, update_receipt
|
||||
|
||||
|
||||
def _lost_run(tmp_path):
|
||||
update_receipt.begin_update_receipt()
|
||||
run = dict(update_receipt._current.get().data)
|
||||
request = {"source": str(tmp_path / "checkout"), "receipt": run, "desktop": False, "gateway_mode": False}
|
||||
return run, update_receipt._run_file(update_receipt._receipt_dir(), run), request
|
||||
|
||||
|
||||
@pytest.mark.parametrize("content", ["null", "[]", "{torn"])
|
||||
def test_lost_completion_over_a_malformed_run_archive_still_closes_the_run(_isolate_hermes_home, tmp_path, content):
|
||||
# The code is committed: an archive that is valid JSON but not a record (hand edit, foreign
|
||||
# writer) must settle exactly like a missing one, never raise out of the post-commit path.
|
||||
run, archive, request = _lost_run(tmp_path)
|
||||
archive.write_text(content, encoding="utf-8")
|
||||
|
||||
result = update_completion.settle_lost_completion(request, "the completion process exited -9")
|
||||
|
||||
assert result["exit_code"] == 0
|
||||
assert result["receipt"]["update_id"] == run["update_id"]
|
||||
assert result["receipt"]["outcome"] == "success"
|
||||
assert [row["step"] for row in result["receipt"]["followups"]] == ["completion"]
|
||||
Reference in New Issue
Block a user