fix(devin): bind tenant hosts to the transmitted key and sanitize Connect trailers

scripts/test-layout/layout.json

@@ -735,6 +735,7 @@
     "desktop-start-at-login-default.test.ts": "clients",
     "destination-policy-resolved.test.ts": "routing",
     "devin-adapter.test.ts": "providers",
+    "devin-adapter-reset-wait.test.ts": "providers",
     "devin-cli-authmode-migration.test.ts": "providers",
     "devin-effort-ladder.test.ts": "providers",
     "devin-live-models.test.ts": "providers",

src/adapters/devin.ts

@@ -612,7 +612,7 @@ export function createDevinAdapter(
       // The signed-in account's tenant decides the host, not the static registry
       // entry: an EU or FedStart account that used provider.baseUrl would send
       // every RPC to the US server it is not provisioned on.
-      const host = resolveDevinApiServer(provider.baseUrl, credentialProviderId);
+      const host = resolveDevinApiServer(provider.baseUrl, credentialProviderId, apiKey);
       // One catalog read per turn serves model-UID resolution, the input
       // ceiling, and the chat pre-flight inside streamChatEvents. Failures are
       // not cached, so a second read would only pay another fetch timeout on
@@ -642,10 +642,11 @@ export function createDevinAdapter(
         const maxOutputTokens = resolveDevinMaxOutputTokens(
           provider, modelUid, parsed.options.maxOutputTokens,
         );
-        // The reset-retry wrapper waits out a 429 that states its own recovery
-        // delay ("limit will reset in 35 seconds") and replays the identical
-        // request — but only while zero events have been yielded, so a
-        // post-output failure still takes the terminal path untouched.
+        // An admitted HTTP turn owns globally shared capacity until this call
+        // emits. Never retain that capacity while waiting out a provider 429:
+        // preserve the typed reset delay in generated diagnostic wording,
+        // never the raw trailer text that may reflect a credential. The
+        // refusal returns immediately so the caller can release its slot.
         for await (const event of streamChatEventsWithResetRetry({
           apiKey,
           apiServerUrl: host,
@@ -664,6 +665,7 @@ export function createDevinAdapter(
           },
           signal: incoming.abortSignal,
         }, {
+          maxWaitMs: 0,
           execution: {
             executor: incoming.providerFetch,
             sendBudget: incoming.sendBudget,

src/adapters/devin/cloud-direct/chat.ts

@@ -37,6 +37,7 @@ import { buildMetadata } from './metadata.js';
 import { getCachedUserJwt } from './auth.js';
 import { getCachedCatalog, ModelNotAvailableError, type CacheEntry } from './catalog.js';
 import { anySignal, cancelBodyOnAbort } from '../../../lib/abort.js';
+import { parseRetryAfterFromMessage } from '../../../lib/retry-delay.js';
 import { resolveDevinApiBaseUrl } from '../../../oauth/devin/api-base.js';
 
 /**
@@ -1071,13 +1072,29 @@ export class CloudChatError extends Error {
      * a live rate limit was classified 502 and core's failover never rotated.
      */
     public readonly status?: number,
+    /**
+     * Provider-stated recovery delay in seconds. The trailer parser extracts
+     * it while the raw upstream text is still available, because the thrown
+     * message is content-free: the stated-reset retry reads this typed field
+     * instead of scraping untrusted trailer text out of error.message.
+     */
+    public readonly retryAfterSeconds?: number,
   ) {
     super(message);
     this.name = 'CloudChatError';
   }
 }
 
 const TRACE_ID_RE = /\(trace ID: ([0-9a-f]+)\)/i;
+const SAFE_CONNECT_CODES = new Set([
+  'canceled', 'unknown', 'invalid_argument', 'deadline_exceeded', 'not_found', 'already_exists',
+  'permission_denied', 'resource_exhausted', 'failed_precondition', 'aborted', 'out_of_range',
+  'unimplemented', 'internal', 'unavailable', 'data_loss', 'unauthenticated',
+]);
+
+function safeConnectCode(value: unknown): string | undefined {
+  return typeof value === 'string' && SAFE_CONNECT_CODES.has(value) ? value : undefined;
+}
 
 /**
  * A quota refusal Cognition delivers as `permission_denied`.
@@ -1296,7 +1313,7 @@ export async function* streamChatEvents(req: CloudChatRequest): AsyncGenerator<C
   // Bun + Node ReadableStream readers diverge on the type-level shape
   // (Bun's includes a `readMany` method); both work the same at runtime.
   const reader = resp.body.getReader() as ReadableStreamDefaultReader<Uint8Array>;
-  let trailerError: { code?: string; message: string; traceId?: string } | null = null;
+  let trailerError: { code?: string; message: string; opaqueDenial: boolean; retryAfterSeconds?: number; traceId?: string } | null = null;
   let sawEos = false;
 
   /**
@@ -1441,14 +1458,23 @@ export async function* streamChatEvents(req: CloudChatRequest): AsyncGenerator<C
           const text = payload.toString('utf8');
           if (text && text.includes('"error"')) {
             let code: string | undefined;
-            let message = text;
+            let message = '';
             try {
               const j = JSON.parse(text) as { error?: { code?: string; message?: string } };
-              code = j.error?.code;
-              if (j.error?.message) message = j.error.message;
-            } catch { /* keep raw */ }
+              code = safeConnectCode(j.error?.code);
+              if (typeof j.error?.message === 'string') message = j.error.message;
+            } catch { /* malformed trailers still become a content-free error */ }
             const traceMatch = message.match(TRACE_ID_RE);
-            trailerError = { code, message, traceId: traceMatch?.[1] };
+            trailerError = {
+              code,
+              message,
+              opaqueDenial: /an internal error occurred/i.test(message),
+              // Parsed here, where the raw text still exists: the thrown error
+              // is content-free, so the stated-reset retry consumes this typed
+              // field rather than the upstream sentence.
+              retryAfterSeconds: parseRetryAfterFromMessage(message),
+              traceId: traceMatch?.[1],
+            };
           }
           continue;
         }
