diff --git a/src/pqn_node/api/main.py b/src/pqn_node/api/main.py index 60ad59e5..8dad6e4e 100644 --- a/src/pqn_node/api/main.py +++ b/src/pqn_node/api/main.py @@ -9,8 +9,8 @@ from pqn_node.api.routes import rng from pqn_node.api.routes import serial from pqn_node.api.routes import timetagger +from pqn_node.api.routes.health import get_effective_availability from pqn_node.core.config import GamesAvailability -from pqn_node.core.config import get_settings from pqn_node.core.config import settings @@ -31,7 +31,7 @@ class NodeConfig(BaseModel): @api_router.get("/games/availability", tags=["games"]) def get_availability() -> GamesAvailability: - return get_settings().games_availability + return get_effective_availability() @api_router.get("/node/config", tags=["node"]) diff --git a/src/pqn_node/api/routes/health.py b/src/pqn_node/api/routes/health.py index dcfdd6e1..e9e711f6 100644 --- a/src/pqn_node/api/routes/health.py +++ b/src/pqn_node/api/routes/health.py @@ -10,6 +10,7 @@ from pydantic import BaseModel from pydantic import Field +from pqn_node.core.config import GamesAvailability from pqn_node.core.config import settings logger = logging.getLogger(__name__) @@ -29,6 +30,31 @@ _probe_executor = concurrent.futures.ThreadPoolExecutor(max_workers=4, thread_name_prefix="health-probe") +class _AvailabilityCache: + """Holds what `/games/availability` reports: the last probe's gated result. + + The invariant, which the sticky-availability bug came from violating: + + value == effective_availability(config.toml, most recent probe) + + `value` is a pure function of those two inputs and carries no history. Every + probe *overwrites* it with a freshly computed result — the previous value is + never read back as an input, so a stale False cannot influence, and cannot + survive, the next probe. This is deliberately a cache beside the settings + singleton rather than a mutation of it: `settings.games_availability` stays + pristine as the configured baseline, because that baseline is what each + recomputation starts from. Mutating it in place (as this code once did) makes + the output its own next input, which latches the flags off permanently. + + `value` is None until the first probe runs; see `get_effective_availability`. + """ + + value: GamesAvailability | None = None + + +_availability_cache = _AvailabilityCache() + + def _run_with_timeout[T](fn: Callable[[], T], timeout_s: float) -> T: return _probe_executor.submit(fn).result(timeout=timeout_s) @@ -212,7 +238,9 @@ def health() -> HealthStatus: else: follower_node = None - _apply_games_override(router_status, follower_node) + # Refresh what /games/availability reports. Recomputed from the pristine config + # (never from the cached value) so games recover once hardware comes back. + _availability_cache.value = effective_availability(settings.games_availability, router_status, follower_node) return HealthStatus( router=router_status, @@ -222,8 +250,32 @@ def health() -> HealthStatus: ) -def _apply_games_override(router_status: ComponentStatus, follower_node: ComponentStatus | None) -> None: - ga = settings.games_availability +def effective_availability( + configured: GamesAvailability, + router_status: ComponentStatus, + follower_node: ComponentStatus | None, +) -> GamesAvailability: + """Gate the configured game availability on live hardware reachability. + + Pure: same inputs always give the same answer, and neither `configured` nor + any shared state is mutated. Callers own the returned copy. + + Two properties follow from starting at `configured` and only ever clearing + flags, and both are load-bearing: + + - **Games recover on their own.** The result is re-derived from config each + call rather than revised from the previous result, so once the hardware is + reachable again the next probe returns True with no explicit re-enable + step. There is nothing to reset by hand and no restart needed. + - **config.toml is an absolute veto.** No branch here assigns True, so a game + disabled in config can never be switched on by a healthy probe. + + `configured` must therefore be the pristine values parsed from config.toml — + pass `settings.games_availability`, never a previously gated result. Passing + a gated result back in reintroduces the latching bug this function replaced: + the flags would ratchet toward all-off and stay there. + """ + ga = configured.model_copy() if not router_status.reachable: ga.chsh = False ga.qf = False @@ -231,3 +283,20 @@ def _apply_games_override(router_status: ComponentStatus, follower_node: Compone elif follower_node is not None and not follower_node.reachable: ga.chsh = False ga.ssm = False + return ga + + +def get_effective_availability() -> GamesAvailability: + """Return the availability computed by the most recent health probe. + + Backs `GET /games/availability`. Read-only: this does not probe hardware, so + the answer is only as fresh as the last `health()` call. Today that means app + startup, the daily report, and any manual hit on `/health/` — so hitting + `/health/` is what re-enables games on a node whose hardware has recovered. + + Falls back to the configured values when no probe has run yet, so the + endpoint reports config rather than claiming everything is disabled. + """ + if _availability_cache.value is None: + return settings.games_availability.model_copy() + return _availability_cache.value diff --git a/tests/pytest/test_games_availability.py b/tests/pytest/test_games_availability.py new file mode 100644 index 00000000..a385b7f0 --- /dev/null +++ b/tests/pytest/test_games_availability.py @@ -0,0 +1,69 @@ +"""Regression tests for the games-availability gate. + +The bug these cover: the health probe used to mutate the process-wide settings +singleton and could only ever assign False, so one unreachable-router probe +disabled every game until the process restarted. +""" + +from pqn_node.api.routes.health import ComponentStatus +from pqn_node.api.routes.health import effective_availability +from pqn_node.core.config import GamesAvailability +from pqn_node.core.config import get_settings + +UP = ComponentStatus(reachable=True) +DOWN = ComponentStatus(reachable=False, error="unreachable") + + +def test_all_games_gated_off_when_router_unreachable() -> None: + result = effective_availability(GamesAvailability(chsh=True, qf=True, ssm=True), DOWN, UP) + assert (result.chsh, result.qf, result.ssm) == (False, False, False) + + +def test_follower_unreachable_leaves_qf_enabled() -> None: + result = effective_availability(GamesAvailability(chsh=True, qf=True, ssm=True), UP, DOWN) + assert (result.chsh, result.qf, result.ssm) == (False, True, False) + + +def test_no_follower_configured_does_not_gate() -> None: + result = effective_availability(GamesAvailability(chsh=True, qf=True, ssm=True), UP, None) + assert (result.chsh, result.qf, result.ssm) == (True, True, True) + + +def test_games_re_enable_once_hardware_recovers() -> None: + """The whole point: the gate is re-derived from config, not latched.""" + configured = GamesAvailability(chsh=True, qf=True, ssm=True) + + while_down = effective_availability(configured, DOWN, UP) + assert not while_down.qf + + after_recovery = effective_availability(configured, UP, UP) + assert (after_recovery.chsh, after_recovery.qf, after_recovery.ssm) == (True, True, True) + + +def test_config_disabled_game_is_never_enabled_by_a_healthy_probe() -> None: + configured = GamesAvailability(chsh=False, qf=True, ssm=False) + + result = effective_availability(configured, UP, UP) + + assert result.chsh is False + assert result.ssm is False + assert result.qf is True + + +def test_gating_does_not_mutate_the_configured_object() -> None: + configured = GamesAvailability(chsh=True, qf=True, ssm=True) + + effective_availability(configured, DOWN, DOWN) + + assert (configured.chsh, configured.qf, configured.ssm) == (True, True, True) + + +def test_gating_does_not_mutate_the_settings_singleton() -> None: + """A failed probe must not poison process-wide state.""" + configured = get_settings().games_availability + before = (configured.chsh, configured.qf, configured.ssm) + + effective_availability(configured, DOWN, DOWN) + + after = get_settings().games_availability + assert (after.chsh, after.qf, after.ssm) == before