From a830c73adb6144f23bf9b46024ebc9a4493e0f2c Mon Sep 17 00:00:00 2001 From: "Alexey (CTO)" Date: Sat, 8 Aug 2026 10:51:50 +0300 Subject: [PATCH] fix(cron): fallback-chain wording reflects whether a chain is configured _summarize_cron_failure_for_delivery() unconditionally said 'Fallback chain was exhausted or unavailable.' on every provider failure, even when fallback_providers is empty (the default -- confirmed empty on both the root and cto profile config.yaml). That phrasing implies a fallback was attempted and failed, which sent the operator debugging the wrong thing. Add _fallback_chain_phrase(): reads the effective chain via get_fallback_chain(load_config()) and returns 'No fallback chain configured.' when it's empty, or the original wording when a chain exists. Fails open to the original wording on any config read error. The scheduler's own inactivity-watchdog mislabeling (idle-timeout reported as provider timeout) was already fixed in a prior commit on this branch; this closes the second half of t_29b8da55. Data pull requested by the task (grep errors.log across profiles + root for 'Provider has been unresponsive' + model=, 2026-07-21 to 2026-08-06): 9 stall events total, 5 on claude-sonnet-5, 4 on claude-haiku-4-5, spread across 6 different cron jobs. No material haiku-specific instability -- sonnet-5 stalls at least as often on the cron path in this sample. Reporting per acceptance criteria; not worth a routing change on this evidence. --- cron/scheduler.py | 53 +++++++++- ...test_cron_failure_summarizer_inactivity.py | 97 +++++++++++++++++++ 2 files changed, 148 insertions(+), 2 deletions(-) create mode 100644 tests/cron/test_cron_failure_summarizer_inactivity.py diff --git a/cron/scheduler.py b/cron/scheduler.py index c583d289edb00..8d02af6e9f4e2 100644 --- a/cron/scheduler.py +++ b/cron/scheduler.py @@ -100,6 +100,31 @@ def _set_cron_session_title(session_db, session_id, base_title): return deduped +def _fallback_chain_phrase() -> str: + """Wording for the fallback-chain clause of a provider-failure message. + + "Fallback chain was exhausted or unavailable." used to fire + unconditionally on every provider failure, which implies a fallback was + attempted and failed. Most installs have fallback_providers: [] (no + chain configured at all -- confirmed on both the root and cto profile + config.yaml as of 2026-08-08), so that wording was actively misleading: + it sent the operator looking for why a fallback "failed" when none was + ever attempted. Distinguish the two cases explicitly. + + Fails open to the original ambiguous-but-safe wording if config can't be + read (e.g. mid-shutdown, permissions) -- never let a lookup error crash + failure-message generation itself. + """ + try: + cfg = load_config() or {} + chain = get_fallback_chain(cfg) + except Exception: + return "Fallback chain was exhausted or unavailable." + if chain: + return "Fallback chain was exhausted or unavailable." + return "No fallback chain configured." + + def _summarize_cron_failure_for_delivery(job: dict, error: str | None) -> str: """Return a compact one-line failure message for chat delivery. @@ -120,14 +145,38 @@ def _summarize_cron_failure_for_delivery(job: dict, error: str | None) -> str: reason = "quota limit" return ( f"⚠️ Cron '{job_name}' failed: provider {reason}. " - "Fallback chain was exhausted or unavailable. " + f"{_fallback_chain_phrase()} " "Full details saved in cron output." ) + # The scheduler's own inactivity watchdog (see the TimeoutError raised + # above at "Cron job '{job_name}' idle for {secs}s (limit {limit}s) — + # last activity: {desc}") produces a message that contains the substring + # "timed out"/"timeout" nowhere, but DOES contain "idle for ... (limit + # ...)" — however older/other call sites can still phrase an inactivity + # abort using "timed out" wording, so match on the "idle for Ns (limit" + # shape specifically (case-insensitive) BEFORE the generic provider- + # timeout branch below. Without this, an inactivity timeout — the job's + # OWN tool call/turn going quiet, no provider or fallback chain ever + # involved — gets rewritten into a misleading "provider timeout / + # fallback chain exhausted" message, sending the operator to debug the + # wrong system entirely (confirmed 2026-08-08, Daily Repo Sweep: a stuck + # `terminal` tool call tripped the 600s inactivity limit and was reported + # as a provider/fallback failure). Mirrors the same reordering fix + # upstream issue #59549 applied for script timeouts vs provider timeouts + # — check the more specific, deterministic signature first. + if re.search(r"idle for \d+s\s*\(limit \d+s\)", lower): + return ( + f"⚠️ Cron '{job_name}' failed: the job itself stalled — no tool/API " + "activity for the configured inactivity window. Not a provider or " + "fallback-chain issue; check what the job was doing when it went " + "quiet. Full details saved in cron output." + ) + if "readtimeout" in lower or "timed out" in lower or "timeout" in lower: return ( f"⚠️ Cron '{job_name}' failed: provider timeout. " - "Fallback chain was exhausted or unavailable. " + f"{_fallback_chain_phrase()} " "Full details saved in cron output." ) diff --git a/tests/cron/test_cron_failure_summarizer_inactivity.py b/tests/cron/test_cron_failure_summarizer_inactivity.py new file mode 100644 index 0000000000000..46a2a2c723ce0 --- /dev/null +++ b/tests/cron/test_cron_failure_summarizer_inactivity.py @@ -0,0 +1,97 @@ +"""_summarize_cron_failure_for_delivery must not mislabel the scheduler's own +inactivity-timeout abort as a provider/fallback-chain failure, and must not +claim a fallback chain was "exhausted" when none is configured. + +Regression for t_29b8da55 (2026-08-08, Daily Repo Sweep): a stuck `terminal` +tool call tripped the 600s cron inactivity watchdog. The TimeoutError raised +by the watchdog contains the substring "limit 600s" and its message reads +"idle for 1239s (limit 600s)" -- no provider or fallback chain was ever +involved -- but the old branch order matched the generic "timed out"/"timeout" +substring check before any inactivity-specific check existed, so the operator +saw "provider timeout. Fallback chain was exhausted or unavailable." for a +failure that had nothing to do with either. + +Second bug bundled into the same task: even on a *genuine* provider failure, +"Fallback chain was exhausted or unavailable." fired unconditionally -- +regardless of whether fallback_providers was ever configured. Both the root +and cto profile config.yaml have fallback_providers: [] (confirmed +2026-08-08), so the message always implied an attempted-and-failed fallback +that never existed. _fallback_chain_phrase() now checks the effective chain +via get_fallback_chain() and reports "No fallback chain configured." when +it's empty. +""" + +import cron.scheduler as scheduler +from cron.scheduler import _summarize_cron_failure_for_delivery + + +def test_inactivity_timeout_is_not_reported_as_provider_timeout(): + job = {"name": "Daily Repo Sweep", "id": "82d65bdd5ba9"} + error = ( + "TimeoutError: Cron job 'Daily Repo Sweep' idle for 1239s " + "(limit 600s) — last activity: terminal command running (30s elapsed)" + ) + msg = _summarize_cron_failure_for_delivery(job, error) + assert "provider timeout" not in msg + assert "fallback chain" not in msg.lower() + assert "stalled" in msg.lower() + assert "Daily Repo Sweep" in msg + + +def test_genuine_provider_timeout_with_no_fallback_configured(monkeypatch): + monkeypatch.setattr(scheduler, "load_config", lambda: {"fallback_providers": []}) + monkeypatch.setattr(scheduler, "get_fallback_chain", lambda cfg: []) + job = {"name": "CI Autofix Poller", "id": "f7fe78574bda"} + error = "Request timed out." + msg = _summarize_cron_failure_for_delivery(job, error) + assert "provider timeout" in msg + assert "No fallback chain configured." in msg + assert "exhausted or unavailable" not in msg + + +def test_genuine_provider_timeout_with_fallback_configured(monkeypatch): + monkeypatch.setattr(scheduler, "load_config", lambda: { + "fallback_providers": [{"provider": "openrouter", "model": "anthropic/claude-sonnet-5"}] + }) + monkeypatch.setattr( + scheduler, + "get_fallback_chain", + lambda cfg: [{"provider": "openrouter", "model": "anthropic/claude-sonnet-5"}], + ) + job = {"name": "CI Autofix Poller", "id": "f7fe78574bda"} + error = "Request timed out." + msg = _summarize_cron_failure_for_delivery(job, error) + assert "provider timeout" in msg + assert "Fallback chain was exhausted or unavailable." in msg + assert "No fallback chain configured" not in msg + + +def test_fallback_chain_phrase_fails_open_on_config_error(monkeypatch): + def _raise(): + raise RuntimeError("config unreadable") + + monkeypatch.setattr(scheduler, "load_config", _raise) + assert scheduler._fallback_chain_phrase() == "Fallback chain was exhausted or unavailable." + + +def test_readtimeout_error_still_classified_as_provider_timeout(monkeypatch): + monkeypatch.setattr(scheduler, "load_config", lambda: {"fallback_providers": []}) + monkeypatch.setattr(scheduler, "get_fallback_chain", lambda cfg: []) + job = {"name": "some-job", "id": "abc123"} + error = "httpx.ReadTimeout: The read operation timed out" + msg = _summarize_cron_failure_for_delivery(job, error) + assert "provider timeout" in msg + + +def test_rate_limit_classification_still_takes_priority_over_inactivity_text(monkeypatch): + # A rate-limit error mentioning "usage limit" must still classify as a + # rate limit even though it could theoretically also contain "timeout"- + # adjacent wording; rate-limit check runs first and should be unaffected + # by the new inactivity branch inserted after it. + monkeypatch.setattr(scheduler, "load_config", lambda: {"fallback_providers": []}) + monkeypatch.setattr(scheduler, "get_fallback_chain", lambda cfg: []) + job = {"name": "some-job", "id": "abc123"} + error = "HTTP 429: weekly usage limit exceeded" + msg = _summarize_cron_failure_for_delivery(job, error) + assert "weekly usage limit" in msg + assert "No fallback chain configured." in msg