diff --git a/server/pkg/taskfailure/classify.go b/server/pkg/taskfailure/classify.go index e073a01256..86dbda1335 100644 --- a/server/pkg/taskfailure/classify.go +++ b/server/pkg/taskfailure/classify.go @@ -16,6 +16,23 @@ import ( // startup rather than per-call matters. var providerHTTP5xxRe = regexp.MustCompile(`(^|[^0-9])5[0-9][0-9]([^0-9]|$)`) +// httpAuthCodeRe / httpQuotaCodeRe / httpCapacityCodeRe match specific 3-digit +// HTTP status codes only when they are NOT embedded in a longer number, using +// the same digit-boundary guard as providerHTTP5xxRe. Without this guard the +// bare substrings "401"/"402"/"403"/"429"/"529" fire on unrelated numbers — +// e.g. "402913 tokens", "15290ms", "exit status 4030" — misclassifying process +// or unknown failures as provider billing / rate-limit errors. That pollutes +// failure observability: a genuine process crash gets filed under a provider +// bucket, masking the real cause on failure dashboards. (It does not change +// auto-retry — Classify only ever returns agent_error.* reasons, none of which +// are in internal/service/task.go's retryableReasons set.) The 5xx bucket was +// already anchored for exactly this reason (MUL-1949); these codes were not. +var ( + httpAuthCodeRe = regexp.MustCompile(`(^|[^0-9])(401|403)([^0-9]|$)`) + httpQuotaCodeRe = regexp.MustCompile(`(^|[^0-9])402([^0-9]|$)`) + httpCapacityCodeRe = regexp.MustCompile(`(^|[^0-9])(429|529)([^0-9]|$)`) +) + // Classify maps a free-form error string from the agent runtime / CLI // to one of the 14 agent_error.* sub-reasons. Always returns a valid // Reason; falls back to ReasonAgentUnknown when no rule matches and for @@ -82,50 +99,49 @@ func Classify(rawError string) Reason { return ReasonAgentMissingConfig // 3. Auth / access. 401 / 403 / "Not logged in" / invalid token - // / lacks access to the model. - case containsAny(lower, - "401", - "403", - "unauthorized", - "login required", - "not logged in", - "please login again", - "refresh token", - "invalid api key", - "access token", - "subscription access", - "does not have access", - "you may not have access", - ): + // / lacks access to the model. Status codes use a digit boundary + // so "4030" / "1401ms" don't spuriously land here. + case httpAuthCodeRe.MatchString(lower), + containsAny(lower, + "unauthorized", + "login required", + "not logged in", + "please login again", + "refresh token", + "invalid api key", + "access token", + "subscription access", + "does not have access", + "you may not have access", + ): return ReasonAgentProviderAuthOrAccess // 4. Quota / billing. 402 / insufficient balance / monthly usage // limit / credits exhausted. - case containsAny(lower, - "402", - "insufficient_balance", - "balance is too low", - "monthly usage limit", - "usage limit", - "you've hit your limit", - // Curly apostrophe variant: providers and copy-pasted error - // strings sometimes use U+2019 instead of ASCII '. SQL ILIKE - // would not match the curly form either, so this is a small - // in-flight improvement on top of the SQL classifier. - "you\u2019ve hit your limit", - "credits", - "quota", - ): + case httpQuotaCodeRe.MatchString(lower), + containsAny(lower, + "insufficient_balance", + "balance is too low", + "monthly usage limit", + "usage limit", + "you've hit your limit", + // Curly apostrophe variant: providers and copy-pasted error + // strings sometimes use U+2019 instead of ASCII '. SQL ILIKE + // would not match the curly form either, so this is a small + // in-flight improvement on top of the SQL classifier. + "you\u2019ve hit your limit", + "credits", + "quota", + ): return ReasonAgentProviderQuotaLimit // 5. Capacity / rate limit. 429 / 529 / overloaded / rate limit. - case containsAny(lower, - "429", - "rate limit", - "overloaded", - "529", - "no capacity available", - ): + case httpCapacityCodeRe.MatchString(lower), + containsAny(lower, + "rate limit", + "overloaded", + "no capacity available", + ): return ReasonAgentProviderCapacityOrRateLimit // 6. Provider 5xx / server error. The 5xx regex is checked here diff --git a/server/pkg/taskfailure/classify_test.go b/server/pkg/taskfailure/classify_test.go index fc1c98e862..181aea14b3 100644 --- a/server/pkg/taskfailure/classify_test.go +++ b/server/pkg/taskfailure/classify_test.go @@ -136,6 +136,21 @@ func TestClassifyRules(t *testing.T) { // 14. Catchall. {"unrecognized", "the agent gave up for reasons unknown", ReasonAgentUnknown}, {"sentence with no marker", "Hello world.", ReasonAgentUnknown}, + + // 15. Digit-boundary regression: 3-digit HTTP status codes must NOT + // match when embedded in a longer number. Before the fix these + // landed in provider auth/quota/capacity buckets, masking hard + // process failures under a provider reason and polluting failure + // observability. + {"402 embedded not quota", "agent consumed 402913 tokens before crashing", ReasonAgentUnknown}, + {"529 embedded not capacity", "request latency was 15290ms; then it panicked: signal killed", ReasonAgentProcessFailure}, + {"403 embedded not auth", "processed 4030 items, then exit status 1", ReasonAgentProcessFailure}, + {"401 embedded not auth", "job 24019 finished, process exited with status 2", ReasonAgentProcessFailure}, + {"429 embedded not capacity", "seq 14290 unknown outcome", ReasonAgentUnknown}, + // Genuine status codes with a boundary still classify correctly. + {"402 boundary still quota", "API Error: 402 Payment Required", ReasonAgentProviderQuotaLimit}, + {"403 boundary still auth", "HTTP 403 Forbidden", ReasonAgentProviderAuthOrAccess}, + {"429 boundary still capacity", "got 429 from provider", ReasonAgentProviderCapacityOrRateLimit}, } for _, c := range cases {