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.
This commit is contained in:
parent
05a84f205e
commit
9f2fb838e2
|
|
@ -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. "
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
|
|
|||
Loading…
Reference in New Issue