diff --git a/CHANGELOG.md b/CHANGELOG.md index aca01f0..ef68059 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -83,6 +83,15 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Successful staged updates now record the exact NixOS generation. The Hub keeps showing "Restart required" until that generation is active, then clears the marker after reboot (with log-based recovery for older updates). + - Pending-reboot state is now derived from NixOS itself — the boot default + (`/nix/var/nix/profiles/system`) versus the running `/run/current-system` + — instead of from Hub-written marker files. Updates performed from a + terminal or support session (which never touch the Hub's status files) + previously left a stale `REBOOT_REQUIRED` marker the Hub could never + clear, pinning the "Restart required" badge forever even after many + reboots. The marker self-heals to `IDLE` on the next status read whenever + boot default and running system agree, and recording the informational + `.generation` marker can no longer fail an otherwise successful update. --- diff --git a/app/sovran_systemsos_web/server.py b/app/sovran_systemsos_web/server.py index 2c96e86..c4bec2a 100644 --- a/app/sovran_systemsos_web/server.py +++ b/app/sovran_systemsos_web/server.py @@ -55,7 +55,7 @@ from .security_helpers import ( load_session_store, save_session_store, ) -from .update_state import staged_generation_is_active +from .update_state import effective_update_status logger = logging.getLogger(__name__) @@ -1648,11 +1648,14 @@ def _write_update_status(status: str): def _read_update_status() -> str: """Read and reconcile the persistent update status. - ``REBOOT_REQUIRED`` intentionally survives Hub/browser restarts before the - reboot. Once the staged generation is the running ``/run/current-system``, - clear that marker so a completed reboot cannot leave the Hub asking for - another reboot forever. The generation helper can recover older updates - from the final nixos-rebuild log line when no explicit marker exists. + ``REBOOT_REQUIRED`` survives Hub/browser restarts before the reboot, but + it is a CLAIM about live NixOS state, not the source of truth: the boot + default (``/nix/var/nix/profiles/system``) versus the running + ``/run/current-system``. Re-validating on every read keeps the Hub + correct when the system was updated from a terminal or support session + (which never writes Hub markers), and lets an old marker that predates + the reconciliation feature self-heal instead of demanding reboots + forever. The stale marker file is removed once cleared. """ try: with open(UPDATE_STATUS, "r") as f: @@ -1660,15 +1663,14 @@ def _read_update_status() -> str: except FileNotFoundError: return "IDLE" - if status == "REBOOT_REQUIRED" and staged_generation_is_active( - UPDATE_GENERATION, UPDATE_LOG - ): - _write_update_status("IDLE") + effective = effective_update_status(status) + if effective != status: + _write_update_status(effective) try: os.remove(UPDATE_GENERATION) except OSError: pass - return "IDLE" + return effective return status diff --git a/app/sovran_systemsos_web/update_state.py b/app/sovran_systemsos_web/update_state.py index 5c68c5e..73fa5b4 100644 --- a/app/sovran_systemsos_web/update_state.py +++ b/app/sovran_systemsos_web/update_state.py @@ -1,81 +1,88 @@ -"""Persistent update-state helpers for the Sovran Hub. +"""Update-state helpers for the Sovran Hub. The full-system updater stages a NixOS generation with ``nixos-rebuild boot``. -That generation is not active until the machine reboots. These helpers let the -Hub distinguish a genuinely pending reboot from an old REBOOT_REQUIRED marker -that survived the reboot. +That generation is not active until the machine reboots — and the same is true +for updates started from a terminal or an SSH support session, which never go +near the Hub's status files. -This module deliberately has no FastAPI or systemd dependencies so its state -reconciliation can be tested without importing the Hub server. +The ONLY reliable indicator that a reboot is pending is NixOS itself: the +system profile (``/nix/var/nix/profiles/system``), which ``nixos-rebuild`` +points at the newest generation on every ``boot`` AND every ``switch``, versus +``/run/current-system``, the generation actually running since the last boot. +When the two differ, a staged generation has not been booted yet. + +Earlier revisions reconstructed this from a marker file and log tails written +by the Hub's own updater. Any system updated by other means — or whose +``REBOOT_REQUIRED`` status was written by an updater older than the marker +feature — left the Hub showing "Restart required" forever: the recorded +generation could never equal the (since advanced) running one, so the marker +could never be cleared. + +This module has no FastAPI or systemd dependencies so the policy can be tested +without importing the Hub server. """ from __future__ import annotations import os -import re + +# The NixOS system profile. ``nixos-rebuild boot`` and ``nixos-rebuild +# switch`` both add a generation here; ``boot`` additionally makes it the +# bootloader default. The path is a symlink chain (``system`` -> +# ``system-N-link`` -> ``/nix/store/...-nixos-system-...``). +BOOT_PROFILE_PATH = "/nix/var/nix/profiles/system" +CURRENT_SYSTEM_PATH = "/run/current-system" -# Nix store hashes use the lower-case Nix base32 alphabet. Keep the output -# name deliberately conservative: a system generation has no path separators. -_SYSTEM_GENERATION_RE = re.compile( - r"^/nix/store/[0-9a-z]{32}-nixos-system-[A-Za-z0-9._+\-]+$" -) -_LOG_GENERATION_RE = re.compile( - r"The new configuration is " - r"(/nix/store/[0-9a-z]{32}-nixos-system-[A-Za-z0-9._+\-]+)" -) +def reboot_is_pending( + boot_profile_path: str = BOOT_PROFILE_PATH, + current_system_path: str = CURRENT_SYSTEM_PATH, +) -> bool: + """Return whether a staged NixOS generation has not been booted yet. + This is deliberately independent of how the update was started — Hub + "Update System", terminal ``nixos-rebuild boot``, or a support session all + move the system profile the same way: -def _valid_generation(value: str) -> str | None: - """Return a normalized NixOS generation path, or ``None`` if invalid.""" - candidate = value.strip() - if _SYSTEM_GENERATION_RE.fullmatch(candidate): - return candidate - return None + * after ``nixos-rebuild boot``: profile -> new, current -> old → pending + * after rebooting: both -> new → cleared + * after ``nixos-rebuild switch``: both move together → no reboot + ever needed (switch activates immediately) + * after a rollback: both point at the rollback target → cleared - -def read_staged_generation(marker_path: str, log_path: str) -> str | None: - """Read the generation staged by the last successful Hub update. - - New updater versions write ``marker_path`` explicitly. For an update that - started with an older updater, recover the same value from the final - ``nixos-rebuild`` log line. Only the tail is needed and bounding the read - avoids loading a potentially large build log during every status poll. + Unreadable or missing paths are treated as "not pending": the Hub must + never demand a reboot it cannot substantiate. """ try: - with open(marker_path, "r", encoding="utf-8") as marker: - generation = _valid_generation(marker.read()) - if generation: - return generation - except OSError: - pass - - try: - with open(log_path, "rb") as log: - log.seek(0, os.SEEK_END) - size = log.tell() - log.seek(max(0, size - 131_072), os.SEEK_SET) - tail = log.read().decode("utf-8", errors="replace") - except OSError: - return None - - matches = list(_LOG_GENERATION_RE.finditer(tail)) - if not matches: - return None - return _valid_generation(matches[-1].group(1)) - - -def staged_generation_is_active( - marker_path: str, - log_path: str, - current_system_path: str = "/run/current-system", -) -> bool: - """Return whether the staged update generation is now the running system.""" - staged = read_staged_generation(marker_path, log_path) - if not staged: - return False - try: + boot_default = os.path.realpath(boot_profile_path) current = os.path.realpath(current_system_path) except OSError: return False - return current == staged + if not os.path.exists(boot_default) or not os.path.exists(current): + return False + return boot_default != current + + +def effective_update_status( + status: str, + boot_profile_path: str = BOOT_PROFILE_PATH, + current_system_path: str = CURRENT_SYSTEM_PATH, +) -> str: + """Map a persisted Hub status to the one that reflects live NixOS state. + + Only ``REBOOT_REQUIRED`` is re-validated: it means "the update staged a + generation the machine has not booted into", a claim that must stay true + no matter which tool performed the last update. When the boot default IS + the running system the claim is stale — the staged generation booted, was + superseded by a newer update, or the marker was written by an updater that + could never clear it — so the effective status is ``IDLE``. + + All other statuses (``RUNNING``, ``FAILED``, ``SUCCESS``, ``IDLE``) pass + through unchanged; RUNNING staleness is handled separately against the + systemd unit itself. + """ + if status == "REBOOT_REQUIRED" and not reboot_is_pending( + boot_profile_path, current_system_path + ): + return "IDLE" + return status diff --git a/modules/core/sovran-hub.nix b/modules/core/sovran-hub.nix index 87492db..82f365c 100644 --- a/modules/core/sovran-hub.nix +++ b/modules/core/sovran-hub.nix @@ -199,9 +199,11 @@ let echo "[ERROR] nixos-rebuild boot failed" RC=1 elif ! readlink -f /nix/var/nix/profiles/system > "$GENERATION"; then - echo "[ERROR] update was built but its staged generation could not be recorded" + # The marker is informational only. The Hub derives pending-reboot + # state from the NixOS system profile itself, so failing to record + # the marker must not fail an otherwise successful update. + echo "[WARNING] update succeeded but its staged generation could not be recorded" rm -f "$GENERATION" - RC=1 fi echo "" fi diff --git a/tests/test_update_state.py b/tests/test_update_state.py index 322896a..f719dd1 100644 --- a/tests/test_update_state.py +++ b/tests/test_update_state.py @@ -17,80 +17,144 @@ if str(_APP_PARENT) not in sys.path: sys.path.insert(0, str(_APP_PARENT)) from sovran_systemsos_web.update_state import ( # noqa: E402 - read_staged_generation, - staged_generation_is_active, + effective_update_status, + reboot_is_pending, ) -GENERATION = ( - "/nix/store/rmi0g35cd8w60k0ig7pm6kb8kzws8b7x-" - "nixos-system-nixos-26.11.20260817.ec2d622" +# Real store paths observed on the incident machine that prompted this rework. +RUNNING_GENERATION = ( + "84rsiqi66nc68jbikd26ms50ap831xf8-nixos-system-nixos-26.11.20260817.ec2d622" ) -OTHER_GENERATION = ( - "/nix/store/aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa-" - "nixos-system-nixos-26.11.20260816.old" +PREVIOUS_GENERATION = ( + "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa-nixos-system-nixos-26.11.20260816.old" +) +# What the stale Hub log still claimed was staged: a two-week-old update cycle. +HUB_LOG_GENERATION = ( + "yis0saq6p8fhqcaii0h2yzqf0blhdwns-nixos-system-nixos-26.11.20260804.e72e4f2" ) -class TestStagedGenerationState(unittest.TestCase): +class TestRebootPendingState(unittest.TestCase): + """Reboot-pending state is derived from NixOS, not from Hub marker files. + + The system profile (what boots next) is compared against + /run/current-system (what is running). This stays correct no matter + which tool performed the update: Hub "Update System", a terminal + ``nixos-rebuild``, or an SSH support session. + """ + def setUp(self): self.tmp = tempfile.TemporaryDirectory() self.addCleanup(self.tmp.cleanup) root = Path(self.tmp.name) - self.marker = root / "update.generation" - self.log = root / "update.log" + self.store = root / "store" + self.store.mkdir() + # /nix/var/nix/profiles/system is a two-hop chain: the diagnostics on + # the incident box showed ``system`` -> ``system-148-link`` -> store. + self.profile = root / "system" + self.profile_entry = root / "system-148-link" + # /run/current-system links straight to the running store path. self.current = root / "current-system" - def test_explicit_generation_marker_is_read(self): - self.marker.write_text(GENERATION + "\n", encoding="utf-8") - self.assertEqual( - read_staged_generation(str(self.marker), str(self.log)), GENERATION + def _store_dir(self, name: str) -> str: + path = self.store / name + path.mkdir(exist_ok=True) + return str(path) + + def _stage(self, booted: str, running: str) -> None: + """Stage ``booted`` as the boot default with ``running`` live.""" + os.symlink(self._store_dir(booted), self.profile_entry) + os.symlink(self.profile_entry, self.profile) + os.symlink(self._store_dir(running), self.current) + + def test_hub_update_staged_and_not_yet_rebooted_is_pending(self): + self._stage(booted=RUNNING_GENERATION, running=PREVIOUS_GENERATION) + self.assertTrue( + reboot_is_pending(str(self.profile), str(self.current)) ) - def test_legacy_updater_generation_is_recovered_from_log_tail(self): - self.log.write_text( - "building the system configuration...\n" - f"Done. The new configuration is {GENERATION}\n" + def test_staged_generation_booted_is_no_longer_pending(self): + self._stage(booted=RUNNING_GENERATION, running=RUNNING_GENERATION) + self.assertFalse( + reboot_is_pending(str(self.profile), str(self.current)) + ) + + def test_terminal_switch_needs_no_reboot(self): + # nixos-rebuild switch moves the profile AND current-system together. + self._stage(booted=RUNNING_GENERATION, running=RUNNING_GENERATION) + self.assertFalse( + reboot_is_pending(str(self.profile), str(self.current)) + ) + + def test_rollback_leaves_nothing_pending(self): + # nixos-rebuild switch --rollback points both at the rollback target. + self._stage(booted=PREVIOUS_GENERATION, running=PREVIOUS_GENERATION) + self.assertFalse( + reboot_is_pending(str(self.profile), str(self.current)) + ) + + def test_unverifiable_state_is_never_a_reboot_demand(self): + # No profile and no running system readable: the Hub must not nag + # about a reboot it cannot substantiate. + self.assertFalse( + reboot_is_pending(str(self.profile), str(self.current)) + ) + + def test_stale_hub_marker_clears_after_terminal_updates(self): + """The exact incident: terminal-updated machine, frozen Hub marker. + + The user's last Hub update (old updater, weeks prior) left + REBOOT_REQUIRED behind. Every update since ran in a terminal and + never touched the Hub's files, so the log still records a staged + generation from 2026-08-04 while the machine runs a 2026-08-17 + build. With the profile and the running system in agreement, the + stale claim must reconcile to IDLE regardless of anything the old + marker/log files say. + """ + root = Path(self.tmp.name) + log = root / "sovran-hub-update.log" + log.write_text( + "Done. The new configuration is " + f"/nix/store/{HUB_LOG_GENERATION}\n" "✓ Update staged successfully\n", encoding="utf-8", ) - self.assertEqual( - read_staged_generation(str(self.marker), str(self.log)), GENERATION - ) - - def test_latest_generation_line_wins(self): - self.log.write_text( - f"Done. The new configuration is {OTHER_GENERATION}\n" - f"Done. The new configuration is {GENERATION}\n", - encoding="utf-8", - ) - self.assertEqual( - read_staged_generation(str(self.marker), str(self.log)), GENERATION - ) - - def test_invalid_marker_is_ignored(self): - self.marker.write_text("/tmp/not-a-generation\n", encoding="utf-8") - self.log.write_text("no completed generation\n", encoding="utf-8") - self.assertIsNone(read_staged_generation(str(self.marker), str(self.log))) - - def test_staged_generation_is_active_after_reboot(self): - self.marker.write_text(GENERATION + "\n", encoding="utf-8") - os.symlink(GENERATION, self.current) - self.assertTrue( - staged_generation_is_active( - str(self.marker), str(self.log), str(self.current) - ) - ) - - def test_staged_generation_remains_pending_before_reboot(self): - self.marker.write_text(GENERATION + "\n", encoding="utf-8") - os.symlink(OTHER_GENERATION, self.current) + # Deliberately no sovran-hub-update.generation marker: the updater + # that produced this state predates the marker feature. self.assertFalse( - staged_generation_is_active( - str(self.marker), str(self.log), str(self.current) - ) + (root / "sovran-hub-update.generation").exists() ) + self._stage(booted=RUNNING_GENERATION, running=RUNNING_GENERATION) + self.assertEqual( + effective_update_status( + "REBOOT_REQUIRED", str(self.profile), str(self.current) + ), + "IDLE", + ) + + def test_genuine_pending_reboot_claim_survives(self): + # A staged generation that has NOT been booted yet: the claim is + # true and must keep surfacing until the reboot really happens. + self._stage(booted=RUNNING_GENERATION, running=PREVIOUS_GENERATION) + self.assertEqual( + effective_update_status( + "REBOOT_REQUIRED", str(self.profile), str(self.current) + ), + "REBOOT_REQUIRED", + ) + + def test_other_statuses_pass_through_unchanged(self): + self._stage(booted=RUNNING_GENERATION, running=RUNNING_GENERATION) + for status in ("RUNNING", "FAILED", "SUCCESS", "IDLE"): + self.assertEqual( + effective_update_status( + status, str(self.profile), str(self.current) + ), + status, + ) + class TestUpdatePollingWiring(unittest.TestCase): """Guard the browser failure modes that caused a permanent spinner."""