From 250af1fdc5881aafd17c013b90ab44b374dcaa0a Mon Sep 17 00:00:00 2001 From: jun Date: Sat, 5 Sep 2026 06:34:14 +0900 Subject: [PATCH] test(oauth): prove the unobservable quorum staleness window is harmless Explicit invalidation covers the roster mutations this module can see, but not one it cannot: a 401 elsewhere flagging an account needsReauth drops the real quorum to one while the cached true survives the TTL. The window is left unplumbed on purpose, so the reason is now proven rather than asserted: a stale true only lets the caller ASK, and pickAlternateAnthropicAccount re-reads the roster, skips the flagged account and returns null. If that ever stops holding, the comment above the cache becomes a lie and this test fails. --- src/oauth/anthropic-routing.ts | 13 +++++++++++ tests/routing/anthropic-quorum-cache.test.ts | 24 +++++++++++++++++++- 2 files changed, 36 insertions(+), 1 deletion(-) diff --git a/src/oauth/anthropic-routing.ts b/src/oauth/anthropic-routing.ts index 0770cbf890..47206335ae 100644 --- a/src/oauth/anthropic-routing.ts +++ b/src/oauth/anthropic-routing.ts @@ -249,6 +249,19 @@ export function getEligibleAnthropicAccounts(now = Date.now()): string[] { * enough that a login in another window is visible before the operator can switch back and send a * prompt, long enough that a burst of requests shares one read. The cache holds a BOOLEAN derived * from a count — never a credential, never an account id. + * + * Staleness is bounded by consequence, not only by the TTL. Explicit invalidation covers the + * roster mutations this module can see (rotation, pool-state reset, affinity clear on account + * removal, manual selection), but not one it cannot: a 401 elsewhere flagging an account + * `needsReauth` drops the real quorum to one while a cached `true` survives for up to 2s. + * + * That window is harmless in both directions, which is why it is left rather than plumbed + * through the store. A stale `true` only lets the caller ASK for an alternate; + * `pickAlternateAnthropicAccount` re-reads the roster through `getEligibleAnthropicAccounts`, + * skips the reauth-flagged account and returns `null`, so the 429 surfaces exactly as it would + * have. A stale `false` costs one un-rotated 429 and self-corrects on the next read. Neither + * can dispatch on an unusable credential, which is the only outcome worth adding a store hook + * to prevent. */ const QUORUM_CACHE_TTL_MS = 2_000; diff --git a/tests/routing/anthropic-quorum-cache.test.ts b/tests/routing/anthropic-quorum-cache.test.ts index 17a6db6096..2d609c0875 100644 --- a/tests/routing/anthropic-quorum-cache.test.ts +++ b/tests/routing/anthropic-quorum-cache.test.ts @@ -24,7 +24,7 @@ import { resetAnthropicRoutingForManualSelection, rotateAnthropicAccountOn429, } from "../../src/oauth/anthropic-routing"; -import { getAccountSet, saveCredential } from "../../src/oauth/store"; +import { getAccountSet, markAccountNeedsReauth, saveCredential } from "../../src/oauth/store"; import { removeTreeWithRetry } from "../helpers/remove-tree"; const originalHome = process.env.OPENCODEX_HOME; @@ -119,6 +119,28 @@ describe("Anthropic failover quorum cache", () => { expect(typeof hasAnthropicFailoverQuorum()).toBe("boolean"); }); + test("a stale quorum cannot dispatch on a reauth-flagged account", async () => { + // The one roster mutation this module cannot observe: a 401 elsewhere flags an account + // needsReauth, dropping the real quorum to one while the cached `true` survives the TTL. + // + // The window is left unplumbed deliberately, so this pins the reason: a stale `true` only + // lets the caller ASK. pickAlternateAnthropicAccount re-reads the roster, skips the flagged + // account and answers null, so the 429 surfaces exactly as it would have. If that ever + // stopped being true, the comment above the cache would be a lie and this fails. + const start = Date.now(); + const ids = await seed(2); + expect(hasAnthropicFailoverQuorum(start)).toBe(true); + + await markAccountNeedsReauth("anthropic", ids[1]!, true); + // Deliberately NOT invalidating: this is the stale-cache state under test. + expect(hasAnthropicFailoverQuorum(start + 1)).toBe(true); + + // The rotator admits the request, then finds nothing usable and refuses. + expect( + rotateAnthropicAccountOn429({ providers: {} } as never, ids[0]!, null, null, start + 1), + ).toBeNull(); + }); + test("removing an account invalidates immediately, not after the TTL", async () => { // The management DELETE route calls clearAnthropicSessionAffinityForAccount for Anthropic. // Without invalidation there, deleting the second account would leave quorum true for up to