fix(devin): map Connect trailer codes onto the status core acts on

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.

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 };
 }

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<C
     // error event and /api/logs, and a Connect error can quote the request that
     // produced it - which is the request holding the api_key.
     //
-    // Only the HTTP status line is carried here. A Connect EOS trailer that
-    // reports resource_exhausted or unavailable still arrives without a status,
-    // so a cap delivered that way keeps the older message-inference path.
-    // Mapping trailer codes onto HTTP statuses is deliberately a follow-up.
+    // The status line is carried on the error. A cap or an expired credential
+    // delivered instead as a Connect EOS trailer is mapped by
+    // connectTrailerHttpStatus at the trailer sites below.
     throw new CloudChatError(`GetChatMessage failed (HTTP ${resp.status})`, undefined, undefined, resp.status);
   }
   if (!resp.body) {
@@ -1316,7 +1354,12 @@ export async function* streamChatEvents(req: CloudChatRequest): AsyncGenerator<C
         `service accepts. If the request is unchanged and this is new, the ` +
         `account's model access is the next thing to check. ` +
         `(cloud trace ID: ${trailerError.traceId ?? 'n/a'})`;
-      throw new CloudChatError(enriched, trailerError.code, trailerError.traceId);
+      throw new CloudChatError(
+        enriched,
+        trailerError.code,
+        trailerError.traceId,
+        connectTrailerHttpStatus(trailerError.code, trailerError.message),
+      );
     }
     // Cognition also returns `permission_denied` when a tool description
     // contains a blocklisted phrase that the sanitizer above did not catch
@@ -1336,9 +1379,19 @@ export async function* streamChatEvents(req: CloudChatRequest): AsyncGenerator<C
         // was a phrase match at all.
         `(cloud message: ${trailerError.message}) ` +
         `(cloud trace ID: ${trailerError.traceId ?? 'n/a'})`;
-      throw new CloudChatError(enriched, trailerError.code, trailerError.traceId);
+      throw new CloudChatError(
+        enriched,
+        trailerError.code,
+        trailerError.traceId,
+        connectTrailerHttpStatus(trailerError.code, trailerError.message),
+      );
     }
-    throw new CloudChatError(trailerError.message, trailerError.code, trailerError.traceId);
+    throw new CloudChatError(
+      trailerError.message,
+      trailerError.code,
+      trailerError.traceId,
+      connectTrailerHttpStatus(trailerError.code, trailerError.message),
+    );
   }
   // Truncation detection: the cloud always terminates a successful stream
   // with an EOS trailer. If we hit `done` from the body reader without one,

tests/providers/devin-hardening.test.ts

@@ -7,6 +7,7 @@ import { anySignal } from "../../src/lib/abort";
 import { buildGetChatMessageRequestForTests } from "../../src/adapters/devin/cloud-direct/chat";
 import { decodeModelUsageStats } from "../../src/adapters/devin/cloud-direct/chat";
 import { CloudChatError, decodeChatFrame } from "../../src/adapters/devin/cloud-direct/chat";
+import { connectTrailerHttpStatus } from "../../src/adapters/devin/cloud-direct/chat";
 import { devinErrorClassification, mergeDevinUsage } from "../../src/adapters/devin";
 import { iterFields } from "../../src/adapters/devin/cloud-direct/wire";
 import { buildMetadata, normalizeDevinSessionToken } from "../../src/adapters/devin/cloud-direct/metadata";
@@ -398,3 +399,48 @@ describe("devin usage merging and error classification", () => {
     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 });
+  });
+});