From 9f2fb838e230a149a90618dbe5d0cd5a81e61b7f Mon Sep 17 00:00:00 2001 From: Teknium <127238744+teknium1@users.noreply.github.com> Date: Thu, 13 Aug 2026 10:54:33 -0700 Subject: [PATCH] fix(cron): classify TERMINAL_CWD lock timeouts; scrub environment-specific comments - Widen the scheduler-internal timeout classification to the sibling TERMINAL_CWD lock-wait TimeoutError (#79768), which also matched the generic 'timed out' branch and was delivered as a provider timeout. - Reconcile the drift-guard alert with #72056's lifecycle-aware remediation: finite one-shots are told to recreate the job, not to update a consumed one. - Scrub environment-specific references from comments/docstrings. --- cron/scheduler.py | 25 ++++++++++++---- tests/cron/test_cron_drift_alert_once.py | 2 +- ...est_cron_failure_alert_remediation_hint.py | 2 +- ...test_cron_failure_summarizer_inactivity.py | 30 +++++++++++++++---- 4 files changed, 45 insertions(+), 14 deletions(-) diff --git a/cron/scheduler.py b/cron/scheduler.py index f22cc453d7182..49b40ed58d769 100644 --- a/cron/scheduler.py +++ b/cron/scheduler.py @@ -106,9 +106,8 @@ def _fallback_chain_phrase() -> str: "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 + chain configured at all), 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 @@ -182,9 +181,9 @@ def _summarize_cron_failure_for_delivery(job: dict, error: str | None) -> str: # 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 + # wrong system entirely (field-reported: 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): @@ -195,6 +194,20 @@ def _summarize_cron_failure_for_delivery(job: dict, error: str | None) -> str: "quiet. Full details saved in cron output." ) + # Sibling scheduler-side timeout (#79768): the TERMINAL_CWD lock-wait + # abort also phrases itself with "Timed out ..." and would fall through + # to the generic provider-timeout branch below. Like the inactivity + # watchdog above, it is entirely scheduler-internal — no provider or + # fallback chain involved — so classify it before the generic match. + if "terminal_cwd" in lower and ("lock" in lower or "timed out" in lower): + return ( + f"⚠️ Cron '{job_name}' failed: could not acquire the scheduler's " + "working-directory lock — another cron job (a workdir writer or " + "long-running readers) held it too long. Not a provider or " + "fallback-chain issue; stagger the holder's schedule or remove " + "its workdir. 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. " diff --git a/tests/cron/test_cron_drift_alert_once.py b/tests/cron/test_cron_drift_alert_once.py index f555f829a364b..89b6aa6965a2d 100644 --- a/tests/cron/test_cron_drift_alert_once.py +++ b/tests/cron/test_cron_drift_alert_once.py @@ -1,6 +1,6 @@ """Drift-guard skips must alert once per job, not once per tick (#44585 + #73506). -Coatue field report (2026-08-11): a profile redeploy changed the global +Field report: a fleet-wide config change moved the global default provider and every unpinned cron started alerting on every tick — 40 jobs x N ticks of identical "Skipped to prevent unintended spend" spam. The #44585 drift guard correctly fails closed; this wires the existing diff --git a/tests/cron/test_cron_failure_alert_remediation_hint.py b/tests/cron/test_cron_failure_alert_remediation_hint.py index a30bc56a077be..bc9308b39082e 100644 --- a/tests/cron/test_cron_failure_alert_remediation_hint.py +++ b/tests/cron/test_cron_failure_alert_remediation_hint.py @@ -1,6 +1,6 @@ """The empty-chain failure alert must tell the operator how to fix it. -Coatue field report (2026-08-11): a user whose cron died with "No fallback +Field report: a user whose cron died with "No fallback chain configured." still cannot self-serve — the alert names the problem but not the remedy. The empty-chain branch of _fallback_chain_phrase() must name the exact commands: `hermes fallback add` for the chain, and the diff --git a/tests/cron/test_cron_failure_summarizer_inactivity.py b/tests/cron/test_cron_failure_summarizer_inactivity.py index b5286af5c00bf..fa12c1f17874c 100644 --- a/tests/cron/test_cron_failure_summarizer_inactivity.py +++ b/tests/cron/test_cron_failure_summarizer_inactivity.py @@ -2,7 +2,7 @@ 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` +Field-reported regression: 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 @@ -11,12 +11,11 @@ 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, +Second bug bundled into the same fix: 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 +regardless of whether fallback_providers was ever configured. Most installs +have fallback_providers: [], 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. """ @@ -95,3 +94,22 @@ def test_rate_limit_classification_still_takes_priority_over_inactivity_text(mon msg = _summarize_cron_failure_for_delivery(job, error) assert "weekly usage limit" in msg assert "No fallback chain configured" in msg + + +def test_terminal_cwd_lock_timeout_is_not_reported_as_provider_timeout(): + """Sibling scheduler-internal timeout (#79768): the TERMINAL_CWD lock-wait + abort says "Timed out ..." and must not fall through to the generic + provider-timeout branch.""" + job = {"name": "Workdir Job", "id": "abc123def456"} + error = ( + "TimeoutError: Timed out waiting for the TERMINAL_CWD write lock " + "after 600s — another cron job (a workdir writer, or long-running " + "readers) has held it for longer than the cron inactivity limit. " + "If a workdir job is the holder, stagger its schedule or remove its " + "workdir to unblock this job (#79768)." + ) + msg = _summarize_cron_failure_for_delivery(job, error) + assert "provider timeout" not in msg + assert "fallback chain" not in msg.lower() + assert "working-directory lock" in msg + assert "Workdir Job" in msg