From f79325498e01caaa1c0adafd74dea30ab8704be2 Mon Sep 17 00:00:00 2001 From: nesquena-hermes Date: Thu, 10 Sep 2026 22:30:57 -0700 Subject: [PATCH] fix(cron): restore messaging platforms in the delivery picker (#7525) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix(cron): restore messaging platforms in the delivery picker The Agent moved _KNOWN_DELIVERY_PLATFORMS from cron.scheduler to cron.scheduler_delivery. _handle_cron_delivery_options imported the old path inside a bare except and fell back to an empty frozenset, so the import error was swallowed and GET /api/crons/delivery-options silently returned only local and origin — telegram, discord, slack, feishu and every other messaging platform disappeared from the cron delivery picker with no error surfaced anywhere. Resolve the authority across both module paths (current first, legacy fallback) so the picker keeps working across Agent versions, and add a regression test that pins the resolution order and cross-checks the endpoint output against the resolved authority. * test(cron): skip the authority-resolution test where the Agent is absent CI runs agent-free (tests/browser_smoke.py), so neither cron.scheduler_delivery nor cron.scheduler is importable there and the authority resolves empty. The new regression test asserted non-empty and therefore failed shard 3 on both Python versions. Skip instead: with no cron package there is no authority to resolve and nothing meaningful to assert. The test still fails loudly wherever the Agent IS installed, which is where a future relocation would bite. * test(cron): register the new test in _AGENT_DEPENDENT_TESTS Use the project's existing skip mechanism (tests/conftest.py) rather than an ad-hoc inline pytest.skip, matching the five sibling delivery-options tests. CI runs agent-free, so there is no cron package and no authority to resolve there; with the Agent installed the test still asserts loudly, which is the environment where a future relocation would actually bite. Verified both directions: 6/6 pass with the Agent present, and forcing AGENT_MODULES_AVAILABLE=False skips all 6 (was 5) as CI will. --------- Co-authored-by: nesquena-hermes --- api/routes.py | 21 ++++++++++++--- tests/conftest.py | 1 + tests/test_cron_delivery_options.py | 40 +++++++++++++++++++++++++++++ 3 files changed, 58 insertions(+), 4 deletions(-) diff --git a/api/routes.py b/api/routes.py index 5f402934..48b0963a 100644 --- a/api/routes.py +++ b/api/routes.py @@ -24707,10 +24707,23 @@ def _handle_cron_create(handler, body): def _handle_cron_delivery_options(handler): """Return available delivery platforms for cron jobs.""" - try: - from cron.scheduler import _KNOWN_DELIVERY_PLATFORMS - except Exception: - _KNOWN_DELIVERY_PLATFORMS = frozenset() + # The Agent moved this authority from ``cron.scheduler`` to + # ``cron.scheduler_delivery``. Try the current location first and fall back + # to the legacy one so the picker keeps working across Agent versions. + # Without the fallback chain a bare ImportError silently degraded this + # endpoint to local/origin only, dropping every messaging platform from the + # cron delivery picker (telegram, discord, slack, feishu, ...). + _KNOWN_DELIVERY_PLATFORMS = frozenset() + import importlib + for _module_name in ("cron.scheduler_delivery", "cron.scheduler"): + try: + _mod = importlib.import_module(_module_name) + except Exception: + continue + _known = getattr(_mod, "_KNOWN_DELIVERY_PLATFORMS", None) + if _known: + _KNOWN_DELIVERY_PLATFORMS = frozenset(_known) + break platforms = [ {"value": "local", "label": "Local (save output only)"}, {"value": "origin", "label": "Origin (reply to creator)"} diff --git a/tests/conftest.py b/tests/conftest.py index 5f37c1f5..7dc77884 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -651,6 +651,7 @@ def pytest_collection_modifyitems(config, items): 'test_delivery_options_structure', 'test_delivery_options_includes_common_platforms', 'test_delivery_options_local_label', + 'test_delivery_options_survives_the_authority_module_move', # Skills endpoints (need tools.skills_tool module) 'test_skills_list', 'test_skills_list_has_required_fields', diff --git a/tests/test_cron_delivery_options.py b/tests/test_cron_delivery_options.py index 9e140ab8..d6ffe358 100644 --- a/tests/test_cron_delivery_options.py +++ b/tests/test_cron_delivery_options.py @@ -65,3 +65,43 @@ def test_delivery_options_local_label(): local_entry = next(p for p in result["platforms"] if p["value"] == "local") # Label should contain "Local" or be an i18n key — just verify it's non-empty assert local_entry["label"], "Local platform label is empty" + + +def test_delivery_options_survives_the_authority_module_move(): + """The platform list must not silently empty when the Agent relocates it. + + ``_KNOWN_DELIVERY_PLATFORMS`` used to live in ``cron.scheduler`` and now + lives in ``cron.scheduler_delivery``. The endpoint previously imported the + old path inside a bare ``except`` and fell back to an EMPTY frozenset, so + the move silently degraded the cron delivery picker to local/origin only — + every messaging platform (telegram, discord, slack, feishu, ...) vanished + from the UI with no error anywhere. Pin the resolution order so a future + relocation fails loudly instead of silently dropping platforms. + + Registered in ``_AGENT_DEPENDENT_TESTS`` (tests/conftest.py) alongside the + other delivery-options tests: CI runs agent-free, so there is no ``cron`` + package and no authority to resolve there. + """ + import importlib + + resolved = frozenset() + for module_name in ("cron.scheduler_delivery", "cron.scheduler"): + try: + mod = importlib.import_module(module_name) + except Exception: + continue + known = getattr(mod, "_KNOWN_DELIVERY_PLATFORMS", None) + if known: + resolved = frozenset(known) + break + + assert resolved, ( + "_KNOWN_DELIVERY_PLATFORMS resolved empty from every known module path " + "— the cron delivery picker would silently show only local/origin" + ) + # The endpoint's own output must agree with the resolved authority. + result, status = get("/api/crons/delivery-options") + assert status == 200 + values = {p["value"] for p in result["platforms"]} + missing = resolved - values + assert not missing, f"platforms resolved but absent from the endpoint: {sorted(missing)}"