From efbbc993f62fd4a697ee45c9aa780293a9358cfa Mon Sep 17 00:00:00 2001 From: cation98 Date: Mon, 10 Aug 2026 21:12:27 +0900 Subject: [PATCH] fix(cron): avoid false provider failure summaries --- cron/scheduler.py | 7 ++++- tests/cron/test_scheduler.py | 53 ++++++++++++++++++++++++++++++++++-- 2 files changed, 57 insertions(+), 3 deletions(-) diff --git a/cron/scheduler.py b/cron/scheduler.py index d519662ac5..4eae63a363 100644 --- a/cron/scheduler.py +++ b/cron/scheduler.py @@ -191,7 +191,12 @@ def _summarize_cron_failure_for_delivery(job: dict, error: str | None) -> str: ) # Provider/API failures are the common noisy path. Keep these short. - if provider_reachable and ("429" in text or "rate limit" in lower or "usage limit" in lower): + # Match 429 as a whole token (#83188 @cation98): bare substring matching + # let identifiers containing those digits (job ids, ports, hashes) trip + # a false "provider rate limit" alert. + if provider_reachable and ( + re.search(r"\b429\b", text) or "rate limit" in lower or "usage limit" in lower + ): reason = "rate limit" if "weekly usage limit" in lower: reason = "weekly usage limit" diff --git a/tests/cron/test_scheduler.py b/tests/cron/test_scheduler.py index 357614a23e..2aade5c04e 100644 --- a/tests/cron/test_scheduler.py +++ b/tests/cron/test_scheduler.py @@ -9,11 +9,61 @@ from unittest.mock import AsyncMock, patch, MagicMock import pytest -from cron.scheduler import _resolve_origin, _resolve_delivery_target, _deliver_result, _send_media_via_adapter, run_job, SILENT_MARKER, _build_job_prompt, _resolve_cron_enabled_toolsets, _merge_mcp_into_per_job_toolsets +from cron.scheduler import ( + SILENT_MARKER, + _build_job_prompt, + _deliver_result, + _merge_mcp_into_per_job_toolsets, + _resolve_cron_enabled_toolsets, + _resolve_delivery_target, + _resolve_origin, + _send_media_via_adapter, + _summarize_cron_failure_for_delivery, + run_job, +) from tools.env_passthrough import clear_env_passthrough from tools.credential_files import clear_credential_files +class TestSummarizeCronFailureForDelivery: + def test_embedded_429_in_source_identifier_is_not_a_rate_limit(self): + summary = _summarize_cron_failure_for_delivery( + {"name": "LLM Wiki Incremental Index", "no_agent": True}, + "Script failed: path/hash429abc.md source snapshot failure", + ) + + assert "provider rate limit" not in summary + assert "hash429abc.md" in summary + + def test_http_429_is_still_classified_as_a_rate_limit(self): + summary = _summarize_cron_failure_for_delivery( + {"name": "provider-backed job"}, + "HTTP 429: Too Many Requests", + ) + + assert "provider rate limit" in summary + assert "Fallback chain was exhausted or unavailable" in summary + + def test_no_agent_rate_limit_does_not_claim_a_fallback_chain(self): + summary = _summarize_cron_failure_for_delivery( + {"name": "script job", "no_agent": True}, + "HTTP 429: Too Many Requests", + ) + + assert "provider rate limit" in summary + assert "Fallback chain" not in summary + + def test_no_agent_timeout_is_identified_as_a_script_timeout(self): + summary = _summarize_cron_failure_for_delivery( + {"name": "script job", "no_agent": True}, + "Script timed out after 3600s", + ) + + assert "script timeout" in summary + assert "provider timeout" not in summary + assert "Fallback chain" not in summary + + class TestPerJobToolsetMcpMerge: """A per-job enabled_toolsets allowlist must not silently drop MCP servers.""" @@ -1975,4 +2025,3 @@ class TestSetCronSessionTitle: assert out == "Nightly Synthesis #2" db.get_next_title_in_lineage.assert_called_once_with("Nightly Synthesis") -