@@ -1477,16 +1503,16 @@ export async function* streamChatEvents(req: CloudChatRequest): AsyncGenerator<C
     // the catalog disagrees with the call, but the catalog can lag (a model
     // that was enabled at fetch time may have been gated between then and
     // now) or be missing (network failure caused a fall-through). When the
-    // raw trailer is this exact shape, swap in a message that names the
+    // trailer is this exact shape, swap in a message that names the
     // model and explains the likely cause rather than re-passing
-    // Cognition's opaque text. The cloud's original message is appended in
-    // parens so users (and bug reports) still have it verbatim.
+    // Cognition's opaque text. Raw trailer messages never leave this
+    // parser: they can reflect the credential carried by the request.
     // Both codes carry this shape. Cognition uses `invalid_argument` for a
     // request it could not accept and `permission_denied` for one it would not,
     // and the message body is the same opaque sentence either way.
     const isOpaqueDenial =
       (trailerError.code === 'permission_denied' || trailerError.code === 'invalid_argument') &&
-      /an internal error occurred/i.test(trailerError.message);
+      trailerError.opaqueDenial;
     if (isOpaqueDenial) {
       const enriched =
         `Cognition denied this request for model "${req.modelUid}" with the opaque ` +
@@ -1504,6 +1530,7 @@ export async function* streamChatEvents(req: CloudChatRequest): AsyncGenerator<C
         trailerError.code,
         trailerError.traceId,
         connectTrailerHttpStatus(trailerError.code, trailerError.message),
+        trailerError.retryAfterSeconds,
       );
     }
     // Cognition also returns `permission_denied` when a tool description
@@ -1518,24 +1545,31 @@ export async function* streamChatEvents(req: CloudChatRequest): AsyncGenerator<C
         `Cognition denied this request (permission_denied). If tool descriptions ` +
         `are present, a blocklisted phrase may have triggered this — see the ` +
         `COGNITION_BLOCKLIST_REWRITES table in cloud-direct/chat.ts. ` +
-        // Keep the cloud's own sentence. Replacing it outright is what made the
-        // two Codex entries in that table expensive to find: the message named
-        // the table but dropped the only text that could have said whether this
-        // 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,
         connectTrailerHttpStatus(trailerError.code, trailerError.message),
+        trailerError.retryAfterSeconds,
       );
     }
+    // Raw trailer messages never leave this parser: they can reflect the
+    // credential carried by the request. The allowlisted code and the hex
+    // trace id are the only upstream-controlled fields that reach the error.
+    // The stated delay rides along in our own words so a client can still
+    // tell how long to wait when local retry gives up, exceeds its cap, or
+    // is disabled; the `~` keeps it from re-parsing as a downstream hint.
     throw new CloudChatError(
-      trailerError.message,
+      `Cognition chat failed${trailerError.code ? ` (${trailerError.code})` : ''} ` +
+      `(cloud trace ID: ${trailerError.traceId ?? 'n/a'})` +
+      (trailerError.retryAfterSeconds === undefined
+        ? ''
+        : `; retry after ~${trailerError.retryAfterSeconds}s`),
       trailerError.code,
       trailerError.traceId,
       connectTrailerHttpStatus(trailerError.code, trailerError.message),
+      trailerError.retryAfterSeconds,
     );
   }
   // Truncation detection: the cloud always terminates a successful stream

