fix(cron): prevent false-positive 401/403 match in no_agent script stdout (#70908) - #70913
fix(cron): prevent false-positive 401/403 match in no_agent script stdout (#70908)#70913webtecnica wants to merge 1 commit into
Conversation
teknium1
left a comment
There was a problem hiding this comment.
Thanks for addressing a real delivery-classification bug: current main still treats any bare 401 or 403 as provider authentication at cron/scheduler.py:128, and failed no_agent script output reaches this summarizer through cron/scheduler.py:2776-2841 and cron/scheduler.py:3989.
Problems
cron/scheduler.py:137changes the status predicate to[45]\d\d, which matches every 4xx/5xx response. ThusHTTP/1.1 404andresponse 500would be delivered as "provider authentication error," contrary to the stated 401/403-only behavior.
Suggested changes
- Restrict the new HTTP-context branch to exact 401/403 status codes.
- Add negative tests for contextual 404 and 500 responses; the current added tests cover only 401/403 positives and bare-number negatives.
Automated hermes-sweeper review.
| r"(?:HTTP(?:/\S+)?\s+[45]\d\d|status\s+[45]\d\d|response\s+[45]\d\d)", | ||
| text, | ||
| re.IGNORECASE, | ||
| ): |
There was a problem hiding this comment.
[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.
|
Closing with credit — the motivating bug (#70908, bare 401/403 tokens in no_agent script stdout tripping a provider-auth alert) is fixed on main via PR #85536: no_agent jobs are now excluded from ALL provider-shaped classification (rate-limit, timeout, auth), which covers your case. Your HTTP-status-context refinement for agent-mode jobs is a reasonable further hardening — if you'd like to rebase just that half onto current main as a small follow-up, we'd review it. Thanks! |
What
_summarize_cron_failure_for_delivery()previously matched bare401/403anywhere in the error text via\b(401|403)\b. This caused misleading "provider authentication error" notifications when:no_agent=Truecron job's script exits non-zero (no provider involved at all)401or403in any context (e.g., test output like "returns 401 on invalid input")Fix
Two layered guards:
no_agentcheck — Skip the entire auth-detection block whenjob.get("no_agent")is truthy. Ano_agentjob has no LLM provider involved, so a provider auth error is impossible regardless of what the script printed to stdout.Tighter HTTP context regex — The 401/403 status code detection now requires an HTTP context prefix (
HTTP/,status, orresponse) instead of matching bare numbers anywhere. This prevents false matches from digits appearing coincidentally in test output or other script stdout.Testing
Added 14 tests covering:
HTTP/1.1 401,HTTP/2 403,status 401,status 403,response 401-> all match auth error