fix(update): a failed dashboard cleanup no longer aborts fleet verification and the receipt
`_finish_dashboard_update_cleanup` runs pulled `dashboard_procs` inside the pre-pull interpreter. A stale-symbol failure there (#112604: `AttributeError: module 'hermes_cli.main_dashboard' has no attribute '_loaded_launchd_backend_jobs'`, reproduced on the maintainer's own box) propagated out of `_verify_fleet_after_update`, skipping the fleet version matrix, plan-vs-execution reconciliation and the inner receipt finalize; the command boundary then stamped the receipt `failed` with the traceback as stop_reason even though the code update and gateway restart had succeeded. The cleanup is now isolated like every sibling post-update step: the exception is printed with a manual-restart hint, logged at WARNING, and recorded as a failed `dashboard_cleanup` step on the receipt. A dashboard/serve left on pre-update code is still escalated by the survivor probe → reconciliation (exit 1). Covers the git and ZIP paths. Refs #112604 (residual class after #112753). Test is red on origin/main.
This commit is contained in:
@@ -129,7 +129,15 @@ it guards. `plan → snapshot → apply → restart-per-kind → verify → repo
|
||||
(`latest.json` pointer; steps, skips WITH reasons, restart outcome, plan, fleet snapshot).
|
||||
Finalization is owned by the `cmd_update` command boundary — early `sys.exit` paths (preflight
|
||||
refusals, fetch failures) still persist a receipt with the real exit code. A begun-but-unwritten
|
||||
receipt is a bug: refused/failed runs are the ones receipts exist for.
|
||||
receipt is a bug: refused/failed runs are the ones receipts exist for. The receipt writer runs in
|
||||
the PRE-pull interpreter after the module purge, so `update_receipt.py` may import only stdlib and
|
||||
purge-protected modules (`hermes_constants`) — a `hermes_cli.config` import there re-executed the
|
||||
pulled config against a stale `utils` and silently dropped the whole receipt; a write failure
|
||||
prints `⚠ Update receipt not written` and logs at WARNING, never debug.
|
||||
- **Post-update steps are isolated**: everything after the code swap that runs pulled code in the
|
||||
pre-pull process (`_finish_dashboard_update_cleanup`, notices, probes) catches its own failure,
|
||||
prints it, and records a failed receipt step — one stale-symbol `AttributeError` must not abort
|
||||
the fleet matrix, reconciliation and receipt finalize that follow it.
|
||||
|
||||
Process-scan coordination between updater, serve/dashboard, and gateway is being replaced by a
|
||||
gateway-owned control socket (#92091); scans are the fallback layer for old/crashed processes — read
|
||||
|
||||
@@ -383,7 +383,7 @@ def _finish_dashboard_update_cleanup(
|
||||
|
||||
See #83595.
|
||||
"""
|
||||
from hermes_cli.update_cmd import _m, _reload_process_scan_modules
|
||||
from hermes_cli.update_cmd import _m, _record_update_step, _reload_process_scan_modules
|
||||
if node_failures:
|
||||
print()
|
||||
print(" ℹ Leaving running dashboard process(es) untouched because the")
|
||||
@@ -392,9 +392,22 @@ def _finish_dashboard_update_cleanup(
|
||||
|
||||
_reload_process_scan_modules()
|
||||
|
||||
stop_result = _m()._kill_stale_dashboard_processes(
|
||||
restart_managed=True, already_restarted_units=already_restarted_units
|
||||
)
|
||||
try:
|
||||
stop_result = _m()._kill_stale_dashboard_processes(
|
||||
restart_managed=True, already_restarted_units=already_restarted_units
|
||||
)
|
||||
except Exception as exc:
|
||||
# Isolated like every sibling post-update step: this runs in the pre-pull interpreter
|
||||
# against pulled code, and a symbol gap here (#112604) used to abort the fleet matrix,
|
||||
# reconciliation and the inner receipt finalize that follow it. A dashboard/serve left
|
||||
# on pre-update code is still caught by the survivor probe → reconciliation (exit 1).
|
||||
logger.warning("Post-update dashboard cleanup failed: %s", exc)
|
||||
_record_update_step("dashboard_cleanup", False, f"{type(exc).__name__}: {exc}")
|
||||
print()
|
||||
print(f"⚠ Could not refresh running dashboard/serve process(es): {exc}")
|
||||
print(" If one is still running, restart it so it serves the updated code:")
|
||||
print(" hermes dashboard --port <port> (or: systemctl --user restart hermes-dashboard)")
|
||||
return
|
||||
if not stop_result.get("unrecovered"):
|
||||
return
|
||||
|
||||
|
||||
@@ -940,6 +940,34 @@ class TestPostUpdateStaleModuleReload:
|
||||
assert "hermes_cli._subprocess_compat" in reloaded
|
||||
assert "hermes_cli.dashboard_procs" in reloaded
|
||||
|
||||
def test_cleanup_failure_is_isolated_and_recorded(self, capsys):
|
||||
"""#112604 class: the cleanup runs pulled ``dashboard_procs`` in the pre-pull interpreter,
|
||||
so a symbol gap raises AttributeError from inside the scan. That exception used to
|
||||
propagate out of ``_finish_dashboard_update_cleanup`` and abort the fleet-verification
|
||||
tail (matrix, reconciliation, inner receipt finalize). It must be contained, made
|
||||
visible, and recorded as a failed step on the open receipt."""
|
||||
import hermes_cli.update_receipt as ur
|
||||
from hermes_cli import update_cmd
|
||||
|
||||
ur._current = None
|
||||
try:
|
||||
ur.begin_update_receipt()
|
||||
with patch.object(update_cmd, "_reload_process_scan_modules"), patch(
|
||||
"hermes_cli.main._kill_stale_dashboard_processes",
|
||||
side_effect=AttributeError(
|
||||
"module 'hermes_cli.main_dashboard' has no attribute '_loaded_launchd_backend_jobs'"
|
||||
),
|
||||
):
|
||||
update_cmd._finish_dashboard_update_cleanup([]) # must not raise
|
||||
|
||||
steps = {s["name"]: s for s in ur._current.data["steps"]}
|
||||
finally:
|
||||
ur._current = None
|
||||
|
||||
assert steps["dashboard_cleanup"]["ok"] is False
|
||||
assert "_loaded_launchd_backend_jobs" in steps["dashboard_cleanup"]["detail"]
|
||||
assert "_loaded_launchd_backend_jobs" in capsys.readouterr().out
|
||||
|
||||
|
||||
class TestLaunchdSupervisedBackends:
|
||||
"""macOS (#111689): a backend supervised by a launchd job must come back through launchd. Respawning
|
||||
|
||||
Reference in New Issue
Block a user