diff --git a/src/agent/stream-recovery.ts b/src/agent/stream-recovery.ts index 3cee8d3d..84084f2b 100644 --- a/src/agent/stream-recovery.ts +++ b/src/agent/stream-recovery.ts @@ -1,5 +1,6 @@ import { ProviderError, STREAM_STALL_MARKER } from "../llm/http.js"; +import { rateLimitWaitMsFor } from "../llm/key-rotation.js"; import { isEmptyCompletionError } from "../llm/router.js"; export type StreamFailureKind = @@ -222,18 +223,6 @@ function pick(delays: readonly number[], attempt: number, max: number): number { return Math.min(delays[attempt] ?? delays[delays.length - 1] ?? 0, max); } -function retryAfterSecondsFrom(error: unknown): number | undefined { - if (error instanceof ProviderError && error.retryAfterSeconds !== undefined) { - return error.retryAfterSeconds; - } - const match = (error instanceof Error ? error.message : String(error ?? "")).match( - /retry after ([0-9.]+)\s*s/i, - ); - if (!match) return undefined; - const seconds = Number.parseFloat(match[1]!); - return Number.isFinite(seconds) && seconds >= 0 ? seconds : undefined; -} - export function planStreamRecovery(input: { error?: unknown; kind?: StreamFailureKind; @@ -284,14 +273,7 @@ export function planStreamRecovery(input: { case "rate-limit": { const n = attempt(state.rateLimit, limits.maxRateLimit); if (n >= limits.maxRateLimit) return giveUp; - const scheduled = pick([10_000, 20_000, 30_000, 40_000, 60_000], n, cap); - const providerSeconds = retryAfterSecondsFrom(input.error); - const delayMs = Math.min( - cap, - providerSeconds === undefined - ? scheduled - : Math.max(scheduled, providerSeconds * 1000), - ); + const delayMs = rateLimitWaitMsFor(input.error, n, cap); return { action: "retry", kind, diff --git a/src/llm/key-rotation.ts b/src/llm/key-rotation.ts index 4f29dc57..10d42160 100644 --- a/src/llm/key-rotation.ts +++ b/src/llm/key-rotation.ts @@ -5,6 +5,51 @@ export const MAX_PROVIDER_KEYS = 10; export const MULTI_KEY_ATTEMPTS = 2; +export const RATE_LIMIT_RETRY_WAIT_MS: readonly number[] = [ + 10_000, 20_000, 30_000, 40_000, 60_000, +]; + +export function rateLimitRetryWaitMs( + attempt: number, + maxMs = Number.POSITIVE_INFINITY, +): number { + const index = Math.min( + Math.max(0, attempt), + RATE_LIMIT_RETRY_WAIT_MS.length - 1, + ); + return Math.min(RATE_LIMIT_RETRY_WAIT_MS[index]!, maxMs); +} + +export function providerRetryAfterMs(error: unknown): number | undefined { + if (error && typeof error === "object" && "retryAfterSeconds" in error) { + const seconds = (error as { retryAfterSeconds?: unknown }) + .retryAfterSeconds; + if ( + typeof seconds === "number" && + Number.isFinite(seconds) && + seconds >= 0 + ) { + return Math.ceil(seconds * 1000); + } + } + const match = errorMessage(error).match(/retry after ([0-9.]+)\s*s/i); + if (!match) return undefined; + const seconds = Number.parseFloat(match[1]!); + return Number.isFinite(seconds) && seconds >= 0 + ? Math.ceil(seconds * 1000) + : undefined; +} + +export function rateLimitWaitMsFor( + error: unknown, + attempt: number, + maxMs = Number.POSITIVE_INFINITY, +): number { + const scheduled = rateLimitRetryWaitMs(attempt, maxMs); + const hint = providerRetryAfterMs(error); + return hint === undefined ? scheduled : Math.min(scheduled, hint); +} + export function buildKeyAttemptPlan(n: number, startIndex: number): number[] { if (n <= 0) return []; if (n === 1) return [0]; diff --git a/src/llm/routing/error-classification.ts b/src/llm/routing/error-classification.ts index 69e8971a..079592ff 100644 --- a/src/llm/routing/error-classification.ts +++ b/src/llm/routing/error-classification.ts @@ -13,6 +13,7 @@ import { ProviderError, STREAM_STALL_MARKER, } from "../http.js"; +import { rateLimitWaitMsFor } from "../key-rotation.js"; import { quotaOrRateLimited } from "../quota-signals.js"; import { resolveBuiltInProfile } from "../provider-profiles.js"; import { EFFORT_SCALE, nearestAcceptedEffort } from "../reasoning-controls.js"; @@ -193,6 +194,9 @@ export function isRetriableError(error: unknown): boolean { } export function retryWaitMs(error: unknown, attempt: number): number { + if (isRateLimited(error)) { + return rateLimitWaitMsFor(error, attempt, MAX_RETRY_WAIT_MS); + } if (error instanceof ProviderError && error.retryAfterSeconds !== undefined) { return Math.ceil(error.retryAfterSeconds * 1000); } diff --git a/src/mcp/auth/provider.ts b/src/mcp/auth/provider.ts index 60444af5..dea946ea 100644 --- a/src/mcp/auth/provider.ts +++ b/src/mcp/auth/provider.ts @@ -389,7 +389,6 @@ class OAuthProvider implements McpAuthProvider { } throw new McpTransportError("network", "MCP OAuth authorization was declined."); } - await this.requireConsent(metadata, scope); const pkce = createPkcePair(); const loopback = this.deps.runLoopback ?? runLoopbackAuthorization; const bootstrapClientId = this.clientId ?? this.deps.clientName ?? DEFAULT_CLIENT_NAME; diff --git a/src/ui-core/bootstrap/composition-root.ts b/src/ui-core/bootstrap/composition-root.ts index ea6688dc..048c1b12 100644 --- a/src/ui-core/bootstrap/composition-root.ts +++ b/src/ui-core/bootstrap/composition-root.ts @@ -142,7 +142,10 @@ export function createCompositionRoot( "plain", ); if (!shown) { - sessionRef?.notice("info", lines.join(" · ")); + sessionRef?.notice( + "info", + lines.filter((line) => line.trim().length > 0).join(" · "), + ); } }, onAuthorizationUrl: (info) => { diff --git a/src/ui-core/commands/mcp-commands.ts b/src/ui-core/commands/mcp-commands.ts index d028413e..1056b97e 100644 --- a/src/ui-core/commands/mcp-commands.ts +++ b/src/ui-core/commands/mcp-commands.ts @@ -123,12 +123,6 @@ async function addServer( draft = input; } const written = await writeProjectMcpServer(draft); - if (written.ok) { - services.session.notice( - "info", - `added MCP server ${written.serverName} to ${written.displayPath} · connecting…`, - ); - } if (!written.ok) { services.session.notice( "warn", @@ -145,16 +139,24 @@ async function addServer( draft = retry; continue; } - const state = await services.mcp.refresh({ force: true }); - const status = state.snapshot.statuses.find( - (candidate) => candidate.name === written.serverName, - ); - if (status?.status === "ready") selectServer(services, status); - services.session.notice( - status?.status === "ready" ? "info" : "warn", - `${written.replaced ? "updated" : "added"} MCP server ${written.serverName} in ${written.displayPath}${status?.status === "ready" ? ` · use ${formatMcpToken(written.serverName)} in your prompt · ${status.toolCount} tool${status.toolCount === 1 ? "" : "s"}` : ` · ${status?.status ?? "not discovered"}${status?.detail ? ` · ${status.detail}` : ""}`}`, + const pendingToast = services.toast.show( + `adding MCP server ${written.serverName}…`, + { level: "info", sticky: true }, ); - return written.serverName; + try { + const state = await services.mcp.refresh({ force: true }); + const status = state.snapshot.statuses.find( + (candidate) => candidate.name === written.serverName, + ); + if (status?.status === "ready") selectServer(services, status); + services.session.notice( + status?.status === "ready" ? "info" : "warn", + `${written.replaced ? "updated" : "added"} MCP server ${written.serverName} in ${written.displayPath}${status?.status === "ready" ? ` · use ${formatMcpToken(written.serverName)} in your prompt · ${status.toolCount} tool${status.toolCount === 1 ? "" : "s"}` : ` · ${status?.status ?? "not discovered"}${status?.detail ? ` · ${status.detail}` : ""}`}`, + ); + return written.serverName; + } finally { + services.toast.dismiss(pendingToast); + } } return undefined; } @@ -212,7 +214,21 @@ async function addKnownServer( if (existing.status === "ready") selectServer(services, existing); return; } - services.session.notice("info", `adding ${known.title} MCP server…`); + const pendingToast = services.toast.show( + `adding ${known.title} MCP server…`, + { level: "info", sticky: true }, + ); + try { + await finishKnownServerAdd(services, known); + } finally { + services.toast.dismiss(pendingToast); + } +} + +async function finishKnownServerAdd( + services: AppServices, + known: KnownMcpServer, +): Promise { const collected: Record = {}; for (const secret of known.secrets) { if (secret.optional) continue; diff --git a/test/stream-recovery.test.ts b/test/stream-recovery.test.ts index 477a6013..c870eac1 100644 --- a/test/stream-recovery.test.ts +++ b/test/stream-recovery.test.ts @@ -188,34 +188,27 @@ describe("planStreamRecovery — bounded escalation", () => { ); }); - it("waits out a longer provider reset window instead of the schedule", () => { + it("caps provider reset windows at the uniform schedule but honors shorter hints", () => { const state = createStreamRecoveryState(); - const error = new ProviderError("rate limited", 429, "", 45); + const long = new ProviderError("rate limited", 429, "", 45); expect( - planStreamRecovery({ kind: "rate-limit", state, error }).delayMs, - ).toBe(45_000); - }); - - it("keeps the schedule as a floor and 60s as the ceiling for resets", () => { - const state = createStreamRecoveryState(); + planStreamRecovery({ kind: "rate-limit", state, error: long }).delayMs, + ).toBe(10_000); + recordRecoveryAttempt(state, "rate-limit"); const short = new ProviderError("rate limited", 429, "", 2); expect( planStreamRecovery({ kind: "rate-limit", state, error: short }).delayMs, - ).toBe(10_000); - const long = new ProviderError("rate limited", 429, "", 600); - expect( - planStreamRecovery({ kind: "rate-limit", state, error: long }).delayMs, - ).toBe(60_000); + ).toBe(2_000); }); - it("derives the reset window from the failure message when needed", () => { + it("keeps the uniform schedule when the failure message carries a reset window", () => { const state = createStreamRecoveryState(); const error = new Error( "hetzner: Provider request failed with HTTP 429 (retry after 35s)", ); expect( planStreamRecovery({ kind: "rate-limit", state, error }).delayMs, - ).toBe(35_000); + ).toBe(10_000); }); it("compacts on context overflow before retrying, then gives up", () => {