fix(coding-agent): fixed usage-limit retry delay to use earliest sibling unblock
- Added auth-storage regression coverage in `packages/ai` for `markUsageLimitReached` returning the earliest sibling unblock time when all credentials are momentarily blocked. - Added an agent-session retry-cap test ensuring usage-limit 429 retries wait for sibling unblock and succeed within `retry.maxDelayMs` instead of giving up. - Updated retry-policy docs and both package changelogs to document the sibling-unblock fallback behavior.
This commit is contained in:
can1357
@@ -63,7 +63,7 @@ Flow (`#handleRetryableError`):
|
||||
4. Create `#retryPromise` once (first attempt in a chain).
|
||||
5. If attempt exceeded `retry.maxRetries`, emit final failure event and stop.
|
||||
6. Compute base delay: `retry.baseDelayMs * 2^(attempt-1)`.
|
||||
7. For usage-limit errors, parse retry hints and call auth storage (`markUsageLimitReached(...)`); if credential switching succeeds, force delay to `0`, otherwise use a larger retry-after/backoff hint when present.
|
||||
7. For usage-limit errors, parse retry hints and call auth storage (`markUsageLimitReached(...)`); if credential switching succeeds, force delay to `0`. Otherwise wait for whichever comes first — the provider's retry-after/backoff hint, or the earliest moment a temporarily blocked sibling credential frees up (`retryAtMs` + 1s buffer) so the next attempt can pick it up.
|
||||
8. If no credential switch occurred, suppress the current model selector for cooldown, try configured retry model fallback chains, and force delay to `0` on model switch.
|
||||
9. If the final delay exceeds `retry.maxDelayMs` and no credential/model switch happened, emit final failure and do not sleep.
|
||||
10. Emit `auto_retry_start`.
|
||||
|
||||
@@ -10,6 +10,7 @@
|
||||
|
||||
### Fixed
|
||||
|
||||
- Fixed `AuthStorage.markUsageLimitReached` collapsing "every sibling is momentarily blocked" into "no sibling exists": it now returns `UsageLimitMarkResult` with the earliest sibling block expiry (`retryAtMs`), so retry layers can wait out a short-lived block (60s post-401, 5-min usage-probe) instead of adopting the provider's multi-hour retry-after. `rotateSessionCredential` and the auth-gateway adapt to the new shape.
|
||||
- Fixed Gemini streaming silently presenting truncated or blocked output as a successful `stop`: in-band `{"error":{...}}` events and `promptFeedback.blockReason` chunks were never inspected, and a stream ending without any `finishReason` kept the initialized `stop` — all three now surface as errors (both the API-key and gemini-cli/Antigravity consumers), and the `toolUse` stop-reason override no longer masks `SAFETY`/`MALFORMED_FUNCTION_CALL` finishes that arrive after a valid tool call.
|
||||
- Fixed Gemini/Bedrock error finishes reporting "An unknown error occurred": the raw finish/stop reason (`MALFORMED_FUNCTION_CALL`, `RECITATION`, `guardrail_intervened`, …) is now recorded into the surfaced error message.
|
||||
- Fixed the Anthropic provider retry loop ignoring server `retry-after` on 429/529 — it now waits `max(headerDelay, backoff)` instead of hammering a rate-limited endpoint three times within ~14s of guaranteed failures.
|
||||
|
||||
@@ -160,4 +160,43 @@ describe("AuthStorage forceRefresh + rotateSessionCredential", () => {
|
||||
// Never resolved a key for this session → nothing to rotate away from.
|
||||
expect(await authStorage.rotateSessionCredential(PROVIDER, "untouched", { error: authError() })).toBe(false);
|
||||
});
|
||||
|
||||
test("markUsageLimitReached reports the earliest sibling unblock time when every sibling is blocked", async () => {
|
||||
if (!authStorage) throw new Error("test setup failed");
|
||||
registerProvider();
|
||||
await authStorage.set(PROVIDER, [
|
||||
{ type: "oauth", access: "acc-A", refresh: "ref-A", expires: farExpiry() },
|
||||
{ type: "oauth", access: "acc-B", refresh: "ref-B", expires: farExpiry() },
|
||||
]);
|
||||
|
||||
// Session A takes one credential and parks it briefly (e.g. a transient
|
||||
// probe block) — a sibling is still free, so this reports switched.
|
||||
await authStorage.getApiKey(PROVIDER, "sess-a");
|
||||
const blockedAt = Date.now();
|
||||
const first = await authStorage.markUsageLimitReached(PROVIDER, "sess-a", { retryAfterMs: 30_000 });
|
||||
expect(first.switched).toBe(true);
|
||||
|
||||
// Session B lands on the remaining credential and hits a multi-hour
|
||||
// usage limit. No sibling is free *right now*, but the result must
|
||||
// carry session A's short unblock time — not the 1h window — so the
|
||||
// retry layer can wait seconds instead of bailing on the long wait.
|
||||
await authStorage.getApiKey(PROVIDER, "sess-b");
|
||||
const second = await authStorage.markUsageLimitReached(PROVIDER, "sess-b", { retryAfterMs: 3_600_000 });
|
||||
expect(second.switched).toBe(false);
|
||||
expect(second.retryAtMs).toBeDefined();
|
||||
expect(second.retryAtMs!).toBeGreaterThan(blockedAt);
|
||||
expect(second.retryAtMs!).toBeLessThanOrEqual(blockedAt + 30_000);
|
||||
});
|
||||
|
||||
test("markUsageLimitReached reports no retry time for a single-credential setup", async () => {
|
||||
if (!authStorage) throw new Error("test setup failed");
|
||||
registerProvider();
|
||||
await authStorage.set(PROVIDER, [
|
||||
{ type: "oauth", access: "only-access", refresh: "only-refresh", expires: farExpiry() },
|
||||
]);
|
||||
|
||||
await authStorage.getApiKey(PROVIDER, "sess");
|
||||
const outcome = await authStorage.markUsageLimitReached(PROVIDER, "sess", { retryAfterMs: 3_600_000 });
|
||||
expect(outcome).toEqual({ switched: false, retryAtMs: undefined });
|
||||
});
|
||||
});
|
||||
|
||||
@@ -34,6 +34,7 @@
|
||||
|
||||
### Fixed
|
||||
|
||||
- Fixed auto-retry giving up after one attempt ("Provider requested Xms wait, exceeds retry.maxDelayMs") on a usage-limit 429 when every sibling account was only momentarily blocked: the retry delay now waits for the earliest sibling unblock when that comes sooner than the provider's multi-hour retry-after, so the next attempt picks up the recovered account instead of failing fast.
|
||||
- Fixed the bundled `explore` agent's `thinking-level: med` frontmatter — not a valid effort (`minimal`/`low`/`medium`/`high`/`xhigh`), so it silently parsed to undefined and the agent ran without its intended thinking level
|
||||
- Discovery context-file reads (`~/.claude`, `~/.cursor`, project trees, `@`-imports) now stat-gate to regular files before reading: a FIFO/socket/char device dropped where a context file is expected previously blocked startup forever on a read that can never see EOF.
|
||||
- Fixed the read tool's provider-visible `path` schema and docs so web URLs and internal URI targets (`omp://`, `issue://`, `pr://`, etc.) are advertised alongside local files ([#2215](https://github.com/can1357/oh-my-pi/issues/2215)).
|
||||
|
||||
@@ -216,6 +216,92 @@ describe("AgentSession retry delay cap", () => {
|
||||
expect(last.content).toContainEqual({ type: "text", text: "recovered after credential switch" });
|
||||
});
|
||||
|
||||
it("waits for the earliest sibling unblock instead of failing the delay cap", async () => {
|
||||
// Regression: with every sibling credential momentarily blocked (e.g. a
|
||||
// short post-401 or usage-probe block), a usage-limit 429 with a
|
||||
// multi-hour retry-after used to adopt the full provider wait and trip
|
||||
// the fail-fast cap ("gave up after 1 attempt") — even though a sibling
|
||||
// would have been usable seconds later. The retry delay must track the
|
||||
// earliest sibling unblock, not the provider window.
|
||||
const model = getBundledModel("anthropic", "claude-sonnet-4-5");
|
||||
if (!model) {
|
||||
throw new Error("Expected bundled Anthropic test model to exist");
|
||||
}
|
||||
|
||||
authStorage.removeRuntimeApiKey("anthropic");
|
||||
await authStorage.set("anthropic", [
|
||||
{ type: "api_key", key: "anthropic-key-1" },
|
||||
{ type: "api_key", key: "anthropic-key-2" },
|
||||
]);
|
||||
|
||||
// Another session holds one credential and parks it for 2s — the test
|
||||
// session lands on the sibling.
|
||||
await modelRegistry.getApiKeyForProvider("anthropic", "other-session");
|
||||
const blocked = await authStorage.markUsageLimitReached("anthropic", "other-session", { retryAfterMs: 2_000 });
|
||||
expect(blocked.switched).toBe(true);
|
||||
|
||||
const rateLimitError =
|
||||
'429 {"type":"error","error":{"type":"rate_limit_error","message":"This request would exceed your account\'s rate limit. Please try again later."}} retry-after-ms=11180000';
|
||||
const mock = createMockModel();
|
||||
let attempts = 0;
|
||||
let agent!: Agent;
|
||||
agent = new Agent({
|
||||
getApiKey: provider => modelRegistry.getApiKeyForProvider(provider, agent.sessionId),
|
||||
initialState: {
|
||||
model,
|
||||
systemPrompt: ["Test"],
|
||||
tools: [],
|
||||
messages: [],
|
||||
},
|
||||
streamFn: (requestedModel, context, options) => {
|
||||
attempts += 1;
|
||||
mock.push(attempts === 1 ? { throw: rateLimitError } : { content: ["recovered after sibling unblock"] });
|
||||
return mock.stream(requestedModel, context, options);
|
||||
},
|
||||
});
|
||||
|
||||
const settings = Settings.isolated({
|
||||
"compaction.enabled": false,
|
||||
"retry.baseDelayMs": 5,
|
||||
"retry.maxDelayMs": 5_000,
|
||||
"retry.maxRetries": 1,
|
||||
});
|
||||
settings.setModelRole("default", `${model.provider}/${model.id}`);
|
||||
|
||||
session = new AgentSession({
|
||||
agent,
|
||||
sessionManager: SessionManager.inMemory(),
|
||||
settings,
|
||||
modelRegistry,
|
||||
});
|
||||
|
||||
const waitSpy = vi.spyOn(scheduler, "wait").mockResolvedValue(undefined);
|
||||
const retryStartEvents: AutoRetryStartEvent[] = [];
|
||||
const retryEndEvents: AutoRetryEndEvent[] = [];
|
||||
session.subscribe(event => {
|
||||
if (event.type === "auto_retry_start") retryStartEvents.push(event);
|
||||
if (event.type === "auto_retry_end") retryEndEvents.push(event);
|
||||
});
|
||||
|
||||
await session.prompt("Trigger account rate limit while the sibling is briefly blocked");
|
||||
await session.waitForIdle();
|
||||
|
||||
expect(attempts).toBe(2);
|
||||
expect(retryStartEvents).toHaveLength(1);
|
||||
// ~2s sibling block + 1s buffer — NOT the provider's 11180s window and
|
||||
// NOT the fail-fast bail (which would emit zero start events).
|
||||
expect(retryStartEvents[0].delayMs).toBeGreaterThanOrEqual(1_000);
|
||||
expect(retryStartEvents[0].delayMs).toBeLessThanOrEqual(3_000);
|
||||
expect(retryEndEvents).toHaveLength(1);
|
||||
expect(retryEndEvents[0]).toMatchObject({ success: true, attempt: 1 });
|
||||
for (const call of waitSpy.mock.calls) {
|
||||
expect(call[0]).toBeLessThanOrEqual(5_000);
|
||||
}
|
||||
const last = lastAssistant(session);
|
||||
expect(last.stopReason).toBe("stop");
|
||||
expect(last.content).toContainEqual({ type: "text", text: "recovered after sibling unblock" });
|
||||
});
|
||||
|
||||
it("still retries normally when the delay is under retry.maxDelayMs", async () => {
|
||||
// Sanity check: a small retry-after MUST still go through the retry
|
||||
// loop so we don't regress the existing transient-error recovery.
|
||||
|
||||
@@ -89,7 +89,7 @@ describe("AuthStorage account rotation", () => {
|
||||
expect(firstKey).toMatch(/^api-acct-/);
|
||||
|
||||
usageExhausted = true;
|
||||
const switched = await authStorage.markUsageLimitReached("openai-codex", sessionId);
|
||||
const { switched } = await authStorage.markUsageLimitReached("openai-codex", sessionId);
|
||||
expect(switched).toBe(true);
|
||||
|
||||
const exhaustedFallbackKey = await authStorage.getApiKey("openai-codex", sessionId);
|
||||
|
||||
Reference in New Issue
Block a user