Skip to content

fix(auth): never auto is_active=0 on unpaid/ban flaps; recover after renew - #15293

Closed
RaviTharuma wants to merge 3 commits into
cheaper-inference:release/v3.8.51from
RaviTharuma:fix/creds-flap-no-auto-deactivate
Closed

RaviTharuma wants to merge 3 commits into
cheaper-inference:release/v3.8.51from
RaviTharuma:fix/creds-flap-no-auto-deactivate

Conversation

@RaviTharuma

Copy link
Copy Markdown
Contributor

Summary

Reopens only the credentials-flap behavior fix from #14854 (closed), without the AGENTS.md edits and with a failing-then-passing regression test per Hard Rules #8 / #18.

Stops OmniRoute from permanently sidelining accounts on temporary unpaid / ban-looking flaps so operators can pick them back up after billing renew / re-auth without a dashboard re-enable.

  • writeTerminalStatus: no longer defaults isActive=false on terminal testStatus (including credits_exhausted). Callers must pass isActive explicitly.
  • chatCore: FORBIDDEN / ACCOUNT_DEACTIVATED record terminal testStatus for selection skip + alerts, and only deactivate via maybeAutoDisableBannedAccount (autoDisableBannedAccounts + scope) — the same gate auth.ts already uses.
  • Unrecoverable OAuth refresh: sets testStatus=expired only (keeps is_active=1); updateProviderCredentials already skips isActive when omitted.
  • Health scheduler: unchanged — never deactivates.
  • Docs: BAN_DETECTION.md updated for unpaid recovery + gated isActive. No AGENTS.md changes.

Behavior after this PR

Signal testStatus is_active
Unpaid / credits_exhausted set (selection skip + alert) stays 1; recovery re-probe can clear after renew
Ban-looking FORBIDDEN / deactivated set 0 only if autoDisableBannedAccounts allows
OAuth refresh death expired stays 1
Health scheduler unchanged never deactivates

Related Issues

Validation

  • Change type: auth / routing behavior
  • Focused tests: node --import tsx/esm --test tests/unit/terminal-status-no-auto-deactivate-14853.test.ts — RED against pre-fix terminalStatus.ts (2 fail), GREEN after fix (3 pass)
  • Existing terminal/auth guards still pass: terminal-status-origin, auth-terminal-status, probe-gate-autodisable (21 pass)
  • npm run typecheck:core clean
  • npm run lint (eslint on changed files) clean; prettier clean
  • Reconciled onto release/v3.8.51 base tip

Tests Added Or Updated

  • tests/unit/terminal-status-no-auto-deactivate-14853.test.ts (new regression):
    • credits_exhausted without explicit isActive keeps is_active=1
    • banned without explicit isActive records testStatus but keeps is_active=1
    • explicit isActive=false still deactivates (gated auto-disable path honored)

Coverage Notes

  • Touches open-sse/handlers/chatCore.ts and src/shared/utils/terminalStatus.ts. The new regression test directly covers the writeTerminalStatus default change; the chatCore paths now route permanent deactivation through the already-tested maybeAutoDisableBannedAccount gate.

Reviewer Notes

  • The chatCore FORBIDDEN / ACCOUNT_DEACTIVATED branches now call maybeAutoDisableBannedAccount({ permanent: true }) instead of inline isActive: false, matching auth.ts. With autoDisableBannedAccounts on (+ scope), true bans still deactivate; otherwise the connection stays active with a terminal testStatus for selection skip + alerts.

…renew

writeTerminalStatus no longer defaults isActive=false on terminal testStatus (incl. credits_exhausted); callers pass isActive explicitly. chatCore FORBIDDEN/ACCOUNT_DEACTIVATED record testStatus for selection skip + alerts and only deactivate via the gated maybeAutoDisableBannedAccount (autoDisableBannedAccounts + scope). Unrecoverable OAuth refresh sets testStatus=expired only, keeping is_active=1. credits_exhausted / expired accounts recover after billing renew / re-auth without a dashboard re-enable.

Regression: tests/unit/terminal-status-no-auto-deactivate-14853.test.ts (RED before fix, GREEN after).

Fixes #14853
Copilot AI balanced review requested due to automatic review settings October 1, 2026 17:23

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

…atCore ban blocks

The #14853 fix made chatCore FORBIDDEN/ACCOUNT_DEACTIVATED record testStatus
only, gating is_active=0 behind autoDisableBannedAccounts (+ scope). Two unit
tests still asserted the old ungated is_active=0:

- anthropic-request-not-allowed-cooldown-escalation: "any other claude 403
  still bans" now enables autoDisableBannedAccounts before the 403; the
  explicit-isActive 3rd-refusal escalation path is unchanged. Adds a flag-OFF
  regression proving a real ban keeps is_active=1 (recover after flap clears).
- probe-testall-isolation RESTORE: probeCanDisable alone no longer deactivates;
  real-traffic path also needs autoDisableBannedAccounts — enables both, resets
  in finally. Default isolation tests unchanged.

Extracts the duplicated FORBIDDEN/ACCOUNT_DEACTIVATED writeTerminalStatus +
maybeAutoDisableBannedAccount blocks into chatCore/gatedTerminalBan.ts
(recordGatedBanTerminalStatus). open-sse/handlers/chatCore.ts 6465 -> 6425,
back under the frozen 6441 ceiling.

Refs #14853
@RaviTharuma

RaviTharuma commented Oct 1, 2026 •

Copy link
Copy Markdown
Contributor Author

CI follow-ups on fork head:

Fast Quality Gates remaining (baseline / unrelated): mutation-test-coverage (25 unrelated stryker tap.testFiles) and vuln-ratchet (41>27, no lockfile change). Happy to sync in a separate hygiene PR; this PR stays credentials-only.

The #14853 DRY extract moved the two FORBIDDEN / ACCOUNT_DEACTIVATED
lastError: persistentMessage writes out of chatCore's classifier block into
chatCore/gatedTerminalBan.ts, so the classifier now holds 7 (was >=9). Follow
the #12864 pattern already used for requestRejectedFailure.ts: lower the
classifier count to >=7 (what stayed) and hold the extracted module to the same
invariant — lastError must use the projected persistentMessage, never a raw
upstream message.

Refs #14853
@RaviTharuma RaviTharuma closed this Oct 2, 2026
@RaviTharuma
RaviTharuma deleted the fix/creds-flap-no-auto-deactivate branch October 2, 2026 08:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants