diff --git a/hermes_cli/config_defaults.py b/hermes_cli/config_defaults.py index 6cfc67692f..08ab1789e8 100644 --- a/hermes_cli/config_defaults.py +++ b/hermes_cli/config_defaults.py @@ -4065,6 +4065,15 @@ DEFAULT_CONFIG = { # settings are non-secret routing config and live here. Both are bridged to # the VERTEX_PROJECT_ID / VERTEX_REGION env vars the adapter reads, so an # explicit env var still wins over config.yaml. + "nous": { + # Upper bound on the Nous auth keepalive tick, in seconds. The tick + # actually used derives from the credential lifetime the server issued + # and is capped by this value, so lowering it makes the keepalive more + # frequent while raising it has no effect below the derived tick. + # 0 disables the keepalive thread entirely. + "keepalive_interval_seconds": 900, + }, + "vertex": { # GCP project ID. Empty → use the project_id embedded in the service # account JSON (or ADC-resolved project). diff --git a/hermes_cli/nous_auth_keepalive.py b/hermes_cli/nous_auth_keepalive.py index 3bf28d2a78..f86acc705c 100644 --- a/hermes_cli/nous_auth_keepalive.py +++ b/hermes_cli/nous_auth_keepalive.py @@ -43,7 +43,7 @@ NOUS_AUTH_KEEPALIVE_MIN_INTERVAL_SECONDS = 60 # expiry without making the thread chatty. NOUS_AUTH_KEEPALIVE_TICKS_PER_LIFETIME = 4 NOUS_AUTH_KEEPALIVE_INITIAL_DELAY_SECONDS = 60 -NOUS_AUTH_KEEPALIVE_INTERVAL_ENV = "HERMES_NOUS_KEEPALIVE_INTERVAL_SECONDS" +NOUS_AUTH_KEEPALIVE_INTERVAL_CONFIG_KEY = "keepalive_interval_seconds" _keepalive_lock = threading.Lock() _keepalive_stop = threading.Event() @@ -59,12 +59,30 @@ def _timeout_seconds(value: Optional[float]) -> float: return 15.0 +def _nous_config() -> dict: + """Return the ``nous:`` section of config.yaml, or {} on any failure. + + Imported lazily: this module is loaded by the gateway and the web server + during startup, and the config loader pulls in a wider dependency graph + than the keepalive itself needs. + """ + try: + from hermes_cli.config import load_config + + section = load_config().get("nous") + return section if isinstance(section, dict) else {} + except Exception: + return {} + + def _interval_seconds(value: Optional[int]) -> int: """Resolve the keepalive tick interval. - Explicit argument wins, then the environment override, then the module - default. A non-positive result disables the keepalive thread entirely, - which is the documented way to turn it off. + Explicit argument wins, then ``nous.keepalive_interval_seconds`` in + config.yaml, then the module default. This is a behavioural threshold + rather than a credential, so it lives in config.yaml and not in .env. + A non-positive result disables the keepalive thread entirely, which is + the documented way to turn it off. """ if value is not None: try: @@ -72,15 +90,15 @@ def _interval_seconds(value: Optional[int]) -> int: except (TypeError, ValueError): return NOUS_AUTH_KEEPALIVE_INTERVAL_SECONDS - raw = os.getenv(NOUS_AUTH_KEEPALIVE_INTERVAL_ENV) - if raw is None or not raw.strip(): + raw = _nous_config().get(NOUS_AUTH_KEEPALIVE_INTERVAL_CONFIG_KEY) + if raw is None or (isinstance(raw, str) and not raw.strip()): return NOUS_AUTH_KEEPALIVE_INTERVAL_SECONDS try: return int(float(raw)) except (TypeError, ValueError): logger.warning( - "Ignoring invalid %s=%r; using %ds", - NOUS_AUTH_KEEPALIVE_INTERVAL_ENV, + "Ignoring invalid nous.%s=%r; using %ds", + NOUS_AUTH_KEEPALIVE_INTERVAL_CONFIG_KEY, raw, NOUS_AUTH_KEEPALIVE_INTERVAL_SECONDS, ) diff --git a/tests/hermes_cli/test_nous_auth_keepalive.py b/tests/hermes_cli/test_nous_auth_keepalive.py index 6006ab41fb..97becdbd45 100644 --- a/tests/hermes_cli/test_nous_auth_keepalive.py +++ b/tests/hermes_cli/test_nous_auth_keepalive.py @@ -5,16 +5,27 @@ from hermes_cli.auth import ACCESS_TOKEN_REFRESH_SKEW_SECONDS OBSERVED_LIFETIMES_SECONDS = (3594, 899) -def test_keepalive_interval_fits_inside_the_token_lifetime(): - """The tick must land before the credential rolls over. +def test_resolved_tick_fits_inside_the_token_lifetime(): + """The tick actually used must land before the credential rolls over. A tick at or above TTL - skew can miss the refresh window entirely, which is what made every hour expire into a 401 plus a re-auth round trip. + + This asserts on the derived tick rather than the configured constant, + because the constant is only a ceiling -- the schedule that ships is + whatever the derivation produces. It therefore fails if the derivation + constants regress (a lower TICKS_PER_LIFETIME or a higher + MIN_INTERVAL_SECONDS both break it), which a bare inequality against the + default interval cannot catch. """ - assert ( - keepalive.NOUS_AUTH_KEEPALIVE_INTERVAL_SECONDS - < min(OBSERVED_LIFETIMES_SECONDS) * 4 - ACCESS_TOKEN_REFRESH_SKEW_SECONDS - ) + for lifetime in OBSERVED_LIFETIMES_SECONDS: + tick = keepalive._tick_seconds( + keepalive.NOUS_AUTH_KEEPALIVE_INTERVAL_SECONDS, lifetime + ) + assert tick < lifetime - ACCESS_TOKEN_REFRESH_SKEW_SECONDS, ( + f"tick={tick}s leaves no room to refresh inside a {lifetime}s " + f"lifetime (skew={ACCESS_TOKEN_REFRESH_SKEW_SECONDS}s)" + ) def test_refresh_always_fires_before_expiry_for_observed_lifetimes(): @@ -98,30 +109,48 @@ def test_observed_lifetime_takes_the_shorter_credential(monkeypatch): def test_interval_precedence_and_disable(monkeypatch): - monkeypatch.delenv(keepalive.NOUS_AUTH_KEEPALIVE_INTERVAL_ENV, raising=False) + def _config(section): + monkeypatch.setattr(keepalive, "_nous_config", lambda: section) + + # An absent section leaves the module default in place. + _config({}) assert ( keepalive._interval_seconds(None) == keepalive.NOUS_AUTH_KEEPALIVE_INTERVAL_SECONDS ) - monkeypatch.setenv(keepalive.NOUS_AUTH_KEEPALIVE_INTERVAL_ENV, "600") + _config({keepalive.NOUS_AUTH_KEEPALIVE_INTERVAL_CONFIG_KEY: 600}) assert keepalive._interval_seconds(None) == 600 - # An explicit argument still outranks the environment. + # An explicit argument still outranks config.yaml. assert keepalive._interval_seconds(300) == 300 - # A malformed override falls back to the default rather than disabling. - monkeypatch.setenv(keepalive.NOUS_AUTH_KEEPALIVE_INTERVAL_ENV, "not-a-number") + # A malformed value falls back to the default rather than disabling. + _config({keepalive.NOUS_AUTH_KEEPALIVE_INTERVAL_CONFIG_KEY: "not-a-number"}) assert ( keepalive._interval_seconds(None) == keepalive.NOUS_AUTH_KEEPALIVE_INTERVAL_SECONDS ) # Zero remains the documented way to turn the keepalive off. - monkeypatch.setenv(keepalive.NOUS_AUTH_KEEPALIVE_INTERVAL_ENV, "0") + _config({keepalive.NOUS_AUTH_KEEPALIVE_INTERVAL_CONFIG_KEY: 0}) assert keepalive._interval_seconds(None) == 0 assert keepalive.start_nous_auth_keepalive() is None +def test_interval_survives_an_unreadable_config(monkeypatch): + """A broken config.yaml must not take the keepalive thread down with it.""" + + def _boom(): + raise RuntimeError("config.yaml is unreadable") + + monkeypatch.setattr("hermes_cli.config.load_config", _boom) + assert keepalive._nous_config() == {} + assert ( + keepalive._interval_seconds(None) + == keepalive.NOUS_AUTH_KEEPALIVE_INTERVAL_SECONDS + ) + + def test_keepalive_refreshes_stale_pool_entry(monkeypatch): class _Entry: access_token = "pooled-access-token"