diff --git a/devlog/_plan/260912_devin_hardening/020_cloud_direct_hardening.md b/devlog/_plan/260912_devin_hardening/020_cloud_direct_hardening.md index 726c5e181e..873b08eb3d 100644 --- a/devlog/_plan/260912_devin_hardening/020_cloud_direct_hardening.md +++ b/devlog/_plan/260912_devin_hardening/020_cloud_direct_hardening.md @@ -76,3 +76,18 @@ already knows, with status 499. ## Verification bun test tests/providers/devin-adapter.test.ts tests/providers/devin-hardening.test.ts + +## 5. A Connect trailer carries no status — closed + +Landed in `connectTrailerHttpStatus`. The three EOS trailer throw sites now pass a status, +so a cap delivered as `permission_denied` with "your limit will reset" reads as 429 rather +than 403, an `unauthenticated` trailer reaches the auth path, and an unrecognised code still +falls back to message inference. `unimplemented` maps to 501 and is explicitly non-retryable, +because the blanket 5xx rule was telling clients to retry a call the service does not +implement. + +Accepted residuals: `internal`, `unknown` and `data_loss` map to 502 rather than Connect's +500 — both are transient here and 502 is what this adapter already reported — and a genuine +ACL denial whose text happens to contain the words "rate limit" would be read as a cap. The +regex reads the raw trailer message, never the enriched text, so the tool-description +blocklist wrapper cannot trip it. diff --git a/src/adapters/devin.ts b/src/adapters/devin.ts index 5bbd65c480..9260976058 100644 --- a/src/adapters/devin.ts +++ b/src/adapters/devin.ts @@ -56,6 +56,10 @@ export function devinErrorClassification(error: unknown): { status?: number; err if (status === 401) return { status, errorType: "authentication_error", retryable: false }; if (status === 403) return { status, errorType: "permission_error", retryable: false }; if (status === 429) return { status, errorType: "rate_limit_error", retryable: true }; + // 501 is the one 5xx that will never succeed on a second attempt: the service + // does not implement the call. Marking it retryable put `retryable: true` on + // the SSE failure a client reads, inviting a retry that cannot change. + if (status === 501) return { status, retryable: false }; if (status >= 500) return { status, retryable: true }; return { status, retryable: false }; } diff --git a/src/adapters/devin/cloud-direct/chat.ts b/src/adapters/devin/cloud-direct/chat.ts index 2c7b7c32a1..d67a7696e1 100644 --- a/src/adapters/devin/cloud-direct/chat.ts +++ b/src/adapters/devin/cloud-direct/chat.ts @@ -969,6 +969,45 @@ export class CloudChatError extends Error { const TRACE_ID_RE = /\(trace ID: ([0-9a-f]+)\)/i; +/** + * A quota refusal Cognition delivers as `permission_denied`. + * + * "Your limit will reset in 13 minutes" and "Reached overall message rate + * limit" are caps, not authorization failures. Classified as 403 they invite + * the client to retry straight into a live cap; as 429 the proxy backs off and + * can rotate. + */ +const TRAILER_QUOTA_RE = /\b(?:limit will reset|rate limit|quota exceeded|out of credits)\b/i; + +/** + * Connect error code to HTTP status. + * + * Without this only the HTTP status line reached the adapter, so a cap or an + * expired credential delivered as an EOS trailer fell through to + * `inferHttpStatusFromAdapterMessage` and became a generic 502 — which is not + * retryable-with-backoff, not an auth prompt, and not something core's failover + * acts on. + */ +export function connectTrailerHttpStatus(code: string | undefined, message: string): number | undefined { + if (code === 'permission_denied' && TRAILER_QUOTA_RE.test(message)) return 429; + switch (code) { + case 'unauthenticated': return 401; + case 'permission_denied': return 403; + case 'resource_exhausted': return 429; + case 'not_found': return 404; + case 'unavailable': return 503; + case 'deadline_exceeded': return 504; + case 'unimplemented': return 501; + case 'invalid_argument': + case 'failed_precondition': + case 'out_of_range': return 400; + case 'internal': + case 'unknown': + case 'data_loss': return 502; + default: return undefined; + } +} + /** * Stream chat events from the cloud. Yields CloudChatEvent (text deltas, tool * call deltas, finish reason). Use `streamChatText` for legacy text-only iteration. @@ -1085,10 +1124,9 @@ export async function* streamChatEvents(req: CloudChatRequest): AsyncGenerator { expect(devinErrorClassification(new CloudChatError("x", "resource_exhausted"))).toEqual({}); }); }); + +describe("connect trailer to HTTP status", () => { + test("a cap delivered as permission_denied is a 429, not a 403", () => { + // Cognition sends the account cap through the same code as an ACL denial. + // Classified 403 the client retries straight into a live cap. + expect(connectTrailerHttpStatus("permission_denied", "Your limit will reset in 13 minutes")).toBe(429); + expect(connectTrailerHttpStatus("permission_denied", "Reached overall message rate limit")).toBe(429); + // An ordinary denial stays a denial. + expect(connectTrailerHttpStatus("permission_denied", "an internal error occurred")).toBe(403); + }); + + test("the remaining Connect codes map to the status core acts on", () => { + expect(connectTrailerHttpStatus("unauthenticated", "")).toBe(401); + expect(connectTrailerHttpStatus("resource_exhausted", "")).toBe(429); + expect(connectTrailerHttpStatus("unavailable", "")).toBe(503); + expect(connectTrailerHttpStatus("deadline_exceeded", "")).toBe(504); + expect(connectTrailerHttpStatus("invalid_argument", "")).toBe(400); + expect(connectTrailerHttpStatus("internal", "")).toBe(502); + // An unknown code keeps the older message-inference path rather than + // asserting a status nobody measured. + expect(connectTrailerHttpStatus("some_new_code", "")).toBeUndefined(); + expect(connectTrailerHttpStatus(undefined, "")).toBeUndefined(); + }); + + test("a trailer status reaches the adapter's structured classification", () => { + const err = new CloudChatError("capped", "permission_denied", "abc", connectTrailerHttpStatus("permission_denied", "Your limit will reset in 3 minutes")); + expect(devinErrorClassification(err)).toEqual({ status: 429, errorType: "rate_limit_error", retryable: true }); + }); +}); + +describe("devin status classification across the newly reachable trailer codes", () => { + const cls = (status: number) => devinErrorClassification(new CloudChatError("x", undefined, undefined, status)); + + test("a request the service will not accept is never retried", () => { + expect(cls(400)).toEqual({ status: 400, retryable: false }); + expect(cls(404)).toEqual({ status: 404, retryable: false }); + // 501 is the one 5xx a second attempt cannot change. + expect(cls(501)).toEqual({ status: 501, retryable: false }); + }); + + test("a timeout or an unavailable service is retryable", () => { + expect(cls(503)).toEqual({ status: 503, retryable: true }); + expect(cls(504)).toEqual({ status: 504, retryable: true }); + }); +});