-
Notifications
You must be signed in to change notification settings - Fork 1.1k
test(oauth): pin the rotator against being re-gated, and fix the last stale guide #3523
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
f03812d
292cb15
f2a78a3
fc05738
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -13,7 +13,10 @@ You can log in multiple Claude accounts via the Providers dashboard (`ocx login | |
| add-account). By default every request uses the **active** account only. | ||
|
|
||
| An **experimental, opt-in** Claude account pool (`anthropicAccountPool.enabled`) adds sticky | ||
| session affinity and 429 cooldown failover across those OAuth accounts. For **new** sessions, | ||
| session affinity and usage-aware new-session selection across those OAuth accounts. It does | ||
| **not** gate 429 failover: with two or more usable accounts stored, a rate-limited request moves | ||
| to another account whether the pool is on or off, and that cannot be switched off. For **new** | ||
|
Comment on lines
+17
to
+18
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
The categorical statement that a rate-limited request “moves to another account” is false when the other stored credential is already cooling or the per-request retry bound has been exhausted; the runtime then surfaces the 429, and the operational bullet below correctly says it only “may rotate.” Say that failover is always enabled but occurs only when an eligible alternate and retry budget remain, and remove the following “when enabled” qualification that now incorrectly scopes this behavior to the optional pool. Apply the same clarification to the edited translations. AGENTS.md reference: docs-site/AGENTS.md:L7-L10 Useful? React with 👍 / 👎. |
||
| sessions, | ||
| `anthropicAccountPool.strategy` selects among eligible accounts: `quota` (default) picks the | ||
| lowest known usage in the window set by `quotaWindow` (`five-hour` by default, or `weekly` / | ||
| `max-utilization`) when above `autoSwitchThreshold`; `round-robin` spreads evenly | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -139,4 +139,21 @@ describe("proactive Anthropic routing stays opt-in", () => { | |
| ); | ||
| expect(picks.every(id => id === ids[0]!)).toBe(true); | ||
| }); | ||
| test("the rotator cannot be re-gated behind the pool flag", async () => { | ||
| // The original defect was ONE line at the top of rotateAnthropicAccountOn429: | ||
| // if (!isAnthropicAccountPoolEnabled(config)) return null; | ||
| // Restoring it would strand every stock install again, and nothing else in this file would | ||
| // fail -- every behavioural test seeds two accounts, which satisfies the quorum either way, | ||
| // so they would keep passing while the feature was dead for the users who never opted in. | ||
| // | ||
| // Pin the shape instead: the flag may still appear in the rotator, but only alongside the | ||
| // presence check, never as a gate of its own. | ||
| const source = await Bun.file("src/oauth/anthropic-routing.ts").text(); | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
This source-oracle read is relative to AGENTS.md reference: AGENTS.md:L22-L23 Useful? React with 👍 / 👎. |
||
| const start = source.indexOf("export function rotateAnthropicAccountOn429"); | ||
| expect(start).toBeGreaterThan(-1); | ||
| const body = source.slice(start, source.indexOf("\n}", start)); | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win Extract the complete function body.
🤖 Prompt for AI Agents |
||
| const gate = body.split("\n").find(line => line.includes("isAnthropicAccountPoolEnabled")); | ||
| expect(gate, "the rotator no longer references the pool flag at all").toBeDefined(); | ||
| expect(gate, "the pool flag became a gate of its own again").toContain("hasAnthropicFailoverQuorum"); | ||
|
coderabbitai[bot] marked this conversation as resolved.
Comment on lines
+156
to
+157
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win Do not require the pool flag to remain in the rotator. The contract only prohibits 🤖 Prompt for AI Agents |
||
| }); | ||
| }); | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Keep 429 failover outside the “when enabled” scope.
The new paragraphs correctly describe 429 failover as independent of
anthropicAccountPool.enabled, but each page still uses an enabled-only heading for the section whose first bullet describes 429 failover.docs-site/src/content/docs/guides/claude-code.md#L17-L18: rename the heading at Line 28 or separate the always-on 429 behavior.docs-site/src/content/docs/fr/guides/claude-code.md#L16: rename the heading at Line 26 or separate the always-on 429 behavior.docs-site/src/content/docs/tr/guides/claude-code.md#L19: rename the heading at Line 30 or separate the always-on 429 behavior.docs-site/src/content/docs/zh-tw/guides/claude-code.md#L16: rename the heading at Line 24 or separate the always-on 429 behavior.As per path instructions, documentation must keep account-pool session behavior separate from mandatory 429 failover.
📍 Affects 4 files
docs-site/src/content/docs/guides/claude-code.md#L17-L18(this comment)docs-site/src/content/docs/fr/guides/claude-code.md#L16-L16docs-site/src/content/docs/tr/guides/claude-code.md#L19-L19docs-site/src/content/docs/zh-tw/guides/claude-code.md#L16-L16🤖 Prompt for AI Agents
Source: Path instructions