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.
This commit is contained in:
parent
0b50c8e48f
commit
a830c73adb
|
|
@ -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."
|
||||
)
|
||||
|
||||
|
|
|
|||
|
|
@ -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
|
||||
Loading…
Reference in New Issue