Skip to content

fix(account-pool): recover from temporary OAuth refresh outages - #3

Closed
smsunarto wants to merge 2 commits into
scott/pool-refresh-singleflightfrom
scott/pool-refresh-recovery
Closed

fix(account-pool): recover from temporary OAuth refresh outages#3
smsunarto wants to merge 2 commits into
scott/pool-refresh-singleflightfrom
scott/pool-refresh-recovery

Conversation

@smsunarto

@smsunarto smsunarto commented Sep 5, 2026

Copy link
Copy Markdown
Owner

Superseded by get-bb#3115 in the correct upstream repository. Closing this misplaced PR.

Human comments

What was wrong

Any OAuth refresh failure became a persistent account error. A temporary HTTP 503 therefore disabled an account even when its access token was still valid. Later requests returned quota 429 and required manual recovery.

What changed

Both adapters classify temporary HTTP and connection failures at a shared refresh boundary with a 15-second timeout. The hub retains only known-unexpired tokens during those failures. Refresh attempts back off from one second to a 60-second cap, including server retry hints. State is bounded and tied to the account and access token. Expired tokens return a temporary 503 when no alternative can serve. Credential rejection, parsing errors, and persistence errors remain terminal.

Updated the plugin overview and CLI guide. This layer builds on get-bb/bb#3112. Its base is that PR's exact source branch in this fork, because upstream access is read-only.

How you verified

Public HTTP regressions failed before the fix with expected 429 to be 200 and expected 429 to be 503 for both providers. All six now pass. Further tests cover expiry during backoff, bounded Retry-After, canceled error bodies, broken connections, and timeout.

  • Full plugin Turbo suite: 86 tests passed across 8 files.
  • Turbo typecheck, explicit-file formatting, and oxlint passed.
  • Original public-hub probe with real SQLite and secret files: refresh outage now returns [200,200], one refresh attempt, and no persisted error.

Summary by cubic

Fixes the account pool so temporary OAuth refresh outages no longer permanently disable accounts. A transient HTTP or connection failure now keeps a valid access token usable and retries the refresh with backoff, instead of marking the account in error. Builds on the shared refresh boundary from get-bb/bb#3112.

What changed

  • Both adapters classify temporary failures through a shared refresh transport with a 15-second timeout.
  • Refresh backoff starts at 1 second and caps at 60, respecting server Retry-After hints.
  • Expired tokens return HTTP 503 when no other account can serve the request.
  • Credential rejection, parse errors, and persistence errors remain terminal account errors.
  • Updated the plugin overview and CLI guide.

Written for commit 1c85def. Summary will update on new commits.

Review in cubic

Stack

Review and merge in order:

  1. Serialize credential refreshes
  2. Recover transient OAuth failures
  3. Recover requests before streaming
  4. Preserve session affinity

All PRs target get-bb/bb:main. The source branches remain stacked in the fork. GitHub diffs are currently cumulative because upstream access is read-only, so the parent branches cannot be published there.

AGENT GENERATED

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

1 issue found across 8 files

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="plugins/account-pool/src/hub.ts">

<violation number="1" location="plugins/account-pool/src/hub.ts:380">
P3: When any selected account hits a transient refresh failure (expired token) during a request, the post-loop `refreshFailure` check returns 503 even if a later account was actually served and rejected only on quota (429), or the real blocker was another account's terminal error. The returned 503 also carries no `retry-after` header, so the client gets no signal about when the refresh outage clears and may retry immediately or too late. Consider only returning the transient 503 when the request could not be served for an actual refresh reason (no other account with a usable token), and otherwise keep the previous 429/noEligible path.</violation>
</file>

Reply with feedback, questions, or to request a fix.

Re-trigger cubic

return this.noEligibleResponse(accounts, family, adapter);
return refreshFailure === null
? this.noEligibleResponse(accounts, family, adapter)
: adapter.errorResponse(503, refreshFailure.message);

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P3: When any selected account hits a transient refresh failure (expired token) during a request, the post-loop refreshFailure check returns 503 even if a later account was actually served and rejected only on quota (429), or the real blocker was another account's terminal error. The returned 503 also carries no retry-after header, so the client gets no signal about when the refresh outage clears and may retry immediately or too late. Consider only returning the transient 503 when the request could not be served for an actual refresh reason (no other account with a usable token), and otherwise keep the previous 429/noEligible path.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At plugins/account-pool/src/hub.ts, line 380:

<comment>When any selected account hits a transient refresh failure (expired token) during a request, the post-loop `refreshFailure` check returns 503 even if a later account was actually served and rejected only on quota (429), or the real blocker was another account's terminal error. The returned 503 also carries no `retry-after` header, so the client gets no signal about when the refresh outage clears and may retry immediately or too late. Consider only returning the transient 503 when the request could not be served for an actual refresh reason (no other account with a usable token), and otherwise keep the previous 429/noEligible path.</comment>

<file context>
@@ -360,7 +375,9 @@ export class AccountPoolHub {
-    return this.noEligibleResponse(accounts, family, adapter);
+    return refreshFailure === null
+      ? this.noEligibleResponse(accounts, family, adapter)
+      : adapter.errorResponse(503, refreshFailure.message);
   }
 
</file context>

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.

1 participant