src/adapters/devin/cloud-direct/stated-reset-retry.ts

@@ -112,8 +112,11 @@ export async function* streamChatEventsWithResetRetry(
         && error.status === 429
         ? error
         : undefined;
+      // Prefer the typed field: trailer-derived errors are content-free now,
+      // so the parser preserved the stated delay there. The message scrape
+      // remains for errors whose text still carries a hint.
       const waitSec = retryableError
-        ? parseRetryAfterFromMessage(retryableError.message)
+        ? retryableError.retryAfterSeconds ?? parseRetryAfterFromMessage(retryableError.message)
         : undefined;
       const waitMs = waitSec === undefined ? undefined : waitSec * 1000;
       if (

src/oauth/devin.ts

@@ -19,7 +19,7 @@ import { DEFAULT_REGION, type WindsurfRegion } from "./devin/types";
 import { registerUser } from "./devin/register-user";
 import { DEVIN_DEFAULT_API_SERVER, resolveDevinApiBaseUrl, validateDevinApiBaseUrl } from "./devin/api-base";
 import { readDevinCliCredentialOutcome } from "./devin/cli-import";
-import { getCredential } from "./store";
+import { getCredential, listAccounts } from "./store";
 import { DEPRECATED_OAUTH_PROVIDER_ALIASES } from "./index";
 
 export { DEVIN_DEFAULT_API_SERVER } from "./devin/api-base";
@@ -50,46 +50,52 @@ function devinAliasCredentialSlots(providerId: string): string[] {
  * configured provider baseUrl is the fallback, and the US default is the last
  * resort; both are re-validated because neither is trusted more than the
  * network value.
+ *
+ * A stored tenant host is used only for the account that owns `apiKey`, the key
+ * this request will transmit. Devin keeps several accounts per provider id and
+ * the request path injects the admitted account's token, which need not be the
+ * active one, while a provider-configured key, a forwarded bearer, or a test
+ * token is not stored at all. Reading the active slot's host for any of those
+ * would send one account's key to another account's EU or FedStart tenant.
  */
-export function resolveDevinApiServer(configuredBaseUrl?: string, providerId = "devin"): string {
-  // Provider-scoped, keyed by the configured provider id verbatim and consulted
-  // FIRST. `devin-cli` is a deprecated alias for `devin`, but an unmigrated
-  // config row still owns its old credential slot until the startup migration
-  // rekeys the row and the slot together — normalizing the id here would read
-  // the wrong slot for that window. An EU or FedStart tenant is recorded on the
-  // credential rather than in the registry, so a fixed "devin" slot would send
-  // the key to the wrong host either way.
-  const literalCredential = getCredential(providerId);
-  const literal = validateDevinApiBaseUrl(literalCredential?.apiBaseUrl);
-  if (literal !== undefined) return literal;
+export function resolveDevinApiServer(configuredBaseUrl?: string, providerId = "devin", apiKey?: string): string {
+  const owner = apiKey ? findDevinCredentialOwner(providerId, apiKey) : undefined;
+  if (owner !== undefined) {
+    // The owning account decides. An owner whose recorded host is missing or
+    // off-allowlist falls through to the configured base URL rather than
+    // borrowing another slot's tenant: the same key stored twice is the rekey
+    // window, and a second slot's host is not more trustworthy than this one.
+    const host = validateDevinApiBaseUrl(owner.apiBaseUrl);
+    if (host !== undefined) return host;
+  }
+  return validateDevinApiBaseUrl(configuredBaseUrl) ?? DEVIN_DEFAULT_API_SERVER;
+}
 
-  // The startup merge saves providers["devin"] synchronously but fires the
-  // credential rekey detached — runDevinProviderMergeStartupMigration cannot
-  // await inside the synchronous startServer window — so the row can already
-  // say "devin" while the credential still sits in the "devin-cli" slot, and it
-  // stays that way for the whole process when the rekey fails or refuses on an
-  // occupied destination slot. Reading the alias-linked slots in both
-  // directions closes that window: "devin" finds the not-yet-rekeyed
-  // "devin-cli" credential, and a lingering "devin-cli" row finds a credential
-  // already rekeyed to "devin". Every candidate passes the same allowlist — an
-  // alias slot is not trusted more than the literal one.
-  // Only when this id owns no credential at all. A present credential whose
-  // apiBaseUrl is missing or off-allowlist is a different situation: the rekey
-  // refuses an occupied destination slot, so both ids can hold credentials that
-  // belong to two different accounts. Borrowing a tenant across that pair would
-  // send this account's key to the other account's EU or FedStart host, which
-  // is the exact misdirection the provider-scoped lookup exists to prevent. An
-  // unusable host on a credential that does exist falls through to the
-  // configured base URL and then the default, as it did before this window was
-  // closed.
-  if (literalCredential === null || literalCredential === undefined) {
-    for (const slot of devinAliasCredentialSlots(providerId)) {
-      const host = validateDevinApiBaseUrl(getCredential(slot)?.apiBaseUrl);
-      if (host !== undefined) return host;
+/**
+ * The stored credential whose access token is exactly `apiKey`.
+ *
+ * The configured provider id is searched first and verbatim: `devin-cli` is a
+ * deprecated alias for `devin`, but an unmigrated config row still owns its old
+ * slot until the startup migration rekeys the row and the slot together. The
+ * startup merge saves providers["devin"] synchronously but fires the credential
+ * rekey detached (runDevinProviderMergeStartupMigration cannot await inside the
+ * synchronous startServer window), so the alias-linked slots are searched next,
+ * in both directions. Within a slot the active account is read first, then the
+ * rest, because the admitted account can be any of them.
+ *
+ * A key refreshed between token resolution and this lookup would match nothing
+ * and use the configured host for that turn. Devin keys are long-lived
+ * RegisterUser API keys, so this window is not a routine refresh race.
+ */
+function findDevinCredentialOwner(providerId: string, apiKey: string): OAuthCredentials | undefined {
+  for (const slot of [providerId, ...devinAliasCredentialSlots(providerId)]) {
+    const active = getCredential(slot);
+    if (active?.access === apiKey) return active;
+    for (const account of listAccounts(slot)) {
+      if (account.credential.access === apiKey) return account.credential;
     }
   }
-
-  return validateDevinApiBaseUrl(configuredBaseUrl) ?? DEVIN_DEFAULT_API_SERVER;
+  return undefined;
 }
 
 function decodeJwtPayload(token: string): Record<string, unknown> | undefined {

structure/adapters/registry.md

@@ -40,8 +40,8 @@ Some adapters share another adapter's routed-tool semantics while retaining inde
   does not. `devin-cli` survives only as a deprecated alias — `ocx login devin-cli` routes to
   `devin`, and a startup merge migration rewrites any saved row still keyed under the old
   provider id, so the registry carries one Devin provider, not two.
-  Its `GetChatMessage` inference POSTs, including the two bounded pre-output stated-reset
-  replays, pass through the request's provider executor and shared physical-send budget.
+  Its `GetChatMessage` inference POSTs pass through the provider executor and shared send budget.
+  The adapter allows no reset wait; the shared helper retains bounded replays for opted-in callers.
   Catalog and JWT RPCs remain adapter support traffic rather than inference sends.
   `AdapterFactoryContext.providerId` still tells the shared adapter which configured row it is
   serving: the Cognition tenant is recorded on the credential, not in the registry, so the
@@ -92,7 +92,9 @@ Some adapters share another adapter's routed-tool semantics while retaining inde
   hints, so `CompletionConfiguration` field #3 no longer serializes the encoder's
   128000 fallback. Smaller operator hints cap live evidence and never enlarge it;
   with no evidence the adapter hint is omitted and the encoder still serializes
-  its own 128000 fallback for field #3. Investigation and limits:
+  its own 128000 fallback for field #3. Connect trailer diagnostics expose only an
+  allowlisted error code, hexadecimal trace id and typed `retryAfterSeconds`, optionally rendered as
+  generated `retry after ~Ns` wording; raw text stays internal because it can reflect credentials. Investigation and limits:
   `devlog/_plan/260917_devin_input_ceiling/000_review.md`.
 
 The registry records those relationships with `contractParent`. A parent relationship does **not** mean the registry recursively constructs a parent adapter and injects it into the child. Azure and MiMo keep owning their existing internal composition. This avoids making production constructors depend on test/conformance needs and keeps this authority refactor behavior-neutral.