Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
29 changes: 21 additions & 8 deletions cron/scheduler.py
Original file line number Diff line number Diff line change
Expand Up @@ -118,14 +118,27 @@ def _summarize_cron_failure_for_delivery(job: dict, error: str | None) -> str:
"Full details saved in cron output."
)

# Match authentication/authorization wording at a word boundary and the
# 401/403 status codes as whole tokens, so "oauth", "4015" and similar do
# not trip a misleading auth message.
if re.search(r"authenticat|authoriz", lower) or re.search(r"\b(401|403)\b", text):
return (
f"⚠️ Cron '{job_name}' failed: provider authentication error. "
"Full details saved in cron output."
)
# no_agent jobs have no LLM provider involved, so an auth error is
# impossible regardless of what the script printed to stdout.
if not job.get("no_agent"):
# Match authentication/authorization wording at a word boundary.
if re.search(r"authenticat|authoriz", lower):
return (
f"⚠️ Cron '{job_name}' failed: provider authentication error. "
"Full details saved in cron output."
)
# Match 401/403 only when they appear in HTTP status context so that
# bare numbers in arbitrary script stdout (e.g. test output) do not
# trigger a misleading provider-auth error.
if re.search(
r"(?:HTTP(?:/\S+)?\s+[45]\d\d|status\s+[45]\d\d|response\s+[45]\d\d)",
text,
re.IGNORECASE,
):

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[45]\d\d matches every 4xx/5xx code, so HTTP/1.1 404 and response 500 take the provider-authentication branch. Please restrict this alternative to exact 401/403 codes and add contextual 404/500 negative coverage.

return (
f"⚠️ Cron '{job_name}' failed: provider authentication error. "
"Full details saved in cron output."
)

# Strip common exception wrappers and collapse provider payloads. Bound
# the input first so a multi-KB provider blob cannot slow the
Expand Down
162 changes: 162 additions & 0 deletions tests/cron/test_scheduler.py
Original file line number Diff line number Diff line change
Expand Up @@ -5509,3 +5509,165 @@ def test_returns_none_without_db_or_session(self):
from cron.scheduler import _set_cron_session_title
assert _set_cron_session_title(None, "sess-1", "X") is None
assert _set_cron_session_title(MagicMock(), "", "X") is None


class TestSummarizeCronFailureForDelivery:
"""Test the _summarize_cron_failure_for_delivery auth/rate-limit detection."""

# ------------------------------------------------------------------
# no_agent=True — no provider involved, so auth errors are impossible
# ------------------------------------------------------------------

def test_no_agent_with_401_does_not_match_auth(self):
"""no_agent=True + bare 401 in output → NOT auth error."""
from cron.scheduler import _summarize_cron_failure_for_delivery

result = _summarize_cron_failure_for_delivery(
{"name": "my-job", "no_agent": True},
"The script returned 401 for some request in its test suite",
)
assert "authentication error" not in result
assert "my-job" in result
assert "failed" in result # should fall through to generic message

def test_no_agent_with_403_does_not_match_auth(self):
"""no_agent=True + bare 403 in output → NOT auth error."""
from cron.scheduler import _summarize_cron_failure_for_delivery

result = _summarize_cron_failure_for_delivery(
{"name": "my-job", "no_agent": True},
"test returns 403 — expected behavior",
)
assert "authentication error" not in result

def test_no_agent_with_auth_word_does_not_match_auth(self):
"""no_agent=True + 'authorization' in output → NOT auth error."""
from cron.scheduler import _summarize_cron_failure_for_delivery

result = _summarize_cron_failure_for_delivery(
{"name": "my-job", "no_agent": True},
"authorization check failed: permission denied",
)
assert "authentication error" not in result

# ------------------------------------------------------------------
# no_agent=False or no key — provider involved, should match HTTP
# ------------------------------------------------------------------

def test_http_401_matches_auth(self):
"""HTTP 401 in error text → auth error (HTTP status context)."""
from cron.scheduler import _summarize_cron_failure_for_delivery

result = _summarize_cron_failure_for_delivery(
{"name": "my-job"},
"HTTP/1.1 401 Unauthorized",
)
assert "authentication error" in result

def test_http_403_matches_auth(self):
"""HTTP 403 in error text → auth error (HTTP status context)."""
from cron.scheduler import _summarize_cron_failure_for_delivery

result = _summarize_cron_failure_for_delivery(
{"name": "my-job"},
"HTTP/2 403 Forbidden",
)
assert "authentication error" in result

def test_status_401_matches_auth(self):
""""status 401" in error text → auth error."""
from cron.scheduler import _summarize_cron_failure_for_delivery

result = _summarize_cron_failure_for_delivery(
{"name": "my-job"},
"Provider responded with status 401 Unauthorized",
)
assert "authentication error" in result

def test_status_403_matches_auth(self):
""""status 403" in error text → auth error."""
from cron.scheduler import _summarize_cron_failure_for_delivery

result = _summarize_cron_failure_for_delivery(
{"name": "my-job"},
"provider status 403 forbidden",
)
assert "authentication error" in result

def test_response_401_matches_auth(self):
""""response 401" in error text → auth error."""
from cron.scheduler import _summarize_cron_failure_for_delivery

result = _summarize_cron_failure_for_delivery(
{"name": "my-job"},
"got response 401 from upstream",
)
assert "authentication error" in result

def test_authenticat_word_matches_auth(self):
"""'authenticat' keyword → auth error (no provider context needed)."""
from cron.scheduler import _summarize_cron_failure_for_delivery

result = _summarize_cron_failure_for_delivery(
{"name": "my-job"},
"Authentication failed for provider",
)
assert "authentication error" in result

def test_authoriz_word_matches_auth(self):
"""'authoriz' keyword → auth error."""
from cron.scheduler import _summarize_cron_failure_for_delivery

result = _summarize_cron_failure_for_delivery(
{"name": "my-job"},
"Authorization token expired",
)
assert "authentication error" in result

# ------------------------------------------------------------------
# Bare 401/403 WITHOUT HTTP context — should NOT match auth
# ------------------------------------------------------------------

def test_bare_401_does_not_match_auth(self):
"""Bare '401' without HTTP/status/response prefix → NOT auth error."""
from cron.scheduler import _summarize_cron_failure_for_delivery

result = _summarize_cron_failure_for_delivery(
{"name": "my-job"},
"script output: returns 401 on invalid input",
)
assert "authentication error" not in result

def test_bare_403_does_not_match_auth(self):
"""Bare '403' without HTTP/status/response prefix → NOT auth error."""
from cron.scheduler import _summarize_cron_failure_for_delivery

result = _summarize_cron_failure_for_delivery(
{"name": "my-job"},
"expected 403, got 200 in test_case_17",
)
assert "authentication error" not in result

# ------------------------------------------------------------------
# Rate limit / timeout detection still works (unaffected by change)
# ------------------------------------------------------------------

def test_rate_limit_still_detected(self):
"""Rate limit detection is unaffected."""
from cron.scheduler import _summarize_cron_failure_for_delivery

result = _summarize_cron_failure_for_delivery(
{"name": "my-job"},
"rate limit exceeded",
)
assert "rate limit" in result

def test_timeout_still_detected(self):
"""Timeout detection is unaffected."""
from cron.scheduler import _summarize_cron_failure_for_delivery

result = _summarize_cron_failure_for_delivery(
{"name": "my-job"},
"ReadTimeout: connection timed out",
)
assert "timeout" in result
Loading