fix(creds): never auto is_active=0 on unpaid/ban flaps

AGENTS.md

@@ -1,5 +1,23 @@
 # OmniRoute agent guide
 
+
+## Testing (HARD)
+Sources: https://x.com/anshnanda/status/2101627891721371971 · https://x.com/nimsbh_ai/status/2102083469362790401 · https://x.com/imrobertjames/status/2100787901701456057
+
+## Credentials / account health (HARD)
+- Health flaps and temporary unpaid/billing lapses are **alerts only** — never auto-set `is_active=0`.
+- `credits_exhausted` / unpaid is **not terminal forever**: keep `is_active=1`, record informative `testStatus` for selection skip + alerts, and let connection recovery / re-probe clear it after billing renew so influencers pick the account up without an OmniRoute UI re-enable.
+- Permanent ban deactivation (`is_active=0`) is opt-in only via `autoDisableBannedAccounts` (+ scope). Do not ungated-flip `isActive` in chatCore / OAuth refresh death / `writeTerminalStatus` defaults.
+- The credential health scheduler must remain non-deactivating (refresh / observe only).
+
+- NEVER write unit tests after you write code.
+- Highly prefer E2E tests as the sole testing mechanism. Use them to verify complex features work. At the end of E2E tests, produce a verifiable and repeatable artifact.
+- If you must test a system in isolation, FIRST write down all the ways it could fail, THEN write the code.
+- When writing E2E tests, do not pick the simplest possible scenario to prove it works — pick a medium-to-hard scenario (models love to cheat).
+- Tautological tests considered harmful.
+- Change-detector tests considered harmful.
+- Do not create regression tests for bug fixes without a genuine gap in behavior testing.
+
 > **Single source of truth.** This file holds ALL project rules, conventions, architecture notes
 > and Hard Rules for every AI assistant working this repository (Claude Code, Gemini, Codex,
 > Copilot, and any other agent). `CLAUDE.md` and `GEMINI.md` only add assistant-specific deltas
@@ -207,9 +225,13 @@ baseCooldownMs * 2 ** failureIndex;
 The anti-thundering-herd guard prevents concurrent failures on the same connection from
 repeatedly extending the cooldown or double-incrementing `backoffLevel`.
 
-Terminal states are not cooldowns. `banned`, `expired` (which becomes terminal only after N bounded retries via `EXPIRED_RETRY_MAX`), and `credits_exhausted` are
-intended to stay unavailable until credentials/settings change or an operator resets
-them. Do not overwrite terminal states with transient cooldown state.
+Terminal states are not cooldowns. `banned` and `expired` (which becomes terminal only after N bounded retries via `EXPIRED_RETRY_MAX`) stay unavailable until
+credentials/settings change or an operator resets them — and even then, flipping
+`is_active=0` is opt-in via `autoDisableBannedAccounts` (see Credentials HARD).
+`credits_exhausted` / temporary unpaid is **selection-skip + alert**, not a forever
+lock and **never** an auto `is_active=0`: connection recovery re-probes on a timer so
+unpaid→renew returns the account to rotation without a UI re-enable. Do not overwrite
+true terminal states with transient cooldown state.
 
 ### Model Lockout
 

docs/security/BAN_DETECTION.md

@@ -42,7 +42,10 @@ Two adjacent, **separate** signal tables live in the same file and are _not_ par
 of banned-keyword detection:
 
 - `CREDITS_EXHAUSTED_SIGNALS` — billing/quota depleted (`insufficient_quota`,
-  `credit_balance_too_low`, `payment required`, …) → terminal `credits_exhausted`.
+  `credit_balance_too_low`, `payment required`, …) → `credits_exhausted` testStatus
+  (selection-skip + alert). **Not** an auto `is_active=0`, and **not** forever:
+  `connectionRecovery` re-probes on a timer so unpaid→renew restores the account
+  without an OmniRoute UI re-enable.
 - `OAUTH_INVALID_TOKEN_SIGNALS` — **non-terminal**; a token refresh can recover.
 
 Note: common transient phrases like **`rate limit`** / `429` are handled by the
@@ -55,21 +58,27 @@ upstream error response
   → body stringified + lowercased
   → isAccountDeactivated(body): getMergedBannedSignals().some(sig => body.includes(sig))   [substring match]
   → match?
