From e62a47ab680bf952b26559e540c28bc18571017c Mon Sep 17 00:00:00 2001 From: kshitijk4poor <82637225+kshitijk4poor@users.noreply.github.com> Date: Thu, 24 Sep 2026 18:56:21 +0530 Subject: [PATCH] fix(curator): floor interval_hours at 1 and warn once per bad value interval_hours <= 0 made should_run_now() true on every idle tick, re-running the review pass each time. Route it through the same floor-with-default helper as the day counts (renamed _bounded_count), and log the fallback warning once per (key, value) since the dashboard status endpoint polls these getters. --- agent/curator.py | 19 +++++++++++++------ tests/agent/test_curator.py | 30 ++++++++++++++++++++++++++++++ 2 files changed, 43 insertions(+), 6 deletions(-) diff --git a/agent/curator.py b/agent/curator.py index d5482c9e20..9f08609278 100644 --- a/agent/curator.py +++ b/agent/curator.py @@ -107,32 +107,39 @@ def is_enabled() -> bool: # default ON when no config says otherwise def get_interval_hours() -> int: - return _config_number("interval_hours", DEFAULT_INTERVAL_HOURS, int) + # < 1 would make should_run_now() true on every idle tick (a review pass each time), so floor it the same way. + return _bounded_count("interval_hours", DEFAULT_INTERVAL_HOURS) def get_min_idle_hours() -> float: return _config_number("min_idle_hours", DEFAULT_MIN_IDLE_HOURS, float) -def _bounded_days(key: str, default: int) -> int: - """*key* (a ``curator.`` day count), floored at 1 like ``curator prune --days`` already +_warned_bad_values: set = set() + + +def _bounded_count(key: str, default: int) -> int: + """*key* (a ``curator.`` day/hour count), floored at 1 like ``curator prune --days`` already refuses (hermes_cli/curator.py::_cmd_prune). A value < 1 collapses stale_cutoff/archive_cutoff onto or past "now" in apply_automatic_transitions(), mass-transitioning every skill with any past activity on the next automatic pass — unlike the manual prune path this runs unconfirmed, so it falls back to the default instead of acting on the bad value.""" value = _config_number(key, default, int) if value < 1: - logger.warning("curator.%s must be >= 1 (got %d); using the default of %d", key, value, default) + # Warn once per (key, bad value): the dashboard status endpoint polls these getters. + if (key, value) not in _warned_bad_values: + _warned_bad_values.add((key, value)) + logger.warning("curator.%s must be >= 1 (got %d); using the default of %d", key, value, default) return default return value def get_stale_after_days() -> int: - return _bounded_days("stale_after_days", DEFAULT_STALE_AFTER_DAYS) + return _bounded_count("stale_after_days", DEFAULT_STALE_AFTER_DAYS) def get_archive_after_days() -> int: - return _bounded_days("archive_after_days", DEFAULT_ARCHIVE_AFTER_DAYS) + return _bounded_count("archive_after_days", DEFAULT_ARCHIVE_AFTER_DAYS) def get_consolidate() -> bool: diff --git a/tests/agent/test_curator.py b/tests/agent/test_curator.py index c15ca6b727..13d63b2a81 100644 --- a/tests/agent/test_curator.py +++ b/tests/agent/test_curator.py @@ -163,6 +163,36 @@ def test_non_positive_stale_after_days_falls_back_to_default(curator_env, monkey assert c.get_stale_after_days() == c.DEFAULT_STALE_AFTER_DAYS +@pytest.mark.parametrize("bad_hours", [0, -3]) +def test_non_positive_interval_hours_falls_back_to_default(curator_env, monkeypatch, bad_hours): + """``curator.interval_hours: 0`` (or negative) made should_run_now() true on every idle tick, + re-running the review pass each time; it must fall back to the default interval instead.""" + c = curator_env["curator"] + monkeypatch.setattr(c, "_load_config", lambda: {"interval_hours": bad_hours}) + now = datetime.now(timezone.utc) + c.save_state({"last_run_at": (now - timedelta(minutes=1)).isoformat()}) + + assert c.get_interval_hours() == c.DEFAULT_INTERVAL_HOURS + assert c.should_run_now(now=now) is False + + +def test_bad_bounded_value_warns_once_per_distinct_value(curator_env, monkeypatch, caplog): + """The dashboard status endpoint polls these getters, so a bad value logs once, not per poll; + a different bad value (the user edited config again) logs again.""" + c = curator_env["curator"] + cfg = {"archive_after_days": 0} + monkeypatch.setattr(c, "_load_config", lambda: cfg) + with caplog.at_level("WARNING", logger=c.logger.name): + for _ in range(3): + c.get_archive_after_days() + cfg["archive_after_days"] = -1 + c.get_archive_after_days() + c.get_archive_after_days() + msgs = [r.getMessage() for r in caplog.records if "archive_after_days" in r.getMessage()] + assert len(msgs) == 2, msgs + assert "got 0" in msgs[0] and "got -1" in msgs[1] + + def test_pinned_skill_is_never_touched(curator_env): c = curator_env["curator"] u = curator_env["usage"]