fix(gateway): finite-bounded watchdog knob validation + wire keys through load_gateway_config
Addresses both review findings from @egilewski on #89134: - Non-finite values: _coerce_int now degrades int(inf) (OverflowError previously ABORTED gateway config loading); the clamp requires math.isfinite plus sane upper bounds (interval <=3600s, timeout <=600s, strikes <=1000), falling back to the shutdown_watchdog constants. - Loader wiring: load_gateway_config builds gw_data FLAT and never forwarded the yaml gateway: section, so loop_watchdog* keys — including the PRE-EXISTING loop_watchdog bool documented in config_defaults — were silently ignored on the real startup path. Bridged with the established top-level-wins/nested-fallback pattern. E2E: config.yaml with loop_watchdog:false + strikes:12 + interval:.inf now yields False/12/30.0 through the real loader.
This commit is contained in:
@@ -9,6 +9,7 @@ Handles loading and validating configuration for:
|
||||
"""
|
||||
|
||||
import logging
|
||||
import math
|
||||
import os
|
||||
import json
|
||||
from pathlib import Path
|
||||
@@ -157,7 +158,9 @@ def _coerce_int(value: Any, default: int) -> int:
|
||||
return default
|
||||
try:
|
||||
return int(value)
|
||||
except (TypeError, ValueError):
|
||||
except (TypeError, ValueError, OverflowError):
|
||||
# OverflowError: int(float("inf")) — a non-finite YAML value must
|
||||
# degrade to the default, not abort gateway config loading.
|
||||
return default
|
||||
|
||||
|
||||
@@ -1251,11 +1254,19 @@ class GatewayConfig:
|
||||
else nested_gateway.get("loop_watchdog_max_strikes"),
|
||||
DEFAULT_LOOP_WATCHDOG_MAX_STRIKES,
|
||||
)
|
||||
if loop_watchdog_probe_interval_s < 1.0:
|
||||
if (
|
||||
not math.isfinite(loop_watchdog_probe_interval_s)
|
||||
or loop_watchdog_probe_interval_s < 1.0
|
||||
or loop_watchdog_probe_interval_s > 3600.0
|
||||
):
|
||||
loop_watchdog_probe_interval_s = DEFAULT_LOOP_WATCHDOG_INTERVAL_S
|
||||
if loop_watchdog_probe_timeout_s < 1.0:
|
||||
if (
|
||||
not math.isfinite(loop_watchdog_probe_timeout_s)
|
||||
or loop_watchdog_probe_timeout_s < 1.0
|
||||
or loop_watchdog_probe_timeout_s > 600.0
|
||||
):
|
||||
loop_watchdog_probe_timeout_s = DEFAULT_LOOP_WATCHDOG_TIMEOUT_S
|
||||
if loop_watchdog_max_strikes < 1:
|
||||
if loop_watchdog_max_strikes < 1 or loop_watchdog_max_strikes > 1000:
|
||||
loop_watchdog_max_strikes = DEFAULT_LOOP_WATCHDOG_MAX_STRIKES
|
||||
if multiplex_profiles is None and isinstance(nested_gateway, dict):
|
||||
# Also honor gateway.multiplex_profiles written by
|
||||
@@ -1523,6 +1534,23 @@ def load_gateway_config() -> GatewayConfig:
|
||||
elif isinstance(gateway_section, dict) and "write_sessions_json" in gateway_section:
|
||||
gw_data["write_sessions_json"] = gateway_section["write_sessions_json"]
|
||||
|
||||
# Loop-liveness watchdog toggle + tuning knobs: top-level wins;
|
||||
# nested gateway.* fallback. GatewayConfig.from_dict has its own
|
||||
# nested fallback, but this loader builds gw_data FLAT and never
|
||||
# forwards the yaml `gateway:` section — without this bridge the
|
||||
# documented keys (including the pre-existing loop_watchdog bool)
|
||||
# were silently ignored on the real gateway startup path.
|
||||
for _wd_key in (
|
||||
"loop_watchdog",
|
||||
"loop_watchdog_probe_interval_s",
|
||||
"loop_watchdog_probe_timeout_s",
|
||||
"loop_watchdog_max_strikes",
|
||||
):
|
||||
if _wd_key in yaml_cfg:
|
||||
gw_data[_wd_key] = yaml_cfg[_wd_key]
|
||||
elif isinstance(gateway_section, dict) and _wd_key in gateway_section:
|
||||
gw_data[_wd_key] = gateway_section[_wd_key]
|
||||
|
||||
if "filter_silence_narration" in yaml_cfg:
|
||||
gw_data["filter_silence_narration"] = yaml_cfg[
|
||||
"filter_silence_narration"
|
||||
|
||||
@@ -220,6 +220,58 @@ def test_gateway_config_loop_watchdog_tuning_round_trip():
|
||||
assert clamped.loop_watchdog_max_strikes == 3
|
||||
|
||||
|
||||
def test_gateway_config_loop_watchdog_nonfinite_values_degrade():
|
||||
"""NaN/Inf tuning values fall back to defaults instead of reaching the
|
||||
watchdog's Event.wait loop (or aborting config load via int(inf))."""
|
||||
from gateway.config import GatewayConfig
|
||||
|
||||
cfg = GatewayConfig.from_dict(
|
||||
{
|
||||
"loop_watchdog_probe_interval_s": float("inf"),
|
||||
"loop_watchdog_probe_timeout_s": float("nan"),
|
||||
"loop_watchdog_max_strikes": float("inf"), # int() would raise
|
||||
}
|
||||
)
|
||||
assert cfg.loop_watchdog_probe_interval_s == 30.0
|
||||
assert cfg.loop_watchdog_probe_timeout_s == 10.0
|
||||
assert cfg.loop_watchdog_max_strikes == 3
|
||||
|
||||
# Oversized-but-finite values also clamp to defaults.
|
||||
big = GatewayConfig.from_dict(
|
||||
{
|
||||
"loop_watchdog_probe_interval_s": 86400,
|
||||
"loop_watchdog_probe_timeout_s": 7200,
|
||||
"loop_watchdog_max_strikes": 10**9,
|
||||
}
|
||||
)
|
||||
assert big.loop_watchdog_probe_interval_s == 30.0
|
||||
assert big.loop_watchdog_probe_timeout_s == 10.0
|
||||
assert big.loop_watchdog_max_strikes == 3
|
||||
|
||||
|
||||
def test_load_gateway_config_bridges_loop_watchdog_keys(tmp_path, monkeypatch):
|
||||
"""The real startup loader must honor gateway.loop_watchdog* from
|
||||
config.yaml — from_dict's nested fallback never sees the yaml gateway
|
||||
section because load_gateway_config builds gw_data flat."""
|
||||
from gateway.config import load_gateway_config
|
||||
|
||||
(tmp_path / "config.yaml").write_text(
|
||||
"gateway:\n"
|
||||
" loop_watchdog: false\n"
|
||||
" loop_watchdog_probe_interval_s: 45\n"
|
||||
" loop_watchdog_probe_timeout_s: 15\n"
|
||||
" loop_watchdog_max_strikes: 12\n",
|
||||
encoding="utf-8",
|
||||
)
|
||||
monkeypatch.setattr("gateway.config.get_hermes_home", lambda: tmp_path)
|
||||
|
||||
cfg = load_gateway_config()
|
||||
assert cfg.loop_watchdog is False
|
||||
assert cfg.loop_watchdog_probe_interval_s == 45.0
|
||||
assert cfg.loop_watchdog_probe_timeout_s == 15.0
|
||||
assert cfg.loop_watchdog_max_strikes == 12
|
||||
|
||||
|
||||
def test_gateway_runner_liveness_guards_start_and_stop():
|
||||
from gateway.run import GatewayRunner
|
||||
|
||||
|
||||
Reference in New Issue
Block a user