From 7b6548f182ca2f0e1ad7b9dde31e12a021a21f28 Mon Sep 17 00:00:00 2001 From: roboomp Date: Sun, 9 Aug 2026 11:03:25 +0000 Subject: [PATCH] fix(agent): fit-check retry fallback before switching models Retry-fallback candidate selection filtered on suppression, effort ceiling, model resolution, and API key, but never compared a candidate's context window with the live context. A large-window primary hitting a retryable error could switch onto a smaller-window fallback and immediately send a predictably oversized request that the provider rejects, stalling the run. This is the forward counterpart of the #7952 cooldown-expiry revert fix. Generalize the existing retry-fit budget check into SessionMaintenance.contextFitsModel(model) and consult it from both retry-fallback selection loops (#tryRetryModelFallback and the usage-aware loop): skip any candidate whose usable window cannot hold the current context and advance to the first configured candidate that fits. The check is independent of compaction.enabled since an oversized request overflows regardless. Fixes #8065 --- packages/coding-agent/CHANGELOG.md | 4 + .../coding-agent/src/session/agent-session.ts | 1 + .../src/session/session-maintenance.ts | 46 +++++---- .../coding-agent/src/session/turn-recovery.ts | 14 +++ .../test/agent-session-retry-fallback.test.ts | 99 +++++++++++++++++++ .../test/turn-recovery-replay-unsafe.test.ts | 1 + 6 files changed, 147 insertions(+), 18 deletions(-) diff --git a/packages/coding-agent/CHANGELOG.md b/packages/coding-agent/CHANGELOG.md index a4d66e8d2..cc873d2e2 100644 --- a/packages/coding-agent/CHANGELOG.md +++ b/packages/coding-agent/CHANGELOG.md @@ -2,6 +2,10 @@ ## [Unreleased] +### Fixed + +- Fixed retry-fallback selection switching a live session from a large-context primary onto a smaller-context fallback and immediately sending a predictably oversized request; candidate selection now skips any fallback whose usable window cannot hold the current context and advances to the first configured candidate that fits ([#8065](https://github.com/can1357/oh-my-pi/issues/8065)). + ## [17.2.12] - 2026-08-08 ### Fixed diff --git a/packages/coding-agent/src/session/agent-session.ts b/packages/coding-agent/src/session/agent-session.ts index 6ccceb043..2f0afce0f 100644 --- a/packages/coding-agent/src/session/agent-session.ts +++ b/packages/coding-agent/src/session/agent-session.ts @@ -1035,6 +1035,7 @@ export class AgentSession { modelRegistry: this.#modelRegistry, configWarnings: this.configWarnings, model: () => this.model, + contextFitsModel: model => this.#maintenance.contextFitsModel(model), textOutputCommitted: () => this.#textOutputCommitted, thinkingLevel: () => this.thinkingLevel, configuredThinkingLevel: () => this.configuredThinkingLevel(), diff --git a/packages/coding-agent/src/session/session-maintenance.ts b/packages/coding-agent/src/session/session-maintenance.ts index f7cae8287..f9399daac 100644 --- a/packages/coding-agent/src/session/session-maintenance.ts +++ b/packages/coding-agent/src/session/session-maintenance.ts @@ -1817,30 +1817,26 @@ export class SessionMaintenance { } /** - * Retry-side counterpart to {@link #compactionCreatedHeadroom}. An - * overflow/incomplete recovery only needs the rebuilt prompt to *fit* the - * window again — it does not have to land under the compaction threshold, let - * alone the stricter `COMPACTION_RECOVERY_BAND × threshold` hysteresis the - * auto-continue thrash guard uses. Reusing the band here turned recoverable - * overflows into manual dead-ends: a 200k-window prompt compacted from - * overflow down to ~150k is comfortably retryable, but sits above - * `0.8 × 170k = 136k` and was wrongly refused (PR #3412 review). + * Whether the current stored context fits `model`'s usable window + * (`contextWindow - reserve`), using the same reserve resolution as + * compaction. This is deliberately independent of `compaction.enabled`: an + * oversized request overflows the provider whether or not compaction would + * have run, so a fit check must judge the raw budget. * - * Measures residual context against the usable budget (`contextWindow - reserve`). * The default absolute reserve can exceed bundled small-context windows, or * nearly consume a 16k-class window; those known-impossible defaults fall * back to the proportional 15% reserve. Explicit valid reserves still define - * the usable prompt budget so retries do not enter headroom the user - * intentionally reserved. Callers MUST - * invoke this AFTER dropping the failed assistant from `this.#host.messages()`, so - * the just-failed turn (which the retry prompt will not include) is excluded - * from the estimate. + * the usable prompt budget so callers do not enter headroom the user + * intentionally reserved. * - * When the model/window is unknown we cannot evaluate the budget, so we - * optimistically allow the retry (preserving prior behavior). + * Used by the retry-fallback selector to skip a candidate whose window cannot + * hold the live context before switching onto it, and (via + * {@link #compactionCreatedRetryFit}) to decide whether an overflow recovery + * produced a retryable prompt. When the window is unknown we cannot evaluate + * the budget, so we optimistically report a fit (preserving prior behavior). */ - #compactionCreatedRetryFit(): boolean { - const contextWindow = this.#model?.contextWindow ?? 0; + contextFitsModel(model: Model): boolean { + const contextWindow = model.contextWindow ?? 0; if (contextWindow <= 0) return true; const compactionSettings = this.#host.settings.getGroup("compaction"); const residualTokens = compactionContextTokens( @@ -1851,6 +1847,20 @@ export class SessionMaintenance { return residualTokens <= fitBudget; } + /** + * Retry-side check: whether an overflow/incomplete recovery rebuilt a prompt + * that fits the active model's window again. Callers MUST invoke this AFTER + * dropping the failed assistant from `this.#host.messages()` so the just-failed + * turn (absent from the retry prompt) is excluded from the estimate. Unlike + * the `COMPACTION_RECOVERY_BAND × threshold` hysteresis the auto-continue + * thrash guard uses, a retry only needs to *fit* — a 200k-window prompt + * compacted from overflow down to ~150k is retryable even though it sits above + * `0.8 × 170k` (PR #3412 review). + */ + #compactionCreatedRetryFit(): boolean { + return this.#model ? this.contextFitsModel(this.#model) : true; + } + /** * Last-resort tiered reducer when {@link runAutoCompaction} would otherwise * dead-end. The summarizer cut at the only available turn boundary, but the diff --git a/packages/coding-agent/src/session/turn-recovery.ts b/packages/coding-agent/src/session/turn-recovery.ts index 3a05bcbc3..83dc031b3 100644 --- a/packages/coding-agent/src/session/turn-recovery.ts +++ b/packages/coding-agent/src/session/turn-recovery.ts @@ -110,6 +110,12 @@ export interface TurnRecoveryHost { modelRegistry: ModelRegistry; configWarnings: string[]; model(): Model | undefined; + /** + * Whether the live context fits `model`'s usable window. Retry-fallback + * selection uses it to skip a candidate whose window cannot hold the current + * context before switching onto it. See `SessionMaintenance.contextFitsModel`. + */ + contextFitsModel(model: Model): boolean; /** Whether streamed text has already been committed to the active output sink. */ textOutputCommitted(): boolean; thinkingLevel(): ThinkingLevel | undefined; @@ -1188,6 +1194,10 @@ export class TurnRecovery { const candidateModel = resolved.model ?? this.#host.modelRegistry.find(candidate.provider, candidate.id); if (!candidateModel || !this.#host.modelRegistry.hasConfiguredAuth(candidateModel)) continue; if (ceiling !== undefined && !modelSupportsEffortCeiling(candidateModel, ceiling)) continue; + // A usage fallback must also fit: skip a candidate whose window cannot + // hold the live context so we never switch onto an oversized request + // (issue #8065). + if (!this.#host.contextFitsModel(candidateModel)) continue; try { const candidateHealth = await this.#host.modelRegistry.authStorage.getModelUsageHealth( candidateModel.provider, @@ -1357,6 +1367,10 @@ export class TurnRecovery { // A candidate whose effort floor exceeds the per-spawn ceiling would be // clamped UP past the cap by its model floor — skip it entirely. if (ceiling !== undefined && !modelSupportsEffortCeiling(candidate, ceiling)) continue; + // Skip a candidate whose window cannot hold the live context: switching + // onto it would send a predictably oversized request. Advance to the + // first candidate whose usable window fits (issue #8065). + if (!this.#host.contextFitsModel(candidate)) continue; const apiKey = await this.#host.modelRegistry.getApiKey(candidate, this.#host.sessionId()); if (!apiKey) continue; return this.applyRetryFallbackCandidate(role, selector, currentSelector, options); diff --git a/packages/coding-agent/test/agent-session-retry-fallback.test.ts b/packages/coding-agent/test/agent-session-retry-fallback.test.ts index fee96de00..5da7a5468 100644 --- a/packages/coding-agent/test/agent-session-retry-fallback.test.ts +++ b/packages/coding-agent/test/agent-session-retry-fallback.test.ts @@ -3517,6 +3517,105 @@ describe("AgentSession retry fallback", () => { expect(requestedModels.filter(id => id === `${primaryModel.provider}/${primaryModel.id}`)).toHaveLength(1); }); + it("does not send oversized context to a smaller retry fallback model", async () => { + // Regression for #8065: the forward counterpart of #7952. A retryable + // error on a large-window primary switches to a retry-fallback candidate, + // but candidate selection never compared the candidate's window with the + // live context. A 1M-window primary could fall onto a 4000-window fallback + // and immediately send a predictably oversized request. The fit gate must + // skip the undersized candidate and advance to the first configured + // candidate whose window can hold the accumulated context. + const modelsConfigPath = path.join(tempDir.path(), "fallback-overflow-models.json"); + await Bun.write( + modelsConfigPath, + JSON.stringify({ + providers: { + anthropic: { + modelOverrides: { + "claude-sonnet-4-5": { contextWindow: 1_000_000 }, + }, + }, + openai: { + modelOverrides: { + "gpt-4o-mini": { contextWindow: 4000, contextPromotionTarget: "openai/gpt-4o" }, + "gpt-4o": { contextWindow: 1_000_000 }, + }, + }, + }, + }), + ); + modelRegistry = new ModelRegistry(authStorage, modelsConfigPath); + + const primaryModel = modelRegistry.find("anthropic", "claude-sonnet-4-5"); + const smallFallback = modelRegistry.find("openai", "gpt-4o-mini"); + const largeFallback = modelRegistry.find("openai", "gpt-4o"); + if (!primaryModel || !smallFallback || !largeFallback) { + throw new Error("Expected override models to resolve"); + } + expect(primaryModel.contextWindow).toBe(1_000_000); + expect(smallFallback.contextWindow).toBe(4000); + expect(largeFallback.contextWindow).toBe(1_000_000); + + // ~15k estimated tokens in the initial prompt: fits the 1M primary and the + // 1M large fallback, but far exceeds the 4000-window small fallback + // (80% => 3200), so the small fallback cannot legally receive the request. + const bigText = "lorem ipsum ".repeat(5000); + const requestedModels: string[] = []; + const mock = createMockModel(); + let primaryAttempts = 0; + const agent = new Agent({ + getApiKey: model => `${model.provider}-test-key`, + initialState: { + model: primaryModel, + systemPrompt: ["Test"], + tools: [], + messages: [], + }, + streamFn: (model, context, options) => { + requestedModels.push(`${model.provider}/${model.id}`); + if (model.id === primaryModel.id && primaryAttempts === 0) { + primaryAttempts += 1; + mock.push({ throw: "rate limit exceeded retry-after-ms=200" }); + } else { + mock.push({ content: ["ok"] }); + } + return mock.stream(model, context, options); + }, + }); + + const settings = Settings.isolated({ + "compaction.enabled": true, + "compaction.strategy": "context-full", + "compaction.thresholdPercent": 80, + "compaction.thresholdTokens": -1, + "contextPromotion.enabled": true, + "retry.baseDelayMs": 5, + "retry.fallbackChains": { + default: [`${smallFallback.provider}/${smallFallback.id}`, `${largeFallback.provider}/${largeFallback.id}`], + }, + }); + settings.setModelRole("default", `${primaryModel.provider}/${primaryModel.id}`); + + session = new AgentSession({ + agent, + sessionManager: SessionManager.inMemory(), + settings, + modelRegistry, + }); + const now = Date.now(); + vi.spyOn(Date, "now").mockImplementation(() => now); + + // Primary rate-limits with a live ~15k context; the retry-fallback path + // must skip the 4000-window candidate and land on the 1M-window one. + await session.prompt(bigText); + await session.waitForIdle(); + + expect(requestedModels).not.toContain(`${smallFallback.provider}/${smallFallback.id}`); + expect(requestedModels).toContain(`${largeFallback.provider}/${largeFallback.id}`); + expect(session.model?.id).toBe(largeFallback.id); + expect(requestedModels.at(-1)).toBe(`${largeFallback.provider}/${largeFallback.id}`); + }); + it("restores routed fallback primaries after cooldown expiry", async () => { const openRouterModel = getBundledModel("openrouter", "z-ai/glm-4.7"); const fallbackModel = getBundledModel("openai", "gpt-4o-mini"); diff --git a/packages/coding-agent/test/turn-recovery-replay-unsafe.test.ts b/packages/coding-agent/test/turn-recovery-replay-unsafe.test.ts index 2d5f91eee..0ed01d8af 100644 --- a/packages/coding-agent/test/turn-recovery-replay-unsafe.test.ts +++ b/packages/coding-agent/test/turn-recovery-replay-unsafe.test.ts @@ -54,6 +54,7 @@ function createHost( modelRegistry, configWarnings: [], model: () => model, + contextFitsModel: () => true, textOutputCommitted: () => options.textOutputCommitted !== false, thinkingLevel: () => undefined, configuredThinkingLevel: () => undefined,