diff --git a/docs/non-compaction-retry-policy.md b/docs/non-compaction-retry-policy.md index ea5f9e114..e0ce2ea7b 100644 --- a/docs/non-compaction-retry-policy.md +++ b/docs/non-compaction-retry-policy.md @@ -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`. diff --git a/packages/ai/CHANGELOG.md b/packages/ai/CHANGELOG.md index 98773ec2c..2345b43f8 100644 --- a/packages/ai/CHANGELOG.md +++ b/packages/ai/CHANGELOG.md @@ -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. diff --git a/packages/ai/test/auth-storage-force-refresh-rotate.test.ts b/packages/ai/test/auth-storage-force-refresh-rotate.test.ts index 1ae31a67b..0330413c3 100644 --- a/packages/ai/test/auth-storage-force-refresh-rotate.test.ts +++ b/packages/ai/test/auth-storage-force-refresh-rotate.test.ts @@ -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 }); + }); }); diff --git a/packages/coding-agent/CHANGELOG.md b/packages/coding-agent/CHANGELOG.md index c9b36b1e7..c6e33f60f 100644 --- a/packages/coding-agent/CHANGELOG.md +++ b/packages/coding-agent/CHANGELOG.md @@ -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)). diff --git a/packages/coding-agent/test/agent-session-retry-cap.test.ts b/packages/coding-agent/test/agent-session-retry-cap.test.ts index 465b80e13..d6a4b8827 100644 --- a/packages/coding-agent/test/agent-session-retry-cap.test.ts +++ b/packages/coding-agent/test/agent-session-retry-cap.test.ts @@ -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. diff --git a/packages/coding-agent/test/auth-storage-rotation.test.ts b/packages/coding-agent/test/auth-storage-rotation.test.ts index 823cf557c..e0433506c 100644 --- a/packages/coding-agent/test/auth-storage-rotation.test.ts +++ b/packages/coding-agent/test/auth-storage-rotation.test.ts @@ -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);