-      → connection testStatus = "banned"      (permanent — 1-year cooldown, never auto-recovers)
+      → connection testStatus = "banned"      (selection-skip / alerts; never auto-recovers by itself)
       → if setting `autoDisableBannedAccounts` is on and `autoDisableBannedScope`
         includes this connection (`all`, or `subscription` for OAuth/cookie/session)
         → also isActive = false. Prepaid API keys stay active when scope is
-        `subscription`.
+        `subscription`. When the setting is off (default posture for temporary
+        unpaid/ban-looking flaps), is_active stays 1 — testStatus alone keeps the
+        account out of rotation until an operator re-tests or credentials change.
       → connection is skipped during account selection (combo QUOTA_BLOCKING statuses)
 ```
 
 - The match is a **case-insensitive substring** search on the response **body**
   (`isAccountDeactivated`, `accountFallback.ts`).
 - The permanent `banned` terminalization fires on a banned-signal body at **any
   HTTP status** (via `markAccountUnavailable` → `checkFallbackError`). The
-  narrower **`deactivated`** label (`isActive=false` when the connection has no
-  spare API keys) is written by the inline `chatCore.ts` path on **HTTP 401 / 403**
-  (classified via `classifyProviderError` → `ACCOUNT_DEACTIVATED`). Note the
+  narrower **`deactivated`** / **`banned`** labels are written by the inline
+  `chatCore.ts` path on **HTTP 401 / 403** (classified via `classifyProviderError`
+  → `ACCOUNT_DEACTIVATED` / `FORBIDDEN`). Those paths record `testStatus` for
+  selection skip + alerts and only flip `isActive=false` when
+  `autoDisableBannedAccounts` (+ scope) allows it — same gate as
+  `maybeAutoDisableBannedAccount` in `auth.ts`. Ungated OAuth-refresh death also
+  keeps `is_active=1` and only sets `testStatus=expired`. Note the
   `markAccountUnavailable()` path writes a _different_ terminal status —
   **`expired`** — for the same `ACCOUNT_DEACTIVATED` signal (via
   `resolveTerminalConnectionStatus`), so the same ban can surface as either
@@ -119,9 +128,10 @@ doubt, watch the connection's `lastError` first, then add the exact wording.
 
 ## Recovering a flagged connection
 
-Terminal `banned` / `deactivated` states **never auto-recover** (they are excluded
-from the proactive-recovery tick — only `unavailable` cooldowns recover on their
-own). An operator must clear them explicitly:
+Terminal `banned` / `deactivated` / `expired` states **never auto-recover** (they
+are excluded from the proactive-recovery tick — only `unavailable` cooldowns and
+`credits_exhausted` re-probes recover on their own). An operator must clear true
+bans explicitly:
 
 1. **Re-test the connection** — the dashboard **Test** action
    (`POST /api/providers/{id}/test`); a successful probe resets `testStatus` to

open-sse/handlers/chatCore.ts

@@ -420,6 +420,7 @@ import { generateRequestId } from "@/shared/utils/requestId";
 import { isLocalStreamLifecycleError } from "@/shared/utils/circuitBreaker";
 import { shouldIsolateProbeFailures } from "@/shared/utils/probeOrigin";
 import { writeTerminalStatus } from "@/shared/utils/terminalStatus";
+import { maybeAutoDisableBannedAccount } from "@/sse/services/autoDisableBannedAccount";
 import { extractFacts } from "@/lib/memory/extraction";
 import { handleToolCallExecution } from "@/lib/skills/interception";
 import { MEMORY_BUILTIN_TOOL_NAMES } from "@/lib/skills/memoryBuiltins";
@@ -3808,11 +3809,13 @@ async function handleChatCoreInner({
       try {
         if (errorType === PROVIDER_ERROR_TYPES.FORBIDDEN) {
           const probeIsolated = await shouldIsolateProbeFailures();
+          // HARD: record terminal testStatus for selection skip / alerts, but do
+          // NOT ungated-flip isActive. Permanent deactivation is opt-in via
+          // autoDisableBannedAccounts (same gate as auth.ts).
           await writeTerminalStatus(
             errorConnectionId,
             {
               testStatus: "banned",
-              isActive: false,
               lastError: persistentMessage,
               lastErrorType: errorType,
               errorCode: String(statusCode),
@@ -3824,8 +3827,15 @@ async function handleChatCoreInner({
               `[provider] Node ${errorConnectionId} probe ${errorType} (${statusCode}) -- connection stays active`
             );
           } else {
+            await maybeAutoDisableBannedAccount({
+              connectionId: errorConnectionId,
+              provider,
+              authType: (credentials as { authType?: string | null } | null | undefined)?.authType,
+              connectionProvider: (credentials as { provider?: string | null } | null | undefined)?.provider,
+              permanent: true,
+            });
             console.warn(
-              `[provider] Node ${errorConnectionId} banned (${statusCode}) -- disabling permanently`
+              `[provider] Node ${errorConnectionId} banned (${statusCode}) -- testStatus=banned; isActive gated by autoDisableBannedAccounts`
             );
           }
         } else if (errorType === PROVIDER_ERROR_TYPES.ACCOUNT_DEACTIVATED) {
@@ -3846,11 +3856,12 @@ async function handleChatCoreInner({
             );
           } else {
             const probeIsolated2 = await shouldIsolateProbeFailures();
+            // HARD: stay is_active=1 through temporary unpaid/ban-looking flaps
+            // unless autoDisableBannedAccounts opts into permanent deactivation.
             await writeTerminalStatus(
               errorConnectionId,
               {
                 testStatus: "deactivated",
-                isActive: false,
                 lastError: persistentMessage,
                 lastErrorType: errorType,
                 errorCode: String(statusCode),
@@ -3862,8 +3873,15 @@ async function handleChatCoreInner({
                 `[provider] Node ${errorConnectionId} probe ${errorType} (${statusCode}) -- connection stays active`
               );
             } else {
+              await maybeAutoDisableBannedAccount({
+                connectionId: errorConnectionId,
+                provider,
+                authType: (credentials as { authType?: string | null } | null | undefined)?.authType,
+                connectionProvider: (credentials as { provider?: string | null } | null | undefined)?.provider,
+                permanent: true,
+              });
               console.warn(
-                `[provider] Node ${errorConnectionId} account deactivated (${statusCode}) -- disabling permanently`
+                `[provider] Node ${errorConnectionId} account deactivated (${statusCode}) -- testStatus=deactivated; isActive gated by autoDisableBannedAccounts`
               );
             }
           }
@@ -4577,11 +4595,11 @@ async function handleChatCoreInner({
       } else {
         log?.warn?.("TOKEN", `${provider?.toUpperCase()} | refresh failed`);
         if (isUnrecoverableRefreshError(newCredentials) && onCredentialsRefreshed) {
-          // Front 3 (reuse-race tolerance): before deactivating, re-read the DB.
+          // Front 3 (reuse-race tolerance): before marking expired, re-read the DB.
           // If a sibling/concurrent refresh already rotated this connection's
           // refresh_token (common for Codex/OpenAI under one shared Auth0 client),
           // the failure we saw was a stale-token reuse — the account is healthy
-          // with the newer token, so keep it active instead of killing it.
+          // with the newer token, so keep it active instead of marking expired.
           let alreadyRotated = false;
           if (typeof connectionId === "string" && connectionId && attemptedRefreshToken) {
             try {
@@ -4598,7 +4616,11 @@ async function handleChatCoreInner({
             }
           }
           if (!alreadyRotated) {
-            await onCredentialsRefreshed({ testStatus: "expired", isActive: false });
+            // HARD: OAuth refresh death is informative (alerts / selection skip via
+            // testStatus=expired), not an auto is_active=0. Access can recover after
+            // re-auth / token rotation without requiring a UI re-enable. Permanent
+            // deactivation stays behind autoDisableBannedAccounts on true bans.
+            await onCredentialsRefreshed({ testStatus: "expired" });
           }
         }
       }

src/shared/utils/terminalStatus.ts

@@ -34,12 +34,20 @@ export async function writeTerminalStatus(
     });
     return;
   }
-  await updateProviderConnection(connectionId, {
-    isActive: patch.isActive ?? (isTerminal ? false : undefined),
+  // HARD: never auto-flip isActive on health flaps / temporary unpaid /
+  // ban-looking errors. Callers that truly need deactivation must pass
+  // isActive explicitly AND gate it behind autoDisableBannedAccounts
+  // (see maybeAutoDisableBannedAccount). credits_exhausted in particular
+  // must stay is_active=1 so unpaid→renew recovers without a UI re-enable.
+  const update: Record<string, unknown> = {
     testStatus: patch.testStatus,
     lastError: persistedLastError,
     lastErrorAt: new Date().toISOString(),
     lastErrorType: patch.lastErrorType ?? null,
     errorCode: patch.errorCode ?? null,
-  });
+  };
+  if (typeof patch.isActive === "boolean") {
+    update.isActive = patch.isActive;
+  }
+  await updateProviderConnection(connectionId, update);
 }